From 889e2c0deb522cfc9859cc6dce5357752742e735 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Thu, 1 Jan 2026 10:56:33 -0600 Subject: [PATCH] Fix label immutability help text (#37748) - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually - [x] Confirmed that the fix is not expected to adversely impact load test results --------- Co-authored-by: RachelElysia <71795832+RachelElysia@users.noreply.github.com> --- .../components/LabelForm/LabelForm.tests.tsx | 73 +++++++++++++++++++ .../labels/components/LabelForm/LabelForm.tsx | 18 +++-- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/frontend/pages/labels/components/LabelForm/LabelForm.tests.tsx b/frontend/pages/labels/components/LabelForm/LabelForm.tests.tsx index 9bc0c2fcbb..63d03b352d 100644 --- a/frontend/pages/labels/components/LabelForm/LabelForm.tests.tsx +++ b/frontend/pages/labels/components/LabelForm/LabelForm.tests.tsx @@ -66,4 +66,77 @@ describe("LabelForm", () => { true ); }); + + it("should not render immutable help text when no immutable fields are provided (ManualLabelForm without team)", () => { + render( + + ); + + // Help text container should not be in the document + expect( + screen.queryByText( + /are immutable\. To make changes, delete this label and create a new one\./ + ) + ).not.toBeInTheDocument(); + }); + + it("should render correct immutable help text for a single field (ManualLabelForm with team)", () => { + render( + + ); + + expect( + screen.getByText( + "Label teams are immutable. To make changes, delete this label and create a new one." + ) + ).toBeInTheDocument(); + }); + + it("should render correct immutable help text for two fields (DynamicLabelForm without team)", () => { + const immutableFields = ["queries", "platforms"]; + + render( + + ); + + expect( + screen.getByText( + "Label queries and platforms are immutable. To make changes, delete this label and create a new one." + ) + ).toBeInTheDocument(); + + expect(immutableFields.length).toBe(2); + }); + + it("should render correct immutable help text for three fields (DynamicLabelForm with team)", () => { + render( + + ); + + expect( + screen.getByText( + "Label teams, queries, and platforms are immutable. To make changes, delete this label and create a new one." + ) + ).toBeInTheDocument(); + }); }); diff --git a/frontend/pages/labels/components/LabelForm/LabelForm.tsx b/frontend/pages/labels/components/LabelForm/LabelForm.tsx index 044c8c74b7..c0dd1bfcc2 100644 --- a/frontend/pages/labels/components/LabelForm/LabelForm.tsx +++ b/frontend/pages/labels/components/LabelForm/LabelForm.tsx @@ -33,11 +33,19 @@ const generateDescriptionHelpText = (immutableFields: string[]) => { const SUFFIX = "are immutable. To make changes, delete this label and create a new one."; - return immutableFields.length === 1 - ? `Label ${immutableFields[0]} ${SUFFIX}` - : `Label ${immutableFields - .slice(0, -1) - .join(", ")} and ${immutableFields.pop()} ${SUFFIX}`; + if (immutableFields.length === 1) { + return `Label ${immutableFields[0]} ${SUFFIX}`; + } + + if (immutableFields.length === 2) { + // No comma for two items: "queries and platforms" + return `Label ${immutableFields[0]} and ${immutableFields[1]} ${SUFFIX}`; + } + + // 3+ items: Oxford comma before "and" + const allButLast = immutableFields.slice(0, -1).join(", "); + const last = immutableFields.slice(-1); + return `Label ${allButLast}, and ${last} ${SUFFIX}`; }; const LabelForm = ({