UI: Replace Client Count Attribution Charts with Table #30678

lane-wetmore · 2025-05-19T21:49:59Z

Description

This PR removes the mount attribution and namespace attribution charts from the overview tab. They are replaced by a table which breaks down attribution according to a selected month.

The month filter options are the months in the current billing period.
Tests for the removed charts have been removed.
Tests have been added for the new attribution table.
The table updates when a namespace filter is applied. (same behavior as previous charts)
The table is hidden when a mount path filter is applied. (same behavior as previous charts)
The table is default sorted by clients column descending and supports sorting across all columns.

Ent tests: ✅

Table state w/ no month selection:

Table state w/ a month selected that has no data:

Table state w/ a month selected that has data:

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 · 2025-05-19T21:53:56Z

Build Results:
All builds succeeded! ✅

github-actions · 2025-05-19T22:14:10Z

CI Results:
All Go tests succeeded! ✅

…esting for old attribution charts

didoo · 2025-05-21T15:57:31Z

ui/app/components/clients/page/overview.hbs

+      </:body>
+    </Hds::Table>
+
+    <Hds::Pagination::Numbered


[suggestion] according to Helios guidance there should be a margin of 16px between the table and the pagination: https://helios.hashicorp.design/components/pagination#spacing

Good catch! I've updated the margin accordingly 😄

hellobontempo

Amazing job tackling this! So cool this will make it into the release. I definitely think moving this to a child component makes sense here. I went back and forth on the query param, but ultimately think it overcomplicates the logic a little more than necessary. Let me know if you want to discuss the approach 😄

Really great job building client count context and addressing these improvements ✨

hellobontempo · 2025-05-21T16:10:12Z

ui/app/components/clients/page/overview.hbs

+          <B.Td>{{B.data.namespace}}</B.Td>
+          <B.Td>{{B.data.label}}</B.Td>
+          {{#if (eq B.data.mount_type "deleted mount")}}
+            <B.Td><Hds::Badge @text="Disabled" @type="outlined" /></B.Td>


I think there was discussion to update the display text to "Deleted" to clarify what disabled means in the api context

hellobontempo · 2025-05-21T18:04:34Z

changelog/30678.txt

@@ -0,0 +1,3 @@
+```release-note:change
+ui: (Client Counts) The mount attribution and namespace attribution charts have been replaced in the overview tab with a table that breaks down client attribution according to month selection.


Can we use the same language and formatting as backend changelog entries (example) to denote which aspect of Vault this applies to? ui/activity instead of ui: (Client Counts)

Also a slight rewording suggestion to explain in a little more detail what the table functionality includes.

ui/activity: Replaces mount and namespace attribution charts with a table to allow sorting client count data by `namespace`, `mount_path`, `mount_type` or number of clients for a selected month.

hellobontempo · 2025-05-21T18:05:52Z

ui/app/components/clients/date-range.ts

@@ -142,9 +138,11 @@ export default class ClientsDateRangeComponent extends Component<Args> {
  // used for CE date picker
  @action handleSave() {
    if (this.validationError) return;
+    // including the month param here as undefined allows the table month selection to reset when a new billing period is chosen


It seems a little strange to do this here, since this is a query param it should probably be reset when the route refreshes

ui/app/components/clients/page/overview.ts

hellobontempo · 2025-05-21T18:16:22Z

ui/app/components/clients/page/overview.hbs

+      }}
+      @sortBy="clients"
+      @sortOrder="desc"
+      @onSort={{this.updateSort}}


Is a custom sort function necessary? All of these columns seem like they can either be sortable alphabetically or numerically (docs).

If this is just for the deleted status...I'd consider disregarding that since they'll technically still be grouped together. I think a follow-on improvement would be eventually removing isSortable for mount_type all together and instead adding that as a filter option for the table data. That way users could filter by a certain type, or filter out deleted mounts. (Since sorting doesn't really seem useful for that data key).

I added the custom sort here to support paginating the data. Otherwise, with the table native sort, it would only sort the current page. I'll see if I can rework the pagination approach so that we can remove this sort function.

I didn't end up seeing a way to support pagination and sorting, without the extra custom sort function.

Yeah, HDS table sorts only the visible rows, not the entire dataset. FYI there are UX implications when sorting the entire dataset for a paginated table, even more if it's filtered, and we wanted to stay out of this and let the consumers handle this kind of logic in the context of what they need/know.

hellobontempo · 2025-05-21T18:18:41Z

ui/app/controllers/vault/cluster/clients/counts.ts

 export default class ClientsCountsController extends Controller {
  queryParams = queryParamKeys;

  start_time: string | number | undefined = undefined;
  end_time: string | number | undefined = undefined;
  ns: string | undefined = undefined;
  mountPath: string | undefined = undefined;
+  month: string | undefined = undefined;


Normally I would say that using a query param here is the right move because it's filtering client-side data, however it's a little confusing since the month only applies to the table and not the entire page. I think it'd make sense to put here if the route data was going to be filtered by that month, but since it's just the table - I think this should move down to the component and just be managed there 🤔

hellobontempo · 2025-05-21T18:27:17Z

ui/app/components/clients/page/overview.hbs

+      {{/each}}
+    </S.Options>
+  </Hds::Form::Select::Base>
+  {{#if this.tableData}}


Could this logic be pulled into a separate component, something like Clients::Table? That would make the template easier to parse and separate concerns a little more 🤔 In the future it could be abstracted to accept an arg for column keys, but I think for now most of this could be moved 1 to 1.

Per my suggestion below to remove the query param, this could then be simplified to just check for whether or not a month has been selected in the parent. The child component would then show the table or empty state accordingly

{{! Clients::Table component }} {{#if @filteredMonth}} {{! HDS Table }} {{else}} {{! Empty state }} {{/if}}

lane-wetmore · 2025-05-21T21:04:17Z

ui/app/components/clients/table.ts

+      : 'View the namespace mount breakdown of clients by selecting a month.';
+  }
+
+  sortTableData(data: TableData[]): TableData[] {


The table can sort these for us, but then we don't have access to the sorted data to subsequently paginate. This sort function allows us to sort the whole table and then paginate off of that.

That makes sense! I figured the pagination would add some fun to this 😅

hellobontempo

Just a few more cleanup items! Can the clients/attribution component also be deleted? I don't think it's used elsewhere. It would be good to add tests for the new table component, but that can be a follow-on since I know we want to get this in before feature freeze.

Accurately updating the clients response and adding a test can happen in follow-ons, but it'd be great to cleanup the remaining tidy items from removing the query param before merging. Great work!

hellobontempo · 2025-05-22T16:15:38Z

ui/app/components/clients/page/overview.hbs

-      @isSecretsSyncActivated={{this.flags.secretsSyncIsActivated}}
-    />
-  {{/if}}
+  <Hds::Form::Select::Base


Much cleaner and easier to follow! 🎉

hellobontempo · 2025-05-22T16:40:52Z

ui/app/components/clients/page/overview.ts

+import { HTMLElementEvent } from 'vault/forms';
+import { parseAPITimestamp } from 'core/utils/date-formatters';
+import { MountClients } from 'core/utils/client-count-utils';
+import RouterService from '@ember/routing/router-service';


nit: I for any types imports can type be included? I also think it's nice to group them together, so all services are together

Suggested change

import RouterService from '@ember/routing/router-service';

import type RouterService from '@ember/routing/router-service';

hellobontempo · 2025-05-22T16:42:49Z

ui/app/components/clients/table.ts

+      : 'View the namespace mount breakdown of clients by selecting a month.';
+  }
+
+  sortTableData(data: TableData[]): TableData[] {


That makes sense! I figured the pagination would add some fun to this 😅

hellobontempo · 2025-05-22T16:51:14Z

ui/app/components/clients/page/overview.ts

+    namespaces?.forEach((n) => {
+      const mounts: TableData[] = n.mounts.map((m) => {
+        // add namespace to mount block
+        return { ...m, namespace: n.label };
+      });
+      data = [...data, ...mounts];
+    });


technically this sort of data manipulation should happen in the serializer, in the client-count-utils (And this getter would just select the month). I know it was here for the POC pr 🙃 - is it a lot of lift to move? That can happen after feature freeze if that's easiest.

hellobontempo · 2025-05-22T16:55:30Z

ui/app/components/clients/table.ts

+  }
+
+  get tableHeaderMessage(): string {
+    return this.args.filteredMonth


It looks like filteredMonth could be removed all together and this could be updated to this.args.data.length?

hellobontempo · 2025-05-22T16:56:18Z

ui/app/components/clients/page/overview.hbs

+      {{/each}}
+    </S.Options>
+  </Hds::Form::Select::Base>
+  <Clients::Table @data={{this.tableData}} @filteredMonth={{this.selectedMonth}} />


Looking at the component - could filteredMonth be removed and the component just check for @data?

hellobontempo · 2025-05-22T16:57:02Z

ui/app/templates/vault/cluster/clients/counts/overview.hbs

@@ -10,4 +10,5 @@
  @endTimestamp={{this.model.endTimestamp}}
  @namespace={{this.countsController.ns}}
  @mountPath={{this.countsController.mountPath}}
+  @updateQueryParams={{this.countsController.updateQueryParams}}


Can this be deleted?

hellobontempo · 2025-05-22T17:01:44Z

ui/lib/core/addon/utils/client-count-utils.ts

+export interface OnChangeParams {
+  start_time?: number | undefined;
+  end_time?: number | undefined;
+}


Since this is no longer used other places, I wonder if moving it back to the component makes the most sense 🤔

hellobontempo · 2025-05-22T17:05:52Z

ui/tests/integration/components/clients/page/overview-test.js

    await render(
-      hbs`<Clients::Page::Overview @activity={{this.activity}} @namespace="ns1" @mountPath={{this.mountPath}} />`
+      hbs`<Clients::Page::Overview @activity={{this.activity}} @updateQueryParams={{this.onChange}}/>`


updateQueryParams can this arg be removed from all the tests?

hellobontempo · 2025-05-22T17:28:02Z

ui/tests/helpers/clients/client-count-helpers.js

@@ -56,6 +57,7 @@ export const ACTIVITY_RESPONSE_STUB = {
        },
        {
          mount_path: 'kvv2-engine-0',
+          mount_type: '',


for testing accuracy, it'd be good to update these to what they actually are instead of empty strings.

ui/app/components/clients/table.ts

hellobontempo · 2025-05-22T23:32:30Z

ui/lib/core/addon/utils/client-count-utils.ts

-      mounts = ns.mounts.map((m) => ({ label: m.mount_path, ...destructureClientCounts(m.counts) }));
+      mounts = ns.mounts.map((m) => ({
+        label: m.mount_path,
+        mount_type: m.mount_type || '',


the || may not be necessary, since it looks like the API should return a placeholder string if it's empty https://github.com/hashicorp/vault/pull/30071/files#diff-9617be4433471ef034fc9f806398327dcf6241dd6945bdd7a61202193179f922R421-R422

Oh, good find!

hellobontempo

🚀 Can tests be added in a follow-on?

It'd be great to make sure the table renders as expected, sorts and paginates. Plus that the empty state views show as expected 😄

lane-wetmore · 2025-05-23T14:14:19Z

🚀 Can tests be added in a follow-on?

It'd be great to make sure the table renders as expected, sorts and paginates. Plus that the empty state views show as expected 😄

There are currently some table tests in ui/tests/integration/components/clients/page/overview-test.js, but I don't think sorting is currently tested. So that one (plus any others that come up) can definitely be added. @hellobontempo

…th-Table

* replace attribution charts with a table by month * update tests to include mount_type * fix another portion of tests that were missing secret sync stat and testing for old attribution charts * add tests for attribution table * add changelog and tidy * remove remaining todos * tidy * reset month query param in ce * fix tests missing month param * add margin to pagination in accordance to helios rec * remove query param, update change log, move table into own comp * remove commented code * remove month query params * tidy * update test mount paths * remove unused client attribution component * update tests

replace attribution charts with a table by month

bfc23f8

lane-wetmore requested a review from a team as a code owner May 19, 2025 21:49

lane-wetmore added this to the 1.20.0-rc milestone May 19, 2025

lane-wetmore added the ui label May 19, 2025

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label May 19, 2025

lane-wetmore added 4 commits May 19, 2025 19:12

update tests to include mount_type

72150bd

fix another portion of tests that were missing secret sync stat and t…

91eddff

…esting for old attribution charts

add tests for attribution table

4d9296f

add changelog and tidy

4eb6e3b

lane-wetmore requested a review from a team as a code owner May 21, 2025 14:30

lane-wetmore requested a review from AnPucel May 21, 2025 14:30

lane-wetmore self-assigned this May 21, 2025

lane-wetmore removed the request for review from AnPucel May 21, 2025 14:53

lane-wetmore added 3 commits May 21, 2025 09:53

remove remaining todos

d932517

tidy

fff1c2a

reset month query param in ce

8be4f0c

didoo reviewed May 21, 2025

View reviewed changes

lane-wetmore added 2 commits May 21, 2025 11:40

fix tests missing month param

b8f77aa

add margin to pagination in accordance to helios rec

7821c5b

hellobontempo reviewed May 21, 2025

View reviewed changes

remove query param, update change log, move table into own comp

dd7f7cf

lane-wetmore commented May 21, 2025

View reviewed changes

remove commented code

7f57dbe

lane-wetmore requested a review from hellobontempo May 21, 2025 21:06

remove month query params

ac68fd1

hellobontempo reviewed May 22, 2025

View reviewed changes

tidy

ee5f96e

lane-wetmore requested a review from hellobontempo May 22, 2025 21:49

update test mount paths

0433c9e

hellobontempo reviewed May 22, 2025

View reviewed changes

ui/app/components/clients/table.ts Show resolved Hide resolved

hellobontempo reviewed May 22, 2025

View reviewed changes

hellobontempo previously approved these changes May 22, 2025

View reviewed changes

remove unused client attribution component

724b15c

lane-wetmore dismissed hellobontempo’s stale review via 724b15c May 23, 2025 14:14

update tests

cf6f1d2

hellobontempo approved these changes May 23, 2025

View reviewed changes

Merge branch 'main' into ui/VAULT-35486-Replace-Attribution-Charts-wi…

0150ef7

…th-Table

lane-wetmore enabled auto-merge (squash) May 23, 2025 15:38

vercel bot deployed to Preview May 23, 2025 15:41 View deployment

lane-wetmore merged commit 2e6d5b0 into main May 23, 2025
33 checks passed

lane-wetmore deleted the ui/VAULT-35486-Replace-Attribution-Charts-with-Table branch May 23, 2025 15:58

lane-wetmore mentioned this pull request Jun 2, 2025

Docs: Update to Reflect Client Count Changes #30798

Merged

6 tasks

hc-github-team-secure-vault-core mentioned this pull request Jun 13, 2025

Backport of Docs: Update to Reflect Client Count Changes into release/1.20.x #30976

Merged

6 tasks

hellobontempo mentioned this pull request Jul 9, 2025

UI: Delete horizontal bar chart component #31241

Merged

9 tasks

		@@ -0,0 +1,3 @@
		```release-note:change
		ui: (Client Counts) The mount attribution and namespace attribution charts have been replaced in the overview tab with a table that breaks down client attribution according to month selection.

	import RouterService from '@ember/routing/router-service';
	import type RouterService from '@ember/routing/router-service';

UI: Replace Client Count Attribution Charts with Table #30678

UI: Replace Client Count Attribution Charts with Table #30678

Uh oh!

Conversation

lane-wetmore commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO only if you're a HashiCorp employee

Uh oh!

github-actions bot commented May 19, 2025

Uh oh!

github-actions bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo left a comment

Choose a reason for hiding this comment

Uh oh!

lane-wetmore commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lane-wetmore commented May 19, 2025 •

edited

Loading

github-actions bot commented May 19, 2025 •

edited

Loading

lane-wetmore commented May 23, 2025 •

edited

Loading