Skip to content

Commit

Permalink
Always show trend graph tab.
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
fniessink committed Jul 3, 2024
1 parent fdba285 commit 2d0d58d
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 28 deletions.
36 changes: 17 additions & 19 deletions components/frontend/src/metric/MetricDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,27 +200,25 @@ export function MetricDetails({
</Tab.Pane>
),
},
{
menuItem: (
<Menu.Item key="trend_graph">
<Icon
className="linegraph"
/* Using name="linegraph" results in "Invalid prop `name` of value `linegraph` supplied to `Icon`."

Check failure on line 208 in components/frontend/src/metric/MetricDetails.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Insert `····`
Using name="line graph" does not show the icon. Using className works around this. */
/>
<FocusableTab>{"Trend graph"}</FocusableTab>
</Menu.Item>
),
render: () => (
<Tab.Pane>
<TrendGraph metric={metric} measurements={measurements} />
</Tab.Pane>
),
},
)
if (measurements.length > 0) {
if (getMetricScale(metric, dataModel) !== "version_number") {
panes.push({
menuItem: (
<Menu.Item key="trend_graph">
<Icon
className="linegraph"
/* Using name="linegraph" results in "Invalid prop `name` of value `linegraph` supplied to `Icon`."
Using name="line graph" does not show the icon. Using className works around this. */
/>
<FocusableTab>{"Trend graph"}</FocusableTab>
</Menu.Item>
),
render: () => (
<Tab.Pane>
<TrendGraph metric={metric} measurements={measurements} />
</Tab.Pane>
),
})
}
last_measurement.sources.forEach((source) => {
const report_source = metric.sources[source.source_uuid]
if (!report_source) {
Expand Down
4 changes: 2 additions & 2 deletions components/frontend/src/metric/MetricDetails.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ 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)
})

it("removes the existing hashtag from the URL to share", async () => {
Expand Down
19 changes: 19 additions & 0 deletions components/frontend/src/metric/TrendGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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)
Expand All @@ -15,6 +16,24 @@ function measurementAttributeAsNumber(metric, measurement, field, dataModel) {
export function TrendGraph({ metric, measurements }) {
const dataModel = useContext(DataModel)
const darkMode = useContext(DarkMode)
if (getMetricScale(metric, dataModel) === "version_number") {
return (
<WarningMessage
content="Sorry, trend graphs are not supported for metrics with a version number scale."
header="Trend graph not supported for version numbers"
showIf={true}
/>
)
}
if (measurements.length === 0) {
return (
<WarningMessage
content="A trend graph can not be displayed until this metric has measurements."
header="No measurements"
showIf={true}
/>
)
}
const metricName = getMetricName(metric, dataModel)
const unit = capitalize(formatMetricScaleAndUnit(metric, dataModel))
const measurementValues = measurements.map((measurement) =>
Expand Down
21 changes: 14 additions & 7 deletions components/frontend/src/metric/TrendGraph.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,16 @@ const dataModel = {
},
}

function renderTrendgraph({ measurements = [], darkMode = false } = {}) {
function renderTrendgraph({ measurements = [], darkMode = false, scale = "count" } = {}) {
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} />
</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" }],
Expand Down Expand Up @@ -67,3 +62,15 @@ it("renders the measurements with missing values for the scale", () => {
})
expect(screen.getAllByText(/Time/).length).toBe(1)
})

it("renders a message if the metric scale is version number", () => {
renderTrendgraph({scale: "version_number"})

Check failure on line 67 in components/frontend/src/metric/TrendGraph.test.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Replace `scale:·"version_number"` with `·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)
})

0 comments on commit 2d0d58d

Please sign in to comment.