bito-code-review[bot] commented on code in PR #43238:
URL: https://github.com/apache/superset/pull/43238#discussion_r3801964797
##########
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", () => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Reduce test data duplication</b></div>
<div id="fix">
Test data duplication detected in series.test.ts. Lines 260-276 and 292-308
contain identical expected arrays for sortRows min tests, with similar
duplication for sum tests (202-212, 218-228, 234-244, 250-260, 266-276,
282-292, 298-308, 314-324). Consider consolidating test data into shared
constants.
</div>
</div>
<small><i>Code Review Run #be849c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>SortSeriesType.Min used instead of Max</b></div>
<div id="fix">
Test calls `sortRows` with `SortSeriesType.Min` (line 314) despite test name
declaring 'max descending'. Consistent with line 298 bug - same typo pattern.
</div>
</div>
<small><i>Code Review Run #be849c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>SortSeriesType.Min used instead of Max</b></div>
<div id="fix">
Test calls `sortRows` with `SortSeriesType.Min` (line 298) despite test name
declaring 'max ascending'. The source implementation correctly supports
`SortSeriesType.Max` via a dedicated switch case (series.ts:570-575), so this
mismatch will cause incorrect behavior when `sortRows` is invoked with
`SortSeriesType.Max`.
</div>
</div>
<small><i>Code Review Run #be849c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -109,66 +109,66 @@ import {
transformIntervalAnnotation,
transformSeries,
transformTimeseriesAnnotation,
-} from './transformers';
+} from "./transformers";
import {
OpacityEnum,
StackControlsValue,
TIMEGRAIN_TO_TIMESTAMP,
TIMESERIES_CONSTANTS,
-} from '../constants';
-import { getDefaultTooltip } from '../utils/tooltip';
+} from "../constants";
+import { getDefaultTooltip } from "../utils/tooltip";
import {
getPercentFormatter,
getTooltipTimeFormatter,
getXAxisFormatter,
getYAxisFormatter,
-} from '../utils/formatters';
-import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser';
-import { mergeCustomEChartOptions } from '../utils/mergeCustomEChartOptions';
+} from "../utils/formatters";
+import { safeParseEChartOptions } from "../utils/safeEChartOptionsParser";
+import { mergeCustomEChartOptions } from "../utils/mergeCustomEChartOptions";
-const visibleDashPatterns: ([number, number] | 'dashed' | 'dotted')[] = [
- 'dashed',
- 'dotted',
+const visibleDashPatterns: ([number, number] | "dashed" | "dotted")[] = [
+ "dashed",
+ "dotted",
[6, 15], // narrow dashed
[2, 10], // wide dotted
[20, 3], // wide dashed
];
const visibleSymbols = [
- 'rect',
- 'triangle',
- 'diamond',
- 'roundRect',
- 'pin',
+ "rect",
+ "triangle",
+ "diamond",
+ "roundRect",
+ "pin",
] as const;
function getSymbolMarker(symbol: string, color: string) {
const size = 10;
switch (symbol) {
- case 'circle':
+ case "circle":
return `<span style="
display:inline-block;width:${size}px;height:${size}px;
border-radius:50%;background:${color};margin-right:5px"></span>`;
- case 'rect':
+ case "rect":
return `<span style="
display:inline-block;width:${size}px;height:${size}px;
background:${color};margin-right:5px"></span>`;
- case 'roundRect':
+ case "roundRect":
return `<span style="
display:inline-block;width:${size}px;height:${size}px;border-radius:2px;
background:${color};margin-right:5px"></span>`;
- case 'triangle':
+ case "triangle":
return `<span style="
display:inline-block;width:0;height:0;
border-left:${size / 2}px solid transparent;
border-right:${size / 2}px solid transparent;
border-bottom:${size}px solid ${color};
margin-right:5px"></span>`;
- case 'diamond':
+ case "diamond":
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Extract marker HTML generation</b></div>
<div id="fix">
Consider extracting the HTML span generation for marker types into a helper
function to reduce duplication. The duplicate patterns appear in
transformProps.ts at lines 166-170 and 171-175 (diamond), with similar
structures for circle (147-150, 151-154, 176-179) and other marker types.
</div>
</div>
<small><i>Code Review Run #be849c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1108,9 +1159,9 @@ export default function transformProps(
// at the axis boundary.
...(showMaxLabel && {
showMaxLabel: true,
- alignMaxLabel: 'right',
+ alignMaxLabel: "right",
showMinLabel: true,
- alignMinLabel: 'left',
+ alignMinLabel: "left",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Share axis config logic</b></div>
<div id="fix">
Duplicate axis configuration logic found in Timeseries/transformProps.ts
(lines 1164-1185) and MixedTimeseries/transformProps.ts (lines 778-799).
Consider extracting the shared minInterval/maxInterval and
getMinAndMaxFromBounds logic into a common utility.
</div>
</div>
<small><i>Code Review Run #be849c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
