AmoghAtreya commented on code in PR #44955:
URL: https://github.com/apache/superset/pull/44955#discussion_r4352639290
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts:
##########
@@ -329,22 +358,50 @@ export function syncTimeComparisonGroups(
* Remap saved comparison-column config onto locale-independent keys.
*
* Charts created before comparison slots were stored as `Main …` may still
- * keep `t('Main') …` entries (for example `Principale revenue`). The column
- * editor lists the stored keys, so those legacy entries must be rewritten
- * before it filters `column_config` down to `colnames`.
+ * keep `t('Main') …` entries (for example `Principale revenue`). A buggy
+ * column editor also wrote `COLUMN_NAME_ALIASES` display names (for example
+ * `Time` for `__timestamp`). The column editor lists the stored keys, so
+ * those legacy entries must be rewritten before it filters `column_config`
+ * down to `colnames`.
*/
export function normalizeColumnConfigKeys<T>(
value: Record<string, T> | null | undefined,
colnames: string[],
+ knownKeys: string[] = colnames,
): Record<string, T> {
if (!value) {
return {};
}
const colnamesSet = new Set(colnames);
- const next: Record<string, T> = {};
+ const knownKeySet = new Set(knownKeys);
+ const aliasToKey = new Map<string, string>();
+ const ambiguousAliases = new Set<string>();
+ colnames.forEach(col => {
+ getColumnNameAliasLabels(col).forEach(alias => {
+ // A saved key that names a real column or metric is not a legacy alias.
+ if (!alias || knownKeySet.has(alias) || ambiguousAliases.has(alias)) {
+ return;
+ }
+ const existing = aliasToKey.get(alias);
+ if (existing && existing !== col) {
+ aliasToKey.delete(alias);
+ ambiguousAliases.add(alias);
+ return;
+ }
+ aliasToKey.set(alias, col);
+ });
+ });
+ const next: Record<string, T> = Object.create(null);
Object.entries(value).forEach(([key, config]) => {
+ if (colnamesSet.has(key)) {
Review Comment:
Yes. The early return now also treats `knownKeys` as real columns/metrics,
so a dataset column like `Previous revenue` is never inferred as a Main slot
just because `# revenue` is in the result. Tests cover that case in
`normalizeColumnConfigKeys` and in table/AG Grid `transformProps`.
--
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]
