diff --git a/components/frontend/src/metric/MetricDetails.js b/components/frontend/src/metric/MetricDetails.js index c57ffd873e..84db263e89 100644 --- a/components/frontend/src/metric/MetricDetails.js +++ b/components/frontend/src/metric/MetricDetails.js @@ -18,7 +18,7 @@ import { } from "../sharedPropTypes" import { SourceEntities } from "../source/SourceEntities" import { Sources } from "../source/Sources" -import { getMetricScale, getSourceName, isMeasurementRequested } from "../utils" +import { getSourceName, isMeasurementRequested } from "../utils" import { ActionButton, DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button" import { changelogTabPane, configurationTabPane, tabPane } from "../widgets/TabPane" import { showMessage } from "../widgets/toast" @@ -81,20 +81,23 @@ Buttons.propTypes = { url: string, } -function fetchMeasurements(reportDate, metric_uuid, setMeasurements) { +function fetchMeasurements(reportDate, metric_uuid, setMeasurements, setMeasurementsStatus) { get_metric_measurements(metric_uuid, reportDate) .then(function (json) { - if (json.ok !== false) { - setMeasurements(json.measurements) - } + setMeasurements(json.measurements ?? []) + setMeasurementsStatus("loaded") return null }) - .catch((error) => showMessage("error", "Could not fetch measurements", `${error}`)) + .catch((error) => { + showMessage("error", "Could not fetch measurements", `${error}`) + setMeasurementsStatus("failed") + }) } fetchMeasurements.propTypes = { - reportDate: datePropType, metric_uuid: string, + reportDate: datePropType, setMeasurements: func, + setMeasurementsStatus: func, } export function MetricDetails({ @@ -103,7 +106,7 @@ export function MetricDetails({ isLastMetric, metric_uuid, reload, - report_date, + reportDate, reports, report, stopFilteringAndSorting, @@ -112,18 +115,19 @@ export function MetricDetails({ }) { const dataModel = useContext(DataModel) const [measurements, setMeasurements] = useState([]) + const [measurementsStatus, setMeasurementsStatus] = useState("loading") useEffect(() => { - fetchMeasurements(report_date, metric_uuid, setMeasurements) + fetchMeasurements(reportDate, metric_uuid, setMeasurements, setMeasurementsStatus) // eslint-disable-next-line - }, [metric_uuid, report_date]) + }, [metric_uuid, reportDate]) function measurementsReload() { reload() - fetchMeasurements(report_date, metric_uuid, setMeasurements) + fetchMeasurements(reportDate, metric_uuid, setMeasurements, setMeasurementsStatus) } const subject = report.subjects[subject_uuid] const metric = subject.metrics[metric_uuid] - const last_measurement = measurements[measurements.length - 1] - let anyError = last_measurement?.sources.some((source) => source.connection_error || source.parse_error) + const lastMeasurement = measurements[measurements.length - 1] + let anyError = lastMeasurement?.sources.some((source) => source.connection_error || source.parse_error) anyError = anyError || Object.values(metric.sources ?? {}).some( @@ -160,28 +164,26 @@ export function MetricDetails({ { iconName: "server", error: Boolean(anyError) }, ), changelogTabPane(), + tabPane( + "Trend graph", + , + { iconName: "linegraph" }, + ), ) if (measurements.length > 0) { - if (getMetricScale(metric, dataModel) !== "version_number") { - panes.push( - tabPane("Trend graph", , { - iconName: "linegraph", - }), - ) - } - last_measurement.sources.forEach((source) => { - const report_source = metric.sources[source.source_uuid] - if (!report_source) { + lastMeasurement.sources.forEach((source) => { + const reportSource = metric.sources[source.source_uuid] + if (!reportSource) { return } // source was deleted, continue - const nr_entities = source.entities?.length ?? 0 - if (nr_entities === 0) { + const nrEntities = source.entities?.length ?? 0 + if (nrEntities === 0) { return } // no entities to show, continue - const source_name = getSourceName(report_source, dataModel) + const sourceName = getSourceName(reportSource, dataModel) panes.push( tabPane( - source_name, + sourceName, - Promise.resolve({ - ok: true, - measurements: [ - { - count: { value: "42" }, - version_number: { value: "1.1" }, - start: "2020-02-29T10:25:52.252Z", - end: "2020-02-29T11:25:52.252Z", - sources: [ - {}, - { source_uuid: "source_uuid" }, - { - source_uuid: "source_uuid", - entities: [{ key: "1" }], - connection_error: connection_error, - }, - ], - }, - ], - }), +async function renderMetricDetails(stopFilteringAndSorting, connection_error, failLoadingMeasurements) { + measurement_api.get_metric_measurements.mockImplementation(() => { + if (failLoadingMeasurements) { + return Promise.reject(new Error("failed to load measurements")) + } else { + return Promise.resolve({ + measurements: [ + { + count: { value: "42" }, + version_number: { value: "1.1" }, + start: "2020-02-29T10:25:52.252Z", + end: "2020-02-29T11:25:52.252Z", + sources: [ + {}, + { source_uuid: "source_uuid" }, + { + source_uuid: "source_uuid", + entities: [{ key: "1" }], + connection_error: connection_error, + }, + ], + }, + ], + }) + } + }) + source_api.set_source_entity_attribute.mockImplementation( + (_metric_uuid, _source_uuid, _entity_key, _attribute, _value, reload) => reload(), ) changelog_api.get_changelog.mockImplementation(() => Promise.resolve({ changelog: [] })) const settings = createTestableSettings() await act(async () => render( - + { + jest.clearAllMocks() history.push("") - fetch_server_api.fetch_server_api = jest.fn().mockReturnValue({ - then: jest.fn().mockReturnValue({ - catch: jest.fn().mockReturnValue({ finally: jest.fn() }), - }), - }) + fetch_server_api.fetch_server_api.mockImplementation(() => Promise.resolve()) }) it("switches tabs", async () => { @@ -135,10 +141,11 @@ it("switches tabs to the trend graph", async () => { expect(screen.getAllByText(/Time/).length).toBe(1) }) -it("does not show the trend graph tab if the metric scale is version number", async () => { +it("shows the trend graph tab even if the metric scale is version number", async () => { report.subjects["subject_uuid"].metrics["metric_uuid"].scale = "version_number" await renderMetricDetails() - expect(screen.queryAllByText(/Trend graph/).length).toBe(0) + expect(screen.queryAllByText(/Trend graph/).length).toBe(1) + report.subjects["subject_uuid"].metrics["metric_uuid"].scale = "count" }) it("removes the existing hashtag from the URL to share", async () => { @@ -188,3 +195,19 @@ it("measures the metric", async () => { expect.objectContaining({}), // Ignore the attribute value, it's new Date().toISOString() ) }) + +it("fails to load measurements", async () => { + await renderMetricDetails(null, null, true) + fireEvent.click(screen.getByText(/Trend graph/)) + expect(screen.queryAllByText(/Loading measurements failed/).length).toBe(1) +}) + +it("reloads the measurements after editing a measurement entity", async () => { + await renderMetricDetails() + expect(measurement_api.get_metric_measurements).toHaveBeenCalledTimes(1) + fireEvent.click(screen.getByText(/The source/)) + fireEvent.click(screen.getByRole("button", { name: "Expand/collapse" })) + fireEvent.click(screen.getAllByText("Unconfirmed")[1]) + await act(async () => fireEvent.click(screen.getByText("Confirm"))) + expect(measurement_api.get_metric_measurements).toHaveBeenCalledTimes(2) +}) diff --git a/components/frontend/src/metric/TrendGraph.js b/components/frontend/src/metric/TrendGraph.js index 507591ea3c..45ca9873f0 100644 --- a/components/frontend/src/metric/TrendGraph.js +++ b/components/frontend/src/metric/TrendGraph.js @@ -1,10 +1,13 @@ +import { oneOf } from "prop-types" import { useContext } from "react" +import { Placeholder, PlaceholderImage } from "semantic-ui-react" import { VictoryAxis, VictoryChart, VictoryLabel, VictoryLine, VictoryTheme } from "victory" import { DarkMode } from "../context/DarkMode" import { DataModel } from "../context/DataModel" import { measurementsPropType, metricPropType } from "../sharedPropTypes" import { capitalize, formatMetricScaleAndUnit, getMetricName, getMetricScale, niceNumber, scaledNumber } from "../utils" +import { WarningMessage } from "../widgets/WarningMessage" function measurementAttributeAsNumber(metric, measurement, field, dataModel) { const scale = getMetricScale(metric, dataModel) @@ -12,9 +15,42 @@ function measurementAttributeAsNumber(metric, measurement, field, dataModel) { return value !== null ? Number(value) : null } -export function TrendGraph({ metric, measurements }) { +export function TrendGraph({ metric, measurements, loading }) { const dataModel = useContext(DataModel) const darkMode = useContext(DarkMode) + const chartHeight = 250 + const estimatedTotalChartHeight = chartHeight + 200 // Estimate of the height including title and axis + if (getMetricScale(metric, dataModel) === "version_number") { + return ( + + ) + } + if (loading === "failed") { + return ( + + ) + } + if (loading === "loading") { + return ( + + + + ) + } + if (measurements.length === 0) { + return ( + + ) + } const metricName = getMetricName(metric, dataModel) const unit = capitalize(formatMetricScaleAndUnit(metric, dataModel)) const measurementValues = measurements.map((measurement) => @@ -44,7 +80,7 @@ export function TrendGraph({ metric, measurements }) { } return ( - + , ) } -it("renders the time axis", () => { - renderTrendgraph() - expect(screen.getAllByText(/Time/).length).toBe(1) -}) - it("renders the measurements", () => { renderTrendgraph({ measurements: [{ count: { value: "1" }, start: "2019-09-29", end: "2019-09-30" }], @@ -67,3 +66,26 @@ it("renders the measurements with missing values for the scale", () => { }) expect(screen.getAllByText(/Time/).length).toBe(1) }) + +it("renders a placeholder while the measurements are loading", () => { + const { container } = renderTrendgraph({ loading: "loading" }) + expect(container.firstChild.className).toContain("placeholder") + expect(screen.queryAllByText(/Time/).length).toBe(0) +}) + +it("renders a message if the measurements failed to load", () => { + renderTrendgraph({ loading: "failed" }) + expect(screen.getAllByText(/Loading measurements failed/).length).toBe(1) +}) + +it("renders a message if the metric scale is version number", () => { + renderTrendgraph({ scale: "version_number" }) + expect(screen.queryAllByText(/Time/).length).toBe(0) + expect(screen.getAllByText(/version number scale/).length).toBe(1) +}) + +it("renders a message if the metric has no measurements", () => { + renderTrendgraph() + expect(screen.queryAllByText(/Time/).length).toBe(0) + expect(screen.getAllByText(/trend graph can not be displayed/).length).toBe(1) +}) diff --git a/components/frontend/src/subject/SubjectTable.test.js b/components/frontend/src/subject/SubjectTable.test.js index c5144af0c7..c4d2d991cb 100644 --- a/components/frontend/src/subject/SubjectTable.test.js +++ b/components/frontend/src/subject/SubjectTable.test.js @@ -43,8 +43,8 @@ const dates = [ new Date("2020-01-13T00:00:00+00:00"), ] -function renderSubjectTable({ dates = [], expandedItems = null } = {}) { - let settings = createTestableSettings() +function renderSubjectTable({ dates = [], expandedItems = null, settings = null } = {}) { + settings = settings ?? createTestableSettings() if (expandedItems) { settings.expandedItems = expandedItems } @@ -207,28 +207,27 @@ it("expands the details via the button", () => { expect(expandedItems.result.current.value).toStrictEqual(["1:0"]) }) -it("collapses the details via the button", () => { +it("collapses the details via the button", async () => { history.push("?expanded=1:0") const expandedItems = renderHook(() => useExpandedItemsSearchQuery()) renderSubjectTable({ expandedItems: expandedItems.result.current }) const expand = screen.getAllByRole("button")[0] - fireEvent.click(expand) + await act(async () => fireEvent.click(expand)) expandedItems.rerender() expect(expandedItems.result.current.value).toStrictEqual([]) }) -it("expands the details via the url", () => { - renderSubjectTable() - expect(screen.queryAllByText("Configuration").length).toBe(0) +it("expands the details via the url", async () => { history.push("?expanded=1:0") renderSubjectTable() + await act(async () => {}) // Wait for hooks to finish expect(screen.queryAllByText("Configuration").length).toBe(1) }) it("moves a metric", async () => { history.push("?expanded=1:2") renderSubjectTable() - await act(async () => fireEvent.click(await screen.findByLabelText("Move metric to the next row"))) + await act(async () => fireEvent.click(screen.getByLabelText("Move metric to the next row"))) expect(fetch_server_api.fetch_server_api).toHaveBeenCalledWith("post", "metric/1/attribute/position", { position: "next", }) diff --git a/docs/src/changelog.md b/docs/src/changelog.md index d485b9ef70..6ce3c7d71e 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -18,6 +18,10 @@ If your currently installed *Quality-time* version is not v5.14.0, please first - Give measurement entity tables sticky headers, like the metric tables already have. Closes [#8879](https://github.com/ICTU/quality-time/issues/8879). +### Changed + +- Always show metric trend graph tabs. Display a loader in the tab while loading measurements. If the trend graph cannot be displayed, explain in the tab why not. Closes [#8825](https://github.com/ICTU/quality-time/issues/8825). + ## v5.14.0 - 2024-07-05 ### Deployment notes