This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 660357c76b0 feat: Persist default folders location when repositioned
in folders editor (#38105)
660357c76b0 is described below
commit 660357c76b032fb6271453d9671d36d3d36bd991
Author: Kamil Gabryjelski <[email protected]>
AuthorDate: Thu Feb 26 15:58:25 2026 +0100
feat: Persist default folders location when repositioned in folders editor
(#38105)
Co-authored-by: Claude Opus 4.6 <[email protected]>
---
.../FoldersEditor/folderOperations.test.ts | 106 +++++++++++++++++++++
.../Datasource/FoldersEditor/folderOperations.ts | 57 +++++++----
.../DatasourceEditor/DatasourceEditor.tsx | 28 +++---
.../transformDatasourceFolders.test.ts | 12 ++-
.../DatasourcePanel/transformDatasourceFolders.ts | 71 +++++++++-----
superset/commands/dataset/update.py | 22 ++++-
tests/unit_tests/commands/dataset/update_test.py | 35 ++++++-
7 files changed, 271 insertions(+), 60 deletions(-)
diff --git
a/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.test.ts
b/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.test.ts
index 0b0c8f7dd6b..79531acdaf0 100644
---
a/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.test.ts
+++
b/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.test.ts
@@ -250,5 +250,111 @@ describe('folderUtils', () => {
expect(result.length).toBeGreaterThan(2);
expect(result.find(f => f.name === 'Custom Folder')).toBeDefined();
});
+
+ test('should preserve default folder positions when saved', () => {
+ const customFolder = createFolder('Custom');
+ customFolder.children = [
+ {
+ uuid: 'metric-1',
+ type: FoldersEditorItemType.Metric,
+ name: 'Test Metric 1',
+ },
+ ];
+ const savedFolders = [
+ {
+ uuid: DEFAULT_METRICS_FOLDER_UUID,
+ type: FoldersEditorItemType.Folder as const,
+ name: 'Metrics',
+ children: [
+ {
+ uuid: 'metric-2',
+ name: 'metric-2',
+ type: FoldersEditorItemType.Metric,
+ },
+ ],
+ },
+ customFolder,
+ {
+ uuid: DEFAULT_COLUMNS_FOLDER_UUID,
+ type: FoldersEditorItemType.Folder as const,
+ name: 'Columns',
+ children: [
+ {
+ uuid: 'column-1',
+ name: 'column-1',
+ type: FoldersEditorItemType.Column,
+ },
+ {
+ uuid: 'column-2',
+ name: 'column-2',
+ type: FoldersEditorItemType.Column,
+ },
+ ],
+ },
+ ];
+ const result = ensureDefaultFolders(
+ savedFolders,
+ mockMetrics,
+ mockColumns,
+ );
+
+ expect(result).toHaveLength(3);
+ expect(result[0].uuid).toBe(DEFAULT_METRICS_FOLDER_UUID);
+ expect(result[1].uuid).toBe(customFolder.uuid);
+ expect(result[2].uuid).toBe(DEFAULT_COLUMNS_FOLDER_UUID);
+ });
+
+ test('should add unassigned items to existing default folders', () => {
+ const savedFolders = [
+ {
+ uuid: DEFAULT_METRICS_FOLDER_UUID,
+ type: FoldersEditorItemType.Folder as const,
+ name: 'Metrics',
+ children: [
+ {
+ uuid: 'metric-1',
+ name: 'metric-1',
+ type: FoldersEditorItemType.Metric,
+ },
+ ],
+ },
+ {
+ uuid: DEFAULT_COLUMNS_FOLDER_UUID,
+ type: FoldersEditorItemType.Folder as const,
+ name: 'Columns',
+ children: [
+ {
+ uuid: 'column-1',
+ name: 'column-1',
+ type: FoldersEditorItemType.Column,
+ },
+ ],
+ },
+ ];
+ const result = ensureDefaultFolders(
+ savedFolders,
+ mockMetrics,
+ mockColumns,
+ );
+
+ const metricsFolder = result.find(
+ f => f.uuid === DEFAULT_METRICS_FOLDER_UUID,
+ );
+ const columnsFolder = result.find(
+ f => f.uuid === DEFAULT_COLUMNS_FOLDER_UUID,
+ );
+
+ // metric-2 was unassigned, should be appended
+ expect(metricsFolder?.children).toHaveLength(2);
+ expect(metricsFolder?.children?.some(c => c.uuid === 'metric-2')).toBe(
+ true,
+ );
+
+ // column-2 was unassigned, should be appended
+ expect(columnsFolder?.children).toHaveLength(2);
+ expect(columnsFolder?.children?.some(c => c.uuid === 'column-2')).toBe(
+ true,
+ );
+ });
});
});
diff --git
a/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.ts
b/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.ts
index e9f0b0d9119..cb810bd4a01 100644
---
a/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.ts
+++
b/superset-frontend/src/components/Datasource/FoldersEditor/folderOperations.ts
@@ -174,8 +174,6 @@ export const ensureDefaultFolders = (
f => f.uuid === DEFAULT_COLUMNS_FOLDER_UUID,
);
- const result = [...enrichedFolders];
-
// Build a Set of all assigned UUIDs in a single pass for O(1) lookups
const assignedIds = new Set<string>();
const collectAssignedIds = (folder: DatasourceFolder) => {
@@ -189,33 +187,60 @@ export const ensureDefaultFolders = (
};
enrichedFolders.forEach(collectAssignedIds);
- if (!hasMetricsFolder) {
- const unassignedMetrics = metrics.filter(m => !assignedIds.has(m.uuid));
+ const unassignedMetrics = metrics
+ .filter(m => !assignedIds.has(m.uuid))
+ .map(m => ({
+ type: FoldersEditorItemType.Metric as const,
+ uuid: m.uuid,
+ name: m.metric_name || '',
+ }));
+ const unassignedColumns = columns
+ .filter(c => !assignedIds.has(c.uuid))
+ .map(c => ({
+ type: FoldersEditorItemType.Column as const,
+ uuid: c.uuid,
+ name: c.column_name || '',
+ }));
+
+ // Add unassigned items to existing default folders (handles new items added
after last save)
+ const result = enrichedFolders.map(folder => {
+ if (
+ folder.uuid === DEFAULT_METRICS_FOLDER_UUID &&
+ unassignedMetrics.length > 0
+ ) {
+ return {
+ ...folder,
+ children: [...(folder.children || []), ...unassignedMetrics],
+ };
+ }
+ if (
+ folder.uuid === DEFAULT_COLUMNS_FOLDER_UUID &&
+ unassignedColumns.length > 0
+ ) {
+ return {
+ ...folder,
+ children: [...(folder.children || []), ...unassignedColumns],
+ };
+ }
+ return folder;
+ });
+ // If default folders don't exist at all, add them at the end (backward
compatibility)
+ if (!hasMetricsFolder) {
result.push({
uuid: DEFAULT_METRICS_FOLDER_UUID,
type: FoldersEditorItemType.Folder,
name: t('Metrics'),
- children: unassignedMetrics.map(m => ({
- type: FoldersEditorItemType.Metric,
- uuid: m.uuid,
- name: m.metric_name || '',
- })),
+ children: unassignedMetrics,
});
}
if (!hasColumnsFolder) {
- const unassignedColumns = columns.filter(c => !assignedIds.has(c.uuid));
-
result.push({
uuid: DEFAULT_COLUMNS_FOLDER_UUID,
type: FoldersEditorItemType.Folder,
name: t('Columns'),
- children: unassignedColumns.map(c => ({
- type: FoldersEditorItemType.Column,
- uuid: c.uuid,
- name: c.column_name || '',
- })),
+ children: unassignedColumns,
});
}
diff --git
a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
index fe1a35cc72e..fd3b899f399 100644
---
a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
+++
b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
@@ -91,9 +91,8 @@ import Field from '../Field';
import { fetchSyncedColumns, updateColumns } from '../../utils';
import DatasetUsageTab from './components/DatasetUsageTab';
import {
- DEFAULT_COLUMNS_FOLDER_UUID,
DEFAULT_FOLDERS_COUNT,
- DEFAULT_METRICS_FOLDER_UUID,
+ isDefaultFolder,
} from '../../FoldersEditor/constants';
import { validateFolders } from '../../FoldersEditor/folderValidation';
import { countAllFolders } from '../../FoldersEditor/treeUtils';
@@ -942,8 +941,14 @@ class DatasourceEditor extends PureComponent<
col => !!col.expression,
),
folders: props.datasource.folders || [],
- folderCount:
- countAllFolders(props.datasource.folders || []) +
DEFAULT_FOLDERS_COUNT,
+ 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
@@ -1031,16 +1036,10 @@ class DatasourceEditor extends PureComponent<
handleFoldersChange(folders: DatasourceFolder[]) {
const folderCount = countAllFolders(folders);
- const userMadeFolders = folders.filter(
- f =>
- f.uuid !== DEFAULT_METRICS_FOLDER_UUID &&
- f.uuid !== DEFAULT_COLUMNS_FOLDER_UUID &&
- (f.children?.length ?? 0) > 0,
- );
- this.setState({ folders: userMadeFolders, folderCount }, () => {
+ this.setState({ folders, folderCount }, () => {
this.onDatasourceChange({
...this.state.datasource,
- folders: userMadeFolders,
+ folders,
});
});
}
@@ -2493,7 +2492,10 @@ class DatasourceEditor extends PureComponent<
metrics={
sortedMetrics as unknown as
import('@superset-ui/chart-controls').Metric[]
}
- columns={this.state.databaseColumns}
+ columns={[
+ ...this.state.databaseColumns,
+ ...this.state.calculatedColumns,
+ ]}
onChange={this.handleFoldersChange}
/>
),
diff --git
a/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.test.ts
b/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.test.ts
index 33f922c69f6..c2bd343649b 100644
---
a/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.test.ts
+++
b/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.test.ts
@@ -21,6 +21,10 @@ import { Metric } from '@superset-ui/core';
import { transformDatasourceWithFolders } from './transformDatasourceFolders';
import { DatasourceFolder, DatasourcePanelColumn } from './types';
import { FoldersEditorItemType } from 'src/components/Datasource/types';
+import {
+ DEFAULT_METRICS_FOLDER_UUID,
+ DEFAULT_COLUMNS_FOLDER_UUID,
+} from 'src/components/Datasource/FoldersEditor/constants';
const mockMetrics: Metric[] = [
{ metric_name: 'metric1', uuid: 'metric1-uuid', expression: 'SUM(col1)' },
@@ -45,13 +49,13 @@ test('transforms data into default folders when no folder
config is provided', (
expect(result).toHaveLength(2);
- expect(result[0].id).toBe('metrics-default');
+ expect(result[0].id).toBe(DEFAULT_METRICS_FOLDER_UUID);
expect(result[0].name).toBe('Metrics');
expect(result[0].items).toHaveLength(3);
expect(result[0].items[0].uuid).toBe('metric1-uuid');
expect(result[0].items[0].type).toBe(FoldersEditorItemType.Metric);
- expect(result[1].id).toBe('columns-default');
+ expect(result[1].id).toBe(DEFAULT_COLUMNS_FOLDER_UUID);
expect(result[1].name).toBe('Columns');
expect(result[1].items).toHaveLength(3);
expect(result[1].items[0].uuid).toBe('column1-uuid');
@@ -118,11 +122,11 @@ test('transforms data according to folder configuration',
() => {
expect(result[1].items).toHaveLength(1);
expect(result[1].items[0].uuid).toBe('column1-uuid');
- expect(result[2].id).toBe('metrics-default');
+ expect(result[2].id).toBe(DEFAULT_METRICS_FOLDER_UUID);
expect(result[2].items).toHaveLength(1);
expect(result[2].items[0].uuid).toBe('metric3-uuid');
- expect(result[3].id).toBe('columns-default');
+ expect(result[3].id).toBe(DEFAULT_COLUMNS_FOLDER_UUID);
expect(result[3].items).toHaveLength(2);
});
diff --git
a/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.ts
b/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.ts
index 414b7f85a62..eb94823069a 100644
---
a/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.ts
+++
b/superset-frontend/src/explore/components/DatasourcePanel/transformDatasourceFolders.ts
@@ -19,6 +19,10 @@
import { t } from '@apache-superset/core';
import { Metric } from '@superset-ui/core';
import { FoldersEditorItemType } from 'src/components/Datasource/types';
+import {
+ DEFAULT_METRICS_FOLDER_UUID,
+ DEFAULT_COLUMNS_FOLDER_UUID,
+} from 'src/components/Datasource/FoldersEditor/constants';
import {
ColumnItem,
DatasourceFolder,
@@ -98,10 +102,36 @@ const transformToFolderStructure = (
return folder;
};
+ const addUnassignedToFolder = (
+ folders: Folder[],
+ items: (MetricItem | ColumnItem)[],
+ folderId: string,
+ folderName: string,
+ allItemsCount: number,
+ inFoldersCount: number,
+ ) => {
+ if (items.length === 0) return;
+ const existing = folders.find(f => f.id === folderId);
+ if (existing) {
+ existing.items.push(...items);
+ existing.totalItems += items.length;
+ existing.showingItems += items.length;
+ } else {
+ folders.push({
+ id: folderId,
+ name: folderName,
+ isCollapsed: false,
+ items,
+ totalItems: allItemsCount - inFoldersCount,
+ showingItems: items.length,
+ });
+ }
+ };
+
if (!folderConfig) {
return [
{
- id: 'metrics-default',
+ id: DEFAULT_METRICS_FOLDER_UUID,
name: t('Metrics'),
isCollapsed: false,
items: metricsToDisplay,
@@ -109,7 +139,7 @@ const transformToFolderStructure = (
showingItems: metricsToDisplay.length,
},
{
- id: 'columns-default',
+ id: DEFAULT_COLUMNS_FOLDER_UUID,
name: t('Columns'),
isCollapsed: false,
items: columnsToDisplay,
@@ -128,27 +158,22 @@ const transformToFolderStructure = (
columnsMap.has(column.uuid),
);
- if (unassignedMetrics.length > 0) {
- folders.push({
- id: 'metrics-default',
- name: t('Metrics'),
- isCollapsed: false,
- items: unassignedMetrics,
- totalItems: allMetrics.length - metricsInFolders,
- showingItems: unassignedMetrics.length,
- });
- }
-
- if (unassignedColumns.length > 0) {
- folders.push({
- id: 'columns-default',
- name: t('Columns'),
- isCollapsed: false,
- items: unassignedColumns,
- totalItems: allColumns.length - columnsInFolders,
- showingItems: unassignedColumns.length,
- });
- }
+ addUnassignedToFolder(
+ folders,
+ unassignedMetrics,
+ DEFAULT_METRICS_FOLDER_UUID,
+ t('Metrics'),
+ allMetrics.length,
+ metricsInFolders,
+ );
+ addUnassignedToFolder(
+ folders,
+ unassignedColumns,
+ DEFAULT_COLUMNS_FOLDER_UUID,
+ t('Columns'),
+ allColumns.length,
+ columnsInFolders,
+ );
return folders;
};
diff --git a/superset/commands/dataset/update.py
b/superset/commands/dataset/update.py
index be86cd876b4..3acab024b9e 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -54,6 +54,14 @@ from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
+# Default folder UUIDs matching the frontend constants.
+# Stored as strings so comparisons work whether obj["uuid"] is str or UUID.
+DEFAULT_METRICS_FOLDER_UUID = "255b537d-58c8-443d-9fc1-4e4dc75047e2"
+DEFAULT_COLUMNS_FOLDER_UUID = "83a7ae8f-2e8a-4f2b-a8cb-ebaebef95b9b"
+DEFAULT_FOLDER_UUIDS = frozenset(
+ {DEFAULT_METRICS_FOLDER_UUID, DEFAULT_COLUMNS_FOLDER_UUID}
+)
+
class UpdateDatasetCommand(UpdateMixin, BaseCommand):
def __init__(
@@ -320,11 +328,19 @@ def validate_folders( # noqa: C901
raise ValidationError(f"Duplicate folder name: {name}")
seen_fqns.add(fqn)
- if name.lower() in {"metrics", "columns"}:
+ # Allow default folders (by UUID) to use reserved names
+ if (
+ name.lower() in {"metrics", "columns"}
+ and str(uuid) not in DEFAULT_FOLDER_UUIDS
+ ):
raise ValidationError(f"Folder cannot have name '{name}'")
- # check if metric/column UUID exists
- elif not name and uuid not in valid_uuids:
+ # check if metric/column UUID exists (skip default folders)
+ elif (
+ not name
+ and uuid not in valid_uuids
+ and str(uuid) not in DEFAULT_FOLDER_UUIDS
+ ):
raise ValidationError(f"Invalid UUID: {uuid}")
# traverse children
diff --git a/tests/unit_tests/commands/dataset/update_test.py
b/tests/unit_tests/commands/dataset/update_test.py
index 5eb26fea4a5..afc33741c51 100644
--- a/tests/unit_tests/commands/dataset/update_test.py
+++ b/tests/unit_tests/commands/dataset/update_test.py
@@ -29,7 +29,12 @@ from superset.commands.dataset.exceptions import (
DatasetNotFoundError,
MultiCatalogDisabledValidationError,
)
-from superset.commands.dataset.update import UpdateDatasetCommand,
validate_folders
+from superset.commands.dataset.update import (
+ DEFAULT_COLUMNS_FOLDER_UUID,
+ DEFAULT_METRICS_FOLDER_UUID,
+ UpdateDatasetCommand,
+ validate_folders,
+)
from superset.commands.exceptions import OwnersNotFoundValidationError
from superset.datasets.schemas import FolderSchema
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
@@ -564,6 +569,34 @@ def test_validate_folders_invalid_names(mocker:
MockerFixture) -> None:
assert str(excinfo.value) == "Folder cannot have name 'Columns'"
+@with_feature_flags(DATASET_FOLDERS=True)
+def test_validate_folders_allows_default_folders(mocker: MockerFixture) ->
None:
+ """
+ Test that default system folders (Metrics/Columns) are allowed when using
+ the well-known default folder UUIDs, so their position can be persisted.
+ """
+ folders = cast(
+ list[FolderSchema],
+ [
+ {
+ "uuid": DEFAULT_METRICS_FOLDER_UUID,
+ "type": "folder",
+ "name": "Metrics",
+ "children": [],
+ },
+ {
+ "uuid": DEFAULT_COLUMNS_FOLDER_UUID,
+ "type": "folder",
+ "name": "Columns",
+ "children": [],
+ },
+ ],
+ )
+
+ # Should not raise - default folders are allowed to use reserved names
+ validate_folders(folders=folders, valid_uuids=set())
+
+
@with_feature_flags(DATASET_FOLDERS=True)
def test_validate_folders_invalid_uuid(mocker: MockerFixture) -> None:
"""
