From 01f724959d74241ef52f062630850c2ba874222b Mon Sep 17 00:00:00 2001
From: Shefali Joshi <simplyrender@gmail.com>
Date: Thu, 26 Jan 2023 09:11:13 -0800
Subject: [PATCH] Ensure limit lines for both the old and new y axes are
 redrawn when a series moves from one y axis to another (#6181)

Optimize initialization of Plot configuration
Ensure the the y axis form correctly saves any changes to the configuration
Fix excluded limits test
---
 .../plot/chart/MCTChartAlarmLineSet.js        |  3 ++
 src/plugins/plot/chart/MctChart.vue           | 11 +++--
 .../configuration/PlotConfigurationModel.js   | 27 ++++++-----
 .../plot/inspector/forms/YAxisForm.vue        |  4 +-
 src/plugins/plot/pluginSpec.js                | 45 ++++++++++---------
 5 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/src/plugins/plot/chart/MCTChartAlarmLineSet.js b/src/plugins/plot/chart/MCTChartAlarmLineSet.js
index 3514941840..23dc82b3ee 100644
--- a/src/plugins/plot/chart/MCTChartAlarmLineSet.js
+++ b/src/plugins/plot/chart/MCTChartAlarmLineSet.js
@@ -105,6 +105,9 @@ export default class MCTChartAlarmLineSet {
 
     reset() {
         this.limits = [];
+        if (this.series.limits) {
+            this.getLimitPoints(this.series);
+        }
     }
 
     destroy() {
diff --git a/src/plugins/plot/chart/MctChart.vue b/src/plugins/plot/chart/MctChart.vue
index 17fed18196..75f7b0aa7c 100644
--- a/src/plugins/plot/chart/MctChart.vue
+++ b/src/plugins/plot/chart/MctChart.vue
@@ -121,6 +121,7 @@ export default {
         hiddenYAxisIds() {
             this.hiddenYAxisIds.forEach(id => {
                 this.resetYOffsetAndSeriesDataForYAxis(id);
+                this.drawLimitLines();
             });
             this.scheduleDraw();
         }
@@ -196,6 +197,7 @@ export default {
             this.listenTo(series, 'change:alarmMarkers', this.changeAlarmMarkers, this);
             this.listenTo(series, 'change:limitLines', this.changeLimitLines, this);
             this.listenTo(series, 'change:yAxisId', this.resetAxisAndRedraw, this);
+            // TODO: Which other changes is the listener below reacting to?
             this.listenTo(series, 'change', this.scheduleDraw);
             this.listenTo(series, 'add', this.onAddPoint);
             this.makeChartElement(series);
@@ -625,9 +627,13 @@ export default {
             alarmSets.forEach(this.drawAlarmPoints, this);
         },
         drawLimitLines() {
+            Array.from(this.$refs.limitArea.children).forEach((el) => el.remove());
             this.config.series.models.forEach(series => {
                 const yAxisId = series.get('yAxisId');
-                this.drawLimitLinesForSeries(yAxisId, series);
+
+                if (this.hiddenYAxisIds.indexOf(yAxisId) < 0) {
+                    this.drawLimitLinesForSeries(yAxisId, series);
+                }
             });
         },
         drawLimitLinesForSeries(yAxisId, series) {
@@ -641,12 +647,11 @@ export default {
                 return;
             }
 
-            Array.from(this.$refs.limitArea.children).forEach((el) => el.remove());
             let limitPointOverlap = [];
             this.limitLines.forEach((limitLine) => {
                 let limitContainerEl = this.$refs.limitArea;
                 limitLine.limits.forEach((limit) => {
-                    if (!series.includes(limit.seriesKey)) {
+                    if (series.keyString !== limit.seriesKey) {
                         return;
                     }
 
diff --git a/src/plugins/plot/configuration/PlotConfigurationModel.js b/src/plugins/plot/configuration/PlotConfigurationModel.js
index 33ad0ed1c0..b56c058fcc 100644
--- a/src/plugins/plot/configuration/PlotConfigurationModel.js
+++ b/src/plugins/plot/configuration/PlotConfigurationModel.js
@@ -68,27 +68,26 @@ export default class PlotConfigurationModel extends Model {
         //Add any axes in addition to the main yAxis above - we must always have at least 1 y-axis
         //Addition axes ids will be the MAIN_Y_AXES_ID + x where x is between 1 and MAX_ADDITIONAL_AXES
         this.additionalYAxes = [];
-        if (Array.isArray(options.model.additionalYAxes)) {
-            const maxLength = Math.min(MAX_ADDITIONAL_AXES, options.model.additionalYAxes.length);
-            for (let yAxisCount = 0; yAxisCount < maxLength; yAxisCount++) {
-                const yAxis = options.model.additionalYAxes[yAxisCount];
+        const hasAdditionalAxesConfiguration = Array.isArray(options.model.additionalYAxes);
+
+        for (let yAxisCount = 0; yAxisCount < MAX_ADDITIONAL_AXES; yAxisCount++) {
+            const yAxisId = MAIN_Y_AXES_ID + yAxisCount + 1;
+            const yAxis = hasAdditionalAxesConfiguration && options.model.additionalYAxes.find(additionalYAxis => additionalYAxis?.id === yAxisId);
+            if (yAxis) {
                 this.additionalYAxes.push(new YAxisModel({
                     model: yAxis,
                     plot: this,
                     openmct: options.openmct,
-                    id: yAxis.id || (MAIN_Y_AXES_ID + yAxisCount + 1)
+                    id: yAxis.id
+                }));
+            } else {
+                this.additionalYAxes.push(new YAxisModel({
+                    plot: this,
+                    openmct: options.openmct,
+                    id: yAxisId
                 }));
             }
         }
-
-        // If the saved options config doesn't include information about all the additional axes, we initialize the remaining here
-        for (let axesCount = this.additionalYAxes.length; axesCount < MAX_ADDITIONAL_AXES; axesCount++) {
-            this.additionalYAxes.push(new YAxisModel({
-                plot: this,
-                openmct: options.openmct,
-                id: MAIN_Y_AXES_ID + axesCount + 1
-            }));
-        }
         // end add additional axes
 
         this.legend = new LegendModel({
diff --git a/src/plugins/plot/inspector/forms/YAxisForm.vue b/src/plugins/plot/inspector/forms/YAxisForm.vue
index 7bcb3e3e93..fc87ef2e40 100644
--- a/src/plugins/plot/inspector/forms/YAxisForm.vue
+++ b/src/plugins/plot/inspector/forms/YAxisForm.vue
@@ -227,8 +227,8 @@ export default {
             let prefix = 'yAxis';
             if (this.isAdditionalYAxis) {
                 let index = -1;
-                if (this.additionalYAxes) {
-                    index = this.additionalYAxes.findIndex((yAxis) => {
+                if (this.domainObject?.configuration?.additionalYAxes) {
+                    index = this.domainObject?.configuration?.additionalYAxes.findIndex((yAxis) => {
                         return yAxis.id === this.id;
                     });
                 }
diff --git a/src/plugins/plot/pluginSpec.js b/src/plugins/plot/pluginSpec.js
index f8467f823e..7e40e56980 100644
--- a/src/plugins/plot/pluginSpec.js
+++ b/src/plugins/plot/pluginSpec.js
@@ -28,7 +28,7 @@ import EventEmitter from "EventEmitter";
 import PlotOptions from "./inspector/PlotOptions.vue";
 import PlotConfigurationModel from "./configuration/PlotConfigurationModel";
 
-const TEST_KEY_ID = 'test-key';
+const TEST_KEY_ID = 'some-other-key';
 
 describe("the plugin", function () {
     let element;
@@ -533,6 +533,30 @@ describe("the plugin", function () {
                 expect(openmct.telemetry.request).toHaveBeenCalledTimes(2);
 
             });
+
+            describe('limits', () => {
+
+                it('lines are not displayed by default', () => {
+                    let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line");
+                    expect(limitEl.length).toBe(0);
+                });
+
+                it('lines are displayed when configuration is set to true', (done) => {
+                    const configId = openmct.objects.makeKeyString(testTelemetryObject.identifier);
+                    const config = configStore.get(configId);
+                    config.yAxis.set('displayRange', {
+                        min: 0,
+                        max: 4
+                    });
+                    config.series.models[0].set('limitLines', true);
+
+                    Vue.nextTick(() => {
+                        let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line");
+                        expect(limitEl.length).toBe(4);
+                        done();
+                    });
+                });
+            });
         });
 
         describe('controls in time strip view', () => {
@@ -867,24 +891,5 @@ describe("the plugin", function () {
                 expect(colorSwatch).toBeDefined();
             });
         });
-
-        describe('limits', () => {
-
-            it('lines are not displayed by default', () => {
-                let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line");
-                expect(limitEl.length).toBe(0);
-            });
-
-            xit('lines are displayed when configuration is set to true', (done) => {
-                config.series.models[0].set('limitLines', true);
-
-                Vue.nextTick(() => {
-                    let limitEl = element.querySelectorAll(".js-limit-area .js-limit-line");
-                    expect(limitEl.length).toBe(4);
-                    done();
-                });
-
-            });
-        });
     });
 });