Ensure custom set axis titles are preserved when loading a saved vis. (…

…elastic#24176)
lukeelmers · Nov 15, 2018 · 9806bd1 · 9806bd1
1 parent 3dfc23d
commit 9806bd1
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 9 deletions.
diff --git a/src/core_plugins/kbn_vislib_vis_types/public/controls/point_series/value_axes.js b/src/core_plugins/kbn_vislib_vis_types/public/controls/point_series/value_axes.js
@@ -136,10 +136,13 @@ module.directive('vislibValueAxes', function () {
         }, 1);
       };
 
-      const lastAxisTitles = {};
+      const lastCustomLabels = {};
+      // We track these so we can know when the agg is changed
+      let lastMatchingSeriesAggType = '';
+      let lastMatchingSeriesAggField = '';
       $scope.updateAxisTitle = function () {
         $scope.editorState.params.valueAxes.forEach((axis, axisNumber) => {
-          let label = '';
+          let newCustomLabel = '';
           const isFirst = axisNumber === 0;
           const matchingSeries = [];
           $scope.editorState.params.seriesParams.forEach((series, i) => {
@@ -154,13 +157,31 @@ module.directive('vislibValueAxes', function () {
               });
             }
           });
+
           if (matchingSeries.length === 1) {
-            label = matchingSeries[0].makeLabel();
+            newCustomLabel = matchingSeries[0].makeLabel();
           }
-          if (lastAxisTitles[axis.id] !== label && label !== '') {
-            lastAxisTitles[axis.id] = label;
-            axis.title.text = label;
+
+          const matchingSeriesAggType = _.get(matchingSeries, '[0]type.name', '');
+          const matchingSeriesAggField = _.get(matchingSeries, '[0]params.field.name', '');
+
+          if (lastCustomLabels[axis.id] !== newCustomLabel && newCustomLabel !== '') {
+            const isFirstRender = Object.keys(lastCustomLabels).length === 0;
+            const aggTypeIsChanged = lastMatchingSeriesAggType !== matchingSeriesAggType;
+            const aggFieldIsChanged = lastMatchingSeriesAggField !== matchingSeriesAggField;
+            const aggIsChanged = aggTypeIsChanged || aggFieldIsChanged;
+            const axisTitleIsEmpty = axis.title.text === '';
+            const lastCustomLabelMatchesAxisTitle = lastCustomLabels[axis.id] === axis.title.text;
+
+            if (!isFirstRender && (aggIsChanged || axisTitleIsEmpty || lastCustomLabelMatchesAxisTitle)) {
+              axis.title.text = newCustomLabel; // Override axis title with new custom label
+            }
+
+            lastCustomLabels[axis.id] = newCustomLabel;
           }
+
+          lastMatchingSeriesAggType = matchingSeriesAggType;
+          lastMatchingSeriesAggField = matchingSeriesAggField;
         });
       };
 

diff --git a/src/core_plugins/kbn_vislib_vis_types/public/editors/__tests__/point_series.js b/src/core_plugins/kbn_vislib_vis_types/public/editors/__tests__/point_series.js
@@ -136,4 +136,41 @@ describe('point series editor', function () {
     $parentScope.updateAxisTitle();
     expect($parentScope.editorState.params.valueAxes[0].title.text).to.equal('Custom Title');
   });
+
+  it('should set the custom title to match the value axis label when only one agg exists for that axis', function () {
+    $parentScope.editorState.aggs[0].params.customLabel = 'Custom Label';
+    $parentScope.updateAxisTitle();
+    expect($parentScope.editorState.params.valueAxes[0].title.text).to.equal('Custom Label');
+  });
+
+  it('should not set the custom title to match the value axis label when more than one agg exists for that axis', function () {
+    const aggConfig = new AggConfig($parentScope.vis.aggs, { type: 'avg', schema: 'metric', params: { field: 'bytes' } });
+    $parentScope.vis.aggs.push(aggConfig);
+    $parentScope.$digest();
+    $parentScope.editorState.aggs[0].params.customLabel = 'Custom Label';
+    $parentScope.updateAxisTitle();
+    expect($parentScope.editorState.params.valueAxes[0].title.text).to.equal('Count');
+  });
+
+  it('should not overwrite the custom title with the value axis label if the custom title has been changed', function () {
+    $parentScope.editorState.params.valueAxes[0].title.text = 'Custom Title';
+    $parentScope.editorState.aggs[0].params.customLabel = 'Custom Label';
+    $parentScope.updateAxisTitle();
+    expect($parentScope.editorState.params.valueAxes[0].title.text).to.equal('Custom Title');
+  });
+
+  it('should overwrite the custom title when the agg type changes', function () {
+    const aggConfig = new AggConfig($parentScope.vis.aggs, { type: 'avg', schema: 'metric', params: { field: 'bytes' } });
+
+    $parentScope.editorState.params.valueAxes[0].title.text = 'Custom Title';
+    $parentScope.editorState.aggs[0].params.customLabel = 'Custom Label';
+    $parentScope.updateAxisTitle();
+
+    $parentScope.vis.aggs.push(aggConfig);
+    $parentScope.vis.aggs.shift();
+    $parentScope.$digest();
+    $parentScope.updateAxisTitle();
+
+    expect($parentScope.editorState.params.valueAxes[0].title.text).to.equal('Average bytes');
+  });
 });
diff --git a/test/functional/apps/visualize/_pie_chart.js b/test/functional/apps/visualize/_pie_chart.js
@@ -248,7 +248,7 @@ export default function ({ getService, getPageObjects }) {
         log.debug('Set the 1st filter value');
         await PageObjects.visualize.setFilterAggregationValue('geo.dest:"US"');
         log.debug('Toggle previous editor');
-        await PageObjects.visualize.toggleAggegationEditor(2);
+        await PageObjects.visualize.toggleAggregationEditor(2);
         log.debug('Add a new series');
         await PageObjects.visualize.clickAddBucket();
         log.debug('select bucket Split Slices');

diff --git a/test/functional/apps/visualize/_point_series_options.js b/test/functional/apps/visualize/_point_series_options.js
@@ -137,5 +137,46 @@ export default function ({ getService, getPageObjects }) {
       });
     });
 
+    describe('custom labels and axis titles', function () {
+      const visName = 'Visualization Point Series Test';
+      const customLabel = 'myLabel';
+      const axisTitle = 'myTitle';
+      before(async function () {
+        await PageObjects.visualize.navigateToNewVisualization();
+        await PageObjects.visualize.clickLineChart();
+        await PageObjects.visualize.clickNewSearch();
+        await PageObjects.visualize.selectYAxisAggregation('Average', 'bytes', customLabel, 1);
+        await PageObjects.visualize.clickGo();
+        await PageObjects.visualize.clickMetricsAndAxes();
+        await PageObjects.visualize.clickYAxisOptions('ValueAxis-1');
+      });
+
+      it('should render a custom label when one is set', async function () {
+        const title = await PageObjects.visualize.getYAxisTitle();
+        expect(title).to.be(customLabel);
+      });
+
+      it('should render a custom axis title when one is set, overriding the custom label', async function () {
+        await pointSeriesVis.setAxisTitle(axisTitle);
+        await PageObjects.visualize.clickGo();
+        const title = await PageObjects.visualize.getYAxisTitle();
+        expect(title).to.be(axisTitle);
+      });
+
+      it('should preserve saved axis titles after a vis is saved and reopened', async function () {
+        await PageObjects.visualize.saveVisualizationExpectSuccess(visName);
+        await PageObjects.visualize.loadSavedVisualization(visName);
+        await PageObjects.visualize.waitForVisualization();
+        await PageObjects.visualize.clickData();
+        await PageObjects.visualize.toggleOpenEditor(1);
+        await PageObjects.visualize.setCustomLabel('test', 1);
+        await PageObjects.visualize.clickGo();
+        await PageObjects.visualize.clickMetricsAndAxes();
+        await PageObjects.visualize.clickYAxisOptions('ValueAxis-1');
+        const title = await PageObjects.visualize.getYAxisTitle();
+        expect(title).to.be(axisTitle);
+      });
+    });
+
   });
 }
diff --git a/test/functional/page_objects/point_series_page.js b/test/functional/page_objects/point_series_page.js
@@ -44,6 +44,14 @@ export function PointSeriesPageProvider({ getService }) {
       return await testSubjects.click('visualizeAddYAxisButton');
     }
 
+    setAxisTitle(title, { index = 0 } = {}) {
+      return remote
+        .setFindTimeout(defaultFindTimeout)
+        .findByCssSelector(`#valueAxisTitle${index}`)
+        .clearValue()
+        .type(title);
+    }
+
     getValueAxesCount() {
       return remote
         .setFindTimeout(defaultFindTimeout)

diff --git a/test/functional/page_objects/visualize_page.js b/test/functional/page_objects/visualize_page.js
@@ -605,7 +605,8 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       await testSubjects.click(`aggregationEditor${agg} disableAggregationBtn`);
       await PageObjects.header.waitUntilLoadingHasFinished();
     }
-    async toggleAggegationEditor(agg) {
+
+    async toggleAggregationEditor(agg) {
       await testSubjects.click(`aggregationEditor${agg} toggleEditor`);
       await PageObjects.header.waitUntilLoadingHasFinished();
     }
@@ -649,7 +650,6 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       await PageObjects.header.waitUntilLoadingHasFinished();
     }
 
-
     async changeHeatmapColorNumbers(value = 6) {
       const input = await testSubjects.find(`heatmapOptionsColorsNumberInput`);
       await input.clearValue();
@@ -686,6 +686,7 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
       toCell.clearValue();
       toCell.type(`${to}`);
     }
+
     async clickYAxisOptions(axisId) {
       await testSubjects.click(`toggleYAxisOptions-${axisId}`);
     }