UI/d3 DOM cleanup hover issue (hashicorp#14493)

* fix duplicate rendering of chart elements * organize SVG char elements into groups, give data-test attrs * update tests * tweak mirage * add fake client counting start date * fix test * add waitUntil * adds changelog * add second waituntil
kstevena · Mar 16, 2022 · bc99b9f · bc99b9f
1 parent f905332
commit bc99b9f
Show file tree

Hide file tree

Showing 7 changed files with 157 additions and 25 deletions.
diff --git a/changelog/14493.txt b/changelog/14493.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes horizontal bar chart hover issue when filtering namespaces and mounts
+```
diff --git a/ui/app/components/clients/horizontal-bar-chart.js b/ui/app/components/clients/horizontal-bar-chart.js
@@ -69,21 +69,26 @@ export default class HorizontalBarChart extends Component {
 
     let chartSvg = select(element);
     chartSvg.attr('width', '100%').attr('viewBox', `0 0 564 ${(dataset.length + 1) * LINE_HEIGHT}`);
-    // chartSvg.attr('viewBox', `0 0 700 300`);
 
-    let groups = chartSvg
+    let dataBarGroup = chartSvg
       .selectAll('g')
       .remove()
       .exit()
       .data(stackedData)
       .enter()
       .append('g')
+      .attr('data-test-group', (d) => `${d.key}`)
       // shifts chart to accommodate y-axis legend
       .attr('transform', `translate(${CHART_MARGIN.left}, ${CHART_MARGIN.top})`)
       .style('fill', (d, i) => LIGHT_AND_DARK_BLUE[i]);
 
     let yAxis = axisLeft(yScale).tickSize(0);
-    yAxis(chartSvg.append('g').attr('transform', `translate(${CHART_MARGIN.left}, ${CHART_MARGIN.top})`));
+
+    let yLabelsGroup = chartSvg
+      .append('g')
+      .attr('data-test-group', 'y-labels')
+      .attr('transform', `translate(${CHART_MARGIN.left}, ${CHART_MARGIN.top})`);
+    yAxis(yLabelsGroup);
 
     chartSvg.select('.domain').remove();
 
@@ -94,8 +99,10 @@ export default class HorizontalBarChart extends Component {
 
     chartSvg.selectAll('.tick text').call(truncate);
 
-    groups
+    dataBarGroup
       .selectAll('rect')
+      .remove()
+      .exit()
       // iterate through the stacked data and chart respectively
       .data((stackedData) => stackedData)
       .enter()
@@ -109,8 +116,12 @@ export default class HorizontalBarChart extends Component {
       .attr('rx', 3)
       .attr('ry', 3);
 
-    let actionBars = chartSvg
+    let actionBarGroup = chartSvg.append('g').attr('data-test-group', 'action-bars');
+
+    let actionBars = actionBarGroup
       .selectAll('.action-bar')
+      .remove()
+      .exit()
       .data(dataset)
       .enter()
       .append('rect')
@@ -124,8 +135,12 @@ export default class HorizontalBarChart extends Component {
       .style('opacity', '0')
       .style('mix-blend-mode', 'multiply');
 
-    let yLegendBars = chartSvg
-      .selectAll('.label-bar')
+    let labelActionBarGroup = chartSvg.append('g').attr('data-test-group', 'label-action-bars');
+
+    let labelActionBar = labelActionBarGroup
+      .selectAll('.label-action-bar')
+      .remove()
+      .exit()
       .data(dataset)
       .enter()
       .append('rect')
@@ -173,10 +188,10 @@ export default class HorizontalBarChart extends Component {
       });
 
     // MOUSE EVENTS FOR Y-AXIS LABELS
-    yLegendBars
+    labelActionBar
       .on('mouseover', (data) => {
         if (data.label.length >= CHAR_LIMIT) {
-          let hoveredElement = yLegendBars.filter((bar) => bar.label === data.label).node();
+          let hoveredElement = labelActionBar.filter((bar) => bar.label === data.label).node();
           this.tooltipTarget = hoveredElement;
           this.isLabel = true;
           this.tooltipText = data.label;
@@ -208,10 +223,13 @@ export default class HorizontalBarChart extends Component {
           .style('opacity', '0');
       });
 
-    // add client count total values to the right
-    chartSvg
+    // client count total values to the right
+    let totalValueGroup = chartSvg
       .append('g')
-      .attr('transform', `translate(${TRANSLATE.left}, ${TRANSLATE.down})`)
+      .attr('data-test-group', 'total-values')
+      .attr('transform', `translate(${TRANSLATE.left}, ${TRANSLATE.down})`);
+
+    totalValueGroup
       .selectAll('text')
       .data(dataset)
       .enter()

diff --git a/ui/mirage/handlers/clients.js b/ui/mirage/handlers/clients.js
@@ -1,3 +1,5 @@
+import { formatISO, isBefore, sub } from 'date-fns';
+
 export default function (server) {
   // 1.10 API response
   server.get('sys/version-history', function () {
@@ -88,6 +90,8 @@ export default function (server) {
 
   server.get('/sys/internal/counters/activity', (schema, req) => {
     const { start_time, end_time } = req.queryParams;
+    // fake client counting start date so warning shows if user queries earlier start date
+    const counts_start = '2020-10-17T00:00:00Z';
     return {
       request_id: '25f55fbb-f253-9c46-c6f0-3cdd3ada91ab',
       lease_id: '',
@@ -182,9 +186,9 @@ export default function (server) {
             ],
           },
         ],
-        end_time: end_time || '2022-01-31T23:59:59Z',
+        end_time: end_time || formatISO(sub(new Date(), { months: 1 })),
         months: [],
-        start_time,
+        start_time: isBefore(new Date(start_time), new Date(counts_start)) ? counts_start : start_time,
         total: {
           distinct_entities: 37389,
           entity_clients: 37389,

diff --git a/ui/tests/acceptance/auth-list-test.js b/ui/tests/acceptance/auth-list-test.js
@@ -1,4 +1,13 @@
-import { click, findAll, fillIn, settled, visit, triggerKeyEvent } from '@ember/test-helpers';
+import {
+  click,
+  findAll,
+  fillIn,
+  settled,
+  visit,
+  triggerKeyEvent,
+  find,
+  waitUntil,
+} from '@ember/test-helpers';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import authPage from 'vault/tests/pages/auth';
@@ -31,7 +40,7 @@ module('Acceptance | auth backend list', function (hooks) {
     await click('[data-test-save-config="true"]');
 
     await visit(`/vault/access/${path1}/item/user/create`);
-
+    await waitUntil(() => find('[data-test-input="username"]') && find('[data-test-textarea]'));
     await fillIn('[data-test-input="username"]', user1);
     await triggerKeyEvent('[data-test-input="username"]', 'keyup', 65);
     await fillIn('[data-test-textarea]', user1);

diff --git a/ui/tests/acceptance/client-current-test.js b/ui/tests/acceptance/client-current-test.js
@@ -7,6 +7,7 @@ import { create } from 'ember-cli-page-object';
 import { clickTrigger } from 'ember-power-select/test-support/helpers';
 import ss from 'vault/tests/pages/components/search-select';
 import {
+  CHART_ELEMENTS,
   generateConfigResponse,
   generateCurrentMonthResponse,
   SELECTORS,
@@ -78,7 +79,7 @@ module('Acceptance | clients current', function (hooks) {
     assert.dom(SELECTORS.activeTab).hasText('Current month', 'current month tab is active');
     assert.dom(SELECTORS.usageStats).exists('usage stats block exists');
     assert.dom('[data-test-stat-text-container]').exists({ count: 3 }, '3 stat texts exist');
-    const { clients, entity_clients, non_entity_clients } = monthly.data;
+    const { clients, entity_clients, non_entity_clients, by_namespace } = monthly.data;
     assert.dom('[data-test-stat-text="total-clients"] .stat-value').hasText(clients.toString());
     assert.dom('[data-test-stat-text="entity-clients"] .stat-value').hasText(entity_clients.toString());
     assert
@@ -87,7 +88,28 @@ module('Acceptance | clients current', function (hooks) {
     assert.dom('[data-test-clients-attribution]').exists('Shows attribution area');
     assert.dom('[data-test-horizontal-bar-chart]').exists('Shows attribution bar chart');
     assert.dom('[data-test-top-attribution]').includesText('Top namespace');
-    // Filter by namespace
+
+    // check chart displays correct elements and values
+    for (const key in CHART_ELEMENTS) {
+      let namespaceNumber = by_namespace.length < 10 ? by_namespace.length : 10;
+      let group = find(CHART_ELEMENTS[key]);
+      let elementArray = Array.from(group.children);
+      assert.equal(elementArray.length, namespaceNumber, `renders correct number of ${key}`);
+      if (key === 'totalValues') {
+        elementArray.forEach((element, i) => {
+          assert.equal(element.innerHTML, `${by_namespace[i].counts.clients}`, 'displays correct value');
+        });
+      }
+      if (key === 'yLabels') {
+        elementArray.forEach((element, i) => {
+          assert
+            .dom(element.children[1])
+            .hasTextContaining(`${by_namespace[i].namespace_path}`, 'displays correct namespace label');
+        });
+      }
+    }
+
+    // FILTER BY NAMESPACE
     await clickTrigger();
     await searchSelect.options.objectAt(0).click();
     await waitUntil(() => {
@@ -98,7 +120,29 @@ module('Acceptance | clients current', function (hooks) {
     assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('10');
     assert.dom('[data-test-horizontal-bar-chart]').exists('Still shows attribution bar chart');
     assert.dom('[data-test-top-attribution]').includesText('Top auth method');
-    // Filter by auth method
+
+    // check chart displays correct elements and values
+    for (const key in CHART_ELEMENTS) {
+      const { mounts } = by_namespace[0];
+      let mountNumber = mounts.length < 10 ? mounts.length : 10;
+      let group = find(CHART_ELEMENTS[key]);
+      let elementArray = Array.from(group.children);
+      assert.equal(elementArray.length, mountNumber, `renders correct number of ${key}`);
+      if (key === 'totalValues') {
+        elementArray.forEach((element, i) => {
+          assert.equal(element.innerHTML, `${mounts[i].counts.clients}`, 'displays correct value');
+        });
+      }
+      if (key === 'yLabels') {
+        elementArray.forEach((element, i) => {
+          assert
+            .dom(element.children[1])
+            .hasTextContaining(`${mounts[i].mount_path}`, 'displays correct auth label');
+        });
+      }
+    }
+
+    // FILTER BY AUTH METHOD
     await clickTrigger();
     await searchSelect.options.objectAt(0).click();
     await waitUntil(() => {

diff --git a/ui/tests/acceptance/client-history-test.js b/ui/tests/acceptance/client-history-test.js
@@ -8,6 +8,7 @@ import { create } from 'ember-cli-page-object';
 import { clickTrigger } from 'ember-power-select/test-support/helpers';
 import ss from 'vault/tests/pages/components/search-select';
 import {
+  CHART_ELEMENTS,
   generateActivityResponse,
   generateConfigResponse,
   generateLicenseResponse,
@@ -127,6 +128,7 @@ module('Acceptance | clients history tab', function (hooks) {
         'Date range shows dates correctly parsed activity response'
       );
     assert.dom('[data-test-stat-text-container]').exists({ count: 3 }, '3 stat texts exist');
+    const { by_namespace } = activity.data;
     const { clients, entity_clients, non_entity_clients } = activity.data.total;
     assert
       .dom('[data-test-stat-text="total-clients"] .stat-value')
@@ -140,6 +142,26 @@ module('Acceptance | clients history tab', function (hooks) {
     assert.dom('[data-test-clients-attribution]').exists('Shows attribution area');
     assert.dom('[data-test-horizontal-bar-chart]').exists('Shows attribution bar chart');
     assert.dom('[data-test-top-attribution]').includesText('Top namespace');
+
+    // check chart displays correct elements and values
+    for (const key in CHART_ELEMENTS) {
+      let namespaceNumber = by_namespace.length < 10 ? by_namespace.length : 10;
+      let group = find(CHART_ELEMENTS[key]);
+      let elementArray = Array.from(group.children);
+      assert.equal(elementArray.length, namespaceNumber, `renders correct number of ${key}`);
+      if (key === 'totalValues') {
+        elementArray.forEach((element, i) => {
+          assert.equal(element.innerHTML, `${by_namespace[i].counts.clients}`, 'displays correct value');
+        });
+      }
+      if (key === 'yLabels') {
+        elementArray.forEach((element, i) => {
+          assert
+            .dom(element.children[1])
+            .hasTextContaining(`${by_namespace[i].namespace_path}`, 'displays correct namespace label');
+        });
+      }
+    }
   });
 
   test('filters correctly on history with full data', async function (assert) {
@@ -164,8 +186,9 @@ module('Acceptance | clients history tab', function (hooks) {
     assert.dom(SELECTORS.activeTab).hasText('History', 'history tab is active');
     assert.dom(SELECTORS.usageStats).exists('usage stats block exists');
     assert.dom('[data-test-stat-text-container]').exists({ count: 3 }, '3 stat texts exist');
-    const { clients } = activity.data.total;
-    // Filter by namespace
+    const { total, by_namespace } = activity.data;
+
+    // FILTER BY NAMESPACE
     await clickTrigger();
     await searchSelect.options.objectAt(0).click();
     await waitUntil(() => {
@@ -177,7 +200,29 @@ module('Acceptance | clients history tab', function (hooks) {
     assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('10');
     assert.dom('[data-test-horizontal-bar-chart]').exists('Shows attribution bar chart');
     assert.dom('[data-test-top-attribution]').includesText('Top auth method');
-    // Filter by auth method
+
+    // check chart displays correct elements and values
+    for (const key in CHART_ELEMENTS) {
+      const { mounts } = by_namespace[0];
+      let mountNumber = mounts.length < 10 ? mounts.length : 10;
+      let group = find(CHART_ELEMENTS[key]);
+      let elementArray = Array.from(group.children);
+      assert.equal(elementArray.length, mountNumber, `renders correct number of ${key}`);
+      if (key === 'totalValues') {
+        elementArray.forEach((element, i) => {
+          assert.equal(element.innerHTML, `${mounts[i].counts.clients}`, 'displays correct value');
+        });
+      }
+      if (key === 'yLabels') {
+        elementArray.forEach((element, i) => {
+          assert
+            .dom(element.children[1])
+            .hasTextContaining(`${mounts[i].mount_path}`, 'displays correct auth label');
+        });
+      }
+    }
+
+    // FILTER BY AUTH METHOD
     await clickTrigger();
     await searchSelect.options.objectAt(0).click();
     await settled();
@@ -192,7 +237,7 @@ module('Acceptance | clients history tab', function (hooks) {
     assert.dom('[data-test-top-attribution]').includesText('Top namespace');
     assert
       .dom('[data-test-stat-text="total-clients"] .stat-value')
-      .hasText(clients.toString(), 'total clients stat is back to unfiltered value');
+      .hasText(total.clients.toString(), 'total clients stat is back to unfiltered value');
   });
 
   test('shows warning if upgrade happened within license period', async function (assert) {
@@ -292,7 +337,7 @@ module('Acceptance | clients history tab', function (hooks) {
     assert.equal(currentURL(), '/vault/clients/history', 'clients/history URL is correct');
     assert
       .dom(SELECTORS.emptyStateTitle)
-      .includesText('No start date found', 'Empty state shows no billing start date');
+      .includesText('start date found', 'Empty state shows no billing start date');
     await click(SELECTORS.monthDropdown);
     await click(this.element.querySelector('[data-test-month-list] button:not([disabled])'));
     await click(SELECTORS.yearDropdown);

diff --git a/ui/tests/helpers/clients.js b/ui/tests/helpers/clients.js
@@ -27,6 +27,15 @@ export const SELECTORS = {
   dateDropdownSubmit: '[data-test-date-dropdown-submit]',
 };
 
+export const CHART_ELEMENTS = {
+  entityClientDataBars: '[data-test-group="entity_clients"]',
+  nonEntityDataBars: '[data-test-group="non_entity_clients"]',
+  yLabels: '[data-test-group="y-labels"]',
+  actionBars: '[data-test-group="action-bars"]',
+  labelActionBars: '[data-test-group="label-action-bars"]',
+  totalValues: '[data-test-group="total-values"]',
+};
+
 export function sendResponse(data, httpStatus = 200) {
   if (httpStatus === 403) {
     return [
@@ -60,7 +69,7 @@ function generateNamespaceBlock(idx = 0, skipMounts = false) {
   let mountCount = 1;
   const nsBlock = {
     namespace_id: `${idx}UUID`,
-    namespace_path: `my-namespace-${idx}/`,
+    namespace_path: `${idx}/namespace`,
     counts: {
       clients: mountCount * 15,
       entity_clients: mountCount * 5,