Skip to content

Commit 228ac5f

Browse files
hellobontempohellobontempo
authored
Backport 1.20.x: Namespace picker capabilities and duplicate selection fixes (#31330)
* remove sys/namespace capabilities check for refresh button (#31308) * link jira VAULT-38250 * UI: Fix namespace picker selecting all namespaces with matching nodes (#31326) * add test * add changelog * VAULT-38241 update tests * restart tests because i closed the PR whyyyy
1 parent 94ce9d1 commit 228ac5f

4 files changed

Lines changed: 93 additions & 137 deletions

File tree

‎changelog/31326.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
ui: Fix selecting multiple namespaces in the namespace picker when the path contains matching nodes
3+
```

‎ui/app/components/namespace-picker.hbs‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88

99
<D.ToggleButton
1010
@icon="org"
11-
@text={{or this.selectedNamespace.id "-"}}
11+
{{! Displays the node of the current namespace context in the toggle }}
12+
{{! For example, if a user navigates to 'parent/child' the toggle just displays 'child' }}
13+
@text={{this.namespace.currentNamespace}}
1214
@isFullWidth={{true}}
1315
data-test-toggle-input="namespace-id"
1416
{{on "click" this.toggleNamespacePicker}}
@@ -58,7 +60,7 @@
5860
<div class="is-overflow-y-auto is-max-drawer-height" {{did-insert this.setupScrollListener}}>
5961
{{#each this.visibleNamespaceOptions as |option|}}
6062
<D.Checkmark
61-
@selected={{eq option.id this.selectedNamespace.id}}
63+
@selected={{eq option.path this.selectedNamespace.path}}
6264
{{on "click" (fn this.onChange option)}}
6365
data-test-namespace-link={{option.path}}
6466
>
@@ -71,22 +73,19 @@
7173

7274
<D.Footer @hasDivider={{true}} class="is-flex-center">
7375
<Hds::ButtonSet class="is-fullwidth">
74-
{{#if this.canRefreshNamespaces}}
75-
<Hds::Button
76-
@color="secondary"
77-
@text="Refresh list"
78-
@isFullWidth={{(not this.canManageNamespaces)}}
79-
@icon="reload"
80-
@size="small"
81-
{{on "click" this.refreshList}}
82-
data-test-refresh-namespaces
83-
/>
84-
{{/if}}
76+
<Hds::Button
77+
@color="secondary"
78+
@text="Refresh list"
79+
@isFullWidth={{not this.canManageNamespaces}}
80+
@icon="reload"
81+
@size="small"
82+
{{on "click" this.refreshList}}
83+
data-test-refresh-namespaces
84+
/>
8585
{{#if this.canManageNamespaces}}
8686
<Hds::Button
8787
@color="tertiary"
8888
@text="Manage"
89-
@isFullWidth={{(not this.canRefreshNamespaces)}}
9089
@icon="settings"
9190
@size="small"
9291
@route="vault.cluster.access.namespaces"

‎ui/app/components/namespace-picker.ts‎

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@ import { action } from '@ember/object';
88
import { tracked } from '@glimmer/tracking';
99
import { service } from '@ember/service';
1010
import keys from 'core/utils/keys';
11+
12+
import type CapabilitiesService from 'vault/services/capabilities';
1113
import type Router from 'vault/router';
1214
import type NamespaceService from 'vault/services/namespace';
1315
import type AuthService from 'vault/vault/services/auth';
1416
import type Store from '@ember-data/store';
1517
import errorMessage from 'vault/utils/error-message';
1618

1719
interface NamespaceOption {
18-
id: string;
1920
path: string;
2021
label: string;
2122
}
@@ -32,6 +33,7 @@ interface NamespaceOption {
3233
*/
3334
export default class NamespacePicker extends Component {
3435
@service declare auth: AuthService;
36+
@service declare capabilities: CapabilitiesService;
3537
@service declare namespace: NamespaceService;
3638
@service declare router: Router;
3739
@service declare store: Store;
@@ -40,7 +42,6 @@ export default class NamespacePicker extends Component {
4042
@tracked batchSize = 200;
4143

4244
@tracked canManageNamespaces = false; // Show/hide manage namespaces button
43-
@tracked canRefreshNamespaces = false; // Show/hide refresh list button
4445
@tracked errorLoadingNamespaces = '';
4546
@tracked hasNamespaces = false;
4647
@tracked searchInput = '';
@@ -50,14 +51,11 @@ export default class NamespacePicker extends Component {
5051
constructor(owner: unknown, args: Record<string, never>) {
5152
super(owner, args);
5253
this.loadOptions();
54+
this.fetchManageCapability();
5355
}
5456

5557
get allNamespaces(): NamespaceOption[] {
56-
return this.getOptions(
57-
this.namespace?.accessibleNamespaces,
58-
this.namespace?.currentNamespace,
59-
this.namespace?.path
60-
);
58+
return this.getOptions(this.namespace?.accessibleNamespaces);
6159
}
6260

6361
get selectedNamespace(): NamespaceOption | null {
@@ -72,45 +70,33 @@ export default class NamespacePicker extends Component {
7270
return options.find((option) => this.matchesPath(option, currentPath));
7371
}
7472

75-
private getOptions(
76-
accessibleNamespaces: string[],
77-
currentNamespace: string,
78-
path: string
79-
): NamespaceOption[] {
80-
/* Each namespace option has 3 properties: { id, path, and label }
81-
* - id: node / namespace name (displayed when the namespace picker is closed)
73+
private getOptions(accessibleNamespaces: string[]): NamespaceOption[] {
74+
/* Each namespace option has 2 properties: { path and label }
8275
* - path: full namespace path (used to navigate to the namespace)
83-
* - label: text displayed inside the namespace picker dropdown (if root, then label = id, else label = path)
76+
* - label: text displayed inside the namespace picker dropdown (if root, then path is "", else label = path)
8477
*
8578
* Example:
86-
* | id | path | label |
87-
* | --- | ---- | ----- |
88-
* | 'root' | '' | 'root' |
89-
* | 'parent' | 'parent' | 'parent' |
90-
* | 'child' | 'parent/child' | 'parent/child' |
79+
* | path | label |
80+
* | ---- | ----- |
81+
* | '' | 'root' |
82+
* | 'parent' | 'parent' |
83+
* | 'parent/child' | 'parent/child' |
9184
*/
92-
const options = [
93-
...(accessibleNamespaces || []).map((ns: string) => {
94-
const parts = ns.split('/');
95-
return { id: parts[parts.length - 1] || '', path: ns, label: ns };
96-
}),
97-
];
85+
const options = (accessibleNamespaces || []).map((ns: string) => ({ path: ns, label: ns }));
9886

9987
// Add the user's root namespace because `sys/internal/ui/namespaces` does not include it.
10088
const userRootNamespace = this.auth.authData?.userRootNamespace;
10189
if (!options?.find((o) => o.path === userRootNamespace)) {
102-
const ns = userRootNamespace === '' ? 'root' : userRootNamespace;
103-
options.unshift({ id: ns, path: userRootNamespace, label: ns });
90+
// the 'root' namespace is technically an empty string so we manually add the 'root' label.
91+
const label = userRootNamespace === '' ? 'root' : userRootNamespace;
92+
options.unshift({ path: userRootNamespace, label });
10493
}
10594

10695
// If there are no namespaces returned by the internal endpoint, add the current namespace
10796
// to the list of options. This is a fallback for when the user has access to a single namespace.
10897
if (options.length === 0) {
109-
options.push({
110-
id: currentNamespace,
111-
path: path,
112-
label: path,
113-
});
98+
// 'path' defined in the namespace service is the full namespace path
99+
options.push({ path: this.namespace.path, label: this.namespace.path });
114100
}
115101

116102
return options;
@@ -177,15 +163,12 @@ export default class NamespacePicker extends Component {
177163
}
178164

179165
@action
180-
async fetchListCapability(): Promise<void> {
181-
try {
182-
const namespacePermission = await this.store.findRecord('capabilities', 'sys/namespaces/');
183-
this.canRefreshNamespaces = namespacePermission.get('canList');
184-
this.canManageNamespaces = true;
185-
} catch (error) {
186-
// If the findRecord call fails, the user lacks permissions to refresh or manage namespaces.
187-
this.canRefreshNamespaces = this.canManageNamespaces = false;
188-
}
166+
async fetchManageCapability(): Promise<void> {
167+
// The namespace picker options are from `sys/internal/ui/namespaces` which all users have permissions to request.
168+
// The UI view for managing namespaces (i.e. CRUD actions) calls `sys/namespaces` and DOES require LIST permissions.
169+
// This is the capability check to hide/show the button that navigates to that route.
170+
const { canList } = await this.capabilities.fetchPathCapabilities('sys/namespaces');
171+
this.canManageNamespaces = canList;
189172
}
190173

191174
@action
@@ -202,8 +185,6 @@ export default class NamespacePicker extends Component {
202185
} catch (error) {
203186
this.errorLoadingNamespaces = errorMessage(error);
204187
}
205-
206-
await this.fetchListCapability();
207188
}
208189

209190
@action

‎ui/tests/integration/components/namespace-picker-test.js‎

Lines changed: 52 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -8,36 +8,10 @@ import { setupRenderingTest } from 'ember-qunit';
88
import { render, fillIn, findAll, waitFor, click, find } from '@ember/test-helpers';
99
import sinon from 'sinon';
1010
import hbs from 'htmlbars-inline-precompile';
11-
import Service from '@ember/service';
1211
import { NAMESPACE_PICKER_SELECTORS } from 'vault/tests/helpers/namespace-picker';
1312
import { GENERAL } from 'vault/tests/helpers/general-selectors';
1413
import { setupMirage } from 'ember-cli-mirage/test-support';
15-
16-
class StoreService extends Service {
17-
findRecord(modelType, id) {
18-
return new Promise((resolve, reject) => {
19-
if (modelType === 'capabilities' && id === 'sys/namespaces/') {
20-
resolve(); // Simulate a successful response
21-
} else {
22-
reject({ httpStatus: 404, message: 'not found' }); // Simulate an error response
23-
}
24-
});
25-
}
26-
}
27-
28-
function getMockCapabilitiesModel(canList) {
29-
// Mock for the Capabilities model
30-
return {
31-
path: 'sys/namespaces/',
32-
capabilities: canList ? ['list'] : [],
33-
get(property) {
34-
if (property === 'canList') {
35-
return this.capabilities.includes('list');
36-
}
37-
return undefined;
38-
},
39-
};
40-
}
14+
import { capabilitiesStub, overrideResponse } from 'vault/tests/helpers/stubs';
4115

4216
module('Integration | Component | namespace-picker', function (hooks) {
4317
setupRenderingTest(hooks);
@@ -52,12 +26,8 @@ module('Integration | Component | namespace-picker', function (hooks) {
5226
// the path in the namespace service denotes the current namespace context a user is in
5327
this.nsService.path = 'parent1/child1';
5428
this.server.get('/sys/internal/ui/namespaces', () => {
55-
return {
56-
data: { keys: ['parent1/', 'parent1/child1/'] },
57-
};
29+
return { data: { keys: ['parent1/', 'parent1/child1/'] } };
5830
});
59-
60-
this.owner.register('service:store', StoreService);
6131
});
6232

6333
hooks.afterEach(function () {
@@ -77,6 +47,16 @@ module('Integration | Component | namespace-picker', function (hooks) {
7747
);
7848
});
7949

50+
test('it selects the current namespace', async function (assert) {
51+
await render(hbs`<NamespacePicker />`);
52+
assert.dom(GENERAL.toggleInput('namespace-id')).hasText('child1', 'it just displays the namespace node');
53+
await click(GENERAL.toggleInput('namespace-id'));
54+
assert
55+
.dom(NAMESPACE_PICKER_SELECTORS.link(this.nsService.path))
56+
.hasAttribute('aria-selected', 'true', 'the current namespace path is selected');
57+
assert.dom(`${NAMESPACE_PICKER_SELECTORS.link(this.nsService.path)} ${GENERAL.icon('check')}`).exists();
58+
});
59+
8060
test('it filters namespace options based on search input', async function (assert) {
8161
await render(hbs`<NamespacePicker/>`);
8262
await click(GENERAL.toggleInput('namespace-id'));
@@ -111,65 +91,43 @@ module('Integration | Component | namespace-picker', function (hooks) {
11191
);
11292
});
11393

114-
test('it shows both action buttons when canList is true', async function (assert) {
115-
const storeStub = this.owner.lookup('service:store');
116-
sinon.stub(storeStub, 'findRecord').callsFake((modelType, id) => {
117-
if (modelType === 'capabilities' && id === 'sys/namespaces/') {
118-
return Promise.resolve(getMockCapabilitiesModel(true));
119-
}
120-
return Promise.reject();
94+
module('capabilities', function (hooks) {
95+
hooks.beforeEach(function () {
96+
// the capabilities service prepends the user's root namespace to API paths when checking permissions.
97+
// For simplicity, test permissions from the "root" namespace context
98+
this.nsService.path = '';
12199
});
122100

123-
await render(hbs`<NamespacePicker />`);
124-
await click(GENERAL.toggleInput('namespace-id'));
101+
test('it shows the "Manage" action when user has canList permissions', async function (assert) {
102+
this.server.post('/sys/capabilities-self', () => capabilitiesStub('sys/namespaces', ['list']));
125103

126-
// Verify that the "Refresh List" button is visible
127-
assert.dom(NAMESPACE_PICKER_SELECTORS.refreshList).exists('Refresh List button is visible');
128-
assert.dom(NAMESPACE_PICKER_SELECTORS.manageButton).exists('Manage button is visible');
129-
});
104+
await render(hbs`<NamespacePicker />`);
105+
await click(GENERAL.toggleInput('namespace-id'));
130106

131-
test('it hides the refresh button when canList is false', async function (assert) {
132-
const storeStub = this.owner.lookup('service:store');
133-
sinon.stub(storeStub, 'findRecord').callsFake((modelType, id) => {
134-
if (modelType === 'capabilities' && id === 'sys/namespaces/') {
135-
return Promise.resolve(getMockCapabilitiesModel(false));
136-
}
137-
return Promise.reject();
107+
assert.dom(NAMESPACE_PICKER_SELECTORS.manageButton).exists('Manage button is visible');
138108
});
139109

140-
await render(hbs`<NamespacePicker />`);
141-
await click(GENERAL.toggleInput('namespace-id'));
110+
test('it hides the "Manage" button when canList is false', async function (assert) {
111+
this.server.post('/sys/capabilities-self', capabilitiesStub(`sys/namespaces`, ['deny']));
112+
await render(hbs`<NamespacePicker />`);
113+
await click(GENERAL.toggleInput('namespace-id'));
142114

143-
// Verify that the buttons are hidden
144-
assert.dom(NAMESPACE_PICKER_SELECTORS.refreshList).doesNotExist('Refresh List button is hidden');
145-
assert.dom(NAMESPACE_PICKER_SELECTORS.manageButton).exists('Manage button is hidden');
146-
});
147-
148-
test('it hides both action buttons when the capabilities store throws an error', async function (assert) {
149-
const storeStub = this.owner.lookup('service:store');
150-
sinon.stub(storeStub, 'findRecord').callsFake(() => {
151-
return Promise.reject();
115+
// Verify that the buttons are hidden
116+
assert.dom(NAMESPACE_PICKER_SELECTORS.manageButton).doesNotExist('Manage button is hidden');
152117
});
153118

154-
await render(hbs`<NamespacePicker />`);
155-
await click(GENERAL.toggleInput('namespace-id'));
119+
test('it shows "Manage" button when the capabilities request throws an error', async function (assert) {
120+
// It's rare for the capabilities request to fail (if it does, it's usually because the request is made in the wrong namespace context).
121+
// If it fails, the UI should show resources and rely on the API to gate them appropriately.
122+
this.server.post('/sys/capabilities-self', () => overrideResponse(403));
156123

157-
// Verify that the buttons are hidden
158-
assert.dom(NAMESPACE_PICKER_SELECTORS.refreshList).doesNotExist('Refresh List button is hidden');
159-
assert.dom(NAMESPACE_PICKER_SELECTORS.manageButton).doesNotExist('Manage button is hidden');
124+
await render(hbs`<NamespacePicker />`);
125+
await click(GENERAL.toggleInput('namespace-id'));
126+
assert.dom(NAMESPACE_PICKER_SELECTORS.manageButton).exists();
127+
});
160128
});
161129

162130
test('it updates the namespace list after clicking "Refresh list"', async function (assert) {
163-
this.owner.lookup('service:namespace').set('hasListPermissions', true);
164-
165-
const storeStub = this.owner.lookup('service:store');
166-
sinon.stub(storeStub, 'findRecord').callsFake((modelType, id) => {
167-
if (modelType === 'capabilities' && id === 'sys/namespaces/') {
168-
return Promise.resolve(getMockCapabilitiesModel(true)); // Return the mock model
169-
}
170-
return Promise.reject();
171-
});
172-
173131
await render(hbs`<NamespacePicker />`);
174132
await click(GENERAL.toggleInput('namespace-id'));
175133

@@ -180,7 +138,7 @@ module('Integration | Component | namespace-picker', function (hooks) {
180138
'Initially, three namespaces are displayed'
181139
);
182140

183-
// Re-stub request with a new namespace
141+
// Re-stub request from beforeEach hook with a new namespace
184142
this.server.get('/sys/internal/ui/namespaces', () => {
185143
return {
186144
data: { keys: ['parent1/', 'parent1/child1/', 'new-namespace/'] },
@@ -221,4 +179,19 @@ module('Integration | Component | namespace-picker', function (hooks) {
221179
assert.dom(NAMESPACE_PICKER_SELECTORS.link('admin')).exists();
222180
assert.dom(NAMESPACE_PICKER_SELECTORS.link('admin/child1')).exists();
223181
});
182+
183+
test('it selects the correct namespace when matching nodes exist', async function (assert) {
184+
// stub response so that two namespaces have matching node names 'child1'
185+
this.server.get('/sys/internal/ui/namespaces', () => {
186+
return { data: { keys: ['parent1/', 'parent1/child1', 'anotherParent/', 'anotherParent/child1'] } };
187+
});
188+
await render(hbs`<NamespacePicker />`);
189+
assert.dom(GENERAL.toggleInput('namespace-id')).hasText('child1', 'it displays the namespace node');
190+
await click(GENERAL.toggleInput('namespace-id'));
191+
assert
192+
.dom(NAMESPACE_PICKER_SELECTORS.link(this.nsService.path))
193+
.hasAttribute('aria-selected', 'true', 'the current namespace path is selected');
194+
assert.dom('[aria-selected="true"]').exists({ count: 1 }, 'only one option is selected');
195+
assert.dom(GENERAL.icon('check')).exists({ count: 1 }, 'only one check mark renders');
196+
});
224197
});

0 commit comments

Comments
 (0)