rusackas commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3430423283
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -67,7 +70,120 @@ import { CHART_WIDTH, CHART_HEIGHT } from
'src/dashboard/constants';
// Session storage key for recent dashboard
const SK_DASHBOARD_ID = 'save_chart_recent_dashboard';
-interface SaveModalProps extends RouteComponentProps {
+/**
+ * Creates URLSearchParams with save action and slice ID, removing
form_data_key.
+ * Exported for testing purposes.
+ */
+export const createRedirectParams = (
+ windowLocationSearch: string,
+ chart: { id: number },
+ action: string,
+): URLSearchParams => {
+ const searchParams = new URLSearchParams(windowLocationSearch);
+ searchParams.set('save_action', action);
+ searchParams.delete('form_data_key');
+ searchParams.set('slice_id', chart.id.toString());
+ return searchParams;
+};
+
+/**
+ * Adds a chart to a dashboard tab by updating the position_json.
+ * Exported for testing purposes.
+ */
+export const addChartToDashboard = async (
+ dashboardId: number,
+ chartId: number,
+ tabId: string,
+ sliceNameParam: string | undefined,
+): Promise<void> => {
+ const dashboardResponse = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${dashboardId}`,
+ });
+
+ const dashboardData = dashboardResponse.json.result;
+
+ let positionJson = dashboardData.position_json;
+ if (typeof positionJson === 'string') {
+ positionJson = JSON.parse(positionJson);
+ }
+ positionJson = positionJson || {};
+
+ const chartKey = `CHART-${chartId}`;
+
+ // Find a row in the tab with available space
+ const tabChildren = positionJson[tabId]?.children || [];
+ let targetRowKey: string | null = null;
+
+ for (const childKey of tabChildren) {
+ const child = positionJson[childKey];
+ if (child?.type === 'ROW') {
+ const rowChildren = child.children || [];
+ const totalWidth = rowChildren.reduce((sum: number, key: string) => {
+ const component = positionJson[key];
+ return sum + (component?.meta?.width || 0);
+ }, 0);
+
+ if (totalWidth + CHART_WIDTH <= GRID_COLUMN_COUNT) {
+ targetRowKey = childKey;
+ break;
+ }
+ }
+ }
+
+ const updatedPositionJson = { ...positionJson };
+
+ // Create a new row if no existing row has space
+ if (!targetRowKey) {
+ targetRowKey = `ROW-${nanoid()}`;
+ updatedPositionJson[targetRowKey] = {
+ type: 'ROW',
+ id: targetRowKey,
+ children: [],
+ parents: ['ROOT_ID', 'GRID_ID', tabId],
+ meta: {
+ background: 'BACKGROUND_TRANSPARENT',
+ },
+ };
+
+ if (positionJson[tabId]) {
+ updatedPositionJson[tabId] = {
+ ...positionJson[tabId],
+ children: [...(positionJson[tabId].children || []), targetRowKey],
+ };
+ } else {
+ throw new Error(`Tab ${tabId} not found in positionJson`);
+ }
+ }
+
+ updatedPositionJson[chartKey] = {
Review Comment:
This parent-chain logic is unchanged from master's class version — the
conversion didn't touch it. If tabbed-dashboard placement is wrong it's a
pre-existing bug worth its own issue, not something I want to fold into a
mechanical conversion.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -883,602 +835,693 @@ const mapStateToProps = (state: RootState) => ({
const connector = connect(mapStateToProps, mapDispatchToProps);
type PropsFromRedux = ConnectedProps<typeof connector>;
-type DatasourceEditorProps = DatasourceEditorOwnProps &
- PropsFromRedux & {
- theme?: SupersetTheme;
- };
-
-class DatasourceEditor extends PureComponent<
- DatasourceEditorProps,
- DatasourceEditorState
-> {
- private isComponentMounted: boolean;
+type DatasourceEditorProps = DatasourceEditorOwnProps & PropsFromRedux;
+
+function DatasourceEditor({
+ datasource: propsDatasource,
+ onChange = () => {},
+ addSuccessToast,
+ addDangerToast,
+ setIsEditing = () => {},
+ database,
+ runQuery,
+ resetQuery,
+ formatQuery: formatQueryAction,
+}: DatasourceEditorProps) {
+ const theme = useTheme();
+ const isComponentMounted = useRef(false);
+ const isInitialMount = useRef(true);
+ const prevPropsDatasourceRef = useRef(propsDatasource);
+ const isSyncingColumnsFromProps = useRef(false);
+ const abortControllers = useRef<AbortControllers>({
+ formatQuery: null,
+ formatSql: null,
+ syncMetadata: null,
+ fetchUsageData: null,
+ });
+
+ // Initialize datasource state with transformed owners and metrics
+ const [datasource, setDatasource] = useState<DatasourceObject>(() => ({
+ ...propsDatasource,
+ owners: propsDatasource.owners.map(owner => ({
+ value: owner.value || owner.id,
+ label: owner.label || `${owner.first_name} ${owner.last_name}`,
+ })),
+ metrics: propsDatasource.metrics?.map(metric => {
+ const {
+ certified_by: certifiedByMetric,
+ certification_details: certificationDetails,
+ } = metric;
+ const {
+ certification: {
+ details = undefined,
+ certified_by: certifiedBy = undefined,
+ } = {},
+ warning_markdown: warningMarkdown,
+ } = JSON.parse(metric.extra || '{}') || {};
+ return {
+ ...metric,
+ certification_details: certificationDetails || details,
+ warning_markdown: warningMarkdown || '',
+ certified_by: certifiedBy || certifiedByMetric,
+ };
+ }),
+ }));
- private abortControllers: AbortControllers;
+ const [errors, setErrors] = useState<string[]>([]);
+ const [isSqla] = useState(
+ propsDatasource.datasource_type === 'table' ||
+ propsDatasource.type === 'table',
+ );
+ const [isEditMode, setIsEditMode] = useState(false);
+ const [databaseColumns, setDatabaseColumns] = useState<Column[]>(
+ propsDatasource.columns.filter(col => !col.expression),
+ );
+ const [calculatedColumns, setCalculatedColumns] = useState<Column[]>(
+ propsDatasource.columns.filter(col => !!col.expression),
+ );
+ const [folders, setFolders] = useState<DatasourceFolder[]>(
+ propsDatasource.folders || [],
+ );
+ const [folderCount, setFolderCount] = useState(() => {
+ const savedFolders = propsDatasource.folders || [];
+ const savedCount = countAllFolders(savedFolders);
+ const hasDefaultsSaved = savedFolders.some(f => isDefaultFolder(f.uuid));
+ return savedCount + (hasDefaultsSaved ? 0 : DEFAULT_FOLDERS_COUNT);
+ });
+ const [metadataLoading, setMetadataLoading] = useState(false);
+ const [activeTabKey, setActiveTabKey] = useState(TABS_KEYS.SOURCE);
+ const [datasourceType, setDatasourceType] = useState(
+ propsDatasource.sql
+ ? DATASOURCE_TYPES.virtual.key
+ : DATASOURCE_TYPES.physical.key,
+ );
+ const [usageCharts, setUsageCharts] = useState<ChartUsageData[]>([]);
+ const [usageChartsCount, setUsageChartsCount] = useState(0);
+ const [metricSearchTerm, setMetricSearchTerm] = useState('');
+ const [columnSearchTerm, setColumnSearchTerm] = useState('');
+ const [calculatedColumnSearchTerm, setCalculatedColumnSearchTerm] =
+ useState('');
+
+ const findDuplicates = useCallback(
+ <T,>(arr: T[], accessor: (obj: T) => string): string[] => {
+ const seen: Record<string, null> = {};
+ const dups: string[] = [];
+ arr.forEach((obj: T) => {
+ const item = accessor(obj);
+ if (item in seen) {
+ dups.push(item);
+ } else {
+ seen[item] = null;
+ }
+ });
+ return dups;
+ },
+ [],
+ );
- static defaultProps = {
- onChange: () => {},
- setIsEditing: () => {},
- };
+ const validate = useCallback(
+ (callback: (validationErrors: string[]) => void) => {
+ let validationErrors: string[] = [];
+ let dups: string[];
- constructor(props: DatasourceEditorProps) {
- super(props);
- this.state = {
- datasource: {
- ...props.datasource,
- owners: props.datasource.owners.map(owner => {
- const ownerName =
- owner.label || `${owner.first_name} ${owner.last_name}`;
- return {
- value: owner.value || owner.id,
- label: OwnerSelectLabel({
- name: typeof ownerName === 'string' ? ownerName : '',
- email: owner.email,
- }),
- [OWNER_TEXT_LABEL_PROP]:
- typeof ownerName === 'string' ? ownerName : '',
- [OWNER_EMAIL_PROP]: owner.email ?? '',
- };
- }),
- metrics: props.datasource.metrics?.map(metric => {
- const {
- certified_by: certifiedByMetric,
- certification_details: certificationDetails,
- } = metric;
- const {
- certification: {
- details = undefined,
- certified_by: certifiedBy = undefined,
- } = {},
- warning_markdown: warningMarkdown,
- } = JSON.parse(metric.extra || '{}') || {};
- return {
- ...metric,
- certification_details: certificationDetails || details,
- warning_markdown: warningMarkdown || '',
- certified_by: certifiedBy || certifiedByMetric,
- };
- }),
- },
- errors: [],
- isSqla:
- props.datasource.datasource_type === 'table' ||
- props.datasource.type === 'table',
- isEditMode: false,
- databaseColumns: props.datasource.columns.filter(col => !col.expression),
- calculatedColumns: props.datasource.columns.filter(
- col => !!col.expression,
- ),
- folders: props.datasource.folders || [],
- folderCount: (() => {
- const savedFolders = props.datasource.folders || [];
- const savedCount = countAllFolders(savedFolders);
- const hasDefaultsSaved = savedFolders.some(f =>
- isDefaultFolder(f.uuid),
- );
- return savedCount + (hasDefaultsSaved ? 0 : DEFAULT_FOLDERS_COUNT);
- })(),
- metadataLoading: false,
- activeTabKey: TABS_KEYS.SOURCE,
- datasourceType: props.datasource.sql
- ? DATASOURCE_TYPES.virtual.key
- : DATASOURCE_TYPES.physical.key,
- usageCharts: [],
- usageChartsCount: 0,
- metricSearchTerm: '',
- columnSearchTerm: '',
- calculatedColumnSearchTerm: '',
- };
+ // Looking for duplicate column_name
+ dups = findDuplicates(datasource.columns, obj => obj.column_name);
+ validationErrors = validationErrors.concat(
+ dups.map(name => t('Column name [%s] is duplicated', name)),
+ );
- this.isComponentMounted = false;
- this.abortControllers = {
- formatQuery: null,
- formatSql: null,
- syncMetadata: null,
- fetchUsageData: null,
- };
+ // Looking for duplicate metric_name
+ dups = findDuplicates(datasource.metrics ?? [], obj => obj.metric_name);
+ validationErrors = validationErrors.concat(
+ dups.map(name => t('Metric name [%s] is duplicated', name)),
+ );
- this.onChange = this.onChange.bind(this);
- this.onChangeEditMode = this.onChangeEditMode.bind(this);
- this.onDatasourcePropChange = this.onDatasourcePropChange.bind(this);
- this.onDatasourceChange = this.onDatasourceChange.bind(this);
- this.tableChangeAndSyncMetadata =
- this.tableChangeAndSyncMetadata.bind(this);
- this.syncMetadata = this.syncMetadata.bind(this);
- this.setColumns = this.setColumns.bind(this);
- this.validateAndChange = this.validateAndChange.bind(this);
- this.handleTabSelect = this.handleTabSelect.bind(this);
- this.formatSql = this.formatSql.bind(this);
- this.fetchUsageData = this.fetchUsageData.bind(this);
- this.handleFoldersChange = this.handleFoldersChange.bind(this);
- }
+ // Making sure calculatedColumns have an expression defined
+ const noFilterCalcCols = calculatedColumns.filter(
+ col => !col.expression && !col.json,
+ );
+ validationErrors = validationErrors.concat(
+ noFilterCalcCols.map(col =>
+ t('Calculated column [%s] requires an expression', col.column_name),
+ ),
+ );
- onChange() {
- // Emptying SQL if "Physical" radio button is selected
- // Currently the logic to know whether the source is
- // physical or virtual is based on whether SQL is empty or not.
- const { datasourceType, datasource } = this.state;
- const sql =
- datasourceType === DATASOURCE_TYPES.physical.key ? '' : datasource.sql;
-
- const columns = [
- ...this.state.databaseColumns,
- ...this.state.calculatedColumns,
- ];
-
- // Remove deleted column/metric references from folders
- const validUuids = new Set<string>();
- for (const col of columns) {
- if (col.uuid) validUuids.add(col.uuid);
- }
- for (const metric of datasource.metrics ?? []) {
- if (metric.uuid) validUuids.add(metric.uuid);
- }
- const folders = filterFoldersByValidUuids(this.state.folders, validUuids);
+ // validate currency code (skip 'AUTO' - it's a placeholder for
auto-detection)
+ try {
+ datasource.metrics?.forEach(
+ metric =>
+ metric.currency?.symbol &&
+ metric.currency.symbol !== 'AUTO' &&
+ new Intl.NumberFormat('en-US', {
+ style: 'currency',
+ currency: metric.currency.symbol,
+ }),
+ );
+ } catch {
+ validationErrors = validationErrors.concat([
+ t('Invalid currency code in saved metrics'),
+ ]);
+ }
- const newDatasource = {
- ...this.state.datasource,
- sql,
- columns,
- folders,
- };
+ // Validate folders
+ if (folders?.length > 0) {
+ const folderValidation = validateFolders(folders);
+ validationErrors = validationErrors.concat(folderValidation.errors);
+ }
- this.props.onChange?.(newDatasource, this.state.errors);
- }
+ setErrors(validationErrors);
+ callback(validationErrors);
+ },
+ [datasource, calculatedColumns, folders, findDuplicates],
+ );
- onChangeEditMode() {
- this.props.setIsEditing?.(!this.state.isEditMode);
- this.setState(prevState => ({ isEditMode: !prevState.isEditMode }));
- }
+ const onChangeInternal = useCallback(
+ (validationErrors: string[] = errors) => {
+ // Emptying SQL if "Physical" radio button is selected
+ const sql =
+ datasourceType === DATASOURCE_TYPES.physical.key ? '' : datasource.sql;
- onDatasourceChange(
- datasource: DatasourceObject,
- callback: () => void = this.validateAndChange,
- ) {
- this.setState({ datasource }, callback);
- }
+ const columns = [...databaseColumns, ...calculatedColumns];
- onDatasourcePropChange(attr: string, value: unknown) {
- if (value === undefined) return; // if value is undefined do not update
state
- const datasource = { ...this.state.datasource, [attr]: value };
- this.setState(
- prevState => ({
- datasource: { ...prevState.datasource, [attr]: value },
- }),
- () =>
- attr === 'table_name'
- ? this.onDatasourceChange(datasource,
this.tableChangeAndSyncMetadata)
- : this.onDatasourceChange(datasource, this.validateAndChange),
- );
- }
+ // Remove deleted column/metric references from folders
+ const validUuids = new Set<string>();
+ for (const col of columns) {
+ if (col.uuid) validUuids.add(col.uuid);
+ }
+ for (const metric of datasource.metrics ?? []) {
+ if (metric.uuid) validUuids.add(metric.uuid);
+ }
+ const filteredFolders = filterFoldersByValidUuids(folders, validUuids);
- onDatasourceTypeChange(datasourceType: string) {
- // Call onChange after setting datasourceType to ensure
- // SQL is cleared when switching to a physical dataset
- this.setState({ datasourceType }, this.onChange);
- }
+ const newDatasource = {
+ ...datasource,
+ sql,
+ columns,
+ folders: filteredFolders,
+ };
- handleFoldersChange(folders: DatasourceFolder[]) {
- const folderCount = countAllFolders(folders);
- this.setState({ folders, folderCount }, () => {
- this.onDatasourceChange({
- ...this.state.datasource,
- folders,
- });
- });
- }
+ onChange(newDatasource, validationErrors);
+ },
+ [
+ datasource,
+ datasourceType,
+ databaseColumns,
+ calculatedColumns,
+ folders,
+ errors,
+ onChange,
+ ],
+ );
- setColumns(
- obj: { databaseColumns?: Column[] } | { calculatedColumns?: Column[] },
- ) {
- // update calculatedColumns or databaseColumns
- this.setState(
- obj as Pick<
- DatasourceEditorState,
- 'databaseColumns' | 'calculatedColumns'
- >,
- this.validateAndChange,
- );
- }
+ const validateAndChange = useCallback(() => {
+ validate(onChangeInternal);
+ }, [validate, onChangeInternal]);
- validateAndChange() {
- this.validate(this.onChange);
- }
+ const onDatasourceChange = useCallback((newDatasource: DatasourceObject) => {
+ setDatasource(newDatasource);
+ }, []);
- async onQueryRun() {
- const databaseId = this.state.datasource.database?.id;
- const { sql } = this.state.datasource;
- if (!databaseId || !sql) {
- return;
- }
- this.props.runQuery({
- client_id: this.props.database?.clientId,
- database_id: databaseId,
- runAsync: false,
- catalog: this.state.datasource.catalog,
- schema: this.state.datasource.schema,
- sql,
- tmp_table_name: '',
- select_as_cta: false,
- ctas_method: 'TABLE',
- queryLimit: 25,
- expand_data: true,
+ const onDatasourcePropChange = useCallback((attr: string, value: unknown) =>
{
+ if (value === undefined) return;
+ setDatasource(prev => {
+ const newDatasource = { ...prev, [attr]: value };
+ return newDatasource;
});
- }
+ }, []);
- /**
- * Formats SQL query using the formatQuery action.
- * Aborts any pending format requests before starting a new one.
- */
- async onQueryFormat() {
- const { datasource } = this.state;
- if (!datasource.sql || !this.state.isEditMode) {
+ // Effect to trigger validation after datasource changes (skip initial mount)
+ useEffect(() => {
+ if (isInitialMount.current) {
return;
}
-
- // Abort previous formatQuery if still pending
- if (this.abortControllers.formatQuery) {
- this.abortControllers.formatQuery.abort();
+ if (isComponentMounted.current) {
+ validateAndChange();
}
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [datasource]);
- this.abortControllers.formatQuery = new AbortController();
- const { signal } = this.abortControllers.formatQuery;
+ const onChangeEditMode = useCallback(() => {
+ setIsEditing(!isEditMode);
+ setIsEditMode(prev => !prev);
+ }, [isEditMode, setIsEditing]);
- try {
- const response = await this.props.formatQuery(datasource.sql, { signal
});
+ const onDatasourceTypeChange = useCallback((newDatasourceType: string) => {
+ setDatasourceType(newDatasourceType);
+ }, []);
- this.onDatasourcePropChange('sql', response.json.result);
- this.props.addSuccessToast(t('SQL was formatted'));
- } catch (error) {
- if (error.name === 'AbortError') return;
+ // Effect to call onChange after datasourceType changes (skip initial mount)
+ useEffect(() => {
+ if (isInitialMount.current) {
+ return;
+ }
+ if (isComponentMounted.current) {
+ onChangeInternal();
+ }
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [datasourceType]);
+
+ const handleFoldersChange = useCallback((newFolders: DatasourceFolder[]) => {
+ const userMadeFolders = newFolders.filter(
+ f =>
+ f.uuid !== DEFAULT_METRICS_FOLDER_UUID &&
+ f.uuid !== DEFAULT_COLUMNS_FOLDER_UUID &&
+ (f.children?.length ?? 0) > 0,
+ );
+ setFolders(userMadeFolders);
+ setFolderCount(countAllFolders(userMadeFolders));
+ setDatasource(prev => ({ ...prev, folders: userMadeFolders }));
+ }, []);
- const { error: clientError, statusText } =
- await getClientErrorObject(error);
+ const setColumns = useCallback(
+ (
+ obj: { databaseColumns?: Column[] } | { calculatedColumns?: Column[] },
+ ) => {
+ if ('databaseColumns' in obj && obj.databaseColumns) {
+ setDatabaseColumns(obj.databaseColumns);
+ }
+ if ('calculatedColumns' in obj && obj.calculatedColumns) {
+ setCalculatedColumns(obj.calculatedColumns);
+ }
+ },
+ [],
+ );
- this.props.addDangerToast(
- clientError ||
- statusText ||
- t('An error occurred while formatting SQL'),
- );
- } finally {
- this.abortControllers.formatQuery = null;
+ // Effect to trigger validation after user-initiated column changes
+ // Skips initial mount and prop-sync updates (which don't need validation)
+ useEffect(() => {
+ if (isInitialMount.current || isSyncingColumnsFromProps.current) {
+ isSyncingColumnsFromProps.current = false;
+ return;
}
- }
+ if (isComponentMounted.current) {
+ validateAndChange();
+ }
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [databaseColumns, calculatedColumns]);
- getSQLLabUrl() {
+ const getSQLLabUrl = useCallback(() => {
const queryParams = new URLSearchParams({
- dbid: String(this.state.datasource.database?.id ?? ''),
- sql: this.state.datasource.sql ?? '',
- name: this.state.datasource.datasource_name ?? '',
- schema: this.state.datasource.schema ?? '',
+ dbid: String(datasource.database?.id ?? ''),
+ sql: datasource.sql ?? '',
+ name: datasource.datasource_name ?? '',
+ schema: datasource.schema ?? '',
autorun: 'true',
isDataset: 'true',
});
return makeUrl(`/sqllab/?${queryParams.toString()}`);
- }
+ }, [datasource]);
- openOnSqlLab() {
- window.open(this.getSQLLabUrl(), '_blank', 'noopener,noreferrer');
- }
+ const openOnSqlLab = useCallback(() => {
+ window.open(getSQLLabUrl(), '_blank', 'noopener,noreferrer');
+ }, [getSQLLabUrl]);
- tableChangeAndSyncMetadata() {
- this.validate(() => {
- this.syncMetadata();
- this.onChange();
+ const onQueryRun = useCallback(async () => {
+ const databaseId = datasource.database?.id;
+ const { sql } = datasource;
+ if (!databaseId || !sql) {
+ return;
+ }
+ runQuery({
+ client_id: database?.clientId,
+ database_id: databaseId,
+ runAsync: false,
+ catalog: datasource.catalog,
+ schema: datasource.schema,
+ sql,
+ tmp_table_name: '',
+ select_as_cta: false,
+ ctas_method: 'TABLE',
+ queryLimit: 25,
+ expand_data: true,
});
- }
+ }, [datasource, database?.clientId, runQuery]);
- /**
- * Formats SQL query using the SQL format API endpoint.
- * Aborts any pending format requests before starting a new one.
- */
- async formatSql() {
- const { datasource } = this.state;
- if (!datasource.sql) {
+ const onQueryFormat = useCallback(async () => {
+ if (!datasource.sql || !isEditMode) {
return;
}
- // Abort previous formatSql if still pending
- if (this.abortControllers.formatSql) {
- this.abortControllers.formatSql.abort();
+ // Abort previous formatQuery if still pending
+ if (abortControllers.current.formatQuery) {
+ abortControllers.current.formatQuery.abort();
}
- this.abortControllers.formatSql = new AbortController();
- const { signal } = this.abortControllers.formatSql;
+ abortControllers.current.formatQuery = new AbortController();
+ const { signal } = abortControllers.current.formatQuery;
try {
- const response = await SupersetClient.post({
- endpoint: '/api/v1/sql/format',
- body: JSON.stringify({ sql: datasource.sql }),
- headers: { 'Content-Type': 'application/json' },
- signal,
- });
+ const response = await formatQueryAction(datasource.sql, { signal });
- this.onDatasourcePropChange('sql', response.json.result);
- this.props.addSuccessToast(t('SQL was formatted'));
- } catch (error) {
- if (error.name === 'AbortError') return;
+ onDatasourcePropChange('sql', response.json.result);
+ addSuccessToast(t('SQL was formatted'));
+ } catch (error: unknown) {
+ if ((error as Error).name === 'AbortError') return;
- const { error: clientError, statusText } =
- await getClientErrorObject(error);
+ const { error: clientError, statusText } = await getClientErrorObject(
+ error as Response,
+ );
- this.props.addDangerToast(
+ addDangerToast(
clientError ||
statusText ||
t('An error occurred while formatting SQL'),
);
} finally {
- this.abortControllers.formatSql = null;
+ abortControllers.current.formatQuery = null;
}
- }
-
- /**
- * Syncs dataset columns with the database schema.
- * Fetches column metadata from the underlying table/view and updates the
dataset.
- * Aborts any pending sync requests before starting a new one.
- */
- async syncMetadata() {
- const { datasource } = this.state;
-
+ }, [
+ datasource.sql,
+ isEditMode,
+ formatQueryAction,
+ onDatasourcePropChange,
+ addSuccessToast,
+ addDangerToast,
+ ]);
+
+ const syncMetadata = useCallback(async () => {
// Abort previous syncMetadata if still pending
- if (this.abortControllers.syncMetadata) {
- this.abortControllers.syncMetadata.abort();
+ if (abortControllers.current.syncMetadata) {
+ abortControllers.current.syncMetadata.abort();
}
- this.abortControllers.syncMetadata = new AbortController();
- const { signal } = this.abortControllers.syncMetadata;
+ abortControllers.current.syncMetadata = new AbortController();
+ const { signal } = abortControllers.current.syncMetadata;
- this.setState({ metadataLoading: true });
+ setMetadataLoading(true);
try {
const newCols = await fetchSyncedColumns(datasource, signal);
const columnChanges = updateColumns(
datasource.columns,
newCols,
- this.props.addSuccessToast,
+ addSuccessToast,
);
Review Comment:
The sync flow here is a straight carry-over from master's class component —
same `updateColumns(datasource.columns, ...)` call. If it's diffing against
stale columns that predates this PR, so I'd rather not change sync behavior in
a conversion.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -883,602 +835,693 @@ const mapStateToProps = (state: RootState) => ({
const connector = connect(mapStateToProps, mapDispatchToProps);
type PropsFromRedux = ConnectedProps<typeof connector>;
-type DatasourceEditorProps = DatasourceEditorOwnProps &
- PropsFromRedux & {
- theme?: SupersetTheme;
- };
-
-class DatasourceEditor extends PureComponent<
- DatasourceEditorProps,
- DatasourceEditorState
-> {
- private isComponentMounted: boolean;
+type DatasourceEditorProps = DatasourceEditorOwnProps & PropsFromRedux;
+
+function DatasourceEditor({
+ datasource: propsDatasource,
+ onChange = () => {},
+ addSuccessToast,
+ addDangerToast,
+ setIsEditing = () => {},
+ database,
+ runQuery,
+ resetQuery,
+ formatQuery: formatQueryAction,
+}: DatasourceEditorProps) {
+ const theme = useTheme();
+ const isComponentMounted = useRef(false);
+ const isInitialMount = useRef(true);
+ const prevPropsDatasourceRef = useRef(propsDatasource);
+ const isSyncingColumnsFromProps = useRef(false);
+ const abortControllers = useRef<AbortControllers>({
+ formatQuery: null,
+ formatSql: null,
+ syncMetadata: null,
+ fetchUsageData: null,
+ });
+
+ // Initialize datasource state with transformed owners and metrics
+ const [datasource, setDatasource] = useState<DatasourceObject>(() => ({
+ ...propsDatasource,
+ owners: propsDatasource.owners.map(owner => ({
+ value: owner.value || owner.id,
+ label: owner.label || `${owner.first_name} ${owner.last_name}`,
+ })),
+ metrics: propsDatasource.metrics?.map(metric => {
+ const {
+ certified_by: certifiedByMetric,
+ certification_details: certificationDetails,
+ } = metric;
+ const {
+ certification: {
+ details = undefined,
+ certified_by: certifiedBy = undefined,
+ } = {},
+ warning_markdown: warningMarkdown,
+ } = JSON.parse(metric.extra || '{}') || {};
+ return {
+ ...metric,
+ certification_details: certificationDetails || details,
+ warning_markdown: warningMarkdown || '',
+ certified_by: certifiedBy || certifiedByMetric,
+ };
+ }),
+ }));
- private abortControllers: AbortControllers;
+ const [errors, setErrors] = useState<string[]>([]);
+ const [isSqla] = useState(
+ propsDatasource.datasource_type === 'table' ||
+ propsDatasource.type === 'table',
+ );
+ const [isEditMode, setIsEditMode] = useState(false);
+ const [databaseColumns, setDatabaseColumns] = useState<Column[]>(
+ propsDatasource.columns.filter(col => !col.expression),
+ );
+ const [calculatedColumns, setCalculatedColumns] = useState<Column[]>(
+ propsDatasource.columns.filter(col => !!col.expression),
+ );
+ const [folders, setFolders] = useState<DatasourceFolder[]>(
+ propsDatasource.folders || [],
+ );
+ const [folderCount, setFolderCount] = useState(() => {
+ const savedFolders = propsDatasource.folders || [];
+ const savedCount = countAllFolders(savedFolders);
+ const hasDefaultsSaved = savedFolders.some(f => isDefaultFolder(f.uuid));
+ return savedCount + (hasDefaultsSaved ? 0 : DEFAULT_FOLDERS_COUNT);
+ });
+ const [metadataLoading, setMetadataLoading] = useState(false);
+ const [activeTabKey, setActiveTabKey] = useState(TABS_KEYS.SOURCE);
+ const [datasourceType, setDatasourceType] = useState(
+ propsDatasource.sql
+ ? DATASOURCE_TYPES.virtual.key
+ : DATASOURCE_TYPES.physical.key,
+ );
+ const [usageCharts, setUsageCharts] = useState<ChartUsageData[]>([]);
+ const [usageChartsCount, setUsageChartsCount] = useState(0);
+ const [metricSearchTerm, setMetricSearchTerm] = useState('');
+ const [columnSearchTerm, setColumnSearchTerm] = useState('');
+ const [calculatedColumnSearchTerm, setCalculatedColumnSearchTerm] =
+ useState('');
+
+ const findDuplicates = useCallback(
+ <T,>(arr: T[], accessor: (obj: T) => string): string[] => {
+ const seen: Record<string, null> = {};
+ const dups: string[] = [];
+ arr.forEach((obj: T) => {
+ const item = accessor(obj);
+ if (item in seen) {
+ dups.push(item);
+ } else {
+ seen[item] = null;
+ }
+ });
+ return dups;
+ },
+ [],
+ );
- static defaultProps = {
- onChange: () => {},
- setIsEditing: () => {},
- };
+ const validate = useCallback(
+ (callback: (validationErrors: string[]) => void) => {
+ let validationErrors: string[] = [];
+ let dups: string[];
- constructor(props: DatasourceEditorProps) {
- super(props);
- this.state = {
- datasource: {
- ...props.datasource,
- owners: props.datasource.owners.map(owner => {
- const ownerName =
- owner.label || `${owner.first_name} ${owner.last_name}`;
- return {
- value: owner.value || owner.id,
- label: OwnerSelectLabel({
- name: typeof ownerName === 'string' ? ownerName : '',
- email: owner.email,
- }),
- [OWNER_TEXT_LABEL_PROP]:
- typeof ownerName === 'string' ? ownerName : '',
- [OWNER_EMAIL_PROP]: owner.email ?? '',
- };
- }),
- metrics: props.datasource.metrics?.map(metric => {
- const {
- certified_by: certifiedByMetric,
- certification_details: certificationDetails,
- } = metric;
- const {
- certification: {
- details = undefined,
- certified_by: certifiedBy = undefined,
- } = {},
- warning_markdown: warningMarkdown,
- } = JSON.parse(metric.extra || '{}') || {};
- return {
- ...metric,
- certification_details: certificationDetails || details,
- warning_markdown: warningMarkdown || '',
- certified_by: certifiedBy || certifiedByMetric,
- };
- }),
- },
- errors: [],
- isSqla:
- props.datasource.datasource_type === 'table' ||
- props.datasource.type === 'table',
- isEditMode: false,
- databaseColumns: props.datasource.columns.filter(col => !col.expression),
- calculatedColumns: props.datasource.columns.filter(
- col => !!col.expression,
- ),
- folders: props.datasource.folders || [],
- folderCount: (() => {
- const savedFolders = props.datasource.folders || [];
- const savedCount = countAllFolders(savedFolders);
- const hasDefaultsSaved = savedFolders.some(f =>
- isDefaultFolder(f.uuid),
- );
- return savedCount + (hasDefaultsSaved ? 0 : DEFAULT_FOLDERS_COUNT);
- })(),
- metadataLoading: false,
- activeTabKey: TABS_KEYS.SOURCE,
- datasourceType: props.datasource.sql
- ? DATASOURCE_TYPES.virtual.key
- : DATASOURCE_TYPES.physical.key,
- usageCharts: [],
- usageChartsCount: 0,
- metricSearchTerm: '',
- columnSearchTerm: '',
- calculatedColumnSearchTerm: '',
- };
+ // Looking for duplicate column_name
+ dups = findDuplicates(datasource.columns, obj => obj.column_name);
+ validationErrors = validationErrors.concat(
+ dups.map(name => t('Column name [%s] is duplicated', name)),
+ );
- this.isComponentMounted = false;
- this.abortControllers = {
- formatQuery: null,
- formatSql: null,
- syncMetadata: null,
- fetchUsageData: null,
- };
+ // Looking for duplicate metric_name
+ dups = findDuplicates(datasource.metrics ?? [], obj => obj.metric_name);
+ validationErrors = validationErrors.concat(
+ dups.map(name => t('Metric name [%s] is duplicated', name)),
+ );
- this.onChange = this.onChange.bind(this);
- this.onChangeEditMode = this.onChangeEditMode.bind(this);
- this.onDatasourcePropChange = this.onDatasourcePropChange.bind(this);
- this.onDatasourceChange = this.onDatasourceChange.bind(this);
- this.tableChangeAndSyncMetadata =
- this.tableChangeAndSyncMetadata.bind(this);
- this.syncMetadata = this.syncMetadata.bind(this);
- this.setColumns = this.setColumns.bind(this);
- this.validateAndChange = this.validateAndChange.bind(this);
- this.handleTabSelect = this.handleTabSelect.bind(this);
- this.formatSql = this.formatSql.bind(this);
- this.fetchUsageData = this.fetchUsageData.bind(this);
- this.handleFoldersChange = this.handleFoldersChange.bind(this);
- }
+ // Making sure calculatedColumns have an expression defined
+ const noFilterCalcCols = calculatedColumns.filter(
+ col => !col.expression && !col.json,
+ );
+ validationErrors = validationErrors.concat(
+ noFilterCalcCols.map(col =>
+ t('Calculated column [%s] requires an expression', col.column_name),
+ ),
+ );
- onChange() {
- // Emptying SQL if "Physical" radio button is selected
- // Currently the logic to know whether the source is
- // physical or virtual is based on whether SQL is empty or not.
- const { datasourceType, datasource } = this.state;
- const sql =
- datasourceType === DATASOURCE_TYPES.physical.key ? '' : datasource.sql;
-
- const columns = [
- ...this.state.databaseColumns,
- ...this.state.calculatedColumns,
- ];
-
- // Remove deleted column/metric references from folders
- const validUuids = new Set<string>();
- for (const col of columns) {
- if (col.uuid) validUuids.add(col.uuid);
- }
- for (const metric of datasource.metrics ?? []) {
- if (metric.uuid) validUuids.add(metric.uuid);
- }
- const folders = filterFoldersByValidUuids(this.state.folders, validUuids);
+ // validate currency code (skip 'AUTO' - it's a placeholder for
auto-detection)
+ try {
+ datasource.metrics?.forEach(
+ metric =>
+ metric.currency?.symbol &&
+ metric.currency.symbol !== 'AUTO' &&
+ new Intl.NumberFormat('en-US', {
+ style: 'currency',
+ currency: metric.currency.symbol,
+ }),
+ );
+ } catch {
+ validationErrors = validationErrors.concat([
+ t('Invalid currency code in saved metrics'),
+ ]);
+ }
- const newDatasource = {
- ...this.state.datasource,
- sql,
- columns,
- folders,
- };
+ // Validate folders
+ if (folders?.length > 0) {
+ const folderValidation = validateFolders(folders);
+ validationErrors = validationErrors.concat(folderValidation.errors);
+ }
- this.props.onChange?.(newDatasource, this.state.errors);
- }
+ setErrors(validationErrors);
+ callback(validationErrors);
+ },
+ [datasource, calculatedColumns, folders, findDuplicates],
+ );
- onChangeEditMode() {
- this.props.setIsEditing?.(!this.state.isEditMode);
- this.setState(prevState => ({ isEditMode: !prevState.isEditMode }));
- }
+ const onChangeInternal = useCallback(
+ (validationErrors: string[] = errors) => {
+ // Emptying SQL if "Physical" radio button is selected
+ const sql =
+ datasourceType === DATASOURCE_TYPES.physical.key ? '' : datasource.sql;
- onDatasourceChange(
- datasource: DatasourceObject,
- callback: () => void = this.validateAndChange,
- ) {
- this.setState({ datasource }, callback);
- }
+ const columns = [...databaseColumns, ...calculatedColumns];
- onDatasourcePropChange(attr: string, value: unknown) {
- if (value === undefined) return; // if value is undefined do not update
state
- const datasource = { ...this.state.datasource, [attr]: value };
- this.setState(
- prevState => ({
- datasource: { ...prevState.datasource, [attr]: value },
- }),
- () =>
- attr === 'table_name'
- ? this.onDatasourceChange(datasource,
this.tableChangeAndSyncMetadata)
- : this.onDatasourceChange(datasource, this.validateAndChange),
- );
- }
+ // Remove deleted column/metric references from folders
+ const validUuids = new Set<string>();
+ for (const col of columns) {
+ if (col.uuid) validUuids.add(col.uuid);
+ }
+ for (const metric of datasource.metrics ?? []) {
+ if (metric.uuid) validUuids.add(metric.uuid);
+ }
+ const filteredFolders = filterFoldersByValidUuids(folders, validUuids);
- onDatasourceTypeChange(datasourceType: string) {
- // Call onChange after setting datasourceType to ensure
- // SQL is cleared when switching to a physical dataset
- this.setState({ datasourceType }, this.onChange);
- }
+ const newDatasource = {
+ ...datasource,
+ sql,
+ columns,
+ folders: filteredFolders,
+ };
- handleFoldersChange(folders: DatasourceFolder[]) {
- const folderCount = countAllFolders(folders);
- this.setState({ folders, folderCount }, () => {
- this.onDatasourceChange({
- ...this.state.datasource,
- folders,
- });
- });
- }
+ onChange(newDatasource, validationErrors);
+ },
+ [
+ datasource,
+ datasourceType,
+ databaseColumns,
+ calculatedColumns,
+ folders,
+ errors,
+ onChange,
+ ],
+ );
- setColumns(
- obj: { databaseColumns?: Column[] } | { calculatedColumns?: Column[] },
- ) {
- // update calculatedColumns or databaseColumns
- this.setState(
- obj as Pick<
- DatasourceEditorState,
- 'databaseColumns' | 'calculatedColumns'
- >,
- this.validateAndChange,
- );
- }
+ const validateAndChange = useCallback(() => {
+ validate(onChangeInternal);
+ }, [validate, onChangeInternal]);
- validateAndChange() {
- this.validate(this.onChange);
- }
+ const onDatasourceChange = useCallback((newDatasource: DatasourceObject) => {
+ setDatasource(newDatasource);
+ }, []);
- async onQueryRun() {
- const databaseId = this.state.datasource.database?.id;
- const { sql } = this.state.datasource;
- if (!databaseId || !sql) {
- return;
- }
- this.props.runQuery({
- client_id: this.props.database?.clientId,
- database_id: databaseId,
- runAsync: false,
- catalog: this.state.datasource.catalog,
- schema: this.state.datasource.schema,
- sql,
- tmp_table_name: '',
- select_as_cta: false,
- ctas_method: 'TABLE',
- queryLimit: 25,
- expand_data: true,
+ const onDatasourcePropChange = useCallback((attr: string, value: unknown) =>
{
+ if (value === undefined) return;
+ setDatasource(prev => {
+ const newDatasource = { ...prev, [attr]: value };
+ return newDatasource;
});
- }
+ }, []);
- /**
- * Formats SQL query using the formatQuery action.
- * Aborts any pending format requests before starting a new one.
- */
- async onQueryFormat() {
- const { datasource } = this.state;
- if (!datasource.sql || !this.state.isEditMode) {
+ // Effect to trigger validation after datasource changes (skip initial mount)
+ useEffect(() => {
+ if (isInitialMount.current) {
return;
}
-
- // Abort previous formatQuery if still pending
- if (this.abortControllers.formatQuery) {
- this.abortControllers.formatQuery.abort();
+ if (isComponentMounted.current) {
+ validateAndChange();
}
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [datasource]);
- this.abortControllers.formatQuery = new AbortController();
- const { signal } = this.abortControllers.formatQuery;
+ const onChangeEditMode = useCallback(() => {
+ setIsEditing(!isEditMode);
+ setIsEditMode(prev => !prev);
+ }, [isEditMode, setIsEditing]);
- try {
- const response = await this.props.formatQuery(datasource.sql, { signal
});
+ const onDatasourceTypeChange = useCallback((newDatasourceType: string) => {
+ setDatasourceType(newDatasourceType);
+ }, []);
- this.onDatasourcePropChange('sql', response.json.result);
- this.props.addSuccessToast(t('SQL was formatted'));
- } catch (error) {
- if (error.name === 'AbortError') return;
+ // Effect to call onChange after datasourceType changes (skip initial mount)
+ useEffect(() => {
+ if (isInitialMount.current) {
+ return;
+ }
+ if (isComponentMounted.current) {
+ onChangeInternal();
+ }
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [datasourceType]);
+
+ const handleFoldersChange = useCallback((newFolders: DatasourceFolder[]) => {
+ const userMadeFolders = newFolders.filter(
+ f =>
+ f.uuid !== DEFAULT_METRICS_FOLDER_UUID &&
+ f.uuid !== DEFAULT_COLUMNS_FOLDER_UUID &&
+ (f.children?.length ?? 0) > 0,
+ );
+ setFolders(userMadeFolders);
+ setFolderCount(countAllFolders(userMadeFolders));
+ setDatasource(prev => ({ ...prev, folders: userMadeFolders }));
+ }, []);
- const { error: clientError, statusText } =
- await getClientErrorObject(error);
+ const setColumns = useCallback(
+ (
+ obj: { databaseColumns?: Column[] } | { calculatedColumns?: Column[] },
+ ) => {
+ if ('databaseColumns' in obj && obj.databaseColumns) {
+ setDatabaseColumns(obj.databaseColumns);
+ }
+ if ('calculatedColumns' in obj && obj.calculatedColumns) {
+ setCalculatedColumns(obj.calculatedColumns);
+ }
+ },
+ [],
+ );
- this.props.addDangerToast(
- clientError ||
- statusText ||
- t('An error occurred while formatting SQL'),
- );
- } finally {
- this.abortControllers.formatQuery = null;
+ // Effect to trigger validation after user-initiated column changes
+ // Skips initial mount and prop-sync updates (which don't need validation)
+ useEffect(() => {
+ if (isInitialMount.current || isSyncingColumnsFromProps.current) {
+ isSyncingColumnsFromProps.current = false;
+ return;
}
- }
+ if (isComponentMounted.current) {
+ validateAndChange();
+ }
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [databaseColumns, calculatedColumns]);
- getSQLLabUrl() {
+ const getSQLLabUrl = useCallback(() => {
const queryParams = new URLSearchParams({
- dbid: String(this.state.datasource.database?.id ?? ''),
- sql: this.state.datasource.sql ?? '',
- name: this.state.datasource.datasource_name ?? '',
- schema: this.state.datasource.schema ?? '',
+ dbid: String(datasource.database?.id ?? ''),
+ sql: datasource.sql ?? '',
+ name: datasource.datasource_name ?? '',
+ schema: datasource.schema ?? '',
autorun: 'true',
isDataset: 'true',
});
return makeUrl(`/sqllab/?${queryParams.toString()}`);
- }
+ }, [datasource]);
- openOnSqlLab() {
- window.open(this.getSQLLabUrl(), '_blank', 'noopener,noreferrer');
- }
+ const openOnSqlLab = useCallback(() => {
+ window.open(getSQLLabUrl(), '_blank', 'noopener,noreferrer');
+ }, [getSQLLabUrl]);
- tableChangeAndSyncMetadata() {
- this.validate(() => {
- this.syncMetadata();
- this.onChange();
+ const onQueryRun = useCallback(async () => {
+ const databaseId = datasource.database?.id;
+ const { sql } = datasource;
+ if (!databaseId || !sql) {
+ return;
+ }
+ runQuery({
+ client_id: database?.clientId,
+ database_id: databaseId,
+ runAsync: false,
+ catalog: datasource.catalog,
+ schema: datasource.schema,
+ sql,
+ tmp_table_name: '',
+ select_as_cta: false,
+ ctas_method: 'TABLE',
+ queryLimit: 25,
+ expand_data: true,
});
- }
+ }, [datasource, database?.clientId, runQuery]);
- /**
- * Formats SQL query using the SQL format API endpoint.
- * Aborts any pending format requests before starting a new one.
- */
- async formatSql() {
- const { datasource } = this.state;
- if (!datasource.sql) {
+ const onQueryFormat = useCallback(async () => {
+ if (!datasource.sql || !isEditMode) {
return;
}
- // Abort previous formatSql if still pending
- if (this.abortControllers.formatSql) {
- this.abortControllers.formatSql.abort();
+ // Abort previous formatQuery if still pending
+ if (abortControllers.current.formatQuery) {
+ abortControllers.current.formatQuery.abort();
}
- this.abortControllers.formatSql = new AbortController();
- const { signal } = this.abortControllers.formatSql;
+ abortControllers.current.formatQuery = new AbortController();
+ const { signal } = abortControllers.current.formatQuery;
try {
- const response = await SupersetClient.post({
- endpoint: '/api/v1/sql/format',
- body: JSON.stringify({ sql: datasource.sql }),
- headers: { 'Content-Type': 'application/json' },
- signal,
- });
+ const response = await formatQueryAction(datasource.sql, { signal });
- this.onDatasourcePropChange('sql', response.json.result);
- this.props.addSuccessToast(t('SQL was formatted'));
- } catch (error) {
- if (error.name === 'AbortError') return;
+ onDatasourcePropChange('sql', response.json.result);
+ addSuccessToast(t('SQL was formatted'));
+ } catch (error: unknown) {
+ if ((error as Error).name === 'AbortError') return;
- const { error: clientError, statusText } =
- await getClientErrorObject(error);
+ const { error: clientError, statusText } = await getClientErrorObject(
+ error as Response,
+ );
- this.props.addDangerToast(
+ addDangerToast(
clientError ||
statusText ||
t('An error occurred while formatting SQL'),
);
} finally {
- this.abortControllers.formatSql = null;
+ abortControllers.current.formatQuery = null;
}
- }
-
- /**
- * Syncs dataset columns with the database schema.
- * Fetches column metadata from the underlying table/view and updates the
dataset.
- * Aborts any pending sync requests before starting a new one.
- */
- async syncMetadata() {
- const { datasource } = this.state;
-
+ }, [
+ datasource.sql,
+ isEditMode,
+ formatQueryAction,
+ onDatasourcePropChange,
+ addSuccessToast,
+ addDangerToast,
+ ]);
+
+ const syncMetadata = useCallback(async () => {
// Abort previous syncMetadata if still pending
- if (this.abortControllers.syncMetadata) {
- this.abortControllers.syncMetadata.abort();
+ if (abortControllers.current.syncMetadata) {
+ abortControllers.current.syncMetadata.abort();
}
- this.abortControllers.syncMetadata = new AbortController();
- const { signal } = this.abortControllers.syncMetadata;
+ abortControllers.current.syncMetadata = new AbortController();
+ const { signal } = abortControllers.current.syncMetadata;
- this.setState({ metadataLoading: true });
+ setMetadataLoading(true);
try {
const newCols = await fetchSyncedColumns(datasource, signal);
const columnChanges = updateColumns(
datasource.columns,
newCols,
- this.props.addSuccessToast,
+ addSuccessToast,
);
- this.setColumns({
+ setColumns({
databaseColumns: columnChanges.finalColumns.filter(
- col => !col.expression, // remove calculated columns
+ col => !col.expression,
) as Column[],
});
if (datasource.id !== undefined) {
clearDatasetCache(datasource.id);
}
- this.props.addSuccessToast(t('Metadata has been synced'));
- this.setState({ metadataLoading: false });
- } catch (error) {
- if (error.name === 'AbortError') {
- // Only update state if still mounted (abort may happen during unmount)
- if (this.isComponentMounted) {
- this.setState({ metadataLoading: false });
+ addSuccessToast(t('Metadata has been synced'));
+ setMetadataLoading(false);
+ } catch (error: unknown) {
+ if ((error as Error).name === 'AbortError') {
+ if (isComponentMounted.current) {
+ setMetadataLoading(false);
}
return;
}
- const { error: clientError, statusText } =
- await getClientErrorObject(error);
-
- this.props.addDangerToast(
- clientError || statusText || t('An error has occurred'),
+ const { error: clientError, statusText } = await getClientErrorObject(
+ error as Response,
);
- this.setState({ metadataLoading: false });
- } finally {
- this.abortControllers.syncMetadata = null;
- }
- }
- /**
- * Fetches chart usage data for this dataset (which charts use this dataset).
- * Aborts any pending fetch requests before starting a new one.
- *
- * @param {number} page - Page number (1-indexed)
- * @param {number} pageSize - Number of results per page
- * @param {string} sortColumn - Column to sort by
- * @param {string} sortDirection - Sort direction ('asc' or 'desc')
- * @returns {Promise<{charts: Array, count: number, ids: Array}>} Chart
usage data
- */
- async fetchUsageData(
- page = 1,
- pageSize = 25,
- sortColumn = 'changed_on_delta_humanized',
- sortDirection = 'desc',
- ) {
- const { datasource } = this.state;
-
- // Abort previous fetchUsageData if still pending
- if (this.abortControllers.fetchUsageData) {
- this.abortControllers.fetchUsageData.abort();
+ addDangerToast(clientError || statusText || t('An error has occurred'));
+ setMetadataLoading(false);
+ } finally {
+ abortControllers.current.syncMetadata = null;
}
+ }, [datasource, addSuccessToast, addDangerToast, setColumns]);
+
+ const fetchUsageData = useCallback(
+ async (
+ page = 1,
+ pageSize = 25,
+ sortColumn = 'changed_on_delta_humanized',
+ sortDirection = 'desc',
+ ) => {
+ // Abort previous fetchUsageData if still pending
+ if (abortControllers.current.fetchUsageData) {
+ abortControllers.current.fetchUsageData.abort();
+ }
- this.abortControllers.fetchUsageData = new AbortController();
- const { signal } = this.abortControllers.fetchUsageData;
+ abortControllers.current.fetchUsageData = new AbortController();
+ const { signal } = abortControllers.current.fetchUsageData;
+
+ try {
+ const queryParams = rison.encode({
+ columns: [
+ 'slice_name',
+ 'url',
+ 'certified_by',
+ 'certification_details',
+ 'description',
+ 'owners.first_name',
+ 'owners.last_name',
+ 'owners.id',
+ 'changed_on_delta_humanized',
+ 'changed_on',
+ 'changed_by.first_name',
+ 'changed_by.last_name',
+ 'changed_by.id',
+ 'dashboards.id',
+ 'dashboards.dashboard_title',
+ 'dashboards.url',
+ ],
+ filters: [
+ {
+ col: 'datasource_id',
+ opr: 'eq',
+ value: datasource.id,
+ },
+ ],
+ order_column: sortColumn,
+ order_direction: sortDirection,
+ page: page - 1,
+ page_size: pageSize,
+ });
- try {
- const queryParams = rison.encode({
- columns: [
- 'slice_name',
- 'url',
- 'certified_by',
- 'certification_details',
- 'description',
- 'owners.first_name',
- 'owners.last_name',
- 'owners.id',
- 'changed_on_delta_humanized',
- 'changed_on',
- 'changed_by.first_name',
- 'changed_by.last_name',
- 'changed_by.id',
- 'dashboards.id',
- 'dashboards.dashboard_title',
- 'dashboards.url',
- ],
- filters: [
- {
- col: 'datasource_id',
- opr: 'eq',
- value: datasource.id,
- },
- ],
- order_column: sortColumn,
- order_direction: sortDirection,
- page: page - 1,
- page_size: pageSize,
- });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `/api/v1/chart/?q=${queryParams}`,
+ signal,
+ });
- const { json = {} } = await SupersetClient.get({
- endpoint: `/api/v1/chart/?q=${queryParams}`,
- signal,
- });
+ const charts = json?.result || [];
+ const ids = json?.ids || [];
- const charts = json?.result || [];
- const ids = json?.ids || [];
+ const chartsWithIds = charts.map(
+ (chart: Omit<ChartUsageData, 'id'>, index: number) => ({
+ ...chart,
+ id: ids[index],
+ }),
+ );
- // Map chart IDs to chart objects
- const chartsWithIds = charts.map(
- (chart: Omit<ChartUsageData, 'id'>, index: number) => ({
- ...chart,
- id: ids[index],
- }),
- );
+ if (!signal.aborted && isComponentMounted.current) {
+ setUsageCharts(chartsWithIds);
+ setUsageChartsCount(json?.count || 0);
+ }
- // Only update state if not aborted and component still mounted
- if (!signal.aborted && this.isComponentMounted) {
- this.setState({
- usageCharts: chartsWithIds,
- usageChartsCount: json?.count || 0,
- });
- }
+ return {
+ charts: chartsWithIds,
+ count: json?.count || 0,
+ ids,
+ };
+ } catch (error: unknown) {
+ if ((error as Error).name === 'AbortError') throw error;
- return {
- charts: chartsWithIds,
- count: json?.count || 0,
- ids,
- };
- } catch (error) {
- // Rethrow AbortError so callers can handle gracefully
- if (error.name === 'AbortError') throw error;
+ const { error: clientError, statusText } = await getClientErrorObject(
+ error as Response,
+ );
- const { error: clientError, statusText } =
- await getClientErrorObject(error);
+ addDangerToast(
+ clientError ||
+ statusText ||
+ t('An error occurred while fetching usage data'),
+ );
+ setUsageCharts([]);
+ setUsageChartsCount(0);
+
+ return {
+ charts: [],
+ count: 0,
+ ids: [],
+ };
+ } finally {
+ abortControllers.current.fetchUsageData = null;
+ }
+ },
+ [datasource.id, addDangerToast],
+ );
- this.props.addDangerToast(
- clientError ||
- statusText ||
- t('An error occurred while fetching usage data'),
- );
- this.setState({
- usageCharts: [],
- usageChartsCount: 0,
- });
+ const handleTabSelect = useCallback((key: string) => {
+ setActiveTabKey(key);
+ }, []);
- return {
- charts: [],
- count: 0,
- ids: [],
- };
- } finally {
- this.abortControllers.fetchUsageData = null;
- }
- }
+ const sortMetrics = useCallback(
+ (metrics: Metric[]) =>
+ [...metrics].sort(
+ ({ id: a }: { id?: number }, { id: b }: { id?: number }) =>
+ (b ?? 0) - (a ?? 0),
+ ),
+ [],
+ );
- findDuplicates<T>(arr: T[], accessor: (obj: T) => string): string[] {
- const seen: Record<string, null> = {};
- const dups: string[] = [];
- arr.forEach((obj: T) => {
- const item = accessor(obj);
- if (item in seen) {
- dups.push(item);
- } else {
- seen[item] = null;
+ // Keep the refs read by the (one-shot) Mousetrap handler up to date.
+ const isEditModeRef = useRef(isEditMode);
+ const onQueryFormatRef = useRef(onQueryFormat);
+ useEffect(() => {
+ isEditModeRef.current = isEditMode;
+ onQueryFormatRef.current = onQueryFormat;
+ }, [isEditMode, onQueryFormat]);
+
+ // componentDidMount
+ useEffect(() => {
+ isComponentMounted.current = true;
+ // Mark initial mount as complete after first render cycle
+ // This prevents useEffect hooks from firing on mount
+ isInitialMount.current = false;
+ // Bind ctrl+shift+f once on mount and route through refs so we don't
+ // unbind/rebind on every SQL-editor keystroke (onQueryFormat's identity
+ // changes with datasource.sql).
+ Mousetrap.bind('ctrl+shift+f', e => {
+ e.preventDefault();
+ if (isEditModeRef.current) {
+ onQueryFormatRef.current?.();
}
+ return false;
+ });
+ fetchUsageData().catch(error => {
+ if (error?.name !== 'AbortError') throw error;
});
- return dups;
- }
- validate(callback: () => void) {
- let errors: string[] = [];
- let dups: string[];
- const { datasource } = this.state;
+ // componentWillUnmount
+ return () => {
+ isComponentMounted.current = false;
- // Looking for duplicate column_name
- dups = this.findDuplicates(datasource.columns, obj => obj.column_name);
- errors = errors.concat(
- dups.map(name => t('Column name [%s] is duplicated', name)),
- );
+ // Abort all pending requests
+ Object.values(abortControllers.current).forEach(controller => {
+ if (controller) controller.abort();
+ });
- // Looking for duplicate metric_name
- dups = this.findDuplicates(
- datasource.metrics ?? [],
- obj => obj.metric_name,
- );
- errors = errors.concat(
- dups.map(name => t('Metric name [%s] is duplicated', name)),
- );
+ Mousetrap.unbind('ctrl+shift+f');
+ resetQuery();
+ };
+ }, []);
- // Making sure calculatedColumns have an expression defined
- const noFilterCalcCols = this.state.calculatedColumns.filter(
- col => !col.expression && !col.json,
- );
- errors = errors.concat(
- noFilterCalcCols.map(col =>
- t('Calculated column [%s] requires an expression', col.column_name),
- ),
+ // componentDidUpdate for props.datasource changes
+ // Only run when the props.datasource reference actually changes from parent
+ useEffect(() => {
+ if (!isComponentMounted.current) return;
+ if (prevPropsDatasourceRef.current === propsDatasource) return;
+ prevPropsDatasourceRef.current = propsDatasource;
+
+ const newCalculatedColumns = propsDatasource.columns.filter(
+ col => !!col.expression,
);
- // validate currency code (skip 'AUTO' - it's a placeholder for
auto-detection)
- try {
- this.state.datasource.metrics?.forEach(
- metric =>
- metric.currency?.symbol &&
- metric.currency.symbol !== 'AUTO' &&
- new Intl.NumberFormat('en-US', {
- style: 'currency',
- currency: metric.currency.symbol,
- }),
- );
- } catch {
- errors = errors.concat([t('Invalid currency code in saved metrics')]);
- }
+ if (newCalculatedColumns.length === calculatedColumns.length) {
+ const orderedCalculatedColumns: Column[] = [];
+ const usedIds = new Set<string | number>();
- // Validate folders
- if (this.state.folders?.length > 0) {
- const folderValidation = validateFolders(this.state.folders);
- errors = errors.concat(folderValidation.errors);
- }
+ calculatedColumns.forEach(currentCol => {
+ const id = currentCol.id || currentCol.column_name;
+ const updatedCol = newCalculatedColumns.find(
+ newCol => (newCol.id || newCol.column_name) === id,
+ );
+ if (updatedCol) {
+ orderedCalculatedColumns.push(updatedCol);
+ usedIds.add(id);
+ }
+ });
- this.setState({ errors }, callback);
- }
+ newCalculatedColumns.forEach(newCol => {
+ const id = newCol.id || newCol.column_name;
+ if (!usedIds.has(id)) {
Review Comment:
This length gate matches master's `componentWillReceiveProps` exactly (the
old `newCalculatedColumns.length === currentCalculatedColumns.length` check) —
preserved as-is by the conversion. Reworking the reconciliation is out of scope
for the FC move.
--
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]
