Secrets sync UI: add sync page component #24982
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
hashicorp-contributed-pr
hellobontempo
changed the base branch from
main
to
ui/VAULT-23095/secrets-sync-client-counts
hellobontempo
changed the base branch from
ui/VAULT-23095/secrets-sync-client-counts
to
ui/VAULT-23107/client-overview
…for monthly-new component
…T-23107/client-overview
…T-23109/add-sync-page-component
…-sync-page-component
|
Build Results: |
hellobontempo
commented
Jan 23, 2024
|
|
||
| interface Args { | ||
| data: SerializedChartData[]; |
There was a problem hiding this comment.
Updated to just the data relevant to the chart, not all of the data returned from the serialized /activity response
hellobontempo
commented
Jan 23, 2024
| @@ -52,40 +50,48 @@ export default class ClientsSyncBarChartComponent extends Component<Args> { | |||
|
|
|||
| get chartData() { | |||
| return this.args.data.map((d): ChartData => { | |||
| const date = parseAPITimestamp(d.timestamp) as Date; | |||
There was a problem hiding this comment.
I didn't see the need to change this to a Date object because below we format it back from a date to M/yy. Plus, scaleBand which is used to build the x-axis just spaces the values evenly based on how many exist. Dates are already sorted by the backend.
Base automatically changed from
ui/VAULT-23107/client-overview
to
ui/VAULT-23095/secrets-sync-client-counts
January 23, 2024 19:29
hellobontempo
commented
Jan 23, 2024
| @@ -3,29 +3,31 @@ | |||
| SPDX-License-Identifier: BUSL-1.1 | |||
| ~}} | |||
|
|
|||
| <div class="chart-wrapper stacked-charts" data-test-chart="monthly new"> | |||
There was a problem hiding this comment.
Display is unchanged, just leverages component to consolidate styling
hellobontempo
commented
Jan 23, 2024
| @@ -0,0 +1,44 @@ | |||
| {{! HBS display template for rendering title, description and stat boxes with a chart on the right }} | |||
|
|
|||
| <div class="chart-wrapper single-chart-grid" ...attributes> | |||
There was a problem hiding this comment.
Provides layout for these
hellobontempo
commented
Jan 23, 2024
| {{yield to="legend"}} | ||
| {{/if}} | ||
|
|
||
| {{else}} |
There was a problem hiding this comment.
Empty state and stat boxes are intentionally mutually exclusive because a separate component should be used to render just stats if the chart doesn't have data
hellobontempo
commented
Jan 23, 2024
| }; | ||
| }); | ||
| } | ||
|
|
||
| get countDomain() { | ||
| const counts: number[] = this.chartData.map((d) => d.y).flatMap((num) => (num ? [num] : [])); |
There was a problem hiding this comment.
because 0 is falsy, if all of the data points were 0 then counts was an empty array means an upper limit of -Infinity.
In another example, when counts contains numbers under 1000 the upper value (domain max) is set to 0 and renders:
The original purpose here was to intervene and add a "buffer" bar so the max plot wouldn't go above the gridlines. As seen in the examples above, interfering with the data scale calculations is risky. For this reason, the max will always be obtained from the dataset. Tampering with the data scales undermines the purpose of these charts which is to accurately display data trends. 4 is only hardcoded if max=0 because it is the default number of tick lines
The bars going slightly above the top gridline is a minimal tradeoff (IMO) and how the charts have rendered since their inception.
There was a problem hiding this comment.
cc @hashishaw - we discussed this a bit last week and so I wanted to loop you in on the updates I made and reasoning.
zofskeez
reviewed
Jan 23, 2024
There was a problem hiding this comment.
I like the use of named blocks and how tidy this component is!
hellobontempo
commented
Jan 23, 2024
| @@ -81,10 +83,12 @@ export default class LineChart extends Component<Args> { | |||
| } | |||
| // Domains | |||
| get yDomain() { | |||
| const setMax = Math.max(...this.data.map((datum) => datum.y ?? 0)); | |||
There was a problem hiding this comment.
See this comment for more context. But I updated this for similar reasoning.
Interfering with the domain made trends more difficult to see. Both of the following charts are rendered with the exact same data, notice how the first chart flattens the data out in order to compress it into an arbitrary fixed range of either 20, 100, 200, or 1000
before
after
zofskeez
approved these changes
Jan 23, 2024
hellobontempo
merged commit 73cadce
into
ui/VAULT-23095/secrets-sync-client-counts
64 checks passed
zofskeez
added a commit
that referenced
this pull request
Feb 1, 2024
* 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]>
Monkeychip
pushed a commit
that referenced
this pull request
Feb 5, 2024
* 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]>
Monkeychip
pushed a commit
that referenced
this pull request
Feb 7, 2024
* 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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements monthly sync client chart in
Secrets sync clientstabsingle month