- Notifications
You must be signed in to change notification settings - Fork906
feat: handle update build for dynamic params#18226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Adds support for dynamic parameters during workspace build updates by wiring a feature flag through the UI, hooks, and API layers.
- Introduce
isDynamicParametersEnabledflag in batch actions, update dialogs, and mutation calls - Provide an opt-out hook (
useDynamicParametersOptOut) and branch between classic/experimental parameter flows in multiple components - Extend API methods to fetch and apply “dynamic parameters” when the feature is enabled
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx | Wire dynamic-params feature flag into update workspace mutation and dialogs |
| site/src/hooks/useDynamicParametersOptOut.ts | New hook to read opt-out state from localStorage |
| site/src/api/api.ts | AddgetDynamicParameters and branch inupdateWorkspace/changeWorkspaceVersion for dynamic vs. rich parameters |
| site/src/api/queries/workspaces.ts | PassisDynamicParametersEnabled throughchangeVersion andupdateWorkspace query definitions |
| site/src/pages/** | Pass feature flag andtemplateVersionId through workspace pages and dialogs, refactor various routers |
| site/src/components/Dialog/Dialog.tsx | Expose additional exports and update spacing/typography in dialog primitives |
Comments suppressed due to low confidence (2)
site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx:60
- [nitpick] The boolean flag is named
dynamicParametersEnabledhere butisDynamicParametersEnabledelsewhere; consider standardizing on theis*prefix for consistency.
const dynamicParametersEnabled = experiments.includes("dynamic-parameters");site/src/hooks/useDynamicParametersOptOut.ts:1
- New feature logic in this hook isn’t covered by tests; consider adding unit tests for the opt-out default behavior and localStorage reads.
import { useQuery } from "react-query";Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
The updateWorkspace function now requires additional parameters:- buildParameters (array, defaults to [])- isDynamicParametersEnabled (boolean, defaults to false)Updated all test calls to include these parameters to fix test failures.
When the dynamic-parameters experiment is not enabled, the parameterdialogs were showing a loader indefinitely instead of the classicparameter dialog. This was causing e2e tests to fail because theycouldn't find the parameter fields.Fixed by:- Only showing loader when experiment is enabled AND optOut data is loading- Using classic dialog when experiment is disabled OR user opted out- This ensures e2e tests work correctly without the experiment flag
508fba8 intomainUh oh!
There was an error while loading.Please reload this page.
resolvescoder/preview#110