bito-code-review[bot] commented on code in PR #43238:
URL: https://github.com/apache/superset/pull/43238#discussion_r3952310177
##########
superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts:
##########
@@ -129,359 +121,359 @@ const expectedThemeProps = {
};
const sortData: DataRecord[] = [
- { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
- { my_x_axis: 'foo', x: null, y: 10, z: 5 },
+ { my_x_axis: "abc", x: 1, y: 0, z: 2 },
+ { my_x_axis: "foo", x: null, y: 10, z: 5 },
{ my_x_axis: null, x: 4, y: 3, z: 7 },
];
const sortDataWithNumbers: DataRecord[] = [
{
- my_x_axis: 'my_axis',
- '9. September': 6,
+ my_x_axis: "my_axis",
+ "9. September": 6,
6: 1,
- '11. November': 8,
+ "11. November": 8,
8: 2,
- '10. October': 1,
+ "10. October": 1,
10: 4,
- '3. March': 2,
- '8. August': 6,
+ "3. March": 2,
+ "8. August": 6,
2: 1,
12: 3,
9: 1,
- '1. January': 1,
- '4. April': 12,
- '2. February': 9,
+ "1. January": 1,
+ "4. April": 12,
+ "2. February": 9,
5: 4,
3: 1,
11: 2,
- '12. December': 4,
+ "12. December": 4,
1: 7,
- '6. June': 1,
+ "6. June": 1,
4: 5,
7: 2,
c: 0,
- '7. July': 2,
+ "7. July": 2,
d: 0,
- '5. May': 4,
+ "5. May": 4,
a: 1,
},
];
const totalStackedValues = [3, 15, 14];
-test('sortRows by name ascending', () => {
+test("sortRows by name ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Name,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
]);
});
-test('sortRows by name descending', () => {
+test("sortRows by name descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Name,
false,
),
).toEqual([
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by sum ascending', () => {
+test("sortRows by sum ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Sum,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by sum descending', () => {
+test("sortRows by sum descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Sum,
false,
),
).toEqual([
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by avg ascending', () => {
+test("sortRows by avg ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Avg,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by avg descending', () => {
+test("sortRows by avg descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Avg,
false,
),
).toEqual([
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by min ascending', () => {
+test("sortRows by min ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by min descending', () => {
+test("sortRows by min descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
false,
),
).toEqual([
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by max ascending', () => {
+test("sortRows by max ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
Review Comment:
<!-- Bito Reply -->
The user has acknowledged the issue identified by the reviewer regarding the
incorrect use of `SortSeriesType.Min` in the test case. The fix involves
updating the test to use `SortSeriesType.Max` as intended by the test name.
**superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts**
```
test("sortRows by max ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
"my_x_axis",
SortSeriesType.Max,
true,
),
).toEqual([
```
##########
superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts:
##########
@@ -129,359 +121,359 @@ const expectedThemeProps = {
};
const sortData: DataRecord[] = [
- { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
- { my_x_axis: 'foo', x: null, y: 10, z: 5 },
+ { my_x_axis: "abc", x: 1, y: 0, z: 2 },
+ { my_x_axis: "foo", x: null, y: 10, z: 5 },
{ my_x_axis: null, x: 4, y: 3, z: 7 },
];
const sortDataWithNumbers: DataRecord[] = [
{
- my_x_axis: 'my_axis',
- '9. September': 6,
+ my_x_axis: "my_axis",
+ "9. September": 6,
6: 1,
- '11. November': 8,
+ "11. November": 8,
8: 2,
- '10. October': 1,
+ "10. October": 1,
10: 4,
- '3. March': 2,
- '8. August': 6,
+ "3. March": 2,
+ "8. August": 6,
2: 1,
12: 3,
9: 1,
- '1. January': 1,
- '4. April': 12,
- '2. February': 9,
+ "1. January": 1,
+ "4. April": 12,
+ "2. February": 9,
5: 4,
3: 1,
11: 2,
- '12. December': 4,
+ "12. December": 4,
1: 7,
- '6. June': 1,
+ "6. June": 1,
4: 5,
7: 2,
c: 0,
- '7. July': 2,
+ "7. July": 2,
d: 0,
- '5. May': 4,
+ "5. May": 4,
a: 1,
},
];
const totalStackedValues = [3, 15, 14];
-test('sortRows by name ascending', () => {
+test("sortRows by name ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Name,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
]);
});
-test('sortRows by name descending', () => {
+test("sortRows by name descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Name,
false,
),
).toEqual([
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by sum ascending', () => {
+test("sortRows by sum ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Sum,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by sum descending', () => {
+test("sortRows by sum descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Sum,
false,
),
).toEqual([
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by avg ascending', () => {
+test("sortRows by avg ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Avg,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by avg descending', () => {
+test("sortRows by avg descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Avg,
false,
),
).toEqual([
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by min ascending', () => {
+test("sortRows by min ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by min descending', () => {
+test("sortRows by min descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
false,
),
).toEqual([
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
]);
});
-test('sortRows by max ascending', () => {
+test("sortRows by max ascending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
true,
),
).toEqual([
- { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: "abc", x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
{ row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
- { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: "foo", x: null, y: 10, z: 5 }, totalStackedValue: 15 },
]);
});
-test('sortRows by max descending', () => {
+test("sortRows by max descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
- 'my_x_axis',
+ "my_x_axis",
SortSeriesType.Min,
Review Comment:
<!-- Bito Reply -->
The suggestion to use `SortSeriesType.Max` instead of `SortSeriesType.Min`
in the test case is correct. The test name 'sortRows by max descending'
indicates that the test should be verifying the maximum value, but the current
implementation incorrectly uses `SortSeriesType.Min`.
**superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts**
```
test("sortRows by max descending", () => {
expect(
sortRows(
sortData,
totalStackedValues,
"my_x_axis",
SortSeriesType.Max,
false,
),
).toEqual([
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
