Copilot commented on code in PR #41403:
URL: https://github.com/apache/superset/pull/41403#discussion_r3476390152
##########
superset-frontend/src/SqlLab/actions/sqlLab.ts:
##########
@@ -689,8 +689,8 @@ export function syncQueryEditor(
(table: Table) =>
table.inLocalStorage && table.queryEditorId === queryEditor.id,
);
- const localStorageQueries = Object.values(queries).filter(
- query => query.inLocalStorage && query.sqlEditorId === queryEditor.id,
+ const queriesToMigrate = Object.values(queries).filter(
+ query => query.sqlEditorId === queryEditor.id && !query.isDataPreview,
);
Review Comment:
`syncQueryEditor` now explicitly excludes preview queries via
`!query.isDataPreview`, but the existing `syncQueryEditor` thunk test data uses
table `dataPreviewQueryId` values without setting `isDataPreview: true` (and
with `sqlEditorId` set to the editor id). In production, preview queries are
created with `isDataPreview: true` and `sqlEditorId: null` (see
`runTablePreviewQuery`), so the current test doesn’t exercise (and can mask
regressions in) the preview-exclusion behavior. Consider updating the test to
model real preview queries and assert they are *not* migrated, while still
asserting non-preview running queries are migrated.
--
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]
