From 1e8eefade13acf0168a12dfbf7af4ef526f3a757 Mon Sep 17 00:00:00 2001 From: claire bontempo <68122737+hellobontempo@users.noreply.github.com> Date: Tue, 7 May 2024 18:45:42 +0100 Subject: [PATCH] UI: wrap client count card in permission conditional (#26848) * consistent timestamp format * wrap client count card in permissions * add test * add changelog * move tests into module, add more! * final test cleanup, stub permissions manually without helper * use current_billing_period for dashboard, add tests * update mirage to handle new client param * Update ui/app/components/dashboard/client-count-card.js --- changelog/26848.txt | 3 + ui/app/adapters/clients/activity.js | 5 + .../dashboard/client-count-card.hbs | 10 +- .../components/dashboard/client-count-card.js | 35 +-- ui/app/components/dashboard/overview.hbs | 8 +- .../components/dashboard/replication-card.hbs | 2 +- ui/mirage/handlers/clients.js | 6 + .../dashboard/client-count-card-test.js | 68 ++--- .../components/dashboard/overview-test.js | 246 ++++++++++-------- .../unit/adapters/clients-activity-test.js | 14 + 10 files changed, 213 insertions(+), 184 deletions(-) create mode 100644 changelog/26848.txt diff --git a/changelog/26848.txt b/changelog/26848.txt new file mode 100644 index 0000000000..6cce04d943 --- /dev/null +++ b/changelog/26848.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Hide dashboard client count card if user does not have permission to view clients. +``` \ No newline at end of file diff --git a/ui/app/adapters/clients/activity.js b/ui/app/adapters/clients/activity.js index 3a3b2b9986..67c4a74786 100644 --- a/ui/app/adapters/clients/activity.js +++ b/ui/app/adapters/clients/activity.js @@ -11,6 +11,11 @@ export default class ActivityAdapter extends ApplicationAdapter { // create date object from user's input using Date.UTC() then send to backend as unix // time params from the backend are formatted as a zulu timestamp formatQueryParams(queryParams) { + if (queryParams?.current_billing_period) { + // { current_billing_period: true } automatically queries the activity log + // from the builtin license start timestamp to the current month + return queryParams; + } let { start_time, end_time } = queryParams; start_time = start_time.timestamp || formatDateObject(start_time); end_time = end_time.timestamp || formatDateObject(end_time, true); diff --git a/ui/app/components/dashboard/client-count-card.hbs b/ui/app/components/dashboard/client-count-card.hbs index 3690e19ab0..fdf7b90a4f 100644 --- a/ui/app/components/dashboard/client-count-card.hbs +++ b/ui/app/components/dashboard/client-count-card.hbs @@ -14,11 +14,7 @@
- {{#if this.noActivityData}} - {{! This will likely not be show since the client activity api was changed to always return data. In the past it - would return no activity data. Adding this empty state here to match the current client count behavior }} - - {{else}} + {{#if this.hasActivity}} {{#if this.fetchClientActivity.isRunning}} {{else}} @@ -44,5 +40,9 @@ {{/if}} + {{else}} + {{! This will likely never show since the clients activity api has changed to always return data. In the past it + would return no activity data. Adding this empty state here to match the current client count behavior }} + {{/if}} \ No newline at end of file diff --git a/ui/app/components/dashboard/client-count-card.js b/ui/app/components/dashboard/client-count-card.js index e9934acbd6..51c2d174dd 100644 --- a/ui/app/components/dashboard/client-count-card.js +++ b/ui/app/components/dashboard/client-count-card.js @@ -9,27 +9,22 @@ import { task } from 'ember-concurrency'; import { waitFor } from '@ember/test-waiters'; import { tracked } from '@glimmer/tracking'; import { service } from '@ember/service'; -import { setStartTimeQuery } from 'core/utils/client-count-utils'; -import { dateFormat } from 'core/helpers/date-format'; +import { parseAPITimestamp } from 'core/utils/date-formatters'; /** * @module DashboardClientCountCard * DashboardClientCountCard component are used to display total and new client count information * * @example - * - * - * - * @param {boolean} isEnterprise - used for setting the start time for the activity log query + * */ export default class DashboardClientCountCard extends Component { @service store; - clientConfig = null; - licenseStartTime = null; @tracked activityData = null; - @tracked updatedAt = timestamp.now().toISOString(); + @tracked hasActivity = false; + @tracked updatedAt = null; constructor() { super(...arguments); @@ -41,12 +36,11 @@ export default class DashboardClientCountCard extends Component { } get statSubText() { - const format = (date) => dateFormat([date, 'MMM yyyy'], {}); - return this.licenseStartTime + const format = (date) => parseAPITimestamp(date, 'MMM yyyy'); + const { startTime, endTime } = this.activityData; + return startTime && endTime ? { - total: `The number of clients in this billing period (${format(this.licenseStartTime)} - ${format( - this.updatedAt - )}).`, + total: `The number of clients in this billing period (${format(startTime)} - ${format(endTime)}).`, new: 'The number of clients new to Vault in the current month.', } : { total: 'No total client data available.', new: 'No new client data available.' }; @@ -58,20 +52,11 @@ export default class DashboardClientCountCard extends Component { if (e) e.preventDefault(); this.updatedAt = timestamp.now().toISOString(); - if (!this.clientConfig) { - // set config and license start time when component initializes - this.clientConfig = yield this.store.queryRecord('clients/config', {}).catch(() => {}); - this.licenseStartTime = setStartTimeQuery(this.args.isEnterprise, this.clientConfig); - } - - // only make the network request if we have a start_time - if (!this.licenseStartTime) return {}; try { this.activityData = yield this.store.queryRecord('clients/activity', { - start_time: { timestamp: this.licenseStartTime }, - end_time: { timestamp: this.updatedAt }, + current_billing_period: true, }); - this.noActivityData = this.activityData.activity.id === 'no-data' ? true : false; + this.hasActivity = this.activityData.id === 'no-data' ? false : true; } catch (error) { this.error = error; } diff --git a/ui/app/components/dashboard/overview.hbs b/ui/app/components/dashboard/overview.hbs index f2cdf2695f..bc3abf1506 100644 --- a/ui/app/components/dashboard/overview.hbs +++ b/ui/app/components/dashboard/overview.hbs @@ -9,10 +9,10 @@
{{#if (and @version.isEnterprise @isRootNamespace)}}
- - {{#if - (and @isRootNamespace (has-permission "status" routeParams="replication") (not (is-empty-value @replication))) - }} + {{#if (has-permission "clients" routeParams="activity")}} + + {{/if}} + {{#if (and (has-permission "status" routeParams="replication") (not (is-empty-value @replication)))}} Updated - {{date-format @updatedAt "MMM dd, yyyy hh:mm:ss"}} + {{date-format @updatedAt "MMM d yyyy, h:mm:ss aaa" withTimeZone=true}}
{{else}} diff --git a/ui/mirage/handlers/clients.js b/ui/mirage/handlers/clients.js index 7450f45f5e..32b16e2a03 100644 --- a/ui/mirage/handlers/clients.js +++ b/ui/mirage/handlers/clients.js @@ -216,6 +216,12 @@ export default function (server) { server.get('/sys/internal/counters/activity', (schema, req) => { let { start_time, end_time } = req.queryParams; + if (req.queryParams.current_billing_period) { + // { current_billing_period: true } automatically queries the activity log + // from the builtin license start timestamp to the current month + start_time = LICENSE_START.toISOString(); + end_time = STATIC_NOW.toISOString(); + } // backend returns a timestamp if given unix time, so first convert to timestamp string here if (!start_time.includes('T')) start_time = fromUnixTime(start_time).toISOString(); if (!end_time.includes('T')) end_time = fromUnixTime(end_time).toISOString(); diff --git a/ui/tests/integration/components/dashboard/client-count-card-test.js b/ui/tests/integration/components/dashboard/client-count-card-test.js index 2d2abb0815..de4a81f07d 100644 --- a/ui/tests/integration/components/dashboard/client-count-card-test.js +++ b/ui/tests/integration/components/dashboard/client-count-card-test.js @@ -9,11 +9,12 @@ import { render, click } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import { setupMirage } from 'ember-cli-mirage/test-support'; import sinon from 'sinon'; -import { LICENSE_START, STATIC_NOW } from 'vault/mirage/handlers/clients'; +import { STATIC_NOW } from 'vault/mirage/handlers/clients'; import timestamp from 'core/utils/timestamp'; import { ACTIVITY_RESPONSE_STUB } from 'vault/tests/helpers/clients/client-count-helpers'; import { formatNumber } from 'core/helpers/format-number'; import { CLIENT_COUNT } from 'vault/tests/helpers/clients/client-count-selectors'; +import { GENERAL } from 'vault/tests/helpers/general-selectors'; module('Integration | Component | dashboard/client-count-card', function (hooks) { setupRenderingTest(hooks); @@ -21,37 +22,31 @@ module('Integration | Component | dashboard/client-count-card', function (hooks) test('it should display client count information', async function (assert) { sinon.replace(timestamp, 'now', sinon.fake.returns(STATIC_NOW)); - assert.expect(6); + assert.expect(5); const { months, total } = ACTIVITY_RESPONSE_STUB; const [latestMonth] = months.slice(-1); - this.server.get('sys/internal/counters/activity', () => { + this.server.get('sys/internal/counters/activity', (schema, req) => { // this assertion should be hit twice, once initially and then again clicking 'refresh' - assert.true(true, 'makes request to sys/internal/counters/activity'); + assert.propEqual( + req.queryParams, + { current_billing_period: 'true' }, + 'it makes request to sys/internal/counters/activity with builtin license start time' + ); return { request_id: 'some-activity-id', data: ACTIVITY_RESPONSE_STUB, }; }); - this.server.get('sys/internal/counters/config', function () { - assert.true(true, 'sys/internal/counters/config'); - return { - request_id: 'some-config-id', - data: { - billing_start_timestamp: LICENSE_START.toISOString(), - }, - }; - }); - await render(hbs``); + await render(hbs``); assert.dom('[data-test-client-count-title]').hasText('Client count'); assert .dom(CLIENT_COUNT.statText('Total')) .hasText( - `Total The number of clients in this billing period (Jul 2023 - Jan 2024). ${formatNumber([ + `Total The number of clients in this billing period (Aug 2023 - Sep 2023). ${formatNumber([ total.clients, ])}` ); - assert .dom(CLIENT_COUNT.statText('New')) .hasText( @@ -64,30 +59,35 @@ module('Integration | Component | dashboard/client-count-card', function (hooks) await click('[data-test-refresh]'); }); - test('it does not query activity for community edition', async function (assert) { - assert.expect(3); - // in the template this component is wrapped in an isEnterprise conditional so this - // state is currently not possible, adding a test to safeguard against introducing - // regressions during future refactors - this.server.get( - 'sys/internal/counters/activity', - () => new Error('uh oh! a request was made to sys/internal/counters/activity') - ); - this.server.get('sys/internal/counters/config', function () { - assert.true(true, 'sys/internal/counters/config'); + test('it shows no data subtext if no start or end timestamp', async function (assert) { + assert.expect(2); + // as far as I know, responses will always have a start/end time + // stubbing this unrealistic response just to test component subtext logic + this.server.get('sys/internal/counters/activity', () => { return { - request_id: 'some-config-id', - data: { - billing_start_timestamp: '0001-01-01T00:00:00Z', - }, + request_id: 'some-activity-id', + data: { by_namespace: [], months: [], total: {} }, }; }); - await render(hbs``); + await render(hbs``); assert.dom(CLIENT_COUNT.statText('Total')).hasText('Total No total client data available. -'); assert.dom(CLIENT_COUNT.statText('New')).hasText('New No new client data available. -'); + }); - // attempt second request to /activity but component task should return instead of hitting endpoint - await click('[data-test-refresh]'); + test('it shows empty state if no activity data', async function (assert) { + // the activity response has changed and now should ALWAYS return something + // but adding this test until we update the adapter to reflect that + assert.expect(3); + this.server.get('sys/internal/counters/activity', () => { + assert.true(true, 'makes request to sys/internal/counters/activity'); + return { data: {} }; + }); + + await render(hbs``); + assert.dom(GENERAL.emptyStateTitle).hasText('No data received'); + assert + .dom(GENERAL.emptyStateMessage) + .hasText('Tracking is turned on and Vault is gathering data. It should appear here within 30 minutes.'); }); }); diff --git a/ui/tests/integration/components/dashboard/overview-test.js b/ui/tests/integration/components/dashboard/overview-test.js index a367e1c281..c64ead6d4e 100644 --- a/ui/tests/integration/components/dashboard/overview-test.js +++ b/ui/tests/integration/components/dashboard/overview-test.js @@ -9,7 +9,6 @@ import { render } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import { setupMirage } from 'ember-cli-mirage/test-support'; import { DASHBOARD } from 'vault/tests/helpers/components/dashboard/dashboard-selectors'; -import { LICENSE_START } from 'vault/mirage/handlers/clients'; module('Integration | Component | dashboard/overview', function (hooks) { setupRenderingTest(hooks); @@ -17,7 +16,11 @@ module('Integration | Component | dashboard/overview', function (hooks) { hooks.beforeEach(function () { this.store = this.owner.lookup('service:store'); - + this.permissions = this.owner.lookup('service:permissions'); + this.version = this.owner.lookup('service:version'); + this.version.version = '1.13.1+ent'; + this.version.type = 'enterprise'; + this.isRootNamespace = true; this.replication = { dr: { clusterId: '123', @@ -61,28 +64,29 @@ module('Integration | Component | dashboard/overview', function (hooks) { ], }; this.refreshModel = () => {}; - this.server.get('sys/internal/counters/config', function () { - return { - request_id: 'some-config-id', - data: { - billing_start_timestamp: LICENSE_START.toISOString(), - }, - }; - }); - }); - - test('it should show dashboard empty states', async function (assert) { - this.version = this.owner.lookup('service:version'); - this.version.version = '1.13.1'; - this.isRootNamespace = true; - await render( - hbs` + this.renderComponent = async () => { + return await render( + hbs` + @refreshModel={{this.refreshModel}} + @replicationUpdatedAt={{this.replicationUpdatedAt}} + /> ` - ); + ); + }; + }); + + test('it should show dashboard empty states in root namespace', async function (assert) { + this.version.version = '1.13.1'; + this.secretsEngines = null; + this.replication = null; + this.vaultConfiguration = null; + await this.renderComponent(); assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); assert.dom(DASHBOARD.cardName('secrets-engines')).exists(); assert.dom(DASHBOARD.emptyState('secrets-engines')).exists(); @@ -94,92 +98,123 @@ module('Integration | Component | dashboard/overview', function (hooks) { assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); }); - test('it should hide client count and replication card on community', async function (assert) { - this.version = this.owner.lookup('service:version'); - this.version.version = '1.13.1'; - this.isRootNamespace = true; + module('client count and replication card', function () { + test('it should hide cards on community in root namespace', async function (assert) { + this.version.version = '1.13.1'; + this.version.type = 'community'; + this.server.get( + 'sys/internal/counters/activity', + () => new Error('uh oh! a request was made to sys/internal/counters/activity') + ); + await this.renderComponent(); - await render( - hbs` - - ` - ); + assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); + assert.dom(DASHBOARD.cardName('secrets-engines')).exists(); + assert.dom(DASHBOARD.cardName('learn-more')).exists(); + assert.dom(DASHBOARD.cardName('quick-actions')).exists(); + assert.dom(DASHBOARD.cardName('configuration-details')).exists(); + assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); + assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); + }); - assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); - assert.dom(DASHBOARD.cardName('secrets-engines')).exists(); - assert.dom(DASHBOARD.cardName('learn-more')).exists(); - assert.dom(DASHBOARD.cardName('quick-actions')).exists(); - assert.dom(DASHBOARD.cardName('configuration-details')).exists(); - assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); - assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); - }); + test('it should hide cards on enterprise if permission but not in root namespace', async function (assert) { + this.permissions.exactPaths = { + 'sys/internal/counters/activity': { + capabilities: ['read'], + }, + 'sys/replication/status': { + capabilities: ['read'], + }, + }; + this.isRootNamespace = false; + await this.renderComponent(); + assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); + assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); + }); - test('it should show client count on enterprise w/ license', async function (assert) { - this.version = this.owner.lookup('service:version'); - this.version.version = '1.13.1+ent'; - this.version.type = 'enterprise'; - this.license = { - autoloaded: { - license_id: '7adbf1f4-56ef-35cd-3a6c-50ef2627865d', - }, - }; + test('it should show cards on enterprise if has permission and in root namespace', async function (assert) { + this.permissions.exactPaths = { + 'sys/internal/counters/activity': { + capabilities: ['read'], + }, + 'sys/replication/status': { + capabilities: ['read'], + }, + }; + await this.renderComponent(); + assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); + assert.dom(DASHBOARD.cardName('secrets-engines')).exists(); + assert.dom(DASHBOARD.cardName('learn-more')).exists(); + assert.dom(DASHBOARD.cardName('quick-actions')).exists(); + assert.dom(DASHBOARD.cardName('configuration-details')).exists(); + assert.dom(DASHBOARD.cardName('client-count')).exists(); + assert.dom(DASHBOARD.cardName('replication')).exists(); + }); - await render( - hbs` - ` - ); - assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); - assert.dom(DASHBOARD.cardName('secrets-engines')).exists(); - assert.dom(DASHBOARD.cardName('learn-more')).exists(); - assert.dom(DASHBOARD.cardName('quick-actions')).exists(); - assert.dom(DASHBOARD.cardName('configuration-details')).exists(); - assert.dom(DASHBOARD.cardName('client-count')).exists(); - }); + test('it should hide cards on enterprise in root namespace but no permission', async function (assert) { + await this.renderComponent(); + assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); + assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); + }); - test('it should hide replication on enterprise not on root namespace', async function (assert) { - this.version = this.owner.lookup('service:version'); - this.version.version = '1.13.1+ent'; - this.version.type = 'enterprise'; - this.isRootNamespace = false; + test('it should hide cards on enterprise if no permission and not in root namespace', async function (assert) { + this.isRootNamespace = false; + await this.renderComponent(); + assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); + assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); + }); - await render( - hbs` - ` - ); + test('it should hide client count on enterprise in root namespace if no activity permission', async function (assert) { + this.permissions.exactPaths = { + 'sys/internal/counters/activity': { + capabilities: ['deny'], + }, + 'sys/replication/status': { + capabilities: ['read'], + }, + }; - assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); - assert.dom('[data-test-badge-namespace]').exists(); - assert.dom(DASHBOARD.cardName('secrets-engines')).exists(); - assert.dom(DASHBOARD.cardName('learn-more')).exists(); - assert.dom(DASHBOARD.cardName('quick-actions')).exists(); - assert.dom(DASHBOARD.cardName('configuration-details')).exists(); - assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); - assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); + await this.renderComponent(); + assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); + assert.dom(DASHBOARD.cardName('replication')).exists(); + }); + + test('it should hide replication on enterprise in root namespace if no replication status permission', async function (assert) { + this.permissions.exactPaths = { + 'sys/internal/counters/activity': { + capabilities: ['read'], + }, + 'sys/replication/status': { + capabilities: ['deny'], + }, + }; + + await this.renderComponent(); + assert.dom(DASHBOARD.cardName('client-count')).exists(); + assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); + }); + + test('it should hide replication on enterprise if has permission and in root namespace but is empty', async function (assert) { + this.permissions.exactPaths = { + 'sys/internal/counters/activity': { + capabilities: ['read'], + }, + 'sys/replication/status': { + capabilities: ['read'], + }, + }; + this.replication = {}; + await this.renderComponent(); + assert.dom(DASHBOARD.cardName('client-count')).exists(); + assert.dom(DASHBOARD.cardName('replication')).doesNotExist(); + }); }); module('learn more card', function () { test('shows the learn more card on community', async function (assert) { - await render( - hbs`` - ); + this.version.version = '1.13.1'; + this.version.type = 'community'; + await this.renderComponent(); assert.dom('[data-test-learn-more-title]').hasText('Learn more'); assert @@ -193,32 +228,13 @@ module('Integration | Component | dashboard/overview', function (hooks) { .hasText("Don't see what you're looking for on this page? Let us know via our feedback form ."); }); test('shows the learn more card on enterprise', async function (assert) { - this.version = this.owner.lookup('service:version'); - this.version.version = '1.13.1+ent'; - this.version.type = 'enterprise'; this.version.features = [ 'Performance Replication', 'DR Replication', 'Namespaces', 'Transform Secrets Engine', ]; - this.isRootNamespace = true; - this.license = { - autoloaded: { - license_id: '7adbf1f4-56ef-35cd-3a6c-50ef2627865d', - }, - }; - await render( - hbs` - - ` - ); + await this.renderComponent(); assert.dom('[data-test-learn-more-title]').hasText('Learn more'); assert .dom('[data-test-learn-more-subtext]') diff --git a/ui/tests/unit/adapters/clients-activity-test.js b/ui/tests/unit/adapters/clients-activity-test.js index 3d9287c26d..7d23d1e975 100644 --- a/ui/tests/unit/adapters/clients-activity-test.js +++ b/ui/tests/unit/adapters/clients-activity-test.js @@ -136,4 +136,18 @@ module('Unit | Adapter | clients activity', function (hooks) { this.store.queryRecord(this.modelName, queryParams); }); + + test('it sends current billing period boolean if provided', async function (assert) { + assert.expect(1); + + this.server.get('sys/internal/counters/activity', (schema, req) => { + assert.propEqual( + req.queryParams, + { current_billing_period: 'true' }, + 'it passes current_billing_period to query record' + ); + }); + + this.store.queryRecord(this.modelName, { current_billing_period: true }); + }); });