The secrets manager has increased in scope a lot during development, and the team prioritized getting features in for the release. We also have upcoming work items for improving the ~UX, which would require restructuring some of our frontend code to accommodate the changes. Let's use this work item to collect all our tech debt in one place. We'll break each one down and figure out priorities after the LA release. ## Testing with CDot ~backend ~frontend Is there an easier way to test the different CDot responses for frontend? Currently we just stub these responses with a YAML file and monkey-patching some of the backend. ## Structural refactor for `SecretsTable` ~frontend We mount `SecretsTable` as the main component for the `/` path. But over time, as the page acquired more states and UI other than the table, this component has become bloated with non-table concerns. **Proposal:** Create a new `SecretsIndexWrapper` component that houses the components for our trial empty state, empty state, and secrets table (and other related components). These three components also rely on the same GraphQL queries, which should live in the wrapper. Currently, these queries live in `secrets_app` (over-scoped) because of the lack of a wrapper component scoped to the needs of the `/` page. This refactor will also remove the failing Vue3 specs introduced in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/249429 (see [failing job logs](https://gitlab.com/gitlab-org/gitlab/-/jobs/15865592869)). The main issue is that events emitted from a component nested inside `<router-view>` do not propagate to the parent template listener under vue-router 4. The real fix to the underlying issue is https://gitlab.com/gitlab-org/gitlab/-/work_items/396472, however this refactor will better "fix" it by removing the need to bubble up events from the `router-view` altogether (which we should try to avoid anyway). When this refactor lands, remove the `itSkipVue3` wrappers on the 4 tests in `ee/spec/frontend/ci/secrets/components/secrets_app_spec.js`. Additionally, we can remove the `wrapper.vm.*` assertions on the `provide` data from `secrets_app_spec.js`. For example, `isBetaReadOnly` moves closer to (or into) `SecretsIndexWrapper`, where its consumers live. Then a colocated `secrets_index_wrapper_spec.js` can assert directly on button visibility / disabled state without stubbing anything. ## Auto-enrolling and provisioning when starting a trial ### For discussion ~backend ~frontend Should we consider having one mutation for this instead of chaining three on the frontend? ### Replace `BENIGN_*_ERRORS` string matching with typed error codes from backend ~backend ~frontend We use these constants to denote benign errors during the trial onboarding. ``` export const BENIGN_ENROLL_ERRORS = ['Namespace is already enrolled.']; export const BENIGN_TRIAL_ERRORS = ['A Secrets Manager trial is already active for this group.']; export const BENIGN_PROVISION_ERRORS = ['Secrets manager already initialized for the group.']; ``` **The problem:** frontend does an exact string match against backend error messages to decide "was this a real error, or a benign 'already done' response we should treat as success?" For example, when the enroll mutation returns "Namespace is already enrolled.", the frontend swallows that error and proceeds to the trial step (idempotent chain). **Why it's fragile:** any backend rewording — even punctuation ("Namespace already enrolled" → dropped period, "Group is already enrolled" → different noun, translation, etc.) — silently flips a benign no-op into a user-facing error, breaking retries on partial progress. This is what the "coupling" comment in the constants block acknowledges. **What we want instead:** typed error codes (or a `success: true` field, or `errors[].code`) from the three mutations: - `namespaceSecretsManagerEnroll` → `ALREADY_ENROLLED` code - `secretsManagerStartTrial` → `TRIAL_ALREADY_ACTIVE` code - `groupSecretsManagerInitialize` → `ALREADY_INITIALIZED` code Once error codes exist, frontend can match on code instead of raw message text, and the `BENIGN_*_ERRORS `constants go away. ## `Authz/PermissionCheck` in secrets helper ~backend In `secrets_helper.rb`, we use `can_manage_secrets_manager: can?(user, :admin_group, namespace)`. This disables `Gitlab/Authz/PermissionCheck` which requires a more granular ability (we should avoid using `admin_group`). ## Better error-handling ~backend ~"Technical Writing" We have https://gitlab.com/gitlab-org/gitlab/-/work_items/584982+ We should also make sure to check with ~Technical Writing since the FE just surfaces the BE errors from the GraphQL queries/mutation. ## Other concerns (to be refined) - Remove 'show-secrets-toast' event - Audit `ee/app/assets/javascripts/ci/secrets/constants.js` for unused status constants and remove - Clean up mock data: https://gitlab.com/gitlab-org/gitlab/-/work_items/589245 - Improve QA tests Suggestions from Duo: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/250373#note_3692872907