bito-code-review[bot] commented on code in PR #37412: URL: https://github.com/apache/superset/pull/37412#discussion_r2797027763
########## superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.test.ts: ########## @@ -0,0 +1,55 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import transformProps from './transformProps'; +import { mockedTimeSeriesProps } from 'spec/fixtures/mockTimeSeries'; + +test('Timeseries Should transform props correctly', () => { + const result = transformProps(mockedTimeSeriesProps); + + expect(result.coltypeMapping).toEqual({ + 'SUM(money_for_learning)': 0, + 'SUM(money_for_learning), 1 day ago': 0, + 'SUM(money_for_learning), 1 week ago': 0, + 'SUM(money_for_learning), 1 year ago': 1, + testing_count: 0, + 'testing_count, 1 day ago': 0, + 'testing_count, 1 week ago': 0, + 'testing_count, 1 year ago': 1, + time_start: 2, + }); + + const expectedResult = [ + 'SUM(money_for_learning)', + 'SUM(money_for_learning), 1 day ago', + 'SUM(money_for_learning), 1 week ago', + 'SUM(money_for_learning), 1 year ago', + 'Testing count', + 'Testing count, 1 day ago', + 'Testing count, 1 week ago', + 'Testing count, 1 year ago', + 'time_start', + ]; Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Incorrect test expectations</b></div> <div id="fix"> The test expects 'time_start' in the legend data, but 'time_start' is the x-axis column and not included in the series legend. With time_compare: [] in the mock, only 2 series are created, but expectedResult has 9 items. Update expectedResult to ['SUM(money_for_learning)', 'Testing count'] sorted alphabetically. </div> </div> <small><i>Code Review Run #175199</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/src/explore/components/DataTablesPane/utils.ts: ########## @@ -0,0 +1,78 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { ensureIsArray, addLabelMapToVerboseMap } from '@superset-ui/core'; +import { QueryResultInterface } from './types'; + +/** + * Transforms the table data by applying formatting logic, such as renaming columns based on a label map. + * + * @param responseArray - An array of query result interfaces possibly containing a label_map. + * @returns An array of query result interfaces with updated colnames and data. + */ +export const transformTableData = ( + responseArray: ReadonlyArray< + QueryResultInterface & { label_map?: Record<string, string[]> } + >, +): QueryResultInterface[] => { + const transformedResponseArray = responseArray.map(response => { + const labelMap = response.label_map || {}; + /** + * Inverts the label map to create a verbose map. + * + * For example if labelMap is { "Sales": ["total_sales"], "Profit": ["total_profit"] }, + * the verboseMap will be { "total_sales": "Sales", "total_profit": "Profit" } + */ + const verboseMap = Object.entries(labelMap) + .filter(([_key, value]) => ensureIsArray(value).length === 1) + .reduce( + (acc, [key, value]) => ({ + ...acc, + [ensureIsArray(value)[0]]: key, + }), + {}, + ); + + const updatedVerboseMap = addLabelMapToVerboseMap(labelMap, verboseMap); + + return { + ...response, + // Updates the response's colnames and data using the verbose map. + colnames: response.colnames.map( + (colname: string) => updatedVerboseMap[colname] || colname, + ), + /** + * Maps through each row of data and updates the keys using the verbose map. + * For example, if a row is { "total_sales": 100, "total_profit": 20 }, + * it will be transformed to { "total_sales": 100, "total_profit": 20, "Sales": 100, "Profit": 20 } + */ + data: response.data.map((row: any) => + Object.entries(row).reduce( + (acc, [key, value]) => ({ + ...acc, + [key]: value, + [updatedVerboseMap[key] || key]: value, + }), + {}, + ), + ), + }; + }); + + return transformedResponseArray as QueryResultInterface[]; Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Type assertion hides mismatch</b></div> <div id="fix"> The type assertion here masks a potential mismatch between the declared type of QueryResultInterface.data (array of arrays) and its actual usage as an array of objects, which could lead to runtime issues or confusion. </div> </div> <small><i>Code Review Run #175199</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]
