From 2b556275383b6ecd91f88f524f15427151ccb07b Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Wed, 1 May 2024 09:58:42 +0100 Subject: [PATCH 01/12] remvoe request tolicense in dashboard client count card --- .../components/dashboard/client-count-card.hbs | 2 +- .../components/dashboard/client-count-card.js | 17 ++++++++++------- ui/app/components/dashboard/overview.hbs | 6 +++--- ui/app/routes/vault/cluster/dashboard.js | 1 - ui/app/templates/vault/cluster/dashboard.hbs | 1 - 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/ui/app/components/dashboard/client-count-card.hbs b/ui/app/components/dashboard/client-count-card.hbs index bdaea93f5b3d..e8a0f1ee97d5 100644 --- a/ui/app/components/dashboard/client-count-card.hbs +++ b/ui/app/components/dashboard/client-count-card.hbs @@ -55,7 +55,7 @@ /> Updated - {{date-format this.updatedAt "MMM dd, yyyy hh:mm:ss"}} + {{date-format this.updatedAt "MMM d yyyy, h:mm:ss aaa" withTimeZone=true}} {{/if}} diff --git a/ui/app/components/dashboard/client-count-card.js b/ui/app/components/dashboard/client-count-card.js index 6df142b6ebab..c0c3dc122768 100644 --- a/ui/app/components/dashboard/client-count-card.js +++ b/ui/app/components/dashboard/client-count-card.js @@ -4,12 +4,12 @@ */ import Component from '@glimmer/component'; -import getStorage from 'vault/lib/token-storage'; import timestamp from 'core/utils/timestamp'; import { task } from 'ember-concurrency'; import { waitFor } from '@ember/test-waiters'; import { tracked } from '@glimmer/tracking'; import { service } from '@ember/service'; +import getUnixTime from 'date-fns/getUnixTime'; /** * @module DashboardClientCountCard @@ -25,29 +25,32 @@ import { service } from '@ember/service'; export default class DashboardClientCountCard extends Component { @service store; + clientConfig = null; + licenseStartTime = null; @tracked activityData = null; - @tracked clientConfig = null; @tracked updatedAt = timestamp.now().toISOString(); constructor() { super(...arguments); this.fetchClientActivity.perform(); - this.clientConfig = this.store.queryRecord('clients/config', {}).catch(() => {}); } get currentMonthActivityTotalCount() { return this.activityData?.byMonth?.lastObject?.new_clients.clients; } - get licenseStartTime() { - return this.args.license.startTime || getStorage().getItem('vault:ui-inputted-start-date') || null; - } - @task @waitFor *fetchClientActivity(e) { 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 = getUnixTime(this.clientConfig.billingStartTimestamp); + } + // only make the network request if we have a start_time if (!this.licenseStartTime) return {}; try { diff --git a/ui/app/components/dashboard/overview.hbs b/ui/app/components/dashboard/overview.hbs index f87059d5fe8d..2244b0ac3c2b 100644 --- a/ui/app/components/dashboard/overview.hbs +++ b/ui/app/components/dashboard/overview.hbs @@ -7,10 +7,10 @@
- {{#if (and @version.isEnterprise (or @license @isRootNamespace))}} + {{#if (and @version.isEnterprise @isRootNamespace)}}
- {{#if @license}} - + {{#if @version.isEnterprise}} + {{/if}} {{#if (and @isRootNamespace (has-permission "status" routeParams="replication") (not (is-empty-value @replication))) diff --git a/ui/app/routes/vault/cluster/dashboard.js b/ui/app/routes/vault/cluster/dashboard.js index a8af612f1720..c7871ab2cdda 100644 --- a/ui/app/routes/vault/cluster/dashboard.js +++ b/ui/app/routes/vault/cluster/dashboard.js @@ -40,7 +40,6 @@ export default class VaultClusterDashboardRoute extends Route.extend(ClusterRout return hash({ replication, secretsEngines: this.store.query('secret-engine', {}), - license: this.store.queryRecord('license', {}).catch(() => null), isRootNamespace: this.namespace.inRootNamespace && !hasChroot, version: this.version, vaultConfiguration: hasChroot ? null : this.getVaultConfiguration(), diff --git a/ui/app/templates/vault/cluster/dashboard.hbs b/ui/app/templates/vault/cluster/dashboard.hbs index b7f212674a0d..52ce5adcf197 100644 --- a/ui/app/templates/vault/cluster/dashboard.hbs +++ b/ui/app/templates/vault/cluster/dashboard.hbs @@ -6,7 +6,6 @@ Date: Wed, 1 May 2024 09:59:14 +0100 Subject: [PATCH 02/12] cleanup jsdoc --- ui/app/components/dashboard/client-count-card.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ui/app/components/dashboard/client-count-card.js b/ui/app/components/dashboard/client-count-card.js index c0c3dc122768..d6dcc8c621a0 100644 --- a/ui/app/components/dashboard/client-count-card.js +++ b/ui/app/components/dashboard/client-count-card.js @@ -16,10 +16,9 @@ import getUnixTime from 'date-fns/getUnixTime'; * DashboardClientCountCard component are used to display total and new client count information * * @example - * ```js - * - * ``` - * @param {object} license - license object passed from the parent + * + * + * */ export default class DashboardClientCountCard extends Component { From 80dd3ed1f96c691a5b7ba5f369cbfd72868806e4 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Wed, 1 May 2024 11:09:42 +0100 Subject: [PATCH 03/12] add changelog --- changelog/26729.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/26729.txt diff --git a/changelog/26729.txt b/changelog/26729.txt new file mode 100644 index 000000000000..4277b50a98ef --- /dev/null +++ b/changelog/26729.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui (enterprise): Update dashboard to make activity log query using the same start time as the metrics overview +``` \ No newline at end of file From 5353fe42e305554e7fafd352886bd1afcafc8cb9 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Wed, 1 May 2024 11:31:36 +0100 Subject: [PATCH 04/12] use helper to set start time --- .../components/dashboard/client-count-card.js | 4 ++-- ui/app/components/dashboard/overview.hbs | 4 +--- ui/app/routes/vault/cluster/clients/counts.ts | 14 ++++---------- ui/lib/core/addon/utils/client-count-utils.ts | 18 +++++++++++++++++- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/ui/app/components/dashboard/client-count-card.js b/ui/app/components/dashboard/client-count-card.js index d6dcc8c621a0..2dfc1f677d47 100644 --- a/ui/app/components/dashboard/client-count-card.js +++ b/ui/app/components/dashboard/client-count-card.js @@ -9,7 +9,7 @@ import { task } from 'ember-concurrency'; import { waitFor } from '@ember/test-waiters'; import { tracked } from '@glimmer/tracking'; import { service } from '@ember/service'; -import getUnixTime from 'date-fns/getUnixTime'; +import { setStartTimeQuery } from 'core/utils/client-count-utils'; /** * @module DashboardClientCountCard @@ -47,7 +47,7 @@ export default class DashboardClientCountCard extends Component { if (!this.clientConfig) { // set config and license start time when component initializes this.clientConfig = yield this.store.queryRecord('clients/config', {}).catch(() => {}); - this.licenseStartTime = getUnixTime(this.clientConfig.billingStartTimestamp); + this.licenseStartTime = setStartTimeQuery(this.args.isEnterprise, this.clientConfig); } // only make the network request if we have a start_time diff --git a/ui/app/components/dashboard/overview.hbs b/ui/app/components/dashboard/overview.hbs index 2244b0ac3c2b..f2cdf2695fdc 100644 --- a/ui/app/components/dashboard/overview.hbs +++ b/ui/app/components/dashboard/overview.hbs @@ -9,9 +9,7 @@
{{#if (and @version.isEnterprise @isRootNamespace)}}
- {{#if @version.isEnterprise}} - - {{/if}} + {{#if (and @isRootNamespace (has-permission "status" routeParams="replication") (not (is-empty-value @replication))) }} diff --git a/ui/app/routes/vault/cluster/clients/counts.ts b/ui/app/routes/vault/cluster/clients/counts.ts index 54012bb6cdc4..43c052e959c6 100644 --- a/ui/app/routes/vault/cluster/clients/counts.ts +++ b/ui/app/routes/vault/cluster/clients/counts.ts @@ -10,13 +10,14 @@ import timestamp from 'core/utils/timestamp'; import { getUnixTime } from 'date-fns'; import type StoreService from 'vault/services/store'; -import type VersionService from 'vault/services/version'; +import VersionService from 'vault/services/version'; import type { ModelFrom } from 'vault/vault/route'; import type ClientsRoute from '../clients'; -import type ClientsConfigModel from 'vault/models/clients/config'; import type ClientsActivityModel from 'vault/models/clients/activity'; import type ClientsCountsController from 'vault/controllers/vault/cluster/clients/counts'; +import { setStartTimeQuery } from 'core/utils/client-count-utils'; + export interface ClientsCountsRouteParams { start_time?: string | number | undefined; end_time?: string | number | undefined; @@ -86,10 +87,7 @@ export default class ClientsCountsRoute extends Route { async model(params: ClientsCountsRouteParams) { const { config, versionHistory } = this.modelFor('vault.cluster.clients') as ModelFrom; // only enterprise versions will have a relevant billing start date, if null users must select initial start time - let startTime = null; - if (this.version.isEnterprise && this._hasConfig(config)) { - startTime = getUnixTime(config.billingStartTimestamp); - } + const startTime = setStartTimeQuery(this.version.isEnterprise, config); const startTimestamp = Number(params.start_time) || startTime; const endTimestamp = Number(params.end_time) || getUnixTime(timestamp.now()); @@ -118,8 +116,4 @@ export default class ClientsCountsRoute extends Route { }); } } - - _hasConfig(model: ClientsConfigModel | object): model is ClientsConfigModel { - return 'billingStartTimestamp' in model; - } } diff --git a/ui/lib/core/addon/utils/client-count-utils.ts b/ui/lib/core/addon/utils/client-count-utils.ts index 54248c6d4fd3..614747c11856 100644 --- a/ui/lib/core/addon/utils/client-count-utils.ts +++ b/ui/lib/core/addon/utils/client-count-utils.ts @@ -6,6 +6,7 @@ import { parseAPITimestamp } from 'core/utils/date-formatters'; import { compareAsc, getUnixTime, isWithinInterval } from 'date-fns'; +import type ClientsConfigModel from 'vault/models/clients/config'; import type ClientsVersionHistoryModel from 'vault/vault/models/clients/version-history'; /* @@ -59,6 +60,18 @@ export const filterVersionHistory = ( return []; }; +export const setStartTimeQuery = ( + isEnterprise: boolean, + config: ClientsConfigModel | Record +) => { + // CE versions have no license and so the start time defaults to "0001-01-01T00:00:00Z" + if (isEnterprise && _hasConfig(config)) { + return getUnixTime(config.billingStartTimestamp); + } + return null; +}; + +// METHODS FOR SERIALIZING ACTIVITY RESPONSE export const formatDateObject = (dateObj: { monthIdx: number; year: number }, isEnd: boolean) => { const { year, monthIdx } = dateObj; // day=0 for Date.UTC() returns the last day of the month before @@ -188,6 +201,10 @@ export const namespaceArrayToObject = ( }; // type guards for conditionals +function _hasConfig(model: ClientsConfigModel | object): model is ClientsConfigModel { + return 'billingStartTimestamp' in model; +} + export function hasMountsKey( obj: ByMonthNewClients | NamespaceNewClients | MountNewClients ): obj is NamespaceNewClients { @@ -201,7 +218,6 @@ export function hasNamespacesKey( } // TYPES RETURNED BY UTILS (serialized) - export interface TotalClients { clients: number; entity_clients: number; From 65421074568c0a21c98761c005646976e4b8e2fb Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Wed, 1 May 2024 16:58:28 +0100 Subject: [PATCH 05/12] update component tests --- .../dashboard/client-count-card.hbs | 19 +---- .../components/dashboard/client-count-card.js | 16 +++- .../dashboard/client-count-card-test.js | 74 ++++++++++++++----- 3 files changed, 71 insertions(+), 38 deletions(-) diff --git a/ui/app/components/dashboard/client-count-card.hbs b/ui/app/components/dashboard/client-count-card.hbs index e8a0f1ee97d5..3690e19ab0e2 100644 --- a/ui/app/components/dashboard/client-count-card.hbs +++ b/ui/app/components/dashboard/client-count-card.hbs @@ -23,23 +23,8 @@ {{else}}
- - + +
diff --git a/ui/app/components/dashboard/client-count-card.js b/ui/app/components/dashboard/client-count-card.js index 2dfc1f677d47..e9934acbd6b7 100644 --- a/ui/app/components/dashboard/client-count-card.js +++ b/ui/app/components/dashboard/client-count-card.js @@ -10,6 +10,7 @@ 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'; /** * @module DashboardClientCountCard @@ -17,8 +18,9 @@ import { setStartTimeQuery } from 'core/utils/client-count-utils'; * * @example * - * + * * + * @param {boolean} isEnterprise - used for setting the start time for the activity log query */ export default class DashboardClientCountCard extends Component { @@ -38,6 +40,18 @@ export default class DashboardClientCountCard extends Component { return this.activityData?.byMonth?.lastObject?.new_clients.clients; } + get statSubText() { + const format = (date) => dateFormat([date, 'MMM yyyy'], {}); + return this.licenseStartTime + ? { + total: `The number of clients in this billing period (${format(this.licenseStartTime)} - ${format( + this.updatedAt + )}).`, + new: 'The number of clients new to Vault in the current month.', + } + : { total: 'No total client data available.', new: 'No new client data available.' }; + } + @task @waitFor *fetchClientActivity(e) { 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 e3e6445f5716..20ff111ae745 100644 --- a/ui/tests/integration/components/dashboard/client-count-card-test.js +++ b/ui/tests/integration/components/dashboard/client-count-card-test.js @@ -13,6 +13,7 @@ import { LICENSE_START, 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'; module('Integration | Component | dashboard/client-count-card', function (hooks) { setupRenderingTest(hooks); @@ -22,18 +23,12 @@ module('Integration | Component | dashboard/client-count-card', function (hooks) sinon.stub(timestamp, 'now').callsFake(() => STATIC_NOW); }); - hooks.beforeEach(function () { - this.license = { - startTime: LICENSE_START.toISOString(), - }; - }); - hooks.after(function () { timestamp.now.restore(); }); test('it should display client count information', async function (assert) { - assert.expect(9); + assert.expect(6); const { months, total } = ACTIVITY_RESPONSE_STUB; const [latestMonth] = months.slice(-1); this.server.get('sys/internal/counters/activity', () => { @@ -44,24 +39,63 @@ module('Integration | Component | dashboard/client-count-card', function (hooks) 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('[data-test-stat-text="total-clients"] .stat-label').hasText('Total'); - assert - .dom('[data-test-stat-text="total-clients"] .stat-text') - .hasText('The number of clients in this billing period (Jul 2023 - Jan 2024).'); assert - .dom('[data-test-stat-text="total-clients"] .stat-value') - .hasText(`${formatNumber([total.clients])}`); - assert.dom('[data-test-stat-text="new-clients"] .stat-label').hasText('New'); - assert - .dom('[data-test-stat-text="new-clients"] .stat-text') - .hasText('The number of clients new to Vault in the current month.'); + .dom(CLIENT_COUNT.statText('Total')) + .hasText( + `Total The number of clients in this billing period (Jul 2023 - Jan 2024). ${formatNumber([ + total.clients, + ])}` + ); + assert - .dom('[data-test-stat-text="new-clients"] .stat-value') - .hasText(`${formatNumber([latestMonth.new_clients.counts.clients])}`); + .dom(CLIENT_COUNT.statText('New')) + .hasText( + `New The number of clients new to Vault in the current month. ${formatNumber([ + latestMonth.new_clients.counts.clients, + ])}` + ); + // fires second request to /activity await click('[data-test-refresh]'); }); + + test('it does not query activity for community edition', async function (assert) { + assert.expect(4); + this.server.get('sys/internal/counters/activity', () => { + // this assertion should NOT be hit in this test + assert.true(true, 'uh oh! makes request to sys/internal/counters/activity'); + 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: '0001-01-01T00:00:00Z', + }, + }; + }); + + 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]'); + }); }); From 0d645e07ae7ff59cad67eb471306d49c9d645439 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Wed, 1 May 2024 17:05:48 +0100 Subject: [PATCH 06/12] update overview test --- .../components/dashboard/overview-test.js | 49 +++++-------------- 1 file changed, 13 insertions(+), 36 deletions(-) diff --git a/ui/tests/integration/components/dashboard/overview-test.js b/ui/tests/integration/components/dashboard/overview-test.js index 6670e4d2ea4a..51d9792727f7 100644 --- a/ui/tests/integration/components/dashboard/overview-test.js +++ b/ui/tests/integration/components/dashboard/overview-test.js @@ -9,6 +9,7 @@ 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); @@ -60,6 +61,14 @@ 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) { @@ -129,7 +138,7 @@ module('Integration | Component | dashboard/overview', function (hooks) { @replication={{this.replication}} @version={{this.version}} @isRootNamespace={{true}} - @license={{this.license}} + @isEnterprise={{this.version.isEnterprise}} @refreshModel={{this.refreshModel}} />` ); assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); @@ -140,43 +149,11 @@ module('Integration | Component | dashboard/overview', function (hooks) { assert.dom(DASHBOARD.cardName('client-count')).exists(); }); - test('it should hide client count on enterprise w/o license ', async function (assert) { - this.version = this.owner.lookup('service:version'); - this.version.version = '1.13.1+ent'; - this.version.type = 'enterprise'; - this.isRootNamespace = true; - - await render( - hbs` - ` - ); - - 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('client-count')).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; - this.license = { - autoloaded: { - license_id: '7adbf1f4-56ef-35cd-3a6c-50ef2627865d', - }, - }; await render( hbs` @@ -186,7 +163,7 @@ module('Integration | Component | dashboard/overview', function (hooks) { @secretsEngines={{this.secretsEngines}} @vaultConfiguration={{this.vaultConfiguration}} @replication={{this.replication}} - @license={{this.license}} + @isEnterprise={{this.version.isEnterprise}} @refreshModel={{this.refreshModel}} />` ); @@ -197,7 +174,7 @@ module('Integration | Component | dashboard/overview', function (hooks) { 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')).exists(); + assert.dom(DASHBOARD.cardName('client-count')).doesNotExist(); }); module('learn more card', function () { @@ -238,7 +215,7 @@ module('Integration | Component | dashboard/overview', function (hooks) { Date: Wed, 1 May 2024 17:17:14 +0100 Subject: [PATCH 07/12] update util tests --- .../utils/client-count-utils-test.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/ui/tests/integration/utils/client-count-utils-test.js b/ui/tests/integration/utils/client-count-utils-test.js index a838924a9bce..0debd172d5d1 100644 --- a/ui/tests/integration/utils/client-count-utils-test.js +++ b/ui/tests/integration/utils/client-count-utils-test.js @@ -12,6 +12,7 @@ import { destructureClientCounts, namespaceArrayToObject, sortMonthsByTimestamp, + setStartTimeQuery, } from 'core/utils/client-count-utils'; import { LICENSE_START } from 'vault/mirage/handlers/clients'; import { @@ -372,4 +373,30 @@ module('Integration | Util | client count utils', function (hooks) { 'it formats combined data for monthly namespaces_by_key spanning upgrade to 1.10' ); }); + + test('setStartTimeQuery: it returns start time query for activity log', async function (assert) { + assert.expect(5); + const apiPath = 'sys/internal/counters/config'; + assert.strictEqual(setStartTimeQuery(true, {}), null, `it returns null if no permission to ${apiPath}`); + assert.strictEqual( + setStartTimeQuery(false, {}), + null, + `it returns null for community edition and no permission to ${apiPath}` + ); + assert.strictEqual( + setStartTimeQuery(true, { billingStartTimestamp: new Date('2022-06-08T00:00:00Z') }), + 1654646400, + 'it returns unix time if enterprise and billing_start_timestamp exists' + ); + assert.strictEqual( + setStartTimeQuery(false, { billingStartTimestamp: new Date('0001-01-01T00:00:00Z') }), + null, + 'it returns null time for community edition even if billing_start_timestamp exists' + ); + assert.strictEqual( + setStartTimeQuery(false, { foo: 'bar' }), + null, + 'it returns null if billing_start_timestamp key does not exist' + ); + }); }); From 3041deae20693e27323de3522897784bb99ec65d Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 2 May 2024 10:18:59 +0100 Subject: [PATCH 08/12] throw error instead, add comment to util file --- .../dashboard/client-count-card-test.js | 17 ++++++++--------- .../utils/client-count-utils-test.js | 3 +++ 2 files changed, 11 insertions(+), 9 deletions(-) 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 20ff111ae745..0cc75566de04 100644 --- a/ui/tests/integration/components/dashboard/client-count-card-test.js +++ b/ui/tests/integration/components/dashboard/client-count-card-test.js @@ -72,15 +72,14 @@ module('Integration | Component | dashboard/client-count-card', function (hooks) }); test('it does not query activity for community edition', async function (assert) { - assert.expect(4); - this.server.get('sys/internal/counters/activity', () => { - // this assertion should NOT be hit in this test - assert.true(true, 'uh oh! makes request to sys/internal/counters/activity'); - return { - request_id: 'some-activity-id', - data: ACTIVITY_RESPONSE_STUB, - }; - }); + 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'); return { diff --git a/ui/tests/integration/utils/client-count-utils-test.js b/ui/tests/integration/utils/client-count-utils-test.js index 0debd172d5d1..5812ba7d64c3 100644 --- a/ui/tests/integration/utils/client-count-utils-test.js +++ b/ui/tests/integration/utils/client-count-utils-test.js @@ -28,6 +28,9 @@ used to normalize the sys/counters/activity response in the clients/activity serializer. these functions are tested individually here, instead of all at once in a serializer test for easier debugging */ + +// TODO refactor tests into a module for each util method, then make each assertion its separate tests + module('Integration | Util | client count utils', function (hooks) { setupTest(hooks); From 2364f05f2930e454f4f58b12f471594449f2b211 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 2 May 2024 10:36:02 +0100 Subject: [PATCH 09/12] fix accidentally removed type from import --- ui/app/routes/vault/cluster/clients/counts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/routes/vault/cluster/clients/counts.ts b/ui/app/routes/vault/cluster/clients/counts.ts index 43c052e959c6..e5be73f97e67 100644 --- a/ui/app/routes/vault/cluster/clients/counts.ts +++ b/ui/app/routes/vault/cluster/clients/counts.ts @@ -10,7 +10,7 @@ import timestamp from 'core/utils/timestamp'; import { getUnixTime } from 'date-fns'; import type StoreService from 'vault/services/store'; -import VersionService from 'vault/services/version'; +import type VersionService from 'vault/services/version'; import type { ModelFrom } from 'vault/vault/route'; import type ClientsRoute from '../clients'; From a5753a44b26124375ffc06801553506413095c34 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 2 May 2024 10:41:23 +0100 Subject: [PATCH 10/12] remove typo arg from test component --- ui/tests/integration/components/dashboard/overview-test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/ui/tests/integration/components/dashboard/overview-test.js b/ui/tests/integration/components/dashboard/overview-test.js index 51d9792727f7..a367e1c2813f 100644 --- a/ui/tests/integration/components/dashboard/overview-test.js +++ b/ui/tests/integration/components/dashboard/overview-test.js @@ -138,7 +138,6 @@ module('Integration | Component | dashboard/overview', function (hooks) { @replication={{this.replication}} @version={{this.version}} @isRootNamespace={{true}} - @isEnterprise={{this.version.isEnterprise}} @refreshModel={{this.refreshModel}} />` ); assert.dom(DASHBOARD.cardHeader('Vault version')).exists(); @@ -163,7 +162,6 @@ module('Integration | Component | dashboard/overview', function (hooks) { @secretsEngines={{this.secretsEngines}} @vaultConfiguration={{this.vaultConfiguration}} @replication={{this.replication}} - @isEnterprise={{this.version.isEnterprise}} @refreshModel={{this.refreshModel}} />` ); @@ -215,7 +213,6 @@ module('Integration | Component | dashboard/overview', function (hooks) { Date: Thu, 2 May 2024 14:26:48 +0100 Subject: [PATCH 11/12] rename token stat getter to avoid future typos --- ui/app/components/clients/page/token.hbs | 8 ++++---- ui/app/components/clients/page/token.ts | 4 ++-- .../components/clients/page/token-test.js | 12 ++++++++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/ui/app/components/clients/page/token.hbs b/ui/app/components/clients/page/token.hbs index c386cf965f04..198fd78783f0 100644 --- a/ui/app/components/clients/page/token.hbs +++ b/ui/app/components/clients/page/token.hbs @@ -15,7 +15,7 @@ @@ -100,21 +100,21 @@ diff --git a/ui/app/components/clients/page/token.ts b/ui/app/components/clients/page/token.ts index 4d653ec3eff9..716e32943e26 100644 --- a/ui/app/components/clients/page/token.ts +++ b/ui/app/components/clients/page/token.ts @@ -37,11 +37,11 @@ export default class ClientsTokenPageComponent extends ActivityComponent { return this.calculateClientAverages(this.byMonthNewClients); } - get tokenUsageCounts() { + get tokenStats() { if (this.totalUsageCounts) { const { entity_clients, non_entity_clients } = this.totalUsageCounts; return { - clients: entity_clients + non_entity_clients, + total: entity_clients + non_entity_clients, entity_clients, non_entity_clients, }; diff --git a/ui/tests/integration/components/clients/page/token-test.js b/ui/tests/integration/components/clients/page/token-test.js index a0553ac11f62..60e562282adf 100644 --- a/ui/tests/integration/components/clients/page/token-test.js +++ b/ui/tests/integration/components/clients/page/token-test.js @@ -69,21 +69,25 @@ module('Integration | Component | clients | Clients::Page::Token', function (hoo test('it should render monthly total chart', async function (assert) { const count = this.activity.byMonth.length; - assert.expect(count + 7); + const { entity_clients, non_entity_clients } = this.activity.total; + assert.expect(count + 8); const getAverage = (data) => { const average = ['entity_clients', 'non_entity_clients'].reduce((count, key) => { return (count += calculateAverage(data, key) || 0); }, 0); return formatNumber([average]); }; - const expectedTotal = getAverage(this.activity.byMonth); + const expectedAvg = getAverage(this.activity.byMonth); + const expectedTotal = formatNumber([entity_clients + non_entity_clients]); const chart = CHARTS.container('Entity/Non-entity clients usage'); - await this.renderComponent(); assert - .dom(CLIENT_COUNT.statTextValue('Average total clients per month')) + .dom(CLIENT_COUNT.statTextValue('Total clients')) .hasText(expectedTotal, 'renders correct total clients'); + assert + .dom(CLIENT_COUNT.statTextValue('Average total clients per month')) + .hasText(expectedAvg, 'renders correct average clients'); // assert bar chart is correct assert.dom(`${chart} ${CHARTS.xAxis}`).hasText('7/23 8/23 9/23 10/23 11/23 12/23 1/24'); From d3fefc494dd66dc8503357fd2dda0651d49be9ad Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 2 May 2024 18:35:28 +0100 Subject: [PATCH 12/12] fix tests, add fallback for util --- ui/lib/core/addon/utils/client-count-utils.ts | 1 + ui/tests/acceptance/dashboard-test.js | 12 +++++------- .../integration/utils/client-count-utils-test.js | 7 ++++++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/ui/lib/core/addon/utils/client-count-utils.ts b/ui/lib/core/addon/utils/client-count-utils.ts index 614747c11856..3d56fa64f7f9 100644 --- a/ui/lib/core/addon/utils/client-count-utils.ts +++ b/ui/lib/core/addon/utils/client-count-utils.ts @@ -202,6 +202,7 @@ export const namespaceArrayToObject = ( // type guards for conditionals function _hasConfig(model: ClientsConfigModel | object): model is ClientsConfigModel { + if (!model) return false; return 'billingStartTimestamp' in model; } diff --git a/ui/tests/acceptance/dashboard-test.js b/ui/tests/acceptance/dashboard-test.js index acdb8e984c20..e3753c4deea1 100644 --- a/ui/tests/acceptance/dashboard-test.js +++ b/ui/tests/acceptance/dashboard-test.js @@ -404,16 +404,14 @@ module('Acceptance | landing page dashboard', function (hooks) { assert.dom(DASHBOARD.cardName('client-count')).exists(); const response = await this.store.peekRecord('clients/activity', 'some-activity-id'); assert.dom('[data-test-client-count-title]').hasText('Client count'); - assert.dom('[data-test-stat-text="total-clients"] .stat-label').hasText('Total'); + assert.dom('[data-test-stat-text="Total"] .stat-label').hasText('Total'); + assert.dom('[data-test-stat-text="Total"] .stat-value').hasText(formatNumber([response.total.clients])); + assert.dom('[data-test-stat-text="New"] .stat-label').hasText('New'); assert - .dom('[data-test-stat-text="total-clients"] .stat-value') - .hasText(formatNumber([response.total.clients])); - assert.dom('[data-test-stat-text="new-clients"] .stat-label').hasText('New'); - assert - .dom('[data-test-stat-text="new-clients"] .stat-text') + .dom('[data-test-stat-text="New"] .stat-text') .hasText('The number of clients new to Vault in the current month.'); assert - .dom('[data-test-stat-text="new-clients"] .stat-value') + .dom('[data-test-stat-text="New"] .stat-value') .hasText(formatNumber([response.byMonth.lastObject.new_clients.clients])); }); }); diff --git a/ui/tests/integration/utils/client-count-utils-test.js b/ui/tests/integration/utils/client-count-utils-test.js index 5812ba7d64c3..d56a37a1a5b9 100644 --- a/ui/tests/integration/utils/client-count-utils-test.js +++ b/ui/tests/integration/utils/client-count-utils-test.js @@ -378,7 +378,7 @@ module('Integration | Util | client count utils', function (hooks) { }); test('setStartTimeQuery: it returns start time query for activity log', async function (assert) { - assert.expect(5); + assert.expect(6); const apiPath = 'sys/internal/counters/config'; assert.strictEqual(setStartTimeQuery(true, {}), null, `it returns null if no permission to ${apiPath}`); assert.strictEqual( @@ -401,5 +401,10 @@ module('Integration | Util | client count utils', function (hooks) { null, 'it returns null if billing_start_timestamp key does not exist' ); + assert.strictEqual( + setStartTimeQuery(false, undefined), + null, + 'fails gracefully if no config model is passed' + ); }); });