diff --git a/src/api.ts b/src/api.ts index 3cd3976..3c16ca7 100644 --- a/src/api.ts +++ b/src/api.ts @@ -81,7 +81,7 @@ type CheckCommon = { published: boolean; output: Output | null; message: string | null; - related_property: string | null; + related_properties: string[] | null; passed: boolean; status: "PASS" | "FAIL" | "WARN"; executed_at: string | null; @@ -105,7 +105,7 @@ export type OutputType = | "custom"; export type CustomOutputOptions = { - unit: "string"; + unit: string; decimals: "auto" | number; }; diff --git a/src/components/CheckResultBadge.test.tsx b/src/components/CheckResultBadge.test.tsx new file mode 100644 index 0000000..a3ad458 --- /dev/null +++ b/src/components/CheckResultBadge.test.tsx @@ -0,0 +1,77 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import { CheckResultBadge, CheckResultBadgeProps } from "./CheckResultBadge"; + +function renderBadge(props: Partial = {}) { + return render( + + ); +} + +describe("CheckResultBadge", () => { + it("renders a custom unit verbatim rather than pluralizing it", () => { + renderBadge({ + outputValue: 4, + outputType: "custom", + outputCustomOptions: { unit: "trace metrics", decimals: 0 }, + }); + + expect(screen.getByText("4 trace metrics")).toBeInTheDocument(); + }); + + it("renders a custom unit verbatim when decimals are automatic", () => { + renderBadge({ + outputValue: 89, + outputType: "custom", + outputCustomOptions: { unit: "chars", decimals: "auto" }, + }); + + expect(screen.getByText("89 chars")).toBeInTheDocument(); + }); + + it("formats a custom value to the requested number of decimals", () => { + renderBadge({ + outputValue: 0, + outputType: "custom", + outputCustomOptions: { unit: "% time > 90% util", decimals: 2 }, + }); + + expect(screen.getByText("0.00 % time > 90% util")).toBeInTheDocument(); + }); + + it("pluralizes built-in duration units", () => { + renderBadge({ outputValue: 3, outputType: "duration_days" }); + + expect(screen.getByText("3 days")).toBeInTheDocument(); + }); + + it("keeps built-in duration units singular for a count of one", () => { + renderBadge({ outputValue: 1, outputType: "duration_days" }); + + expect(screen.getByText("1 day")).toBeInTheDocument(); + }); + + it("falls back to the status text when output is disabled", () => { + renderBadge({ + status: "FAIL", + outputEnabled: false, + outputValue: 4, + outputType: "number", + }); + + expect(screen.getByText("Not passed")).toBeInTheDocument(); + }); + + it("shows a no-data placeholder when output is enabled but empty", () => { + renderBadge({ outputType: "number" }); + + expect(screen.getByText("(No data)")).toBeInTheDocument(); + }); +}); diff --git a/src/components/CheckResultBadge.tsx b/src/components/CheckResultBadge.tsx index b94951c..62fb2db 100644 --- a/src/components/CheckResultBadge.tsx +++ b/src/components/CheckResultBadge.tsx @@ -230,13 +230,15 @@ function formatCustomOutputValue( outputValue: number, outputCustomOptions: CustomOutputOptions ): string { + // The unit is author-supplied and already in its intended form, so it is + // rendered verbatim rather than pluralized. if (outputCustomOptions.decimals === "auto") { - return `${outputValue} ${pluralize(outputCustomOptions.unit, outputValue)}`; + return `${outputValue} ${outputCustomOptions.unit}`; } const valueWithDecimals = outputValue.toFixed(outputCustomOptions.decimals); - return `${valueWithDecimals} ${pluralize(outputCustomOptions.unit, outputValue)}`; + return `${valueWithDecimals} ${outputCustomOptions.unit}`; } function pluralize(text: string, count: number | null = null) { diff --git a/src/components/CheckResultDrawer.test.tsx b/src/components/CheckResultDrawer.test.tsx new file mode 100644 index 0000000..69c2878 --- /dev/null +++ b/src/components/CheckResultDrawer.test.tsx @@ -0,0 +1,94 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import { CheckResultDrawer } from "./CheckResultDrawer"; +import { LevelBasedScorecardCheck } from "../api"; +import { COLORS } from "../styles"; + +const check: LevelBasedScorecardCheck = { + id: "rcw3pkmrxp8j", + name: "Internal dependencies", + description: "Set the internal-dependency-documentation DX property.", + published: true, + output: { type: "string", value: "Needs doc link" }, + message: null, + related_properties: null, + passed: false, + status: "FAIL", + executed_at: null, + level: { id: "92ktdhy45tls", name: "Required" }, +}; + +function renderDrawer({ + onEditRelatedProperty = () => {}, + ...overrides +}: Partial & { + onEditRelatedProperty?: () => void; +} = {}) { + return render( + {}} + onEditRelatedProperty={onEditRelatedProperty} + /> + ); +} + +describe("CheckResultDrawer", () => { + it("lists a single related property", () => { + renderDrawer({ + related_properties: ["internal-dependency-documentation"], + }); + + expect(screen.getByText("Related property:")).toBeInTheDocument(); + expect( + screen.getByText("internal-dependency-documentation") + ).toBeInTheDocument(); + }); + + it("lists every related property when a check has several", () => { + renderDrawer({ + related_properties: ["scaling-thresholds", "task-count"], + }); + + expect(screen.getByText("Related properties:")).toBeInTheDocument(); + expect(screen.getByText("scaling-thresholds")).toBeInTheDocument(); + expect(screen.getByText("task-count")).toBeInTheDocument(); + }); + + it("sets an explicit text color on the property chip so dark themes stay legible", () => { + renderDrawer({ + related_properties: ["internal-dependency-documentation"], + }); + + expect( + screen.getByText("internal-dependency-documentation") + ).toHaveStyle({ color: COLORS.GRAY_700 }); + }); + + it("offers an edit affordance for the related properties", async () => { + const onEditRelatedProperty = jest.fn(); + renderDrawer({ + related_properties: ["internal-dependency-documentation"], + onEditRelatedProperty, + }); + + await userEvent.click(screen.getByRole("button", { name: "Edit in DX" })); + + expect(onEditRelatedProperty).toHaveBeenCalledTimes(1); + }); + + it.each([ + ["null", null], + ["empty", []], + ])("omits the related property section when %s", (_label, related) => { + renderDrawer({ related_properties: related as string[] | null }); + + expect(screen.queryByText(/^Related propert/)).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Edit in DX" }) + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/components/CheckResultDrawer.tsx b/src/components/CheckResultDrawer.tsx index 4d0e8cf..6bcfdeb 100644 --- a/src/components/CheckResultDrawer.tsx +++ b/src/components/CheckResultDrawer.tsx @@ -99,7 +99,7 @@ export function CheckResultDrawer({ )} - {check.related_property && ( + {!!check.related_properties?.length && ( - - Related property:{" "} - - {check.related_property} - - + + + {check.related_properties.length === 1 + ? "Related property:" + : "Related properties:"} + + {check.related_properties.map((relatedProperty) => ( + + {relatedProperty} + + ))} +