When adding a new source to a metric, don't show the spinner until th…

…e source has been configured sufficiently to start collecting data.

Fixes #9994.

components/frontend/src/__fixtures__/fixtures.js

@@ -28,6 +28,9 @@ export const dataModel = {
     metrics: {
         metric_type: { name: "Metric type", tags: [] },
     },
+    sources: {
+        source_type: { name: "Source type", metrics: ["metric_type"], parameters: {} },
+    },
 }
 
 export const report = {
@@ -42,7 +45,7 @@ export const report = {
                     type: "metric_type",
                     tags: ["other tag"],
                     target: "1",
-                    sources: { source_uuid: { name: "Source" } },
+                    sources: { source_uuid: { name: "Source", type: "source_type" } },
                     status: "target_not_met",
                     recent_measurements: [],
                     latest_measurement: { count: 1 },
@@ -54,7 +57,7 @@ export const report = {
                     tags: ["tag"],
                     target: "2",
                     issue_ids: ["ABC-42"],
-                    sources: { source_uuid2: { name: "Source 2" } },
+                    sources: { source_uuid2: { name: "Source 2", type: "source_type" } },
                     status: "informative",
                     recent_measurements: [],
                     latest_measurement: { count: 2 },

components/frontend/src/measurement/MeasurementValue.jsx

@@ -15,6 +15,7 @@ import {
     isMeasurementOutdated,
     isMeasurementRequested,
     isMeasurementStale,
+    isSourceConfigurationComplete,
     sum,
 } from "../utils"
 import { IgnoreIcon, LoadingIcon } from "../widgets/icons"
@@ -104,8 +105,10 @@ export function MeasurementValue({ metric, reportDate }) {
     value = formatMetricValue(scale, value)
     const unit = getMetricUnit(metric, dataModel)
     const stale = isMeasurementStale(metric, reportDate)
+    const complete = isSourceConfigurationComplete(dataModel, metric)
     const outdated = isMeasurementOutdated(metric)
     const requested = isMeasurementRequested(metric)
+    const updating = complete && (outdated || requested)
     const hasIgnoredEntities = sum(ignoredEntitiesCount(metric.latest_measurement)) > 0
     return (
         <Tooltip
@@ -115,6 +118,10 @@ export function MeasurementValue({ metric, reportDate }) {
                     <WarningMessage showIf={stale} title="This metric was not recently measured">
                         This may indicate a problem with Quality-time itself. Please contact a system administrator.
                     </WarningMessage>
+                    <WarningMessage showIf={!complete} title="Source configuration incomplete">
+                        The source configuration of this metric is not complete. Add at least one source and make sure
+                        all mandatory parameters for all sources have been provided.
+                    </WarningMessage>
                     <WarningMessage showIf={outdated} title="Latest measurement out of date">
                         The source configuration of this metric was changed after the latest measurement.
                     </WarningMessage>
@@ -143,7 +150,7 @@ export function MeasurementValue({ metric, reportDate }) {
                 </div>
             }
         >
-            {measurementValueLabel(hasIgnoredEntities, stale, outdated || requested, value)}
+            {measurementValueLabel(hasIgnoredEntities, stale, updating, value)}
         </Tooltip>
     )
 }

components/frontend/src/measurement/MeasurementValue.test.jsx

@@ -13,14 +13,21 @@ function renderMeasurementValue({
     status = null,
     type = "violations",
     unit = null,
+    url = "https://example.org",
 } = {}) {
     return render(
-        <DataModel.Provider value={{ metrics: { violations: { unit: "violations" } } }}>
+        <DataModel.Provider
+            value={{
+                metrics: { violations: { unit: "violations" } },
+                sources: { source_type: { parameters: { url: { mandatory: true, metrics: ["violations"] } } } },
+            }}
+        >
             <MeasurementValue
                 metric={{
                     latest_measurement: latest_measurement,
                     measurement_requested: measurement_requested,
                     scale: scale,
+                    sources: { source_uuid: { type: "source_type", parameters: { url: url } } },
                     status: status,
                     type: type,
                     unit: unit,
@@ -40,12 +47,29 @@ it("renders the value", async () => {
 it("renders an unkown value", async () => {
     const { container } = renderMeasurementValue({ latest_measurement: { count: { value: null } } })
     expect(screen.getAllByText(/\?/).length).toBe(1)
+    expect(screen.queryAllByTestId("LoopIcon").length).toBe(0)
     await expectNoAccessibilityViolations(container)
 })
 
 it("renders a value that has not been measured yet", async () => {
     const { container } = renderMeasurementValue()
     expect(screen.getAllByText(/\?/).length).toBe(1)
+    expect(screen.queryAllByTestId("LoopIcon").length).toBe(0)
+    await expectNoAccessibilityViolations(container)
+})
+
+it("renders a value that can not be measured yet", async () => {
+    const { container } = renderMeasurementValue({
+        latest_measurement: {
+            count: { value: 1 },
+            outdated: true,
+            start: new Date().toISOString(),
+            end: new Date().toISOString(),
+        },
+        url: "",
+    })
+    expect(screen.getAllByText(/1/).length).toBe(1)
+    expect(screen.queryAllByTestId("LoopIcon").length).toBe(0)
     await expectNoAccessibilityViolations(container)
 })
 

components/frontend/src/metric/MetricDetails.jsx

@@ -23,7 +23,7 @@ import {
 import { Logo } from "../source/Logo"
 import { SourceEntities } from "../source/SourceEntities"
 import { Sources } from "../source/Sources"
-import { getSourceName, isMeasurementRequested } from "../utils"
+import { getSourceName, isMeasurementRequested, isSourceConfigurationComplete } from "../utils"
 import { ButtonRow } from "../widgets/ButtonRow"
 import { ActionButton } from "../widgets/buttons/ActionButton"
 import { DeleteButton } from "../widgets/buttons/DeleteButton"
@@ -37,16 +37,22 @@ import { MetricDebtParameters } from "./MetricDebtParameters"
 import { TrendGraph } from "./TrendGraph"
 
 function RequestMeasurementButton({ metric, metric_uuid, reload }) {
+    const dataModel = useContext(DataModel)
+    const configurationComplete = isSourceConfigurationComplete(dataModel, metric)
     const measurementRequested = isMeasurementRequested(metric)
     return (
         <ActionButton
             action="Measure"
-            disabled={measurementRequested}
+            disabled={!configurationComplete || measurementRequested}
             icon={<RefreshIcon />}
             itemType="metric"
             loading={measurementRequested}
             onClick={() => set_metric_attribute(metric_uuid, "measurement_requested", new Date().toISOString(), reload)}
-            popup={`Measure this metric as soon as possible`}
+            popup={
+                configurationComplete
+                    ? "Measure this metric as soon as possible"
+                    : "The source configuration of this metric is not complete. Add at least one source and make sure all mandatory parameters for all sources have been provided."
+            }
         />
     )
 }

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

@@ -76,31 +76,35 @@ const dataModel = {
     subjects: { subject_type: { metrics: ["violations"] } },
 }
 
-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: [
+function getMetricMeasurementsSuccessfully(connection_error) {
+    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_uuid2" },
                     {
-                        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_uuid2" },
-                            {
-                                source_uuid: "source_uuid",
-                                entities: [{ key: "1" }],
-                                connection_error: connection_error,
-                            },
-                        ],
+                        source_uuid: "source_uuid",
+                        entities: [{ key: "1" }],
+                        connection_error: connection_error,
                     },
                 ],
-            })
-        }
+            },
+        ],
+    })
+}
+
+async function renderMetricDetails({
+    stopFilteringAndSorting = null,
+    connection_error = null,
+    getMetricMeasurements = null,
+} = {}) {
+    measurement_api.get_metric_measurements.mockImplementation(() => {
+        return getMetricMeasurements ? getMetricMeasurements() : getMetricMeasurementsSuccessfully(connection_error)
     })
     source_api.set_source_entity_attribute.mockImplementation(
         (_metric_uuid, _source_uuid, _entity_key, _attribute, _value, reload) => reload(),
@@ -152,7 +156,7 @@ it("removes the existing hashtag from the URL to share", async () => {
 })
 
 it("displays whether sources have errors", async () => {
-    const { container } = await renderMetricDetails(null, "Connection error")
+    const { container } = await renderMetricDetails({ connection_error: "Connection error" })
     expect(screen.getByText(/Sources/)).toHaveClass("error")
     await expectNoAccessibilityViolations(container)
 })
@@ -165,7 +169,7 @@ it("displays whether sources have warnings", async () => {
 
 it("moves the metric", async () => {
     const mockCallback = vi.fn()
-    await renderMetricDetails(mockCallback)
+    await renderMetricDetails({ stopFilteringAndSorting: mockCallback })
     await act(async () => fireEvent.click(screen.getByRole("button", { name: /Move metric to the last row/ })))
     expect(mockCallback).toHaveBeenCalled()
     expect(measurement_api.get_metric_measurements).toHaveBeenCalled()
@@ -189,9 +193,18 @@ it("measures the metric", async () => {
     )
 })
 
+it("does not measure the metric if the metric source configuration is incomplete", async () => {
+    dataModel.sources["sonarqube"].parameters = { url: { mandatory: true, metrics: ["violations"] } }
+    await renderMetricDetails()
+    fireEvent.click(screen.getByText(/Measure metric/))
+    expect(fetch_server_api.fetch_server_api).not.toHaveBeenCalled()
+})
+
 it("fails to load measurements", async () => {
     history.push("?expanded=metric_uuid:5")
-    const { container } = await renderMetricDetails(null, null, true)
+    const { container } = await renderMetricDetails({
+        getMetricMeasurements: () => Promise.reject(new Error("failed to load measurements")),
+    })
     expect(screen.queryAllByText(/Loading measurements failed/).length).toBe(1)
     await expectNoAccessibilityViolations(container)
 })

components/frontend/src/utils.jsx

@@ -4,6 +4,7 @@ import { PERMISSIONS } from "./context/Permissions"
 import { defaultDesiredResponseTimes } from "./defaults"
 import { STATUSES_NOT_REQUIRING_ACTION } from "./metric/status"
 import {
+    dataModelPropType,
     datePropType,
     metricPropType,
     metricsPropType,
@@ -104,12 +105,34 @@ export function isMeasurementOutdated(metric) {
     if (metric.latest_measurement) {
         return metric.latest_measurement.outdated ?? false
     }
-    return Object.keys(metric.sources ?? {}).length > 0 // If there are sources, measurement is needed
+    return false
 }
 isMeasurementOutdated.propTypes = {
     metric: metricPropType,
 }
 
+export function isSourceConfigurationComplete(dataModel, metric) {
+    // Return whether the metric can be measured, meaning that there is at least one source and all sources have
+    // all mandatory parameters configured
+    const sources = Object.values(metric.sources ?? {})
+    if (sources.length === 0) {
+        return false
+    }
+    return sources.every((source) => {
+        const parameters = dataModel.sources[source.type].parameters
+        return Object.entries(parameters).every(([parameterKey, parameter]) => {
+            if (parameter.mandatory && parameter.metrics.includes(metric.type) && !parameter.default_value) {
+                return source?.parameters?.[parameterKey]
+            }
+            return true
+        })
+    })
+}
+isSourceConfigurationComplete.propTypes = {
+    dataModel: dataModelPropType,
+    metric: metricPropType,
+}
+
 export function isMeasurementRequested(metric) {
     if (metric.measurement_requested) {
         if (metric.latest_measurement) {

components/frontend/src/utils.test.jsx

@@ -17,6 +17,7 @@ import {
     isMeasurementOutdated,
     isMeasurementRequested,
     isMeasurementStale,
+    isSourceConfigurationComplete,
     niceNumber,
     nrMetricsInReport,
     nrMetricsInReports,
@@ -509,14 +510,43 @@ it("returns whether a metric's measurement is outdated", () => {
     expect(isMeasurementOutdated({})).toBe(false)
     // A metric without measurements with an empty collection of sources is not outdated:
     expect(isMeasurementOutdated({ sources: {} })).toBe(false)
-    // A metric without measurements but with a source is outdated:
-    expect(isMeasurementOutdated({ sources: { source_uuid: {} } })).toBe(true)
+    // A metric without measurements but with a source is not outdated:
+    expect(isMeasurementOutdated({ sources: { source_uuid: {} } })).toBe(false)
     // A metric with an up-to-date measurement is not outdated:
     expect(isMeasurementOutdated({ latest_measurement: {} })).toBe(false)
     // A metric with an outdated measurement is outdated:
     expect(isMeasurementOutdated({ latest_measurement: { outdated: true } })).toBe(true)
 })
 
+it("returns whether a metric can be measured", () => {
+    const dataModel = {
+        sources: { source_type: { parameters: { url: { mandatory: true, metrics: ["metric_type"] } } } },
+    }
+    const metric = { type: "metric_type" }
+    // A metric without sources is not measurable:
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(false)
+    // A metric with a source without parameters is not measurable:
+    metric.sources = { source_uuid: { type: "source_type" } }
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(false)
+    // A metric with a source with empty parameters is measurable:
+    metric.sources["source_uuid"].parameters = {}
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(false)
+    // A metric with a source with mandatory parameters configured is measurable:
+    metric.sources["source_uuid"].parameters["url"] = "https://example.org"
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(true)
+    // Non-mandatory parameters that are not configured don't make the metric unmeasurable:
+    dataModel.sources["source_type"].parameters["username"] = { mandatory: false, metrics: ["metric_type"] }
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(true)
+    // Mandatory parameters for other metrics that are not configured don't make the metric unmeasurable:
+    metric.sources["source_uuid"].parameters["password"] = { mandatory: true, metrics: ["other_metric_type"] }
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(true)
+    // A metric with a source with mandatory parameters that has a default value for a mandatory parameter is
+    // measurable:
+    dataModel.sources["source_type"].parameters["url"]["default_value"] = "https://example.org"
+    metric.sources["source_uuid"].parameters = {}
+    expect(isSourceConfigurationComplete(dataModel, metric)).toBe(true)
+})
+
 it("returns the reference documentation URL", () => {
     const url = `${DOCUMENTATION_URL}/reference.html`
     // Simple name

docs/src/changelog.md

@@ -21,6 +21,7 @@ If your currently installed *Quality-time* version is not the latest version, pl
 
 ### Fixed
 
+- When adding a new source to a metric, don't show the spinner until the source has been configured sufficiently to start collecting data. Fixes [#9994](https://github.com/ICTU/quality-time/issues/9994).
 - Increase contrast for disabled items in the menu bar. Fixes [#10840](https://github.com/ICTU/quality-time/issues/10840).
 - Links to documentation on Read the Docs for subjects, metrics, or sources with hyphens in their name wouldn't scroll to the right location. Fixes [#10843](https://github.com/ICTU/quality-time/issues/10843).
 - Metric details were not shown in exports to PDF. Fixes [#10845](https://github.com/ICTU/quality-time/issues/10845).