refactor(site): convert OrganizationAutocomplete to fully controlled component#24211
Conversation
…ent business The component was living a double life: fetching data, filtering permissions, auto-selecting defaults, AND rendering a dropdown. Now it just renders the dropdown. Callers bring the data, own the state, and handle the auto-select logic themselves. - Accept value/onChange/options props (fully controlled) - Remove internal selected state, useQuery, useEffect, check prop - Update CreateTemplateForm and CreateUserForm to own org fetching and permission filtering inline - Memoize orgOptions in callers for stable useEffect deps Closes coder/internal#1440
… callers Danielle would not approve of useEffect for derived state. Convert auto-select and invalid-value clearing to React's render-time adjustment pattern (track prevOrgOptions, set state during render). Also drop the unnecessary OnChangeFn type alias from stories.
The render-time adjustment was gated behind orgOptions !== prevOrgOptions which is always false on the first render (same reference from useMemo and useState). Separate auto-select (runs every render, guarded by length === 1 && null) from invalid-value clearing (runs on change). Adds assertion to StarterTemplatePermissionsCheck story to verify the org picker pre-selects the only authorized org.
There was a problem hiding this comment.
Pull request overview
Refactors the frontend org picker so OrganizationAutocomplete becomes a fully controlled, presentational component, with callers responsible for fetching organizations, permission filtering, and selection behavior. This supports upcoming org-scoped features by making org selection state explicit in parent forms.
Changes:
- Convert
OrganizationAutocompleteto acceptvalue,onChange, andoptions, removing internal state, data fetching, and permission filtering. - Update
CreateUserFormandCreateTemplateFormto fetch orgs, run authorization checks, filter options, and implement auto-select / invalid-selection clearing. - Update Storybook stories to match the new controlled API and add assertions around auto-selection behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/pages/CreateUserPage/CreateUserForm.tsx | Moves org fetching + permission filtering into the form and wires the new controlled OrganizationAutocomplete API. |
| site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx | Same refactor for template creation, plus permission-based org option filtering and auto-selection logic. |
| site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx | Adjusts play assertions to wait for controlled auto-selection behavior. |
| site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.tsx | Makes the component purely presentational/controlled (no queries, no internal selected state). |
| site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx | Rewrites stories to pass options/value and validate controlled behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Picker No point fetching orgs and running permission checks when the org picker isn't even rendered. Copilot caught us wasting API calls.
Code Review — 1 verified finding6 reviewers tested 24 hypotheses across correctness, design, and robustness. The controlled-component conversion itself is clean — no correctness or robustness issues found. RDT001 [P2 Improvement] — ~55 lines of org-selection logic duplicated across two formsFile: The org-fetching → permission-checking → option-filtering → render-time state adjustment → auto-select pipeline is near-identical between both forms. A diff of the two blocks shows exactly 4 semantic differences: (1) the Suggestion: The codebase already has query-composition functions for this exact pattern in A similar query-config function (e.g.
|
… callers The org-fetch + permission-check + filter pipeline was copy-pasted between CreateTemplateForm and CreateUserForm (55 lines each, differing in 4 spots). Extract it into a query-config function in organizations.ts following the existing organizationsPermissions() pattern. - Add permittedOrganizations(check) to api/queries/organizations.ts - Add unit tests for the query-config function - Replace duplicated code in both forms with a single useQuery call
|
🤖 Good catch from Danielle — the ~55 lines of duplicated org-fetch + permission-check + filter pipeline has been extracted into Each form now does: const permittedOrgsQuery = useQuery({
...permittedOrganizations({ object: { resource_type: "template" }, action: "create" }),
enabled: Boolean(showOrganizationPicker),
});
const orgOptions = permittedOrgsQuery.data ?? [];Unit tests added for the query-config function. The render-time auto-select/clear logic (~10 lines) stays in each caller since it depends on form-specific state. |
The `?? []` fallback creates a new array reference on every render when query data is undefined. The prevOrgOptions tracking interprets each new reference as a change, calls setPrevOrgOptions, which triggers another render — infinite loop. Fix: use a module-level const empty array so the reference is stable.
Stories were still mocking the old separate organizationsKey + authorization query keys. The form now uses the combined permittedOrganizations query, so the mocks need to match its key and provide the already-filtered org list.
Same query key mismatch as the CreateTemplateForm stories — the WithOrganizations story was mocking the old separate query keys. Updated to mock the combined permittedOrganizations key. Also renamed 'SingleOrg' back to 'OneOrg' to avoid unnecessary Chromatic diff churn. Un-exported organizationsKey since no external consumers remain.
…lete Now that #24211 landed, replace the temporary Radix Select with the real OrganizationAutocomplete component using its new controlled API (value, onChange, options).
Fixes coder/internal#1440
OrganizationAutocompleteto a purely presentational, fully controlled componentvalue,onChange,optionsfrom parent; remove internal state, data fetching, and permission filteringCreateTemplateFormandCreateUserFormto own org fetching, permission checks, auto-select, and invalid-value clearing inlineorgOptionsin callers for stableuseEffectdepsEnabling change for #23906 (which can now be rebased to a much smaller diff) and #23827 (org-scoped chats).