Skip to content

feature(secrets-overview): single-env move secrets environment selection#5775

Merged
scott-ray-wilson merged 6 commits intomainfrom
single-env-move-secrets-env
Mar 20, 2026
Merged

feature(secrets-overview): single-env move secrets environment selection#5775
scott-ray-wilson merged 6 commits intomainfrom
single-env-move-secrets-env

Conversation

@scott-ray-wilson
Copy link
Copy Markdown
Contributor

Context

This PR adds the ability to move secrets across environments to the overview page when a single env is selected. It also updates the move secrets modal to v3 components

Screenshots

multi:
CleanShot 2026-03-20 at 11 22 00@2x

CleanShot 2026-03-20 at 11 22 06@2x

single:
CleanShot 2026-03-20 at 11 22 26@2x

Steps to verify the change

  • verify moving multi-env to different path still works as expected (with and without overwrite)
  • verify new single-env move to different path and/or env works as expected (with and without overwrite)

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Updated CLAUDE.md files (if needed)
  • Read the contributing guide

@maidul98
Copy link
Copy Markdown
Collaborator

maidul98 commented Mar 20, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@scott-ray-wilson scott-ray-wilson changed the title feature(secrets-overview) :single-env move secrets environment selection feat(secrets-overview) :single-env move secrets environment selection Mar 20, 2026
@scott-ray-wilson scott-ray-wilson changed the title feat(secrets-overview) :single-env move secrets environment selection feature(secrets-overview): single-env move secrets environment selection Mar 20, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 20, 2026

Greptile Summary

This PR adds the ability to move secrets across environments from the overview page when a single environment is selected, and upgrades the Move Secrets dialog from v2 to v3 components with react-hook-form / Zod validation. The implementation correctly branches between SingleEnvContent (supports env + path selection) and MultiEnvContent (path-only, same-env move) based on visibleEnvs.length.

Key findings:

  • Carried-over createNotification bug (now in two places): Both SingleEnvContent (line 224) and MultiEnvContent (line 459) call createNotification({ text: "error", title: "..." }) — the literal string "error" ends up as the visible notification body, while the helpful message is in title. Should be { type: "error", text: "..." }.
  • Default environment pre-selects a potentially different env: SingleEnvContent defaults environment to environments[0]?.slug (first alphabetical env) rather than sourceEnv.slug. If these differ, the Move button is immediately enabled on dialog open, which could lead to accidental cross-environment moves without the user noticing a different destination environment was pre-selected.
  • Dead code from removed "missing path" check: The faEyeSlash icon, the "missing" union member in the warning type, and the ternary that references them are now unreachable since the "missing path" warning was removed.

Confidence Score: 3/5

  • Safe to merge after addressing the createNotification bug and reviewing the default environment selection behavior.
  • The core feature logic is sound and the refactor is well-structured, but a carried-over createNotification bug now exists in two places and shows unhelpful UI feedback. The default environment pre-selection in SingleEnvContent could lead to accidental moves. These are actionable fixes before shipping.
  • MoveSecretsDialog.tsx — the createNotification calls and default environment value in SingleEnvContent.

Important Files Changed

Filename Overview
frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx Major refactor that splits the old single Content component into SingleEnvContent, MultiEnvContent, FolderPathSelect, and MoveResultsView. Upgrades from v2 to v3 components and adds react-hook-form/Zod validation. Issues: a carried-over createNotification bug (text/type swap) now duplicated in both new content components; default environment pre-selection could trigger accidental moves; dead code from the removed "missing path" check remains.
frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/SelectionPanel.tsx Minor change — adds visibleEnvs: ProjectEnv[] to the component's Props type and threads it through to MoveSecretsModal. No issues found.
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx Adds a useRef-based effect to reset selected entries when the environment filter changes, and passes visibleEnvs down to SelectionPanel. useRef is already imported. No issues found.

Comments Outside Diff (2)

  1. frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx, line 459-465 (link)

    P1 Swapped text/type in createNotification (MultiEnvContent)

    Same issue as in SingleEnvContenttext: "error" passes the literal string "error" as the visible notification body. The message "You must specify a secret path…" is placed in title but should be in text, and type: "error" should control the notification severity.

  2. frontend/src/pages/secret-manager/OverviewPage/components/SelectionPanel/components/MoveSecretsDialog/MoveSecretsDialog.tsx, line 604-614 (link)

    P2 Dead code branch — "missing" type is never pushed

    The environmentsToBeSkipped array now only ever contains entries with type: "permission" (the previous "missing" path check was removed in this PR). The ternary env.type === "permission" ? faBan : faEyeSlash and faEyeSlash (imported at the top of the file) are therefore unreachable. The TypeScript union "permission" | "missing" in the warning type is also misleading.

    Consider removing "missing" from the union type, removing the faEyeSlash fallback branch, and dropping the faEyeSlash import to keep the code clean.

Last reviewed commit: "improvement: add inf..."

Copy link
Copy Markdown
Contributor

@IgorHorta IgorHorta left a comment

Choose a reason for hiding this comment

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

Lets gooo 🚀

@scott-ray-wilson scott-ray-wilson merged commit 95cd4ca into main Mar 20, 2026
7 checks passed
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.

3 participants