sadpandajoe commented on code in PR #36196: URL: https://github.com/apache/superset/pull/36196#discussion_r2583874128
########## superset-frontend/playwright/tests/experimental/dataset/dataset-list.spec.ts: ########## @@ -0,0 +1,201 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { test, expect } from '@playwright/test'; +import { DatasetListPage } from '../../../pages/DatasetListPage'; +import { ExplorePage } from '../../../pages/ExplorePage'; +import { DeleteConfirmationModal } from '../../../components/modals/DeleteConfirmationModal'; +import { DuplicateDatasetModal } from '../../../components/modals/DuplicateDatasetModal'; +import { Toast } from '../../../components/core/Toast'; +import { + apiDeleteDataset, + apiGetDataset, + getDatasetByName, + duplicateDataset, + ENDPOINTS, +} from '../../../helpers/api/dataset'; + +test.describe('Dataset List', () => { + let datasetListPage: DatasetListPage; + let explorePage: ExplorePage; + let testResources: { datasetIds: number[] } = { datasetIds: [] }; + + test.beforeEach(async ({ page }) => { + datasetListPage = new DatasetListPage(page); + explorePage = new ExplorePage(page); + testResources = { datasetIds: [] }; // Reset for each test + + // Navigate to dataset list page + await datasetListPage.goto(); + await datasetListPage.waitForTableLoad(); + }); + + test.afterEach(async ({ page }) => { + // Cleanup any resources created during the test + const promises = []; + for (const datasetId of testResources.datasetIds) { + promises.push( + apiDeleteDataset(page, datasetId, { + failOnStatusCode: false, + }).catch(() => {}), + ); + } + await Promise.all(promises); + }); + + test('should navigate to Explore when dataset name is clicked', async ({ + page, + }) => { + // Use existing example dataset (hermetic - loaded in CI via --load-examples) + const datasetName = 'members_channels_2'; + const dataset = await getDatasetByName(page, datasetName); + expect(dataset).not.toBeNull(); + + // Verify dataset is visible in list (uses page object + Playwright auto-wait) + await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible(); + + // Click on dataset name to navigate to Explore + await datasetListPage.clickDatasetName(datasetName); + + // Wait for Explore page to load (validates URL + datasource control) + await explorePage.waitForPageLoad(); + + // Verify correct dataset is loaded in datasource control + const loadedDatasetName = await explorePage.getDatasetName(); + expect(loadedDatasetName).toContain(datasetName); + + // Verify visualization switcher shows default viz type (indicates full page load) + await expect(explorePage.getVizSwitcher()).toBeVisible(); + await expect(explorePage.getVizSwitcher()).toContainText('Table'); + }); + + test('should delete a dataset with confirmation', async ({ page }) => { + // Get example dataset to duplicate + const originalDataset = await getDatasetByName(page, 'members_channels_2'); + expect(originalDataset).not.toBeNull(); + + // Create throwaway copy for deletion (hermetic - uses duplicate API) + const datasetName = `test_delete_${Date.now()}`; + const duplicateDatasetResult = await duplicateDataset( + page, + originalDataset!.id, + datasetName, + ); + testResources = { datasetIds: [duplicateDatasetResult.id] }; Review Comment: Already addressed - delete test now uses response interception to track IDs immediately (matching duplicate test pattern). ########## superset-frontend/playwright/components/core/Table.ts: ########## @@ -0,0 +1,83 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Locator, Page } from '@playwright/test'; + +/** + * Table component for Superset ListView tables. + */ +export class Table { + private readonly page: Page; + private readonly tableSelector: string; + + private static readonly SELECTORS = { + TABLE_ROW: '[data-test="table-row"]', + }; + + constructor(page: Page, tableSelector = '[data-test="listview-table"]') { + this.page = page; + this.tableSelector = tableSelector; + } + + /** + * Gets the table element locator + */ + get element(): Locator { + return this.page.locator(this.tableSelector); + } + + /** + * Gets a table row by exact text match in the first cell (dataset name column). + * Uses exact match to avoid substring collisions (e.g., 'members_channels_2' vs 'duplicate_members_channels_2_123'). + * @param rowText - Exact text to find in the row's first cell + */ + getRow(rowText: string): Locator { + return this.element.locator(Table.SELECTORS.TABLE_ROW).filter({ + has: this.page.getByRole('cell', { name: rowText, exact: true }), + }); + } Review Comment: Already addressed - added documentation noting that getRow returns a Locator which has auto-wait behavior (timeout on interaction if not found). ########## superset-frontend/playwright/helpers/api/requests.ts: ########## @@ -0,0 +1,178 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Page, APIResponse } from '@playwright/test'; + +export interface ApiRequestOptions { + headers?: Record<string, string>; + params?: Record<string, string>; + failOnStatusCode?: boolean; +} + +/** + * Get base URL for Referer header + * Reads from environment variable configured in playwright.config.ts + * Preserves full base URL including path prefix (e.g., /app/prefix) + */ +function getBaseUrl(_page: Page): string { + // Use environment variable which includes path prefix if configured + // This matches playwright.config.ts baseURL setting exactly + return process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8088'; +} + +/** + * Get CSRF token from the API endpoint + * Superset provides a CSRF token via api/v1/security/csrf_token/ + * The session cookie is automatically included by page.request + */ +async function getCsrfToken(page: Page): Promise<string> { + try { + const response = await page.request.get('api/v1/security/csrf_token/', { + failOnStatusCode: false, + }); + + if (!response.ok()) { + console.warn('[CSRF] Failed to fetch CSRF token:', response.status()); Review Comment: Already addressed - error now includes `statusText()` for context: `HTTP 401 Unauthorized`. -- 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]
