rusackas commented on code in PR #37396:
URL: https://github.com/apache/superset/pull/37396#discussion_r4042130391
##########
superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx:
##########
@@ -17,61 +17,38 @@
* under the License.
*/
import { useMemo, useCallback, useRef, useState } from 'react';
-import {
- getTimeFormatter,
- safeHtmlSpan,
- TimeFormats,
- getMetricLabel,
- QueryFormMetric,
-} from '@superset-ui/core';
-import { t } from '@apache-superset/core/translation';
+import { getTimeFormatter, safeHtmlSpan, TimeFormats } from
'@superset-ui/core';
import { Constants } from '@superset-ui/core/components';
import { GenericDataType } from '@apache-superset/core/common';
import type { IRowNode } from 'ag-grid-community';
const timeFormatter = getTimeFormatter(TimeFormats.DATABASE_DATETIME);
-const CONTRIBUTION_SUFFIX = '__contribution';
+/**
+ * Builds Grid column definitions from query result metadata.
+ * Assumes {@link colnames}, {@link coltypes} and {@link collabels}
+ * have the same length and align. Only columns present in the first
+ * data row are included.
+ */
export function useGridColumns(
colnames: string[] | undefined,
coltypes: GenericDataType[] | undefined,
+ collabels: string[] | undefined,
data: Record<string, any>[] | undefined,
- columnDisplayNames?: Record<string, string>,
) {
return useMemo(
() =>
colnames && data?.length
? colnames
- .filter((column: string) => Object.keys(data[0]).includes(column))
- .map((key, index) => {
- const colType = coltypes?.[index];
-
- const rawHeader = columnDisplayNames?.[key] ?? key;
- let cleaned = rawHeader;
- let suffix = '';
-
- if (rawHeader.endsWith(CONTRIBUTION_SUFFIX)) {
- cleaned = rawHeader.slice(
- 0,
- rawHeader.length - CONTRIBUTION_SUFFIX.length,
- );
- suffix = ` (${t('contribution')})`;
- }
-
- try {
- const parsed = JSON.parse(cleaned);
- if (parsed && typeof parsed === 'object') {
- cleaned = getMetricLabel(parsed as QueryFormMetric);
- }
- } catch {
- /* not a JSON-encoded metric – keep original display name */
- }
-
- const cleanHeader = `${cleaned}${suffix}`;
+ .map((column, originalIndex) => [column, originalIndex] as const)
+ .filter(([column]) => Object.keys(data[0]).includes(column))
Review Comment:
Fair catch, `Object.keys(data[0])` does get recomputed on every column in
that filter. Worth hoisting it out once, though minor for typical column counts.
--
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]
