Skip to content

refactor: env vars section for project overview#3946

Merged
chronark merged 7 commits intomainfrom
refactor-env-vars
Sep 12, 2025
Merged

refactor: env vars section for project overview#3946
chronark merged 7 commits intomainfrom
refactor-env-vars

Conversation

@ogzhanolguncu
Copy link
Contributor

@ogzhanolguncu ogzhanolguncu commented Sep 11, 2025

What does this PR do?

This PR refactors environment variable section in project's overview for deploy. This won't be necessary for demo(we can keep that fake version to tell our users we'll support env/secrets when we are GA), but we needed to clean up that section for later iterations. We can easily replace the vault encryption/decryption easily when required coz now we support decrypting and editing properly.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • New Features

    • Form-driven add/edit for env vars with validation (including duplicate-key checks), Enter to save and Esc to cancel.
    • Secret support: masked/decrypted display with toggle, loading states, and clear Save/Cancel actions.
    • UI improvements: per-item animations, refined layout, empty state, and env count in header.
  • Refactor

    • Unified, manager-based env var flow with centralized data access.
    • Environment options simplified to production and preview only.

@changeset-bot
Copy link

changeset-bot bot commented Sep 11, 2025

⚠️ No Changeset found

Latest commit: 28140eb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Sep 11, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
dashboard Ready Ready Preview Comment Sep 11, 2025 1:12pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
engineering Ignored Ignored Preview Sep 11, 2025 1:12pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

📝 Walkthrough

Walkthrough

Refactors the env-variables UI to a form-driven system: adds EnvVarForm and related components, introduces a useEnvVarsManager hook, replaces inline editing/add flows and a legacy hook, adds Zod schemas/types, and updates TRPC env list schema (type field, removed development).

Changes

Cohort / File(s) Summary
Form components
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx, .../env-var-inputs.tsx, .../env-var-save-actions.tsx, .../env-var-secret-switch.tsx
New reusable form system: EnvVarForm (react-hook-form + zod), EnvVarInputs (key/value with secret-aware masking), EnvVarSaveActions (Save/Cancel buttons), and EnvVarSecretSwitch (secret toggle). Save/delete mutate logic is scaffolding/mocked and TRPC utils invalidation is used.
Add / Row components
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx, .../env-var-row.tsx
Replace inline value-based props with form-driven APIs. Signatures now accept projectId and getExistingEnvVar. Duplicate-key validation, secret/decrypt scaffolding, mock save/delete with trpc invalidation, and edit flow now uses EnvVarForm.
Section container
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx
Replaces old useEnvVars usage with useEnvVarsManager, simplifies add/edit state (isAddingNew), introduces animation/layout configs, adds EmptyState/ConcaveSeparator helpers, and maps envVars → EnvVarRow/AddEnvVarRow.
Hooks
.../hooks/use-env-var-manager.tsx, removed .../hooks/use-env-var.tsx
Added useEnvVarsManager to read envs via trpc and provide getExistingEnvVar helper. Removed legacy useEnvVars hook that managed local edit/add/delete state.
Types & validation
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts
Adds Zod schemas and TS types: EnvVar, EnvVarFormSchema/EnvVarFormData, EnvVarType ("env"
TRPC envs list
apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts
Replaced local schema with imported envVarSchema, changed returned data shape from isSecret to `type: "env"

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Section as Env Vars Section
  participant AddRow as AddEnvVarRow
  participant Form as EnvVarForm
  participant TRPC as trpc.utils

  User->>Section: Click "Add variable"
  Section->>AddRow: render add row
  AddRow->>Form: mount with defaults
  User->>Form: fill key/value, toggle Secret
  Form->>Form: validate (zod + getExistingEnvVar)
  User->>Form: submit
  alt valid
    Form->>TRPC: invalidate getEnvs(projectId)
    TRPC-->>Form: invalidated
    Form->>AddRow: onSuccess → onCancel
    AddRow-->>Section: close add row
  else invalid
    Form-->>User: show errors
  end
Loading
sequenceDiagram
  autonumber
  actor User
  participant Row as EnvVarRow
  participant Form as EnvVarForm
  participant TRPC as trpc.utils

  User->>Row: Click "Edit"
  Row->>Form: open with initialData (excludeId for uniqueness)
  User->>Form: edit fields
  Form->>Form: validate (zod + excludeId)
  User->>Form: save
  Form->>TRPC: invalidate getEnvs(projectId)
  TRPC-->>Form: invalidated
  Form-->>Row: onSuccess (close)

  User->>Row: Click "Show Secret"
  Row->>Row: mock decrypt (loading)
  Row-->>User: show decryptedValue

  User->>Row: Click "Delete"
  Row->>TRPC: invalidate getEnvs(projectId) (mock delete)
  TRPC-->>Row: invalidated
  Row-->>User: removed from list
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

UI

Suggested reviewers

  • perkinsjr
  • mcstepp
  • MichaelUnkey
  • chronark

Pre-merge checks (1 passed, 2 warnings)

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description includes a high-level summary, a selected "Type of change", and a filled checklist, but it is missing required template items: there is no "Fixes # (issue)" line and the "How should this be tested?" section with concrete test steps is absent or not present in the provided description. Because the repository template explicitly requires an issue reference and a testing section, the description is incomplete relative to the required template. Please add a "Fixes #" line or create/link the corresponding issue, and include a filled "How should this be tested?" section with step‑by‑step test instructions, environment/setup details, and expected outcomes; also ensure the checklist entries reflect those test steps and attach screenshots or recording if the UI changed. Once those required sections are present and sufficiently detailed, re-run the check.
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "refactor: env vars section for project overview" is concise, focuses on the main change (a refactor of the environment variables section), and contains no noisy file lists or emojis; it conveys the primary intent clearly for a teammate scanning history.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-env-vars

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ogzhanolguncu ogzhanolguncu marked this pull request as ready for review September 11, 2025 11:37
@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2025

Thank you for following the naming conventions for pull request titles! 🙏

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 27

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts (1)

68-80: Security posture for secrets in API responses (even mocks).

Consider masking secret values in VARIABLES (e.g., value: null, masked: "••••") to avoid normalizing plaintext secrets in responses.

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (2)

67-77: Add accessible labels to the add button.

Apply:

-          <Button
+          <Button
             size="icon"
             variant="ghost"
             onClick={startAdding}
             className={cn(
               "size-7 text-gray-9 hover:text-gray-11",
               showPlusButton ? "visible" : "invisible",
             )}
+            aria-label="Add environment variable"
           >

78-85: Add accessible labels to the expand/collapse control.

Apply:

-          <Button size="icon" variant="ghost" onClick={toggleExpanded}>
+          <Button
+            size="icon"
+            variant="ghost"
+            onClick={toggleExpanded}
+            aria-label={isExpanded ? "Collapse section" : "Expand section"}
+          >
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (1)

120-135: Add accessible labels to the secret toggle.

Apply:

-          {envVar.type === "secret" && (
+          {envVar.type === "secret" && (
             <Button
               size="icon"
               variant="outline"
               onClick={handleToggleSecret}
               disabled={isSecretLoading}
               className="size-7 text-gray-9 hover:text-gray-11 shrink-0"
               loading={isSecretLoading}
+              aria-label={isDecrypted ? "Hide secret value" : "Reveal secret value"}
             >
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db5041b and 36ac28c.

📒 Files selected for processing (11)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-secret-switch.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (2 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var.tsx (0 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (3 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-04T17:27:09.821Z
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx
📚 Learning: 2025-08-31T19:30:38.171Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3853
File: apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts:75-87
Timestamp: 2025-08-31T19:30:38.171Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts, the getEnvs procedure currently uses mock data (VARIABLES) and ignores the projectId input parameter. This is intentional temporary behavior - the user ogzhanolguncu indicated they plan to hook this up to the database later.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx
  • apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts
📚 Learning: 2025-06-02T11:09:58.791Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/trpc/routers/key/create.ts:11-14
Timestamp: 2025-06-02T11:09:58.791Z
Learning: In the unkey codebase, TypeScript and the env() function implementation already provide sufficient validation for environment variables, so additional runtime error handling for missing env vars is not needed.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx
  • apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts
🧬 Code graph analysis (8)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • EnvVarFormData (20-20)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (4)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • Environment (23-23)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1)
  • useEnvVarsManager (9-27)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (1)
  • EnvVarRow (15-153)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (1)
  • AddEnvVarRow (15-108)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (5)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (3)
  • EnvVarFormData (20-20)
  • EnvVar (13-13)
  • EnvVarFormSchema (15-19)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
  • EnvVarInputs (16-46)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-secret-switch.tsx (1)
  • EnvVarSecretSwitch (3-24)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1)
  • EnvVarSaveActions (3-41)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (3)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • Environment (23-23)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var.tsx (1)
  • useEnvVars (10-91)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (5)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (3)
  • EnvVar (13-13)
  • EnvVarFormData (20-20)
  • EnvVarFormSchema (15-19)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
  • EnvVarInputs (16-46)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-secret-switch.tsx (1)
  • EnvVarSecretSwitch (3-24)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1)
  • EnvVarSaveActions (3-41)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts (1)
  • EnvVar (11-11)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-secret-switch.tsx (1)
apps/dashboard/components/ui/switch.tsx (1)
  • Switch (33-33)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (2)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • EnvVar (13-13)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (1)
  • EnvVarForm (21-123)
🪛 Gitleaks (8.27.2)
apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts

[high] 24-24: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.

(stripe-access-token)


[high] 56-56: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.

(stripe-access-token)

⏰ 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). (3)
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
🔇 Additional comments (3)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (1)

44-57: Duplicate-key policy clarity.

Current check is trim + case-sensitive compare. Confirm whether “FOO” and “foo” should be allowed in the same environment.

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1)

12-16: Prime envs via useQuery before using cache-only getData

trpcUtils.deploy.project.envs.getEnvs.getData({ projectId }) reads only from the TRPC cache; if the query hasn't been fetched yet envVars will be empty and duplicate-key checks can be wrong. Call deploy.project.envs.getEnvs.useQuery({ projectId }) upstream or in this hook (gated by projectId) to ensure data is loaded before running client-side dedupe validation.

apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts (1)

68-80: Intentional mock — safe to remove "development" from the output schema.

Repo-wide search for dot/bracket access to environmentVariables.development returned no matches; only '.development' occurrences are unrelated (.gitignore, wrangler.toml, Vercel selectedApis) and environmentVariables is defined/used only in the mocked list.ts and DB/proto types.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (1)

77-85: A11y: connect toggle to region and announce state.

Add aria-expanded and aria-controls to the chevron button; give the panel an id.

-          <Button size="icon" variant="ghost" onClick={toggleExpanded}>
+          <Button
+            size="icon"
+            variant="ghost"
+            onClick={toggleExpanded}
+            aria-expanded={isExpanded}
+            aria-controls={`envvars-${environment}`}
+          >
             <ChevronDown
               className={cn(
                 "text-grayA-9 !size-3 transition-transform duration-200",
                 isExpanded && "rotate-180",
               )}
             />
           </Button>
@@
-      <div
+      <div
+        id={`envvars-${environment}`}
         className={cn(
           "bg-gray-1 relative overflow-hidden transition-all duration-300 ease-in",
           isExpanded ? `${LAYOUT_CONFIG.maxContentHeight} opacity-100` : "h-0 opacity-0 py-0",
         )}
       >

Also applies to: 91-96

♻️ Duplicate comments (18)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1)

28-36: A11y: mirror disabled state with aria-disabled on buttons.

Add aria-disabled so AT reflects disabled state consistently (same pattern used elsewhere in @unkey/ui).

       <Button
         type="submit"
         variant="outline"
         className="text-xs"
         disabled={save.disabled}
+        aria-disabled={save.disabled}
         loading={isSubmitting}
       >
         Save
       </Button>
       <Button
         type="button"
         variant="outline"
-        disabled={cancel.disabled}
+        disabled={cancel.disabled}
+        aria-disabled={cancel.disabled}
         onClick={cancel.onClick}
         className="text-xs"
       >
         Cancel
       </Button>

Also applies to: 19-27

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)

16-54: Inputs look solid; previous a11y/UX nits addressed.

Autocomplete/spellcheck/capitalize and invalid states are wired correctly. LGTM.

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (3)

113-121: Save onClick removed from actions prop.

This addresses the earlier double-submit risk. ✅


83-89: Remove Enter handler to prevent double submit.

Enter already submits the form; calling handleSubmit here can trigger duplicates.

-  const handleKeyDown = (e: React.KeyboardEvent) => {
-    if (e.key === "Enter" && isValid && !isSubmitting) {
-      handleSubmit(handleSave)();
-    } else if (e.key === "Escape") {
-      onCancel();
-    }
-  };
+  const handleKeyDown = (e: React.KeyboardEvent) => {
+    if (e.key === "Escape") onCancel();
+  };

34-37: Follow-up: wire real mutation and surface server errors.

Replace TODO with the upsert mutation, map server errors to fields via setError, and toast non-field errors.

Do you want me to scaffold the mutation + error mapping?

Also applies to: 61-81

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1)

15-17: Duplicate check trims both sides.

Good fix; avoids whitespace false-negatives.

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (2)

23-25: Dead config removed.

maxScrollHeight is gone; config is lean now. ✅


135-149: Reduced-motion respected; stagger logic is solid.

Nice addition; meets a11y expectations.

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (3)

89-96: Nice: RHF validation is propagated on toggle.

setValue correctly marks field dirty and validates.


98-107: Good: no Save onClick to avoid duplicate submit.

Relies on form submit only; prevents double-submit.


69-75: Prevent double-submit: handle only Escape in keydown.

Intercepts Enter while the form already has onSubmit, causing duplicate submits.

Apply:

-  const handleKeyDown = (e: React.KeyboardEvent) => {
-    if (e.key === "Enter" && isValid && !isSubmitting) {
-      handleSubmit(handleSave)();
-    } else if (e.key === "Escape") {
-      onCancel();
-    }
-  };
+  const handleKeyDown = (e: React.KeyboardEvent) => {
+    if (e.key === "Escape") onCancel();
+  };
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (3)

14-22: Good guard: decryptedValue only for secrets.

The superRefine correctly enforces decryptedValue presence only on secret types.


26-35: Split create vs update schemas to allow editing secrets without value.

Current schema requires value for all cases; editing an undecrypted secret sets value="" and can’t save.

Apply:

-export const EnvVarFormSchema = z.object({
-  key: z
-    .string()
-    .trim()
-    .min(1, "Variable name is required")
-    .regex(/^[A-Za-z][A-Za-z0-9_]*$/, "Use letters, numbers, and underscores; start with a letter"),
-  value: z.string().min(1, "Variable value is required"),
-  type: EnvVarTypeSchema,
-});
-export type EnvVarFormData = z.infer<typeof EnvVarFormSchema>;
+const EnvVarFormBase = z.object({
+  key: z
+    .string()
+    .trim()
+    .transform((s) => s.toUpperCase())
+    .min(1, "Variable name is required")
+    .regex(/^[A-Z][A-Z0-9_]*$/, "Use letters, numbers, and underscores; start with a letter"),
+  type: EnvVarTypeSchema,
+  value: z.string().optional(),
+});
+
+export const EnvVarFormCreateSchema = EnvVarFormBase.extend({
+  value: z.string().min(1, "Variable value is required"),
+});
+
+export const EnvVarFormUpdateSchema = EnvVarFormBase.superRefine((data, ctx) => {
+  if (data.type === "env" && (!data.value || data.value.trim().length === 0)) {
+    ctx.addIssue({ code: "custom", path: ["value"], message: "Variable value is required" });
+  }
+});
+
+export type EnvVarFormData = z.infer<typeof EnvVarFormCreateSchema>;

Follow-ups (outside this file):

  • In AddEnvVarRow, import EnvVarFormCreateSchema and use it for the resolver.
  • In EnvVarForm (update flow), switch resolver to EnvVarFormUpdateSchema when excludeId is set.

27-31: Normalize keys to uppercase to prevent duplicates (API_KEY vs Api_Key).

Adds transform and stricter regex to enforce normalized keys.

Note: Ensure getExistingEnvVar compares normalized keys (uppercase) to keep uniqueness checks consistent.

apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (4)

84-88: Good: don’t prefill ciphertext in edit form.

Leaves value empty when secret isn’t decrypted.


51-51: Professional tone in comment.

Reword the comment.

Apply:

-    // This stupid nested branching won't be necessary once we have the actual tRPC. So disregard this when reviewing
+    // Temporary nested branching for mock behavior; will be removed once tRPC is wired

120-128: UX: disable edit/delete while decrypting.

Prevent conflicting actions during secret toggle.

Apply:

-          {envVar.type === "secret" && (
+          {envVar.type === "secret" && (
             <Button
               size="icon"
               variant="outline"
               onClick={handleToggleSecret}
               disabled={isSecretLoading}
               className="size-7 text-gray-9 hover:text-gray-11 shrink-0"
               loading={isSecretLoading}
             >
         <Button
           size="icon"
           variant="outline"
-          onClick={() => setIsEditing(true)}
+          onClick={() => setIsEditing(true)}
           className="size-7 text-gray-9"
+          disabled={isSecretLoading}
           aria-label="Edit environment variable"
         >
-        <Button
+        <Button
           size="icon"
           variant="outline"
           onClick={handleDelete}
           className="size-7 text-gray-9"
+          disabled={isSecretLoading}
           aria-label="Delete environment variable"
         >

Also applies to: 139-151


139-151: Don’t force-decrypt on Edit.

Open edit immediately; decryption should be optional, not a prerequisite.

Apply:

         <Button
           size="icon"
           variant="outline"
-          onClick={() => {
-            handleToggleSecret().then(() => setIsEditing(true));
-          }}
+          onClick={() => setIsEditing(true)}
           className="size-7 text-gray-9"
+          aria-label="Edit environment variable"
         >
           <PenWriting3 className="!size-[14px]" size="sm-medium" />
         </Button>
-        <Button size="icon" variant="outline" onClick={handleDelete} className="size-7 text-gray-9">
+        <Button
+          size="icon"
+          variant="outline"
+          onClick={handleDelete}
+          className="size-7 text-gray-9"
+          aria-label="Delete environment variable"
+        >
           <Trash className="!size-[14px]" size="sm-medium" />
         </Button>

Pair with the schema split (UpdateSchema) so secrets can be edited without revealing/changing the value.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36ac28c and 28140eb.

📒 Files selected for processing (8)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (2 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (3 hunks)
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-31T19:30:38.171Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3853
File: apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts:75-87
Timestamp: 2025-08-31T19:30:38.171Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts, the getEnvs procedure currently uses mock data (VARIABLES) and ignores the projectId input parameter. This is intentional temporary behavior - the user ogzhanolguncu indicated they plan to hook this up to the database later.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx
  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx
📚 Learning: 2025-05-16T16:16:02.286Z
Learnt from: mcstepp
PR: unkeyed/unkey#3258
File: apps/dashboard/components/dashboard/feedback-component.tsx:28-35
Timestamp: 2025-05-16T16:16:02.286Z
Learning: When validating string inputs in forms using Zod, it's best practice to use `.trim()` before length checks to prevent submissions consisting only of whitespace characters, particularly for feedback forms where meaningful content is expected.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts
📚 Learning: 2024-10-04T17:27:09.821Z
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx
📚 Learning: 2025-06-02T11:09:58.791Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/trpc/routers/key/create.ts:11-14
Timestamp: 2025-06-02T11:09:58.791Z
Learning: In the unkey codebase, TypeScript and the env() function implementation already provide sufficient validation for environment variables, so additional runtime error handling for missing env vars is not needed.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx
📚 Learning: 2025-07-28T20:36:36.865Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/branch/getByName.ts:0-0
Timestamp: 2025-07-28T20:36:36.865Z
Learning: In apps/dashboard/lib/trpc/routers/branch/getByName.ts, mcstepp prefers to keep mock data (gitCommitMessage, buildDuration, lastCommitAuthor, etc.) in the branch procedure during POC phases to demonstrate what the UI would look like with proper schema changes, rather than returning null/undefined values.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx
📚 Learning: 2025-07-25T19:11:00.208Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts:110-147
Timestamp: 2025-07-25T19:11:00.208Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts, the user mcstepp prefers to keep mock data fallbacks in POC/demonstration code for simplicity, even if it wouldn't be production-ready. This aligns with the PR being work-in-progress for demonstration purposes.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx
📚 Learning: 2024-12-03T14:07:45.173Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/components/ui/group-button.tsx:21-31
Timestamp: 2024-12-03T14:07:45.173Z
Learning: In the `ButtonGroup` component (`apps/dashboard/components/ui/group-button.tsx`), avoid suggesting the use of `role="group"` in ARIA attributes.

Applied to files:

  • apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx
🧬 Code graph analysis (7)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/index.tsx (4)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • Environment (38-38)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1)
  • useEnvVarsManager (9-23)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (1)
  • EnvVarRow (15-155)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (1)
  • AddEnvVarRow (15-112)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (5)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (3)
  • EnvVarFormData (35-35)
  • EnvVar (24-24)
  • EnvVarFormSchema (26-34)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
  • EnvVarInputs (16-54)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-secret-switch.tsx (1)
  • EnvVarSecretSwitch (3-24)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1)
  • EnvVarSaveActions (3-39)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts (1)
  • EnvVar (11-11)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/add-env-var-row.tsx (4)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (3)
  • EnvVar (24-24)
  • EnvVarFormData (35-35)
  • EnvVarFormSchema (26-34)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
  • EnvVarInputs (16-54)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-secret-switch.tsx (1)
  • EnvVarSecretSwitch (3-24)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1)
  • EnvVarSaveActions (3-39)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • EnvVarFormData (35-35)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (3)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • Environment (38-38)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var.tsx (1)
  • useEnvVars (10-91)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/env-var-row.tsx (3)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/types.ts (1)
  • EnvVar (24-24)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (1)
  • EnvVarForm (21-127)
⏰ 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). (4)
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/components/env-var-save-actions.tsx (1)

19-27: Submit semantics are correct (no onClick on submit).

Good move making Save a pure submit button and relying on the form handler; avoids double-submit.

@chronark chronark merged commit 7e6061b into main Sep 12, 2025
18 checks passed
@chronark chronark deleted the refactor-env-vars branch September 12, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants