fix: dashboard stacked chart sorting issue #3517
Conversation
ktx-abhay
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
5 times, most recently
from
6e68bb5
to
85b7e16
Compare
ktx-abhay
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
3 times, most recently
from
8af4471
to
59ad6ca
Compare
WalkthroughThe recent updates primarily focus on enhancing data handling within the dashboard utilities. The Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant Dashboard as Dashboard
participant ConvertPanelData as convertPanelData
participant ConvertSQLData as convertSQLData
participant SQLUtils as sqlUtils
Dashboard->>ConvertPanelData: Request to convert panel data
ConvertPanelData->>ConvertSQLData: await convertSQLData()
ConvertSQLData->>SQLUtils: isGivenFieldInOrderBy()
SQLUtils-->>ConvertSQLData: Return field order status
ConvertSQLData-->>ConvertPanelData: Return converted data
ConvertPanelData-->>Dashboard: Return panel data
This diagram illustrates the updated control flow involving the asynchronous Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (4)
web/src/utils/query/sqlUtils.ts (2)
Line range hint
1-163: Consider replacinganytype with more specific types for better type safety.- let parser: any; + let parser: ParserType; // Assuming ParserType is the correct type, replace with actual type - const useSqlParser: any = await import("@/composables/useParser"); + const useSqlParser: UseSqlParserType = await import("@/composables/useParser"); // Replace UseSqlParserType with actual type - const astOfOriginalQuery: any = parser.astify(originalQuery); + const astOfOriginalQuery: AstType = parser.astify(originalQuery); // Replace AstType with actual type - const astOfDummy: any = parser.astify(dummyQuery); + const astOfDummy: AstType = parser.astify(dummyQuery); // Replace AstType with actual type - const ast: any = parser.astify(originalQuery); + const ast: AstType = parser.astify(originalQuery); // Replace AstType with actual type
Line range hint
53-61: Theelseclause inaddLabelToSQlQueryfunction is redundant and can be omitted for cleaner code.- } else { - const newCondition = { - type: "binary_expr", - operator: "AND", - left: { - ...ast.where, - }, - right: { - type: "binary_expr", - operator: operator, - left: { - type: "column_ref", - table: null, - column: label, - }, - right: { - type: "string", - value: value, - }, - }, - }; - - const newAst = { - ...ast, - where: newCondition, - }; - - const sql = parser.sqlify(newAst); - const quotedSql = sql.replace(/`/g, '"'); - - query = quotedSql; - } + const newCondition = { + type: "binary_expr", + operator: "AND", + left: { + ...ast.where, + }, + right: { + type: "binary_expr", + operator: operator, + left: { + type: "column_ref", + table: null, + column: label, + }, + right: { + type: "string", + value: value, + }, + }, + }; + + const newAst = { + ...ast, + where: newCondition, + }; + + const sql = parser.sqlify(newAst); + const quotedSql = sql.replace(/`/g, '"'); + + query = quotedSql;web/src/utils/dashboard/convertSQLData.ts (2)
Line range hint
142-142: Use strict equality===instead of==for comparisons to avoid unexpected type coercion.- if (params.axisDimension == "x") + if (params.axisDimension === "x")
Line range hint
143-144: Use template literals for string concatenation to improve readability and maintainability.- return `${lineBreaks} ${params.value}`; + return `${lineBreaks} ${params.value}`;
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/src/utils/dashboard/convertSQLData.ts (4 hunks)
- web/src/utils/query/sqlUtils.ts (1 hunks)
Additional Context Used
Biome (39)
web/src/utils/dashboard/convertSQLData.ts (20)
33-33: Unexpected any. Specify a different type.
34-34: Unexpected any. Specify a different type.
35-35: Unexpected any. Specify a different type.
36-36: Unexpected any. Specify a different type.
37-37: Unexpected any. Specify a different type.
59-59: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
73-73: Unexpected any. Specify a different type.
80-80: Unexpected any. Specify a different type.
82-82: Unexpected any. Specify a different type.
83-83: Unexpected any. Specify a different type.
90-90: Unexpected any. Specify a different type.
93-93: Unexpected any. Specify a different type.
113-113: Unexpected any. Specify a different type.
124-124: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
142-142: Use === instead of ==.
== is only allowed when comparing againstnull
143-143: Template literals are preferred over string concatenation.
144-144: Template literals are preferred over string concatenation.
158-158: Unexpected any. Specify a different type.
web/src/utils/query/sqlUtils.ts (19)
1-1: Unexpected any. Specify a different type.
6-6: Unexpected any. Specify a different type.
7-7: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
29-29: Unexpected any. Specify a different type.
30-30: Unexpected any. Specify a different type.
53-61: This else clause can be omitted because previous branches break early.
65-65: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
67-67: Unexpected any. Specify a different type.
68-68: Unexpected any. Specify a different type.
72-72: Unexpected any. Specify a different type.
137-137: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
152-152: Unexpected any. Specify a different type.
154-154: Change to an optional chain.
155-155: This let declares a variable that is only assigned once.
Additional comments not posted (1)
web/src/utils/dashboard/convertSQLData.ts (1)
1482-1527: Ensure the new sorting logic for stacked charts is robust and handles edge cases, especially whenisYAxisExistInOrderByreturnsnull.
web/src/utils/query/sqlUtils.ts
Outdated
| await importSqlParser(); | ||
| const ast: any = parser.astify(sqlQuery); | ||
|
|
||
| if (ast && ast.orderby) { |
There was a problem hiding this comment.
Use optional chaining for better safety when accessing properties of potentially null objects.
- if (ast && ast.orderby) {
+ if (ast?.orderby) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (ast && ast.orderby) { | |
| if (ast?.orderby) { |
web/src/utils/query/sqlUtils.ts
Outdated
| const ast: any = parser.astify(sqlQuery); | ||
|
|
||
| if (ast && ast.orderby) { | ||
| for (let item of ast.orderby) { |
There was a problem hiding this comment.
Consider using const instead of let for variables that are not reassigned.
- for (let item of ast.orderby) {
+ for (const item of ast.orderby) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| for (let item of ast.orderby) { | |
| for (const item of ast.orderby) { |
ktx-abhay
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
2 times, most recently
from
247b217
to
a90637f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
web/src/utils/query/sqlUtils.ts (2)
Line range hint
14-61: Consider refactoring to reduce redundancy in the conditional branches.- if (astOfOriginalQuery.where) { - const newWhereClause = { - type: "binary_expr", - operator: "AND", - left: { - ...astOfOriginalQuery.where, - parentheses: true, - }, - right: { - ...astOfDummy.where, - parentheses: true, - }, - }; - const newAst = { - ...astOfOriginalQuery, - where: newWhereClause, - }; - const sql = parser.sqlify(newAst); - const quotedSql = sql.replace(/`/g, '"'); - return quotedSql; - } else { - const newAst = { - ...astOfOriginalQuery, - where: astOfDummy.where, - }; - const sql = parser.sqlify(newAst); - const quotedSql = sql.replace(/`/g, '"'); - return quotedSql; - } + const newWhereClause = astOfOriginalQuery.where ? { + type: "binary_expr", + operator: "AND", + left: { + ...astOfOriginalQuery.where, + parentheses: true, + }, + right: { + ...astOfDummy.where, + parentheses: true, + }, + } : astOfDummy.where; + + const newAst = { + ...astOfOriginalQuery, + where: newWhereClause, + }; + const sql = parser.sqlify(newAst); + const quotedSql = sql.replace(/`/g, '"'); + return quotedSql;
Line range hint
65-137: Consider refactoring to reduce redundancy in the conditional branches.- if (!ast.where) { - // If there is no WHERE clause, create a new one - const newWhereClause = { - type: "binary_expr", - operator: operator, - left: { - type: "column_ref", - table: null, - column: label, - }, - right: { - type: "string", - value: value, - }, - }; - - const newAst = { - ...ast, - where: newWhereClause, - }; - - const sql = parser.sqlify(newAst); - const quotedSql = sql.replace(/`/g, '"'); - query = quotedSql; - } else { - const newCondition = { - type: "binary_expr", - operator: "AND", - // parentheses: true, - left: { - // parentheses: true, - ...ast.where, - }, - right: { - type: "binary_expr", - operator: operator, - left: { - type: "column_ref", - table: null, - column: label, - }, - right: { - type: "string", - value: value, - }, - }, - }; - - const newAst = { - ...ast, - where: newCondition, - }; - - const sql = parser.sqlify(newAst); - the quotedSql = sql.replace(/`/g, '"'); - - query = quotedSql; - } + const newCondition = { + type: "binary_expr", + operator: operator, + left: ast.where ? { + ...ast.where, + parentheses: true, + } : { + type: "column_ref", + table: null, + column: label, + }, + right: { + type: "string", + value: value, + }, + }; + + const newAst = { + ...ast, + where: newCondition, + }; + + const sql = parser.sqlify(newAst); + const quotedSql = sql.replace(/`/g, '"'); + return quotedSql;web/src/utils/dashboard/convertSQLData.ts (2)
Line range hint
142-142: Use strict equality comparison (===) instead of loose equality (==) to avoid type coercion errors.- if (params.axisDimension == "x") + if (params.axisDimension === "x")
Line range hint
143-144: Use template literals for string concatenation to improve readability and maintainability.- return "{a|" + name + "}" - return "{b|" + name + "}"; + return `{a|${name}}` + return `{b|${name}}`;
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/src/utils/dashboard/convertSQLData.ts (4 hunks)
- web/src/utils/query/sqlUtils.ts (1 hunks)
Additional Context Used
Biome (39)
web/src/utils/dashboard/convertSQLData.ts (20)
33-33: Unexpected any. Specify a different type.
34-34: Unexpected any. Specify a different type.
35-35: Unexpected any. Specify a different type.
36-36: Unexpected any. Specify a different type.
37-37: Unexpected any. Specify a different type.
59-59: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
73-73: Unexpected any. Specify a different type.
80-80: Unexpected any. Specify a different type.
82-82: Unexpected any. Specify a different type.
83-83: Unexpected any. Specify a different type.
90-90: Unexpected any. Specify a different type.
93-93: Unexpected any. Specify a different type.
113-113: Unexpected any. Specify a different type.
124-124: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
142-142: Use === instead of ==.
== is only allowed when comparing againstnull
143-143: Template literals are preferred over string concatenation.
144-144: Template literals are preferred over string concatenation.
158-158: Unexpected any. Specify a different type.
web/src/utils/query/sqlUtils.ts (19)
1-1: Unexpected any. Specify a different type.
6-6: Unexpected any. Specify a different type.
7-7: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
29-29: Unexpected any. Specify a different type.
30-30: Unexpected any. Specify a different type.
53-61: This else clause can be omitted because previous branches break early.
65-65: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
67-67: Unexpected any. Specify a different type.
68-68: Unexpected any. Specify a different type.
72-72: Unexpected any. Specify a different type.
137-137: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
152-152: Unexpected any. Specify a different type.
154-154: Change to an optional chain.
155-155: This let declares a variable that is only assigned once.
Additional comments not posted (6)
web/src/utils/query/sqlUtils.ts (2)
Line range hint
5-13: LGTM! The dynamic import and parser initialization are handled correctly.
Line range hint
141-149: LGTM! The use of optional chaining here is appropriate and safe.web/src/utils/dashboard/convertSQLData.ts (4)
31-31: Consider replacinganytype with more specific types for better type safety.
32-32: ConvertconvertSQLDatafunction parameters fromanyto more specific types to enhance type safety and code readability.
309-310: Ensure correct handling of theinverseproperty for "h-stacked" and "h-bar" chart types.
1482-1527: Review the sorting logic for stacked charts to ensure it handles non-time series data correctly.Verification successful
The search results show that the function
isGivenFieldInOrderByis defined inweb/src/utils/query/sqlUtils.tsand used inweb/src/utils/dashboard/convertSQLData.ts. This confirms that the function is being utilized correctly in the context of the sorting logic for stacked charts.
web/src/utils/query/sqlUtils.ts: Function definition.web/src/utils/dashboard/convertSQLData.ts: Function import and usage.The sorting logic for stacked charts appears to be correctly implemented, handling non-time series data as intended.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the sorting logic for stacked charts. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type typescript $'isGivenFieldInOrderBy'Length of output: 79
Script:
#!/bin/bash # Description: Verify the sorting logic for stacked charts. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts 'isGivenFieldInOrderBy'Length of output: 361
ktx-abhay
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
from
a90637f
to
2cef1d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (5)
web/src/utils/query/sqlUtils.ts (2)
Line range hint
1-1: Consider replacinganytype with more specific types to enhance type safety.TypeScript's
anytype is essentially a wildcard that can be anything, which defeats the purpose of using a statically typed language like TypeScript. It's better to define more specific types or at least useunknownif the type is not known ahead of time and perform runtime checks.Also applies to: 6-7, 14-14, 29-30, 65-68, 72-72, 137-137, 141-141, 143-143, 152-152
Line range hint
53-61: Consider refactoring to remove the unnecessary else clause.The else clause can be omitted because the previous branches (if conditions) break early. This can simplify the code and reduce nesting.
web/src/utils/dashboard/convertSQLData.ts (3)
Line range hint
33-37: Replaceanytype with more specific types to enhance type safety and code readability.- panelSchema: any, - searchQueryData: any, - store: any, - chartPanelRef: any, - hoveredSeriesState: any + panelSchema: PanelSchemaType, + searchQueryData: SearchQueryDataType[], + store: StoreType, + chartPanelRef: ChartPanelRefType, + hoveredSeriesState: HoveredSeriesStateTypeAlso applies to: 59-59, 66-66, 73-73, 80-80, 82-82, 83-83, 90-90, 93-93, 113-113, 124-124, 141-141, 158-158
Line range hint
142-142: Use strict equality comparison (===) instead of loose equality (==).- if (panelSchema.type == "h-bar" || panelSchema.type == "h-stacked") + if (panelSchema.type === "h-bar" || panelSchema.type === "h-stacked")
Line range hint
143-144: Prefer template literals over string concatenation for better readability and performance.- return formatDate(utcToZonedTime(it + "Z", store.state.timezone)); + return formatDate(utcToZonedTime(`${it}Z`, store.state.timezone));
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/src/utils/dashboard/convertSQLData.ts (4 hunks)
- web/src/utils/query/sqlUtils.ts (1 hunks)
Additional Context Used
Biome (39)
web/src/utils/dashboard/convertSQLData.ts (20)
33-33: Unexpected any. Specify a different type.
34-34: Unexpected any. Specify a different type.
35-35: Unexpected any. Specify a different type.
36-36: Unexpected any. Specify a different type.
37-37: Unexpected any. Specify a different type.
59-59: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
73-73: Unexpected any. Specify a different type.
80-80: Unexpected any. Specify a different type.
82-82: Unexpected any. Specify a different type.
83-83: Unexpected any. Specify a different type.
90-90: Unexpected any. Specify a different type.
93-93: Unexpected any. Specify a different type.
113-113: Unexpected any. Specify a different type.
124-124: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
142-142: Use === instead of ==.
== is only allowed when comparing againstnull
143-143: Template literals are preferred over string concatenation.
144-144: Template literals are preferred over string concatenation.
158-158: Unexpected any. Specify a different type.
web/src/utils/query/sqlUtils.ts (19)
1-1: Unexpected any. Specify a different type.
6-6: Unexpected any. Specify a different type.
7-7: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
29-29: Unexpected any. Specify a different type.
30-30: Unexpected any. Specify a different type.
53-61: This else clause can be omitted because previous branches break early.
65-65: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
67-67: Unexpected any. Specify a different type.
68-68: Unexpected any. Specify a different type.
72-72: Unexpected any. Specify a different type.
137-137: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
152-152: Unexpected any. Specify a different type.
154-154: Change to an optional chain.
155-155: This let declares a variable that is only assigned once.
Additional comments not posted (6)
web/src/utils/query/sqlUtils.ts (4)
154-154: Use optional chaining for better safety when accessing properties of potentially null objects.- if (ast && ast.orderby) { + if (ast?.orderby) {
155-155: Consider usingconstinstead ofletfor variables that are not reassigned.- for (let item of ast.orderby) { + for (const item of ast.orderby) {
Line range hint
3-12: TheimportSqlParserfunction logic appears sound and well-implemented.
Line range hint
137-143: ThegetStreamFromQueryfunction is well-implemented, handling potential parsing errors gracefully.web/src/utils/dashboard/convertSQLData.ts (2)
309-310: Ensure the logic for inverse data handling aligns with the new requirements for "h-stacked" and "h-bar" chart types.// Verify that the inverse logic correctly applies to the specified chart types and does not affect others. if (["h-stacked", "h-bar"].includes(panelSchema.type)) { console.assert(options.xAxis[0].inverse === true, "Inverse should be true for h-stacked and h-bar"); } else { console.assert(options.xAxis[0].inverse === false, "Inverse should be false for other types"); }
1482-1527: Review the new sorting logic for stacked charts to ensure it handles all edge cases, especially when the y-axis field is not in the order by clause.// Ensure that the sorting logic is robust and handles cases where the y-axis field is not part of the order by clause. if (!isYAxisExistInOrderBy) { console.warn("Y-axis field is not part of the ORDER BY clause. Sorting might not behave as expected."); }
| @@ -28,7 +28,8 @@ import { | |||
| getUnitValue, | |||
| } from "./convertDataIntoUnitValue"; | |||
| import { calculateGridPositions } from "./calculateGridForSubPlot"; | |||
| export const convertSQLData = ( | |||
| import { isGivenFieldInOrderBy } from "../query/sqlUtils"; | |||
There was a problem hiding this comment.
Consider using more specific types instead of any for better type safety.
import { isGivenFieldInOrderBy } from "../query/sqlUtils";
ktx-abhay
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
from
6e9149a
to
e768863
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (5)
web/src/utils/dashboard/convertPanelData.ts (1)
Line range hint
29-33: Consider specifying more specific types instead ofanyto enhance type safety and code maintainability.interface PanelSchema { type: string; queryType?: string; // other properties } interface Data { // Define structure } interface Store { // Define structure } interface ChartPanelRef { // Define structure } interface HoveredSeriesState { // Define structure } export const convertPanelData = async ( panelSchema: PanelSchema, data: Data, store: Store, chartPanelRef: ChartPanelRef, hoveredSeriesState: HoveredSeriesState ): Promise<any> => { // function body remains the same }web/src/utils/query/sqlUtils.ts (1)
Line range hint
1-1: Consider specifying more specific types instead ofanyto enhance type safety and code maintainability.// Define specific types for parser, sqlQuery, label, value, operator, etc.Also applies to: 6-7, 14-14, 29-30, 65-68, 72-72, 137-137, 141-141, 143-143, 155-155
web/src/utils/dashboard/convertSQLData.ts (3)
31-31: Consider using absolute imports to avoid potential issues with relative paths, especially in larger projects.
Line range hint
142-144: Use template literals instead of string concatenation for better readability and performance.- return formatDate(utcToZonedTime(it + "Z", store.state.timezone)); + return formatDate(utcToZonedTime(`${it}Z`, store.state.timezone));
Line range hint
142-142: Use strict equality (===) instead of loose equality (==) to avoid type coercion errors.- if (it.aggregationFunction == "histogram" && it.column == store.state.zoConfig.timestamp_column) + if (it.aggregationFunction === "histogram" && it.column === store.state.zoConfig.timestamp_column)
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- web/src/utils/dashboard/convertPanelData.ts (1 hunks)
- web/src/utils/dashboard/convertSQLData.ts (4 hunks)
- web/src/utils/query/sqlUtils.ts (1 hunks)
Additional Context Used
Biome (44)
web/src/utils/dashboard/convertPanelData.ts (7)
29-29: Unexpected any. Specify a different type.
30-30: Unexpected any. Specify a different type.
31-31: Unexpected any. Specify a different type.
32-32: Unexpected any. Specify a different type.
33-33: Unexpected any. Specify a different type.
54-54: Use === instead of ==.
== is only allowed when comparing againstnull
67-79: This else clause can be omitted because previous branches break early.
web/src/utils/dashboard/convertSQLData.ts (20)
33-33: Unexpected any. Specify a different type.
34-34: Unexpected any. Specify a different type.
35-35: Unexpected any. Specify a different type.
36-36: Unexpected any. Specify a different type.
37-37: Unexpected any. Specify a different type.
59-59: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
73-73: Unexpected any. Specify a different type.
80-80: Unexpected any. Specify a different type.
82-82: Unexpected any. Specify a different type.
83-83: Unexpected any. Specify a different type.
90-90: Unexpected any. Specify a different type.
93-93: Unexpected any. Specify a different type.
113-113: Unexpected any. Specify a different type.
124-124: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
142-142: Use === instead of ==.
== is only allowed when comparing againstnull
143-143: Template literals are preferred over string concatenation.
144-144: Template literals are preferred over string concatenation.
158-158: Unexpected any. Specify a different type.
web/src/utils/query/sqlUtils.ts (17)
1-1: Unexpected any. Specify a different type.
6-6: Unexpected any. Specify a different type.
7-7: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
29-29: Unexpected any. Specify a different type.
30-30: Unexpected any. Specify a different type.
53-61: This else clause can be omitted because previous branches break early.
65-65: Unexpected any. Specify a different type.
66-66: Unexpected any. Specify a different type.
67-67: Unexpected any. Specify a different type.
68-68: Unexpected any. Specify a different type.
72-72: Unexpected any. Specify a different type.
137-137: Unexpected any. Specify a different type.
141-141: Unexpected any. Specify a different type.
143-143: Unexpected any. Specify a different type.
155-155: Unexpected any. Specify a different type.
Additional comments not posted (3)
web/src/utils/query/sqlUtils.ts (2)
148-166: The logic for checking the field in theORDER BYclause is correct.
158-161: Use optional chaining andconstfor better safety and immutability.- for (const item of ast.orderby) { + for (const item of ast?.orderby) { + if (item?.expr?.column === fieldAlias) { + return item.type; // 'ASC' or 'DESC' + } }Likely invalid or redundant comment.
web/src/utils/dashboard/convertSQLData.ts (1)
1482-1527: Ensure the sorting logic for stacked charts handles edge cases correctly, especially whenisGivenFieldInOrderByreturnsnull.
| @@ -28,7 +28,8 @@ import { | |||
| getUnitValue, | |||
| } from "./convertDataIntoUnitValue"; | |||
| import { calculateGridPositions } from "./calculateGridForSubPlot"; | |||
| export const convertSQLData = ( | |||
| import { isGivenFieldInOrderBy } from "../query/sqlUtils"; | |||
| export const convertSQLData = async ( | |||
There was a problem hiding this comment.
Replace any type with more specific types to enhance type safety and code readability.
- panelSchema: any,
- searchQueryData: any,
- store: any,
- chartPanelRef: any,
- hoveredSeriesState: any
+ panelSchema: PanelSchemaType,
+ searchQueryData: SearchQueryDataType[],
+ store: StoreType,
+ chartPanelRef: ChartPanelRefType,
+ hoveredSeriesState: HoveredSeriesStateTypeCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export const convertSQLData = async ( | |
| export const convertSQLData = async ( | |
| panelSchema: PanelSchemaType, | |
| searchQueryData: SearchQueryDataType[], | |
| store: StoreType, | |
| chartPanelRef: ChartPanelRefType, | |
| hoveredSeriesState: HoveredSeriesStateType | |
| ): Promise<any> => { | |
| // Function implementation... | |
| } |
| @@ -28,7 +28,8 @@ | |||
| getUnitValue, | |||
| } from "./convertDataIntoUnitValue"; | |||
| import { calculateGridPositions } from "./calculateGridForSubPlot"; | |||
| export const convertSQLData = ( | |||
| import { isGivenFieldInOrderBy } from "../query/sqlUtils"; | |||
| export const convertSQLData = async ( | |||
| panelSchema: any, | |||
There was a problem hiding this comment.
Replace any type with specific types across various parameters and variables to improve type safety.
- let data: any = ...
+ let data: DataType = ...
- const keys: any = ...
+ const keys: KeyType[] = ...
- const keyArrays: any = ...
+ const keyArrays: KeyArrayType = ...
- let result: any = ...
+ let result: ResultType = ...Also applies to: 34-34, 35-35, 36-36, 37-37, 59-59, 66-66, 73-73, 80-80, 82-82, 83-83, 90-90, 93-93, 113-113, 124-124, 141-141, 158-158
Committable suggestion was skipped due low confidence.
| ...(await convertSQLData( | ||
| panelSchema, | ||
| data, | ||
| store, | ||
| chartPanelRef, | ||
| hoveredSeriesState | ||
| ), | ||
| )), |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure proper error handling for asynchronous calls to convertSQLData in the convertPanelData function. Currently, there are no try-catch blocks or other error handling mechanisms present.
- Location:
web/src/utils/dashboard/convertPanelData.ts
Analysis chain
Ensure proper error handling for asynchronous calls to convertSQLData.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify proper error handling for asynchronous calls in `convertPanelData`.
# Test: Search for error handling patterns around asynchronous calls.
rg --type typescript $'try {.*await convertSQLData.*} catch'
Length of output: 185
Script:
#!/bin/bash
# Description: Verify proper error handling for asynchronous calls in `convertPanelData`.
# Test: Search for error handling patterns around asynchronous calls.
rg --type typescript 'try \{[^}]*await convertSQLData[^}]*\} catch'
Length of output: 104
Script:
#!/bin/bash
# Description: Verify proper error handling for asynchronous calls in `convertPanelData`.
# Test: Search for error handling patterns around asynchronous calls in TypeScript files.
rg 'try \{[^}]*await convertSQLData[^}]*\} catch' --glob '*.ts'
Length of output: 65
Script:
#!/bin/bash
# Description: Verify error handling in the `convertPanelData` function.
# Test: Search for the `convertPanelData` function implementation and inspect for error handling.
ast-grep --lang typescript --pattern $'async function convertPanelData($_) {
$$$
}'
Length of output: 86
Script:
#!/bin/bash
# Description: Verify error handling in the `convertPanelData` function.
# Test: Search for the `convertPanelData` function implementation and inspect for error handling.
rg 'async function convertPanelData' --glob '*.ts' -A 20
Length of output: 58
Script:
#!/bin/bash
# Description: Verify error handling in the `convertPanelData` function.
# Test: Search for the `convertPanelData` function implementation and inspect for error handling.
rg 'convertPanelData' --glob '*.ts' -A 20
Length of output: 1508
Script:
#!/bin/bash
# Description: Verify error handling in the `convertPanelData` function.
# Test: Search for try-catch blocks within the `convertPanelData` function.
rg 'export const convertPanelData = async' --glob '*.ts' -A 50 | rg 'try \{[^}]*\} catch'
Length of output: 91
ktx-abhay
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
from
e768863
to
90c7089
Compare
ktx-vaidehi
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
from
90c7089
to
2af4144
Compare
ktx-vaidehi
force-pushed
the
fix/dashboard-stacked-chart-sorting-issue
branch
from
b0e2a99
to
bca61bb
Compare
- #3499 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved handling and sorting for "h-stacked" and "h-bar" chart types. - Added sorting functionality for stacked charts based on the y-axis field when not a time series chart. - **Improvements** - Enhanced data processing performance with asynchronous operations for data conversion. - **Bug Fixes** - Corrected data inversion issues for "h-stacked" and "h-bar" chart types. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: ktx-vaidehi <vaidehi.akhani@kiara.tech>
Summary by CodeRabbit
New Features
Improvements
Bug Fixes