From 863f4c8078ed7d1e908c9ce4d7535f54b37b17f4 Mon Sep 17 00:00:00 2001 From: Ashish Agrawal Date: Wed, 12 Jul 2023 11:11:14 -0700 Subject: [PATCH 1/5] Fix line to vega conversion bug Signed-off-by: Ashish Agrawal --- .../public/expressions/helpers.ts | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/src/plugins/vis_type_vega/public/expressions/helpers.ts b/src/plugins/vis_type_vega/public/expressions/helpers.ts index 1fffd91b0bd..eaf2136eb95 100644 --- a/src/plugins/vis_type_vega/public/expressions/helpers.ts +++ b/src/plugins/vis_type_vega/public/expressions/helpers.ts @@ -198,35 +198,40 @@ export const createSpecFromXYChartDatatable = ( if (datatable.rows.length > 0 && dimensions.x !== null) { const xAxisId = getXAxisId(dimensions, datatable.columns); const xAxisTitle = cleanString(dimensions.x.label); - let seriesParamSkipCount = 0; datatable.columns.forEach((column, index) => { - // Don't add a layer for x axis column - if (isXAxisColumn(column)) { - seriesParamSkipCount++; - // Don't add a layer for vis layer column - } else if (!isVisLayerColumn(column)) { - const currentSeriesParams = visParams.seriesParams[index - seriesParamSkipCount]; - const currentValueAxis = valueAxis.get(currentSeriesParams.valueAxis.toString()); - let tooltip: Array<{ field: string; type: string; title: string }> = []; - if (visParams.addTooltip) { - tooltip = [ - { field: xAxisId, type: 'temporal', title: xAxisTitle }, - { field: column.id, type: 'quantitative', title: column.name }, - ]; - } - spec.layer.push({ - mark: buildLayerMark(currentSeriesParams), - encoding: { - x: buildXAxis(xAxisTitle, xAxisId, visParams), - y: buildYAxis(column, currentValueAxis, visParams), - tooltip, - color: { - // This ensures all the different metrics have their own distinct and unique color - datum: column.name, - }, - }, - }); + // Ignore columns that are for the x axis and events (eg. Alerts/Anomalies) + if (isXAxisColumn(column) || isVisLayerColumn(column)) return; + // Get the series param id which is the 2nd value in the column id + // Example: 'col-1-3', the seriesParamId is 3. 'col-1-2-6', the seriesParamId is 2. + const seriesParamsId = column.id.split('-')[2]; + const currentSeriesParams = visParams.seriesParams.find( + (param: { data: { id: string } }) => param.data.id === seriesParamsId + ); + if (!currentSeriesParams) { + // eslint-disable-next-line no-console + console.error(`Failed to find matching series param for column of id: ${column.id}`); + return; + } + const currentValueAxis = valueAxis.get(currentSeriesParams.valueAxis.toString()); + let tooltip: Array<{ field: string; type: string; title: string }> = []; + if (visParams.addTooltip) { + tooltip = [ + { field: xAxisId, type: 'temporal', title: xAxisTitle }, + { field: column.id, type: 'quantitative', title: column.name }, + ]; } + spec.layer.push({ + mark: buildLayerMark(currentSeriesParams), + encoding: { + x: buildXAxis(xAxisTitle, xAxisId, visParams), + y: buildYAxis(column, currentValueAxis, visParams), + tooltip, + color: { + // This ensures all the different metrics have their own distinct and unique color + datum: column.name, + }, + }, + }); }); } From e7b7678890f31c78f92dc7e8d73b1d8e7f9d2308 Mon Sep 17 00:00:00 2001 From: Ashish Agrawal Date: Wed, 12 Jul 2023 11:18:03 -0700 Subject: [PATCH 2/5] Update CHANGELOG and release notes Signed-off-by: Ashish Agrawal --- CHANGELOG.md | 1 + release-notes/opensearch-dashboards.release-notes-2.9.0.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfdb7161dcf..741a39d74b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Saved Objects Management] Fix relationships header overflow ([#4070](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4070)) - Update main menu to display 'Dashboards' for consistency ([#4453](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4453)) - [Multiple DataSource] Retain the original sample data API ([#4526](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4526)) +- Fix line to vega conversion bug ([#4554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4554)) ### 🚞 Infrastructure diff --git a/release-notes/opensearch-dashboards.release-notes-2.9.0.md b/release-notes/opensearch-dashboards.release-notes-2.9.0.md index aeff6c0af5d..a6039a84535 100644 --- a/release-notes/opensearch-dashboards.release-notes-2.9.0.md +++ b/release-notes/opensearch-dashboards.release-notes-2.9.0.md @@ -33,6 +33,7 @@ - Update main menu to display 'Dashboards' for consistency ([#4453](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4453)) - [Multiple DataSource] Retain the original sample data API ([#4526](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4526)) - Remove `lmdb-store` to fix backport issue ([#4266](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4266)) +- Fix line to vega conversion bug ([#4554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4554)) ### 🚞 Infrastructure From 9b8a692fb54d04987314c46d9c8f632ab64070c3 Mon Sep 17 00:00:00 2001 From: Ashish Agrawal Date: Wed, 12 Jul 2023 11:36:47 -0700 Subject: [PATCH 3/5] Address comments Signed-off-by: Ashish Agrawal --- src/plugins/vis_type_vega/public/expressions/helpers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/vis_type_vega/public/expressions/helpers.ts b/src/plugins/vis_type_vega/public/expressions/helpers.ts index eaf2136eb95..5f65fe5f5bd 100644 --- a/src/plugins/vis_type_vega/public/expressions/helpers.ts +++ b/src/plugins/vis_type_vega/public/expressions/helpers.ts @@ -199,13 +199,13 @@ export const createSpecFromXYChartDatatable = ( const xAxisId = getXAxisId(dimensions, datatable.columns); const xAxisTitle = cleanString(dimensions.x.label); datatable.columns.forEach((column, index) => { - // Ignore columns that are for the x axis and events (eg. Alerts/Anomalies) + // Ignore columns that are for the x-axis and visLayers if (isXAxisColumn(column) || isVisLayerColumn(column)) return; // Get the series param id which is the 2nd value in the column id // Example: 'col-1-3', the seriesParamId is 3. 'col-1-2-6', the seriesParamId is 2. const seriesParamsId = column.id.split('-')[2]; const currentSeriesParams = visParams.seriesParams.find( - (param: { data: { id: string } }) => param.data.id === seriesParamsId + (param: { data: { id: string } }) => param?.data?.id === seriesParamsId ); if (!currentSeriesParams) { // eslint-disable-next-line no-console From 33918525c8d255e7fdf04dd777d77c099641696d Mon Sep 17 00:00:00 2001 From: Ashish Agrawal Date: Wed, 12 Jul 2023 11:52:11 -0700 Subject: [PATCH 4/5] Add more details to comment Signed-off-by: Ashish Agrawal --- src/plugins/vis_type_vega/public/expressions/helpers.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/vis_type_vega/public/expressions/helpers.test.js b/src/plugins/vis_type_vega/public/expressions/helpers.test.js index 8630b3c69ad..bf8bf28e9ca 100644 --- a/src/plugins/vis_type_vega/public/expressions/helpers.test.js +++ b/src/plugins/vis_type_vega/public/expressions/helpers.test.js @@ -194,6 +194,8 @@ describe('helpers', function () { describe('createSpecFromXYChartDatatable()', function () { // Following 3 tests fail since they are persisting temporal data // which can cause snapshots to fail depending on the test env they are run on. + // Tracking issue: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4555 + // TODO: Add a test for the fix in this PR: https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4554 it.skip('build simple line chart"', function () { expect( JSON.stringify( From 7cbb3fd0ab4e7e256722d7349167d6bd9da0daaf Mon Sep 17 00:00:00 2001 From: Ashish Agrawal Date: Wed, 12 Jul 2023 12:09:07 -0700 Subject: [PATCH 5/5] Not a released changed, so no need to document Signed-off-by: Ashish Agrawal --- CHANGELOG.md | 1 - release-notes/opensearch-dashboards.release-notes-2.9.0.md | 1 - 2 files changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 741a39d74b0..cfdb7161dcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Saved Objects Management] Fix relationships header overflow ([#4070](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4070)) - Update main menu to display 'Dashboards' for consistency ([#4453](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4453)) - [Multiple DataSource] Retain the original sample data API ([#4526](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4526)) -- Fix line to vega conversion bug ([#4554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4554)) ### 🚞 Infrastructure diff --git a/release-notes/opensearch-dashboards.release-notes-2.9.0.md b/release-notes/opensearch-dashboards.release-notes-2.9.0.md index a6039a84535..aeff6c0af5d 100644 --- a/release-notes/opensearch-dashboards.release-notes-2.9.0.md +++ b/release-notes/opensearch-dashboards.release-notes-2.9.0.md @@ -33,7 +33,6 @@ - Update main menu to display 'Dashboards' for consistency ([#4453](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4453)) - [Multiple DataSource] Retain the original sample data API ([#4526](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4526)) - Remove `lmdb-store` to fix backport issue ([#4266](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4266)) -- Fix line to vega conversion bug ([#4554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4554)) ### 🚞 Infrastructure