Always show trend graph tab.

- Show warning message when the metric has a version number scale that trend graphs are not supported.
- Show warning message when the metric has no measurements yet.
- Show loader when loading measurements.
- Show warning message when loading failed.

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(<ChangeLog timestamp={report.timestamp} metric_uuid={metric_uuid} />),
+        tabPane(
+            "Trend graph",
+            <TrendGraph metric={metric} measurements={measurements} loading={measurementsStatus} />,
+            { iconName: "linegraph" },
+        ),
     )
     if (measurements.length > 0) {
-        if (getMetricScale(metric, dataModel) !== "version_number") {
-            panes.push(
-                tabPane("Trend graph", <TrendGraph metric={metric} measurements={measurements} />, {
-                    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,
                     <SourceEntities
                         report={report}
                         metric={metric}
@@ -220,7 +222,7 @@ MetricDetails.propTypes = {
     isLastMetric: bool,
     metric_uuid: string,
     reload: func,
-    report_date: datePropType,
+    reportDate: datePropType,
     reports: reportsPropType,
     report: reportPropType,
     stopFilteringAndSorting: func,

components/frontend/src/metric/MetricDetails.test.js

@@ -5,13 +5,15 @@ import { createTestableSettings } from "../__fixtures__/fixtures"
 import * as changelog_api from "../api/changelog"
 import * as fetch_server_api from "../api/fetch_server_api"
 import * as measurement_api from "../api/measurement"
+import * as source_api from "../api/source"
 import { DataModel } from "../context/DataModel"
-import { EDIT_REPORT_PERMISSION, Permissions } from "../context/Permissions"
+import { EDIT_ENTITY_PERMISSION, EDIT_REPORT_PERMISSION, Permissions } from "../context/Permissions"
 import { MetricDetails } from "./MetricDetails"
 
 jest.mock("../api/fetch_server_api")
 jest.mock("../api/changelog.js")
 jest.mock("../api/measurement.js")
+jest.mock("../api/source.js")
 
 const report = {
     report_uuid: "report_uuid",
@@ -55,37 +57,44 @@ const dataModel = {
     subjects: { subject_type: { metrics: ["violations"] } },
 }
 
-async function renderMetricDetails(stopFilteringAndSorting, connection_error) {
-    measurement_api.get_metric_measurements.mockImplementation(() =>
-        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(
-            <Permissions.Provider value={[EDIT_REPORT_PERMISSION]}>
+            <Permissions.Provider value={[EDIT_ENTITY_PERMISSION, EDIT_REPORT_PERMISSION]}>
                 <DataModel.Provider value={dataModel}>
                     <MetricDetails
                         metric_uuid="metric_uuid"
+                        reload={jest.fn()}
                         report={report}
                         reports={[report]}
                         stopFilteringAndSorting={stopFilteringAndSorting}
@@ -99,12 +108,9 @@ async function renderMetricDetails(stopFilteringAndSorting, connection_error) {
 }
 
 beforeEach(() => {
+    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)
+})

components/frontend/src/metric/TrendGraph.js

@@ -1,20 +1,56 @@
+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)
     const value = measurement[scale]?.[field] ?? null
     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 (
+            <WarningMessage
+                content="Trend graphs are not supported for metrics with a version number scale."
+                header="Trend graph not supported for version numbers"
+            />
+        )
+    }
+    if (loading === "failed") {
+        return (
+            <WarningMessage
+                content="Loading the measurements from the API-server failed."
+                header="Loading measurements failed"
+            />
+        )
+    }
+    if (loading === "loading") {
+        return (
+            <Placeholder fluid inverted={darkMode} style={{ height: estimatedTotalChartHeight }}>
+                <PlaceholderImage />
+            </Placeholder>
+        )
+    }
+    if (measurements.length === 0) {
+        return (
+            <WarningMessage
+                content="A trend graph can not be displayed until this metric has measurements."
+                header="No measurements"
+            />
+        )
+    }
     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 (
         <VictoryChart
-            height={250}
+            height={chartHeight}
             scale={{ x: "time", y: "linear" }}
             style={{
                 parent: { height: "100%", background: darkMode ? "rgb(40, 40, 40)" : "white" },
@@ -81,6 +117,7 @@ export function TrendGraph({ metric, measurements }) {
     )
 }
 TrendGraph.propTypes = {
+    loading: oneOf(["failed", "loaded", "loading"]),
     metric: metricPropType,
     measurements: measurementsPropType,
 }

components/frontend/src/metric/TrendGraph.test.js

@@ -11,21 +11,20 @@ const dataModel = {
     },
 }
 
-function renderTrendgraph({ measurements = [], darkMode = false } = {}) {
+function renderTrendgraph({ measurements = [], darkMode = false, scale = "count", loading = "loaded" } = {}) {
     return render(
         <DarkMode.Provider value={darkMode}>
             <DataModel.Provider value={dataModel}>
-                <TrendGraph metric={{ type: "violations", scale: "count" }} measurements={measurements} />
+                <TrendGraph
+                    metric={{ type: "violations", scale: scale }}
+                    measurements={measurements}
+                    loading={loading}
+                />
             </DataModel.Provider>
         </DarkMode.Provider>,
     )
 }
 
-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)
+})