Clients Counts Parent Route #24899

zofskeez · 2024-01-17T06:47:38Z

This PR adds a parent route named counts to handle fetching the activity as well as rendering the date, namespace and auth mount filters. The overview, token and sync routes have been moved to child routes of counts and any other new client types can be added in the future.

…rect to counts overview route

…ounts

…T-23106/client-counts-parent-route

…dates counts route to always attempt to fetch activity

zofskeez · 2024-01-17T17:08:54Z

ui/app/app.js

@@ -112,7 +112,7 @@ export default class App extends Application {
        services: ['flash-messages', 'router', 'store', 'version'],
        externalRoutes: {
          kvSecretDetails: 'vault.cluster.secrets.backend.kv.secret.details',
-          clientCountOverview: 'vault.cluster.clients.overview',
+          clientCountOverview: 'vault.cluster.clients',


Rather than having to change this again if the target route changes I updated to the top level route which has a redirect so we will only need to update in one place in the future.

zofskeez · 2024-01-17T17:10:21Z

ui/app/adapters/clients/activity.js

-    start_time = start_time.timestamp || getUnixTime(Date.UTC(start_time.year, start_time.monthIdx, 1));
-    // day=0 for Date.UTC() returns the last day of the month before
-    // increase monthIdx by one to get last day of queried month
-    end_time = end_time.timestamp || getUnixTime(Date.UTC(end_time.year, end_time.monthIdx + 1, 0));


Moved the date formatting logic to the utils file so it could be used elsewhere but it is otherwise unchanged.

zofskeez · 2024-01-17T17:12:35Z

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

This is close to a straight copy and paste from the token (formerly dashboard) page component with changes to the args and property names where needed.

zofskeez · 2024-01-17T17:14:56Z

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

+    <div class="is-subtitle-gray has-bottom-margin-m">
+      FILTERS
+      <Toolbar data-test-clients-filter-bar>
+        <ToolbarFilters>
+          <CalendarWidget
+            @startTimestamp={{this.startDate}}
+            @endTimestamp={{this.endDate}}
+            @selectMonth={{this.onDateChange}}
+          />
+          {{#if (or @namespace this.namespaces)}}
+            <SearchSelect
+              @id="namespace-search-select"
+              @options={{this.namespaces}}
+              @inputValue={{if @namespace (array @namespace)}}
+              @selectLimit="1"
+              @disallowNewItems={{true}}
+              @fallbackComponent="input-search"
+              @onChange={{fn this.setFilterValue "ns"}}
+              @placeholder="Filter by namespace"
+              @displayInherit={{true}}
+              class="is-marginless"
+            />
+          {{/if}}
+          {{#if (or @authMount this.authMounts)}}
+            <SearchSelect
+              @id="auth-method-search-select"
+              @options={{this.authMounts}}
+              @inputValue={{if @authMount (array @authMount)}}
+              @selectLimit="1"
+              @disallowNewItems={{true}}
+              @fallbackComponent="input-search"
+              @onChange={{fn this.setFilterValue "authMount"}}
+              @placeholder="Filter by auth method"
+              @displayInherit={{true}}
+            />
+          {{/if}}
+        </ToolbarFilters>
+      </Toolbar>
+    </div>


One notable difference is that this block used to be behind a conditional where if the empty states were showing then the filters would be hidden. It was decided to remove that so the user could make changes to the filters to trigger a new query and also be able to see what was input that returned no results.

zofskeez · 2024-01-17T17:16:33Z

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

+        </Hds::Alert>
+      {{/if}}
+
+      {{yield}}


A block is yielded if there are counts to show. In the consuming route template there is an outlet placed in the block so that the child route can render the specific count information in the charts.

zofskeez · 2024-01-17T17:18:15Z

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

+      <EmptyState
+        @title="No data received  {{if this.dateRangeMessage this.dateRangeMessage}}"
+        @message="Update the filter values or click the button to reset them"
+      >
+        <Hds::Button @text="Reset filters" @color="tertiary" @icon="reload" {{on "click" this.resetFilters}} />
+      </EmptyState>


This had a date dropdown in the yielded block which is redundant now that the filters are always visible. A Reset filters action was added so the user can quickly go back to the default state (current billing period) and clear the namespace and auth mount selections.

zofskeez · 2024-01-17T17:19:56Z

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

+  onDateChange(dateObject: { dateType: string; monthIdx: string; year: string }) {
+    const { dateType, monthIdx, year } = dateObject;
+    const { currentTimestamp } = this.args;
+    const selectedDate = formatDateObject({ monthIdx, year }, dateType === 'endDate');


Converts the selections to unix time for the query params. The activity endpoint supports passing the timestamps in this format.

Could we add a comment that formatDateObject converts to unix? The date formatting has changed so much over with each iteration, it's helpful to know here that this is the format when it's selected

Yep for sure!

zofskeez · 2024-01-17T17:21:45Z

ui/app/routes/vault/cluster/clients.ts

-  @action
-  deactivate() {
-    // when navigating away from parent route, delete manually inputted license start date
-    getStorage().removeItem(INPUTTED_START_DATE);
-  }


start_time value is now held in the query param.

zofskeez · 2024-01-17T17:24:06Z

ui/app/routes/vault/cluster/clients/token.js

Originally converted this to a base route before the counts parent route was added. This restores it to the original dashboard implementation to keep for reference until we move token under the counts route.

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

hellobontempo · 2024-01-17T17:44:04Z

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

TYPESCRIPT! 🎉 🥇

hellobontempo

Amazing job! All of these improvements are going to help us out so much as we continue to expand and improve the dashboard.

Left just a couple comments, before approving I'd like to check out the branch and click around to make sure we didn't miss anything

hellobontempo · 2024-01-17T17:56:02Z

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

+
+    if (dateType !== 'cancel') {
+      const start_time = {
+        reset: null, // clicked 'Current billing period' in calendar widget -> defaults to licenseStartTime


Instead of setting the start time for reset here, it looks like you're doing it in the route? Could we add a comment to clarify that? We can also remove the part about clicking current billing period because there are now two ways to "reset" with the updates to the empty state. So something like:

// resets to billing start date in the route file (which is the default query value)

hellobontempo · 2024-01-17T18:02:22Z

ui/app/routes/vault/cluster/clients.ts

@@ -30,14 +37,8 @@ export default class ClientsRoute extends Route {
  model() {
    // swallow config error so activity can show if no config permissions
    return hash({
-      config: this.store.queryRecord('clients/config', {}).catch(() => {}),
+      config: this.store.queryRecord('clients/config', {}).catch(() => null),


Out of curiosity - why update this from {} to null?

When I converted to ts it was flagging this because it's not actually returning an object but rather it's an empty arrow function which is not allowed so that's why I changed it to return null. I'll add a test for the page component to make sure nothing errors out if their is no config passed in.

Cool - thanks for explaining!

hellobontempo · 2024-01-17T18:13:34Z

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

+  onDateChange(dateObject: { dateType: string; monthIdx: string; year: string }) {
+    const { dateType, monthIdx, year } = dateObject;
+    const { currentTimestamp } = this.args;
+    const selectedDate = formatDateObject({ monthIdx, year }, dateType === 'endDate');


Could we add a comment that formatDateObject converts to unix? The date formatting has changed so much over with each iteration, it's helpful to know here that this is the format when it's selected

hellobontempo · 2024-01-17T18:15:16Z

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

+    let activity, activityError;
+    try {
+      activity = await this.store.queryRecord('clients/activity', {
+        start_time: { timestamp: start_time },


the query params are always formatted as unix now, right? Like when a user selects a date we immediately convert to unix? Originally this object was added because the date was formatted differently depending on how it was set. We can just pass these query params directly, if they're always unix time now and then remove the extra logic (formatQueryParams) in the activity adapter

Though the comment explaining that javascript localizes Date() may be good to keep above the queryRecord method so we don't repeat our past mistake of sending the query with a timezone

That's something to keep in mind once we fully move over the token logic and delete the original route but for now it will be necessary so we can cross reference things as we continue to make changes. I agree though that we should go over things and simplify and remove unnecessary left overs of the original implementation.

Ahh good point - I forgot we've left token.js alone mostly for now

hellobontempo · 2024-01-17T18:20:33Z

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

+
+  async model(params: { start_time: string; end_time: string }) {
+    const { config } = this.modelFor('vault.cluster.clients') as ClientsRouteModel;
+    const startTimestamp = Number(params.start_time) || getUnixTime(config.billingStartTimestamp);


previously we returned null for licenseStartTimestamp so the user could input a date if we didn't have one to make the initial request with. It's possible a user may not have permissions for cluster config and so this value would be undefined.

How is this handled now?

That may have deviated then. I think I mistakenly removed the condition to only query activity if there is a start time thinking that we should go ahead and then display the error. Let me add that back in. The template should still be setup to handle that case but I'll write a test to cover it as well.

…s triggering client counts route model hook

github-actions · 2024-01-18T19:14:08Z

Build Results:
All builds succeeded! ✅

…T-23106/client-counts-parent-route

Co-authored-by: claire bontempo <[email protected]>

zofskeez · 2024-01-18T19:24:36Z

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

+  // using router.transitionTo to update the query params results in the model hook firing each time
+  // this happens when the queryParams object is not added to the route or refreshModel is explicitly set to false
+  // updating the bound properties does however respect the refreshModel settings and functions expectedly
+  @action
+  updateQueryParams(updatedParams: ClientsCountsRouteParams) {
+    if (!updatedParams) {
+      this.queryParams.forEach((key) => (this[key as keyof ClientsCountsRouteParams] = undefined));
+    } else {
+      Object.keys(updatedParams).forEach((key) => {
+        if (queryParamKeys.includes(key)) {
+          const value = updatedParams[key as keyof ClientsCountsRouteParams];
+          set(this, key as keyof ClientsCountsRouteParams, value as keyof ClientsCountsRouteParams);
+        }
+      });
+    }
+  }


This was something interesting where totally removing the queryParams block from the route file and not having refreshModel set was still resulting in the model hook firing when doing a transitionTo and updating the query params.

There have been talks to do the actual filtering of the activity in the model hook in which case this can be removed but for now updating ns and authMount query params does not trigger a full transition.

hellobontempo

Nice work on this - thanks for tackling those test failures

* Client Count Routing Updates (#24733) * updates client count routing for sync and future additions * adds copyright header to clients sync template * adds missing copyright headers * UI: Adds secret_syncs to mirage /activity endpoint (#24846) * add secret_syncs to mirage endpoint * import clients handler * UI: Set up client charts for incoming sync data (#24852) * sum stacked bar values for tooltip total * make tooltip dynamic based on chartLegend * remove redundant helper * add secret_syncs to client count utils * move sum function to helper * update horizontal bar chart to include sync_clients * calculate sum of bars in tooltip * rename color palette const, define chart legends in each parent component instead of token.js * update tooltips * update mirage handler to add sys/ namespace * update mirage handler to add sys/ namespace * use pushObject * update test * UI: Secret sync bar chart (#24926) * install lineal * add ember-style-modifier dep * Add client count types for serialized data * Add sync bar chart component with tests * Chart is responsive * address comments * Clients Counts Parent Route (#24899) * adds interfaces for clients models * moves date formatting logic from clients activity adapter to utils file * adds clients counts route * updates links to clients route to point to top level and updates redirect to counts overview route * removes clients base route and moves overview and sync routes under counts * adds clients counts page component * converts clients route to ts * adds billing start timestamp to clients config mirage response and updates counts route to always attempt to fetch activity * fixes issue with updating namespace and auth mount query params always triggering client counts route model hook * adds tests for clients counts page component * adds missing copyright header to client-counts type file * Update ui/app/components/clients/page/counts.hbs Co-authored-by: claire bontempo <[email protected]> * fixes bad import in sync-bar-chart * updates clients counts route to bypass query if there is not start_time * pins d3-shape to 1.3.7 for now -- makes lineal play nice with old charts * fixes sync bar chart tooltip assertion --------- Co-authored-by: claire bontempo <[email protected]> * UI: convert line-chart to lineal (#24961) * lineal chart alongside svg * Add version-history to sync handler for testing * line chart is TS, test updated * remove d3-shape resolution * fix clients/token-test * use chartHeight in running-total template * use M/yy key instead of timestamp, chart is responsive * Add test for swapping datasets * add more edge case tests * more test * remove untrue assertion * fix weird decimal when between 1.1k and 2k * address feedback * Update line-chart to use timestamp instead of month key * Add timestamp to all places where month is on the clients activity response * Client Counts Overview (#24969) * adds counts base component for use in client counts child routes * adds clients counts overview page component * splits out monthly new chart from clients running total component * adds missing copyright headers * moves running total related assertions from token to overview acceptance test * removes new client assertions from running-total test and adds tests for monthly-new component * updates copy in running-total component * fixes clients overview tests * fixes timestamp stub not being restored in monthly-new test * fixes mfa-login test * renames counts component to activity * removes unused selectedAuthMethod arg from running-total component * adds timestamp back to running-total component * Secrets sync UI: add sync page component (#24982) * adds counts base component for use in client counts child routes * adds clients counts overview page component * splits out monthly new chart from clients running total component * adds missing copyright headers * move sync-bar-chart to charts/ folder * update types and rename chart * rename template file * moves running total related assertions from token to overview acceptance test * removes new client assertions from running-total test and adds tests for monthly-new component * updates copy in running-total component * fixes clients overview tests * fixes timestamp stub not being restored in monthly-new test * fixes mfa-login test * fix 0 values erroring charts * separate timestamp again * address merge conflicts * finish building sync chart component WIP css * renames counts component to activity * update import * revert name to dataKey * update styling for charts without legends * use monthly stat chart component for layout * use monthly chart stats in monthly new * implement stat wrapper; * remove extra grid div * rename component * fix legend css; * update test[ * remove arbitrarily setting max * add single month view * use stat text * update line chart tests * rename line chart * update tests --------- Co-authored-by: Jordan Reimer <[email protected]> * update selectors * add sync page tests * Secrets Sync UI: Add secrets syncs to csv export (#25056) * update mirage and add sync clients to export csv * fix sync legend label * remove word * update copy in modal * update mirage * fix attribution tooltip text * Clients Counts Token Route (#25019) * renames token route and page component back to dashboard * adds client counts token route and page component * updates charts in token page to use ChartContainer component * adds tests for clients token page component * restore clients dashboard test * use var for chart title sync page * updates clients token page to show usage stats when querying single month * updates token page clients averages to only include entity and non-entity clients in calculation * fixes monthly total counts lower than new clients in mirage handler * fixes token test --------- Co-authored-by: [email protected] <[email protected]> * Clients Usage Stats/Running Total Updates (#25094) * updates clients usage counts and running totals * updates usage stats total copy * fixes client counts overview tests * Secrets sync UI: cleanup and consolidation of components (#25090) * rename authMethod to mountPath * generalize count template copy * add todo to delete monthly new component * rename to tokenTab * wrap filters in conditional checking for start timestamp * some users may not have access to /config endpoint * fix querying when user has no billing date permissions and clicks current billing period * extend activity component from counts page * Revert "extend activity component from counts page" This reverts commit 1d0e85c. * rename to startTimestampISO * remove timestamp from route and just use activity model responseTimestamp * fix chart y domain max * fix typos in usage stat and running totals component * delete backing class for display only template; * updates tests * adds comment for fetching license to get start date for billing * cleans up unused client counts files (#25157) * adds changelog * fix assertion copy * adds changelog description * updates enterprise sidebar nav test --------- Co-authored-by: [email protected] <[email protected]> Co-authored-by: claire bontempo <[email protected]> Co-authored-by: Chelsea Shaw <[email protected]>

zofskeez added 7 commits January 16, 2024 23:35

adds interfaces for clients models

38ed068

moves date formatting logic from clients activity adapter to utils file

f5b59da

adds clients counts route

583547e

updates links to clients route to point to top level and updates redi…

f89cdb5

…rect to counts overview route

removes clients base route and moves overview and sync routes under c…

a5ec58b

…ounts

adds clients counts page component

5fd32ec

converts clients route to ts

d077544

zofskeez added ui pr/no-changelog labels Jan 17, 2024

zofskeez added this to the 1.16.0-rc1 milestone Jan 17, 2024

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jan 17, 2024

Merge branch 'ui/VAULT-23095/secrets-sync-client-counts' into ui/VAUL…

6b9551e

…T-23106/client-counts-parent-route

vercel bot deployed to Preview January 17, 2024 16:41 View deployment

adds billing start timestamp to clients config mirage response and up…

ffca205

…dates counts route to always attempt to fetch activity

zofskeez commented Jan 17, 2024

View reviewed changes

hellobontempo reviewed Jan 17, 2024

View reviewed changes

ui/app/components/clients/page/counts.hbs Outdated Show resolved Hide resolved

hellobontempo reviewed Jan 17, 2024

View reviewed changes

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

Copy link

Contributor

hellobontempo Jan 17, 2024

Choose a reason for hiding this comment

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

TYPESCRIPT! 🎉 🥇

hellobontempo reviewed Jan 17, 2024

View reviewed changes

zofskeez added 2 commits January 17, 2024 16:47

fixes issue with updating namespace and auth mount query params alway…

3e45c8b

…s triggering client counts route model hook

adds tests for clients counts page component

50b66df

Merge branch 'ui/VAULT-23095/secrets-sync-client-counts' into ui/VAUL…

3292696

…T-23106/client-counts-parent-route

zofskeez marked this pull request as ready for review January 18, 2024 19:18

zofskeez and others added 2 commits January 18, 2024 12:20

adds missing copyright header to client-counts type file

88b4a76

Update ui/app/components/clients/page/counts.hbs

0083506

Co-authored-by: claire bontempo <[email protected]>

zofskeez commented Jan 18, 2024

View reviewed changes

zofskeez added 4 commits January 18, 2024 12:54

fixes bad import in sync-bar-chart

a03de52

updates clients counts route to bypass query if there is not start_time

8a3364c

pins d3-shape to 1.3.7 for now -- makes lineal play nice with old charts

bbae632

fixes sync bar chart tooltip assertion

67019dd

hellobontempo approved these changes Jan 18, 2024

View reviewed changes

zofskeez merged commit 8697a1f into ui/VAULT-23095/secrets-sync-client-counts Jan 18, 2024
64 checks passed

zofskeez deleted the ui/VAULT-23106/client-counts-parent-route branch January 18, 2024 22:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clients Counts Parent Route #24899

Clients Counts Parent Route #24899

zofskeez commented Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

hellobontempo Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

zofskeez Jan 17, 2024

hellobontempo Jan 17, 2024

hellobontempo left a comment

hellobontempo Jan 17, 2024

hellobontempo Jan 17, 2024

zofskeez Jan 17, 2024

hellobontempo Jan 18, 2024

hellobontempo Jan 17, 2024

hellobontempo Jan 17, 2024

zofskeez Jan 18, 2024

hellobontempo Jan 18, 2024

hellobontempo Jan 17, 2024

zofskeez Jan 18, 2024

github-actions bot commented Jan 18, 2024

zofskeez Jan 18, 2024

hellobontempo left a comment

Clients Counts Parent Route #24899

Clients Counts Parent Route #24899

Conversation

zofskeez commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellobontempo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2024

Choose a reason for hiding this comment

hellobontempo left a comment

Choose a reason for hiding this comment