codeant-ai-for-open-source[bot] commented on code in PR #38145:
URL: https://github.com/apache/superset/pull/38145#discussion_r2838578650
##########
superset-frontend/plugins/plugin-chart-table/src/transformProps.ts:
##########
@@ -384,89 +384,87 @@ const processComparisonColumns = (
props: TableChartProps,
comparisonSuffix: string,
) =>
- columns
- .map(col => {
- const {
- datasource: { columnFormats, currencyFormats },
- rawFormData: { column_config: columnConfig = {} },
- } = props;
- const savedFormat = columnFormats?.[col.key];
- const savedCurrency = currencyFormats?.[col.key];
- const originalLabel = col.label;
- if (
- (col.isMetric || col.isPercentMetric) &&
- !col.key.includes(comparisonSuffix) &&
- col.isNumeric
- ) {
- return [
- {
- ...col,
- originalLabel,
- label: t('Main'),
- key: `${t('Main')} ${col.key}`,
- config: getComparisonColConfig(t('Main'), col.key, columnConfig),
- formatter: getComparisonColFormatter(
- t('Main'),
- col,
- columnConfig,
- savedFormat,
- savedCurrency,
- ),
- },
- {
- ...col,
- originalLabel,
- label: `#`,
- key: `# ${col.key}`,
- config: getComparisonColConfig(`#`, col.key, columnConfig),
- formatter: getComparisonColFormatter(
- `#`,
- col,
- columnConfig,
- savedFormat,
- savedCurrency,
- ),
- },
- {
- ...col,
- originalLabel,
- label: `△`,
- key: `△ ${col.key}`,
- config: getComparisonColConfig(`△`, col.key, columnConfig),
- formatter: getComparisonColFormatter(
- `△`,
- col,
- columnConfig,
- savedFormat,
- savedCurrency,
- ),
- },
- {
- ...col,
- originalLabel,
- label: `%`,
- key: `% ${col.key}`,
- config: getComparisonColConfig(`%`, col.key, columnConfig),
- formatter: getComparisonColFormatter(
- `%`,
- col,
- columnConfig,
- savedFormat,
- savedCurrency,
- ),
- },
- ];
- }
- if (
- !col.isMetric &&
- !col.isPercentMetric &&
- !col.key.includes(comparisonSuffix)
- ) {
- return [col];
- }
- return [];
- })
- .flat();
+ columns.flatMap(col => {
+ const {
+ datasource: { columnFormats, currencyFormats },
+ rawFormData: { column_config: columnConfig = {} },
+ } = props;
+ const savedFormat = columnFormats?.[col.key];
+ const savedCurrency = currencyFormats?.[col.key];
+ const originalLabel = col.label;
+ if (
+ (col.isMetric || col.isPercentMetric) &&
+ !col.key.includes(comparisonSuffix) &&
+ col.isNumeric
+ ) {
+ return [
+ {
+ ...col,
+ originalLabel,
+ label: t('Main'),
+ key: `Main ${col.key}`,
+ config: getComparisonColConfig('Main', col.key, columnConfig),
Review Comment:
**Suggestion:** The comparison column config lookup for the "Main" column
uses the hard-coded string 'Main' instead of the localized label used when
generating column names in the control panel (`t('Main')`), so in non-English
locales any custom column configuration (formatting, currency, etc.) for these
"Main" comparison columns will never be matched or applied. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Main comparison columns ignore custom formatting in localized UIs.
- ⚠️ Main comparison columns cannot be hidden via column_config settings.
```
</details>
```suggestion
config: getComparisonColConfig(t('Main'), col.key, columnConfig),
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Explore, create or edit a Table chart using the legacy table plugin
(`plugins/plugin-chart-table`) with time comparison enabled so
`comparison_type ===
ComparisonType.Values` and `time_compare` is non-empty (checked in
`transformProps.ts:535-538`).
2. With the UI language set to a non-English locale, open the chart's
Customize columns
control (`column_config` control defined in
`plugins/plugin-chart-table/src/controlPanel.tsx:556-649`).
3. For a metric like `metric_1`, configure formatting/visibility on its
"Main" comparison
column; the control panel generates column_config keys using the localized
label, e.g.
`${t('Main')} metric_1` in `generateComparisonColumns` at
`controlPanel.tsx:158-166`, and
uses those names when building `column_config`.
4. Run the chart; `transformProps` (`transformProps.ts:490-795`) calls
`processComparisonColumns` (`transformProps.ts:382-467`), which creates a
Main column with
`label: t('Main')` but looks up config via `getComparisonColConfig('Main',
col.key,
columnConfig)` at `transformProps.ts:406`; `getComparisonColConfig`
(`transformProps.ts:340-347`) builds the key `${label} ${parentColKey}`
which becomes
`'Main metric_1'` instead of the localized `${t('Main')} metric_1`, so it
returns `{}` and
the custom column_config for the Main comparison column is never applied in
non-English
locales.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
**Line:** 406:406
**Comment:**
*Logic Error: The comparison column config lookup for the "Main" column
uses the hard-coded string 'Main' instead of the localized label used when
generating column names in the control panel (`t('Main')`), so in non-English
locales any custom column configuration (formatting, currency, etc.) for these
"Main" comparison columns will never be matched or applied.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38145&comment_hash=38c3f007470a4e20977e5f9a8faf4d7a449c2c315fa324ac0cc7cbbe633526f6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38145&comment_hash=38c3f007470a4e20977e5f9a8faf4d7a449c2c315fa324ac0cc7cbbe633526f6&reaction=dislike'>👎</a>
--
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]
