Skip to content

Commit

Permalink
Fix line to vega conversion bug (#4554)
Browse files Browse the repository at this point in the history
* Fix line to vega conversion bug

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>

* Update CHANGELOG and release notes

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>

* Address comments

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>

* Add more details to comment

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>

* Not a released changed, so no need to document

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>

---------

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>
(cherry picked from commit b94a62f)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] authored and lezzago committed Jul 12, 2023
1 parent df28cbd commit 9faf339
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 27 deletions.
2 changes: 2 additions & 0 deletions src/plugins/vis_type_vega/public/expressions/helpers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
59 changes: 32 additions & 27 deletions src/plugins/vis_type_vega/public/expressions/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 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

Check warning on line 208 in src/plugins/vis_type_vega/public/expressions/helpers.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/expressions/helpers.ts#L206-L208

Added lines #L206 - L208 were not covered by tests
);
if (!currentSeriesParams) {
// eslint-disable-next-line no-console
console.error(`Failed to find matching series param for column of id: ${column.id}`);
return;

Check warning on line 213 in src/plugins/vis_type_vega/public/expressions/helpers.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/expressions/helpers.ts#L212-L213

Added lines #L212 - L213 were not covered by tests
}
const currentValueAxis = valueAxis.get(currentSeriesParams.valueAxis.toString());
let tooltip: Array<{ field: string; type: string; title: string }> = [];

Check warning on line 216 in src/plugins/vis_type_vega/public/expressions/helpers.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/expressions/helpers.ts#L215-L216

Added lines #L215 - L216 were not covered by tests
if (visParams.addTooltip) {
tooltip = [

Check warning on line 218 in src/plugins/vis_type_vega/public/expressions/helpers.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/expressions/helpers.ts#L218

Added line #L218 was not covered by tests
{ field: xAxisId, type: 'temporal', title: xAxisTitle },
{ field: column.id, type: 'quantitative', title: column.name },
];
}
spec.layer.push({

Check warning on line 223 in src/plugins/vis_type_vega/public/expressions/helpers.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/expressions/helpers.ts#L223

Added line #L223 was not covered by tests
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,
},
},
});
});
}

Expand Down

0 comments on commit 9faf339

Please sign in to comment.