Skip to content

[VAULT-35780] address TODOs#30468

Merged
beagins merged 8 commits intoVAULT-34216/namespace-picker-feature-branchfrom
VAULT-34216/address-todos
May 1, 2025
Merged

[VAULT-35780] address TODOs#30468
beagins merged 8 commits intoVAULT-34216/namespace-picker-feature-branchfrom
VAULT-34216/address-todos

Conversation

@beagins
Copy link
Copy Markdown
Contributor

@beagins beagins commented Apr 30, 2025

🗒️ Merging into sidebranch not main
🗒️ Recommend hiding whitespace changes when reviewing file changes

Description

  • address outstanding TODOs
  • styling updates
  • stabilize flaky 🥐 namespace enterprise & integration tests
  • enterprise tests passing ✅

    248 tests completed in 163372 milliseconds, with 0 failed, 10 skipped, and 0 todo.
    1468 assertions of 1468 passed, 0 failed.

Screenshot 2025-05-01 at 3 30 24 PM Screenshot 2025-05-01 at 3 30 38 PM Screenshot 2025-05-01 at 3 30 51 PM Screenshot 2025-05-01 at 3 31 33 PM

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Apr 30, 2025
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🗒️ Note: traced this back to this PR. The test in question was added to verify that the namespace picker works whether the user logs in with '/namespace' or 'namespace'. The namespace service strips out any prefixed '/', so option?.path will not have a proceeding '/'. We still have an enterprise test to ensure that the namespace picker works whether the user logs in w/ a '/' prefixed namespace or not.

@beagins beagins force-pushed the VAULT-34216/address-todos branch from 25d7b7f to df29600 Compare April 30, 2025 23:06
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 30, 2025

CI Results:
All Go tests succeeded! ✅

@beagins beagins force-pushed the VAULT-34216/address-todos branch from df29600 to b31623d Compare April 30, 2025 23:12
@beagins beagins marked this pull request as ready for review May 1, 2025 18:41
@beagins beagins requested a review from a team as a May 1, 2025 18:41
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 1, 2025

Build Results:
All builds succeeded! ✅

@beagins beagins force-pushed the VAULT-34216/namespace-picker-feature-branch branch from 7af7cf4 to ba2237f Compare May 1, 2025 18:51
@beagins beagins requested review from a team and prakashlinga and removed request for a team May 1, 2025 18:51
@beagins beagins force-pushed the VAULT-34216/address-todos branch from 3371445 to f59945f Compare May 1, 2025 18:52
Copy link
Copy Markdown
Contributor Author

@beagins beagins May 1, 2025

Choose a reason for hiding this comment

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

🗒️ Note: This test case is now covered by the three new tests added above:

test('it shows both action buttons when canList is true', async function (assert) {
test('it hides the refresh button when canList is false', async function (assert) {
test('it hides both action buttons when the capabilities store throws an error', async function (assert) {

@beagins beagins force-pushed the VAULT-34216/namespace-picker-feature-branch branch from ba2237f to 78bed19 Compare May 1, 2025 21:47
@beagins beagins force-pushed the VAULT-34216/address-todos branch from c382d0f to 3d3b4a5 Compare May 1, 2025 21:50
<D.ToggleButton @icon="org" @text={{or this.selected.id "-"}} @isFullWidth={{true}} data-test-namespace-toggle />

<D.Header>
{{#if this.errorLoadingNamespaces}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

awesome to see we're displaying the error

import { login, loginNs, logout } from 'vault/tests/helpers/auth/auth-helpers';
import { AUTH_FORM } from 'vault/tests/helpers/auth/auth-form-selectors';
import { GENERAL } from '../helpers/general-selectors';
import { NAMESPACE_PICKER_SELECTORS } from '../helpers/namespace-picker';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Up to you, but I like to shorten then. For Secret Engine Selectors I used SES, or maybe here just NAMESPACE_PICKER. Shortening it saves the smallest amount of time when typing these out, those co-pilot kind of does it for you. Again, totally up to you.

Copy link
Copy Markdown
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Really great job on this!

@beagins beagins merged commit bc03e54 into VAULT-34216/namespace-picker-feature-branch May 1, 2025
25 checks passed
@beagins beagins deleted the VAULT-34216/address-todos branch May 1, 2025 22:59
beagins added a commit that referenced this pull request May 5, 2025
* [VAULT-35780] address TODOs

* wrap manage & refresh buttons in Hds::ButtonSet

* fix failing integration tests

* remove unnecessary import mirage

* fix undefined

* fix flaky tests

* styling updates

* don't show search help text when there are no namespaces matching the search criteria
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants