From c879fedca9498fef1f8d38d053e0876add58338a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 1 Apr 2026 13:10:04 +0000 Subject: [PATCH 1/6] feat(site): add organizationId prop to OrganizationAutocomplete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The component was fully uncontrolled — it managed its own selected state internally. Callers that pre-selected an org (like AgentCreateForm for single-org users) had no way to display that selection in the picker, resulting in a visual desync where the button shows 'Select an organization…' while the form already knows the answer. Adding organizationId as an optional controlled prop lets the parent set which org is visually selected. When provided, the component syncs from options on load. The existing auto-select-on-single-option behavior is preserved when the prop is omitted. --- .../OrganizationAutocomplete.tsx | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx index fd0ca267415..b4e9563bf54 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx @@ -26,6 +26,8 @@ type OrganizationAutocompleteProps = { id?: string; required?: boolean; check?: AuthorizationCheck; + /** When provided, controls which organization is displayed as selected. */ + organizationId?: string; }; export const OrganizationAutocomplete: FC = ({ @@ -33,6 +35,7 @@ export const OrganizationAutocomplete: FC = ({ id, required, check, + organizationId, }) => { const [open, setOpen] = useState(false); const [selected, setSelected] = useState(null); @@ -66,10 +69,25 @@ export const OrganizationAutocomplete: FC = ({ : []; } - // Unfortunate: this useEffect sets a default org value - // if only one is available and is necessary as the autocomplete loads - // its own data. Until we refactor, proceed cautiously! + // Sync internal selection state from the controlled `organizationId` prop + // when the options finish loading. This ensures the button shows + // the correct org name instead of the placeholder text. useEffect(() => { + if (organizationId === undefined || options.length === 0) { + return; + } + const match = options.find((o) => o.id === organizationId); + if (match && match.id !== selected?.id) { + setSelected(match); + } + }, [organizationId, options, selected?.id]); + + // Auto-select when only one option exists and no controlled organizationId + // was provided. This preserves the original single-org behavior. + useEffect(() => { + if (organizationId !== undefined) { + return; + } const org = options[0]; if (options.length !== 1 || org === selected) { return; @@ -77,7 +95,7 @@ export const OrganizationAutocomplete: FC = ({ setSelected(org); onChange(org); - }, [options, selected, onChange]); + }, [options, selected, onChange, organizationId]); return ( From e0c6e5a296516acc76cbb7dc18e4751976f2ace2 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 1 Apr 2026 15:18:13 +0000 Subject: [PATCH 2/6] fix(site): exorcise useEffect demon from OrganizationAutocomplete Derive the displayed selection during render instead of syncing via useEffect. This eliminates the selection-revert loop, the missing onChange notification, and the unnecessary extra render cycle. - Replace first useEffect with render-time derivation - Guard auto-select effect to only run in uncontrolled mode - Handle empty string organizationId as 'not provided' - Add three new Storybook stories for controlled pre-selection --- .../OrganizationAutocomplete.stories.tsx | 85 ++++++++++++++++++- .../OrganizationAutocomplete.tsx | 43 +++++----- 2 files changed, 106 insertions(+), 22 deletions(-) diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx index e809a7505bf..e6df4ddd963 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { action } from "storybook/actions"; -import { userEvent, within } from "storybook/test"; +import { expect, fn, userEvent, waitFor, within } from "storybook/test"; import { MockOrganization, MockOrganization2, @@ -53,3 +53,86 @@ export const OneOrg: Story = { ], }, }; + +export const PreselectedOrg: Story = { + args: { + organizationId: MockOrganization2.id, + onChange: fn<(value: unknown) => void>(), + }, + parameters: { + showOrganizations: true, + user: MockUserOwner, + features: ["multiple_organizations"], + permissions: { viewDeploymentConfig: true }, + queries: [ + { + key: ["organizations"], + data: [MockOrganization, MockOrganization2], + }, + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const button = canvas.getByRole("button"); + await waitFor(() => + expect(button).toHaveTextContent(MockOrganization2.display_name), + ); + const onChangeSpy = args.onChange as ReturnType< + typeof fn<(value: unknown) => void> + >; + expect(onChangeSpy).not.toHaveBeenCalled(); + }, +}; + +export const PreselectedOrgNotFound: Story = { + args: { + organizationId: "nonexistent-id", + }, + parameters: { + showOrganizations: true, + user: MockUserOwner, + features: ["multiple_organizations"], + permissions: { viewDeploymentConfig: true }, + queries: [ + { + key: ["organizations"], + data: [MockOrganization, MockOrganization2], + }, + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const button = canvas.getByRole("button"); + expect(button).toHaveTextContent("Select an organization"); + }, +}; + +export const OneOrgWithControlledId: Story = { + args: { + organizationId: MockOrganization.id, + onChange: fn<(value: unknown) => void>(), + }, + parameters: { + showOrganizations: true, + user: MockUserOwner, + features: ["multiple_organizations"], + permissions: { viewDeploymentConfig: true }, + queries: [ + { + key: ["organizations"], + data: [MockOrganization], + }, + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const button = canvas.getByRole("button"); + await waitFor(() => + expect(button).toHaveTextContent(MockOrganization.display_name), + ); + const onChangeSpy = args.onChange as ReturnType< + typeof fn<(value: unknown) => void> + >; + expect(onChangeSpy).not.toHaveBeenCalled(); + }, +}; diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx index b4e9563bf54..45a06f83c27 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx @@ -26,7 +26,12 @@ type OrganizationAutocompleteProps = { id?: string; required?: boolean; check?: AuthorizationCheck; - /** When provided, controls which organization is displayed as selected. */ + /** + * Pre-selects an organization by ID. When provided, the + * displayed selection is derived from this prop. The parent + * is responsible for updating this prop in response to user + * selections via onChange. + */ organizationId?: string; }; @@ -69,23 +74,17 @@ export const OrganizationAutocomplete: FC = ({ : []; } - // Sync internal selection state from the controlled `organizationId` prop - // when the options finish loading. This ensures the button shows - // the correct org name instead of the placeholder text. - useEffect(() => { - if (organizationId === undefined || options.length === 0) { - return; - } - const match = options.find((o) => o.id === organizationId); - if (match && match.id !== selected?.id) { - setSelected(match); - } - }, [organizationId, options, selected?.id]); + // In controlled mode, derive the displayed selection from the + // prop so we never need to sync prop → state via an effect. + const displayedSelection = organizationId + ? (options.find((o) => o.id === organizationId) ?? null) + : selected; - // Auto-select when only one option exists and no controlled organizationId - // was provided. This preserves the original single-org behavior. + // Auto-select when only one option exists. Only active in + // uncontrolled mode — when the parent controls the value via + // organizationId it is responsible for the initial selection. useEffect(() => { - if (organizationId !== undefined) { + if (organizationId) { return; } const org = options[0]; @@ -108,14 +107,16 @@ export const OrganizationAutocomplete: FC = ({ data-testid="organization-autocomplete" className="w-full justify-start gap-2 font-normal" > - {selected ? ( + {displayedSelection ? ( <> - {selected.display_name} + + {displayedSelection.display_name} + ) : ( @@ -152,7 +153,7 @@ export const OrganizationAutocomplete: FC = ({ {org.display_name || org.name} - {selected?.id === org.id && ( + {displayedSelection?.id === org.id && ( )} From 5933b94acf772a9a00a1191daf3542bebd5b5c41 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 1 Apr 2026 16:34:17 +0000 Subject: [PATCH 3/6] fix(site): address self-review findings for OrganizationAutocomplete - Guard setSelected in onSelect to prevent shadow state in controlled mode - Fix PreselectedOrgNotFound test to prove data loaded before asserting - Add PreselectedOrgUserSelects story for controlled-mode interaction - Extract OnChangeFn type alias to deduplicate spy casts - Document placeholder flash edge case when check + organizationId combine --- .../OrganizationAutocomplete.stories.tsx | 74 ++++++++++++++++--- .../OrganizationAutocomplete.tsx | 11 ++- 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx index e6df4ddd963..37d6a2aa7a6 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx @@ -1,6 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { action } from "storybook/actions"; -import { expect, fn, userEvent, waitFor, within } from "storybook/test"; +import { expect, fn, screen, userEvent, waitFor, within } from "storybook/test"; +import type { Organization } from "#/api/typesGenerated"; import { MockOrganization, MockOrganization2, @@ -8,6 +9,8 @@ import { } from "#/testHelpers/entities"; import { OrganizationAutocomplete } from "./OrganizationAutocomplete"; +type OnChangeFn = (org: Organization | null) => void; + const meta: Meta = { title: "components/OrganizationAutocomplete", component: OrganizationAutocomplete, @@ -57,7 +60,7 @@ export const OneOrg: Story = { export const PreselectedOrg: Story = { args: { organizationId: MockOrganization2.id, - onChange: fn<(value: unknown) => void>(), + onChange: fn(), }, parameters: { showOrganizations: true, @@ -77,9 +80,7 @@ export const PreselectedOrg: Story = { await waitFor(() => expect(button).toHaveTextContent(MockOrganization2.display_name), ); - const onChangeSpy = args.onChange as ReturnType< - typeof fn<(value: unknown) => void> - >; + const onChangeSpy = args.onChange as ReturnType>; expect(onChangeSpy).not.toHaveBeenCalled(); }, }; @@ -103,14 +104,69 @@ export const PreselectedOrgNotFound: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); const button = canvas.getByRole("button"); - expect(button).toHaveTextContent("Select an organization"); + // Open the dropdown to verify data has loaded. + await userEvent.click(button); + await waitFor(() => + expect( + screen.getByText(MockOrganization.display_name), + ).toBeInTheDocument(), + ); + // Close and verify the button still shows placeholder + // (the org ID doesn't match any loaded option). + await userEvent.keyboard("{Escape}"); + await waitFor(() => + expect(button).toHaveTextContent("Select an organization"), + ); + }, +}; + +export const PreselectedOrgUserSelects: Story = { + args: { + organizationId: MockOrganization2.id, + onChange: fn(), + }, + parameters: { + showOrganizations: true, + user: MockUserOwner, + features: ["multiple_organizations"], + permissions: { viewDeploymentConfig: true }, + queries: [ + { + key: ["organizations"], + data: [MockOrganization, MockOrganization2], + }, + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const button = canvas.getByRole("button"); + // Wait for the preselected org to appear. + await waitFor(() => + expect(button).toHaveTextContent(MockOrganization2.display_name), + ); + const onChangeSpy = args.onChange as ReturnType>; + onChangeSpy.mockClear(); + // Open dropdown and select a different org. + await userEvent.click(button); + await waitFor(() => + expect( + screen.getByText(MockOrganization.display_name), + ).toBeInTheDocument(), + ); + await userEvent.click(screen.getByText(MockOrganization.display_name)); + // Verify onChange was called with the new org. + await waitFor(() => + expect(onChangeSpy).toHaveBeenCalledWith( + expect.objectContaining({ id: MockOrganization.id }), + ), + ); }, }; export const OneOrgWithControlledId: Story = { args: { organizationId: MockOrganization.id, - onChange: fn<(value: unknown) => void>(), + onChange: fn(), }, parameters: { showOrganizations: true, @@ -130,9 +186,7 @@ export const OneOrgWithControlledId: Story = { await waitFor(() => expect(button).toHaveTextContent(MockOrganization.display_name), ); - const onChangeSpy = args.onChange as ReturnType< - typeof fn<(value: unknown) => void> - >; + const onChangeSpy = args.onChange as ReturnType>; expect(onChangeSpy).not.toHaveBeenCalled(); }, }; diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx index 45a06f83c27..022d6f47e9e 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx @@ -76,6 +76,10 @@ export const OrganizationAutocomplete: FC = ({ // In controlled mode, derive the displayed selection from the // prop so we never need to sync prop → state via an effect. + // Note: when `check` is also provided, options may be empty + // until permissions load, causing a brief placeholder flash. + // The follow-up refactor (passing the full Organization object + // instead of just an ID) will eliminate this. const displayedSelection = organizationId ? (options.find((o) => o.id === organizationId) ?? null) : selected; @@ -140,7 +144,12 @@ export const OrganizationAutocomplete: FC = ({ key={org.id} value={`${org.display_name} ${org.name}`} onSelect={() => { - setSelected(org); + // Only update internal state in uncontrolled mode. + // In controlled mode, displayedSelection is derived + // from the organizationId prop. + if (!organizationId) { + setSelected(org); + } onChange(org); setOpen(false); }} From 6f2ef598b2ae3e3c01ecc402c7eb02a733c3f56c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 1 Apr 2026 16:46:06 +0000 Subject: [PATCH 4/6] fix(site): add display-revert assertion and JSDoc warning for auth edge case - Assert button reverts to controlled value after user selection in PreselectedOrgUserSelects story - Warn in JSDoc that organizationId must reference an authorized org when combined with check prop --- .../OrganizationAutocomplete.stories.tsx | 5 +++++ .../OrganizationAutocomplete/OrganizationAutocomplete.tsx | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx index 37d6a2aa7a6..329a65d1c22 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx @@ -160,6 +160,11 @@ export const PreselectedOrgUserSelects: Story = { expect.objectContaining({ id: MockOrganization.id }), ), ); + // Button should still show the prop-controlled value since + // the parent hasn't updated organizationId. + await waitFor(() => + expect(button).toHaveTextContent(MockOrganization2.display_name), + ); }, }; diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx index 022d6f47e9e..d6bc734fbce 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx @@ -31,6 +31,12 @@ type OrganizationAutocompleteProps = { * displayed selection is derived from this prop. The parent * is responsible for updating this prop in response to user * selections via onChange. + * + * When combined with `check`, the ID must reference an org + * the user is authorized for — if the org fails the check, + * the button silently shows placeholder text without firing + * onChange(null). The follow-up full-object refactor will + * address this. */ organizationId?: string; }; From 778b9bfb846ad2dbb59836719f8f52a0f787aecd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 1 Apr 2026 16:47:38 +0000 Subject: [PATCH 5/6] fix(site): link follow-up issue in organizationId JSDoc --- .../OrganizationAutocomplete/OrganizationAutocomplete.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx index d6bc734fbce..2f56e7fffb6 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx @@ -35,7 +35,8 @@ type OrganizationAutocompleteProps = { * When combined with `check`, the ID must reference an org * the user is authorized for — if the org fails the check, * the button silently shows placeholder text without firing - * onChange(null). The follow-up full-object refactor will + * onChange(null). The follow-up full-object refactor + * (https://github.com/coder/internal/issues/1440) will * address this. */ organizationId?: string; From 03191e0d272975ce00f57c8cddd40b8d3a124cdf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 1 Apr 2026 16:57:06 +0000 Subject: [PATCH 6/6] fix(site): use ID comparison in auto-select to survive react-query refetches Reference equality (org === selected) breaks on background refetch since react-query returns new object instances. Compare by ID instead to prevent spurious onChange calls for single-org users. --- .../OrganizationAutocomplete/OrganizationAutocomplete.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx index 2f56e7fffb6..2b023d050d3 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx @@ -99,7 +99,7 @@ export const OrganizationAutocomplete: FC = ({ return; } const org = options[0]; - if (options.length !== 1 || org === selected) { + if (options.length !== 1 || org.id === selected?.id) { return; }