feat: make dynamic parameters opt-in by default for new templates #19006
Conversation
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe changes update the default value of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateWorkspacePage
participant TemplatePermissionsQuery
participant CreateWorkspacePageView
participant ClassicParameterFlowDeprecationWarning
User->>CreateWorkspacePage: Load page
CreateWorkspacePage->>TemplatePermissionsQuery: Fetch canUpdateTemplate
TemplatePermissionsQuery-->>CreateWorkspacePage: Return permission data
CreateWorkspacePage->>CreateWorkspacePageView: Render with templatePermissions
CreateWorkspacePageView->>ClassicParameterFlowDeprecationWarning: Render warning if enabled and canUpdateTemplate
sequenceDiagram
participant User
participant WorkspaceParametersPage
participant TemplatePermissionsQuery
participant WorkspaceParametersPageView
participant WorkspaceParametersForm
participant ClassicParameterFlowDeprecationWarning
User->>WorkspaceParametersPage: Load page
WorkspaceParametersPage->>TemplatePermissionsQuery: Fetch canUpdateTemplate
TemplatePermissionsQuery-->>WorkspaceParametersPage: Return permission data
WorkspaceParametersPage->>WorkspaceParametersPageView: Render with templatePermissions
WorkspaceParametersPageView->>WorkspaceParametersForm: Pass templatePermissions
WorkspaceParametersForm->>ClassicParameterFlowDeprecationWarning: Render warning if enabled and canUpdateTemplate
Estimated code review effort3 (~45 minutes) ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx (1)
545-548: Same duplication as aboveRefer to the previous comment – same literal, same recommendation.
🧹 Nitpick comments (6)
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx (1)
213-214: Centralise docs URL to avoid future driftThe same
/admin/templates/extending-templates/dynamic-parametersslug is now hard-coded here and in several other files. Expose it via a shared constant (e.g.DOCS_DYNAMIC_PARAMETERS) or extend thedocs()helper to accept a well-named key.
This prevents silent divergence if the path changes again.site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx (1)
396-399: Duplicate literal docs path – extract to constantTwo identical string literals were introduced in this component (see also the Parameters section below). Moving the slug to a shared constant keeps the UI and docs in sync and avoids scattered hot-fixes next time the URL changes.
coderd/insights_test.go (1)
668-672: Consider adding a clarifying comment for consistency.For better maintainability and consistency with the similar change at line 1564, consider adding a comment explaining why
UseClassicParameterFlow: trueis explicitly set here.createdTemplate := dbgen.Template(t, db, database.Template{ ID: template.id, ActiveVersionID: version.ID, OrganizationID: firstUser.OrganizationID, CreatedBy: firstUser.UserID, - UseClassicParameterFlow: true, + UseClassicParameterFlow: true, // Required for testing legacy parameter flow behavior GroupACL: database.TemplateACL{site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx (1)
1-1: Note: MUI components present - consider migration.This file uses deprecated MUI components (Checkbox, FormControlLabel, TextField, etc.). While not introduced by this PR, consider migrating these to newer component alternatives in future work.
site/src/testHelpers/entities.ts (1)
829-829: Avoid hard-coded duplicate default – reuse a shared constantThe literal
falseis now duplicated across mocks. Extracting aDEFAULT_USE_CLASSIC_PARAMETER_FLOWconstant (or deriving the value from the template when constructing the workspace) will keep test helpers in sync the next time this default changes.- use_classic_parameter_flow: false, + use_classic_parameter_flow: DEFAULT_USE_CLASSIC_PARAMETER_FLOW,site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.tsx (1)
28-30: Improve the deprecation message clarity.The phrase "dynamic improved functionality" is awkward and unclear. Consider rephrasing for better user experience.
This template is using the classic parameter flow, which will be{" "} <strong>deprecated</strong> in a future release. Please migrate to -dynamic improved functionality. +the improved dynamic parameter flow.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
coderd/database/dbgen/dbgen.go(1 hunks)coderd/insights_test.go(2 hunks)coderd/templates.go(1 hunks)coderd/templates_test.go(2 hunks)site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.test.tsx(1 hunks)site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.tsx(1 hunks)site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx(3 hunks)site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx(1 hunks)site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx(4 hunks)site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx(2 hunks)site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx(1 hunks)site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx(4 hunks)site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx(5 hunks)site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx(2 hunks)site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx(1 hunks)site/src/testHelpers/entities.ts(2 hunks)
🧠 Learnings (9)
site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx (3)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Leverage custom color tokens: content-primary, surface-secondary, etc.
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx (2)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.test.tsx (4)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to site/src/**/*.test.{ts,tsx,js,jsx} : React components and pages are organized in the site/src/ directory, with Jest used for testing.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx (9)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use shadcn/ui components first - check site/src/components for existing implementations
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never pass Hooks around as values or mutate them dynamically. Keep Hook usage static and local to each component.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Don’t call Hooks (including useRef) inside loops, conditions, or map(). Extract a child component instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Only call Hooks from React functions. Regular JS functions, classes, event handlers, useMemo, etc. are off-limits.
site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx (1)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.tsx (6)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Replace Emotion css prop with Tailwind classes
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never pass Hooks around as values or mutate them dynamically. Keep Hook usage static and local to each component.
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx (4)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use shadcn/ui components first - check site/src/components for existing implementations
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx (1)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
coderd/templates_test.go (2)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
🧰 Additional context used
🧠 Learnings (9)
site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx (3)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Leverage custom color tokens: content-primary, surface-secondary, etc.
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx (2)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.test.tsx (4)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to site/src/**/*.test.{ts,tsx,js,jsx} : React components and pages are organized in the site/src/ directory, with Jest used for testing.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx (9)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use shadcn/ui components first - check site/src/components for existing implementations
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never pass Hooks around as values or mutate them dynamically. Keep Hook usage static and local to each component.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Don’t call Hooks (including useRef) inside loops, conditions, or map(). Extract a child component instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Only call Hooks from React functions. Regular JS functions, classes, event handlers, useMemo, etc. are off-limits.
site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx (1)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.tsx (6)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Replace Emotion css prop with Tailwind classes
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never pass Hooks around as values or mutate them dynamically. Keep Hook usage static and local to each component.
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx (4)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : MUI components are deprecated - migrate away from these when encountered
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Emotion CSS is deprecated. Use Tailwind CSS instead.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Components and custom Hooks must be pure and idempotent—same inputs → same output; move side-effects to event handlers or Effects.
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use shadcn/ui components first - check site/src/components for existing implementations
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx (1)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.{ts,tsx} : Never mutate props, state, or values returned by Hooks. Always create new objects or use the setter from useState.
coderd/templates_test.go (2)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test-e2e
- GitHub Check: test-go-race-pg
- GitHub Check: build-dylib
- GitHub Check: test-go-pg-17
- GitHub Check: test-go-pg (windows-2022)
- GitHub Check: test-go-pg (macos-latest)
- GitHub Check: test-go-pg (ubuntu-latest)
- GitHub Check: chromatic
- GitHub Check: gen
🔇 Additional comments (33)
coderd/database/dbgen/dbgen.go (1)
150-151: Auditdbgen.Templatecalls for missingUseClassicParameterFlow
The previous script misparsed file paths (it included the code line in the filename). Please use the following to correctly extractfile:lineand then scan the next ~20 lines forUseClassicParameterFlow:#!/usr/bin/env bash # Find dbgen.Template invocations without UseClassicParameterFlow configured. rg --type go -n 'dbgen\.Template' | while IFS=":" read -r file line _; do if ! sed -n "${line},$((line+20))p" "$file" | grep -q 'UseClassicParameterFlow'; then echo "$file:$line" fi done | headRerun this and audit any test that now depends on the flipped default.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx (1)
33-34: Storybook arg looks goodAdding
templatePermissionskeeps the stories compiling with the new prop signature – no issues spotted.coderd/insights_test.go (1)
1560-1564: Good practice with explanatory comment.Excellent approach to explicitly set
UseClassicParameterFlow: truewith a clear comment explaining why this override is necessary for the test's parameter usage tracking functionality. This ensures the test remains functional despite the default value change.coderd/templates_test.go (2)
80-80: LGTM: Test assertion correctly updated for new default.The test assertion has been properly updated to expect
falseforUseClassicParameterFlow, which aligns with the PR objective of making dynamic parameters the default for new templates.
1554-1554: LGTM: Default expectation correctly updated.The assertion correctly expects
falseas the default value forUseClassicParameterFlow, confirming that dynamic parameters are now enabled by default for new templates. The comment clearly documents this behavior change.site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx (2)
229-229: LGTM: Documentation link updated to canonical path.The documentation link has been cleanly updated from a fragment-based URL to a dedicated dynamic parameters path, improving maintainability and user experience.
264-266: LGTM: Consistent documentation link update.The EmptyState documentation link has been updated to match the same canonical dynamic parameters path, maintaining consistency across the component.
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx (3)
248-248: LGTM: UI label appropriately updated to reflect recommended option.The addition of "(recommended)" to the dynamic parameters label clearly communicates the preferred choice to users, aligning with the PR's objective of making dynamic parameters the default.
252-256: LGTM: Helper text updated to reflect new default behavior.The updated helper text effectively communicates:
- Dynamic parameters as the default for new templates
- Additional capabilities (form types, conditional parameters)
- Future deprecation of classic flow
This provides clear guidance to users about the direction of the platform.
261-261: LGTM: Documentation link updated to canonical path.The documentation link has been updated to the cleaner, dedicated dynamic parameters path, improving consistency across the application.
site/src/testHelpers/entities.ts (1)
1392-1392: LGTM – flag value updated to reflect new default
The workspace mock correctly mirrors the template’s default (false). ✅site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx (4)
30-30: LGTM: Clean import of the new deprecation warning component.The import is properly structured and follows the project's naming conventions.
70-70: LGTM: Proper TypeScript interface extension.The new
templatePermissionsprop is correctly typed with the expected structure.
96-96: LGTM: Consistent prop destructuring.The new prop is properly destructured in the component signature.
251-255: LGTM: Proper component integration with correct props.The deprecation warning component is well-positioned between the header and main form, and receives the appropriate props from the template and permissions data.
site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx (4)
72-85: LGTM: Well-structured authorization query for template permissions.The query follows the established pattern for permission checks and correctly uses the template ID as the resource identifier. The enablement condition ensures the query only runs when template data is available.
107-107: LGTM: Proper integration of new query into loading state.The loading state correctly includes the new permissions query alongside existing queries.
110-113: LGTM: Consistent error handling pattern.The error handling follows the existing pattern of using nullish coalescing to check multiple query errors.
229-231: LGTM: Type-safe prop passing with proper assertion.The template permissions data is correctly passed to the view component with appropriate type assertion, following the same pattern as other permission data.
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.test.tsx (3)
4-7: LGTM: Appropriate mocking of navigation dependencies.The navigation modules are properly mocked with consistent return values for testing.
17-27: Good test coverage for enabled state.The test properly verifies that the warning text and action link are rendered when the component is enabled.
29-38: LGTM: Proper test for disabled state.The test correctly verifies that nothing is rendered when the component is disabled.
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.tsx (4)
6-10: LGTM: Well-defined TypeScript interface.The props interface is clear and uses appropriate types for the component's requirements.
15-19: LGTM: Proper conditional rendering pattern.The early return for disabled state is clean and follows React best practices.
21-23: LGTM: Clean link generation logic.The template settings URL is constructed properly using the navigation utilities.
26-35: LGTM: Proper use of modern styling and component structure.The component correctly uses Tailwind CSS classes (following the learning about avoiding Emotion) and maintains a clean Alert + Link structure.
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx (4)
17-17: LGTM: Consistent import of deprecation warning component.The import follows the same pattern used in other files.
37-37: LGTM: Proper optional prop typing.The templatePermissions prop is correctly typed as optional with undefined, allowing for graceful handling when permissions aren't available.
51-51: LGTM: Consistent prop destructuring.The templatePermissions prop is properly added to the destructuring pattern.
87-91: LGTM: Well-positioned deprecation warning with proper fallback.The warning component is appropriately placed after the workspace disabled alert and uses nullish coalescing to provide a safe fallback when templatePermissions is undefined.
Also applies to: 92-96
site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx (3)
54-67: LGTM! Well-structured permission query implementation.The template permissions query follows the established pattern used for workspace permissions and correctly implements conditional enabling. The authorization check structure is appropriate for template update permissions.
69-71: LGTM! Consistent type casting pattern.The type assertion follows the same pattern used for the existing
permissionsvariable and correctly handles the potential undefined state.
82-82: LGTM! Proper prop threading implementation.The
templatePermissionsprop is correctly added to the type definitions, component signatures, and passed through the component hierarchy without any mutations. This follows React best practices and maintains type safety throughout.Also applies to: 117-117, 130-130, 154-154
| // Default is false as dynamic parameters are now the preferred approach. | ||
| useClassicParameterFlow := ptr.NilToDefault(createTemplate.UseClassicParameterFlow, false) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Breaking-default flip – double-check upstream callers
Switching the default from true → false silently changes behaviour for every client that omits use_classic_parameter_flow when calling the POST /organizations/:org/templates endpoint.
Please verify that:
- All first-party callers (UI, CLI, Terraform provider, integration tests, seed data, etc.) now explicitly set the flag to
truewhen the classic flow is expected. - Public API docs are updated so external consumers are aware of the new default.
- No inserts rely on the column’s DB-level default anymore (we pass the value explicitly here, but raw SQL elsewhere might not).
Quick scan helper:
🏁 Script executed:
#!/bin/bash
# Find CreateTemplateRequest literals that do NOT specify UseClassicParameterFlow
rg --type-go -n --pretty $'CreateTemplateRequest\\s*{[^}]*}' | \
grep -v 'UseClassicParameterFlow'Length of output: 532
🏁 Script executed:
#!/usr/bin/env bash
# Scan all .go files (excluding vendor/testdata) for CreateTemplateRequest literals missing the UseClassicParameterFlow field
python3 - << 'EOF'
import re, glob, os
for path in glob.glob('**/*.go', recursive=True):
if any(excl in path for excl in ('vendor/', 'testdata/')):
continue
with open(path, 'r', encoding='utf-8') as f:
src = f.read()
for m in re.finditer(r'CreateTemplateRequest\s*\{', src):
# Walk the braces to extract the full literal block
idx = m.end()
depth = 1
while depth > 0 and idx < len(src):
if src[idx] == '{':
depth += 1
elif src[idx] == '}':
depth -= 1
idx += 1
block = src[m.start():idx]
if 'UseClassicParameterFlow' not in block:
lineno = src.count('\n', 0, m.start()) + 1
print(f"{path}:{lineno}")
EOFLength of output: 542
Explicitly set UseClassicParameterFlow on all CreateTemplateRequest call sites
We flipped the default from true → false, so any caller that omits UseClassicParameterFlow will now use the new dynamic‐parameter flow. The following literals need to be updated to include UseClassicParameterFlow: ptr.Bool(true) where the classic flow is expected:
• cli/templatecreate.go:152
• cli/templatepush.go:183
• coderd/templates_test.go:99, 116, 132, 150, 183, 219, 242, 262, 312, 362, 393, 419, 435
• coderd/coderdtest/coderdtest.go:969
• codersdk/toolsdk/toolsdk.go:1284
• enterprise/coderd/users_test.go:282
Additionally:
- Update the public API docs to reflect the new default (
false) foruse_classic_parameter_flow. - Audit any raw‐SQL inserts/migrations to ensure they no longer rely on the old DB default.
🤖 Prompt for AI Agents
In coderd/templates.go around lines 200 to 202, the default for
UseClassicParameterFlow was changed from true to false, affecting all
CreateTemplateRequest call sites. Update all listed call sites in
cli/templatecreate.go, cli/templatepush.go, coderd/templates_test.go,
coderd/coderdtest/coderdtest.go, codersdk/toolsdk/toolsdk.go, and
enterprise/coderd/users_test.go to explicitly set UseClassicParameterFlow:
ptr.Bool(true) where the classic flow is required. Also, update the public API
documentation to show the new default as false and audit any raw-SQL inserts or
migrations to ensure they do not depend on the old default value.
| const defaultProps = { | ||
| enabled: true, | ||
| organizationName: "test-org", | ||
| templateName: "test-template", | ||
| canUpdateTemplate: true, | ||
| }; |
There was a problem hiding this comment.
Fix prop naming inconsistency in test setup.
The default props define enabled: true but the test uses isEnabled={true}. This creates confusion about the actual prop interface.
const defaultProps = {
- enabled: true,
organizationName: "test-org",
templateName: "test-template",
- canUpdateTemplate: true,
+ isEnabled: true,
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
site/src/modules/workspaces/ClassicParameterFlowDeprecationWarning/ClassicParameterFlowDeprecationWarning.test.tsx
between lines 10 and 15, the defaultProps object uses the prop name 'enabled'
while the test uses 'isEnabled'. To fix this inconsistency, rename the prop in
defaultProps from 'enabled' to 'isEnabled' so it matches the prop used in the
test, ensuring clarity and consistency in the prop interface.
No description provided.