diff --git a/.github/playwright/impact-map.json b/.github/playwright/impact-map.json index 016b83afa017..e08a5634d76e 100644 --- a/.github/playwright/impact-map.json +++ b/.github/playwright/impact-map.json @@ -522,6 +522,25 @@ "playwright/e2e/Features/ActivityFeedTabBadge.spec.ts", "playwright/e2e/Flow/CustomizeWidgets.spec.ts" ] + }, + { + "_comment": "The customize-landing-page and widget-filter playwright helpers are shared by every landing-page/customization spec. Without this entry a helper-only change (e.g. a wait fix in customizeLandingPage.ts or widgetFilters.ts) counted as an unmapped file, so CustomizeWidgets/CustomizeLandingPage and the other specs that execute the changed helpers never ran on the PR. Specs listed are the exact importers of these modules; DataInsight.spec.ts runs in the Data Insight project.", + "sources": [ + "openmetadata-ui/src/main/resources/ui/playwright/utils/customizeLandingPage.ts", + "openmetadata-ui/src/main/resources/ui/playwright/utils/widgetFilters.ts" + ], + "projects": ["chromium", "Data Insight"], + "specs": [ + "playwright/e2e/Features/ActivityFeed.spec.ts", + "playwright/e2e/Features/CuratedAssets.spec.ts", + "playwright/e2e/Features/LandingPageWidgets/**/*.spec.ts", + "playwright/e2e/Features/NavigationBlocker.spec.ts", + "playwright/e2e/Features/SettingsNavigationPage.spec.ts", + "playwright/e2e/Flow/CustomizeLandingPage.spec.ts", + "playwright/e2e/Flow/CustomizeWidgets.spec.ts", + "playwright/e2e/Flow/PersonaFlow.spec.ts", + "playwright/e2e/Pages/DataInsight.spec.ts" + ] } ] } diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/CustomizeLandingPage.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/CustomizeLandingPage.spec.ts index 40be28361c91..d633b89ec41b 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/CustomizeLandingPage.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/CustomizeLandingPage.spec.ts @@ -26,41 +26,85 @@ import { openAddCustomizeWidgetModal, removeAndCheckWidget, saveCustomizeLayoutPage, - setUserDefaultPersona, waitForLandingPageWidget, } from '../../utils/customizeLandingPage'; import { waitForAllLoadersToDisappear } from '../../utils/entity'; -const adminUser = new UserClass(); -const persona = new PersonaClass(); -const persona2 = new PersonaClass(); +type LandingPageTestFixtures = { + adminPage: Page; + testUser: UserClass; + persona: PersonaClass; +}; + +// Issue #31407 (same class of bug as CustomizeWidgets). Every test here rewrites +// the whole `persona.` layout document, and the landing page resolves its +// layout from `currentUser.defaultPersona`. Under `fullyParallel` this file's +// tests run in different workers, so both the persona and the user have to be +// per test: a shared persona makes layout saves last-write-wins across tests, +// and previously only one test set the default persona, leaving the others to +// assert the persona layout against a session that rendered the stock layout. +const test = base.extend({ + testUser: async ({ browser }, use) => { + const { apiContext, afterAction } = await performAdminLogin(browser); + const user = new UserClass(); + await user.create(apiContext); + await user.setAdminRole(apiContext); + await afterAction(); + + await use(user); + + const { apiContext: cleanupContext, afterAction: cleanupAfterAction } = + await performAdminLogin(browser); + await user.delete(cleanupContext); + await cleanupAfterAction(); + }, + + persona: async ({ browser, testUser }, use) => { + const { apiContext, afterAction } = await performAdminLogin(browser); + const testPersona = new PersonaClass(); + await testPersona.create(apiContext, [testUser.responseData.id]); + + const personaReference = { + id: testPersona.responseData.id, + type: 'persona', + name: testPersona.responseData.name, + fullyQualifiedName: testPersona.responseData.fullyQualifiedName, + description: testPersona.responseData.description, + displayName: testPersona.responseData.displayName, + }; + + await apiContext.patch(`/api/v1/users/${testUser.responseData.id}`, { + data: [ + { op: 'add', path: '/personas/0', value: personaReference }, + { op: 'add', path: '/defaultPersona', value: personaReference }, + ], + headers: { + 'Content-Type': 'application/json-patch+json', + }, + }); + await afterAction(); + + await use(testPersona); + + const { apiContext: cleanupContext, afterAction: cleanupAfterAction } = + await performAdminLogin(browser); + await testPersona.delete(cleanupContext); + await cleanupAfterAction(); + }, + + adminPage: async ({ browser, testUser, persona }, use) => { + // `persona` is depended on for its side effect - the default persona has to + // be attached to the user before login, otherwise the session starts with + // the stock layout instead of the persona's customizable one. + void persona; -const test = base.extend<{ adminPage: Page; userPage: Page }>({ - adminPage: async ({ browser }, use) => { const adminPage = await browser.newPage(); - await adminUser.login(adminPage); + await testUser.login(adminPage); await use(adminPage); await adminPage.close(); }, }); -base.beforeAll('Setup pre-requests', async ({ browser }) => { - const { afterAction, apiContext } = await performAdminLogin(browser); - await adminUser.create(apiContext); - await adminUser.setAdminRole(apiContext); - await persona.create(apiContext, [adminUser.responseData.id]); - await persona2.create(apiContext); - await afterAction(); -}); - -base.afterAll('Cleanup', async ({ browser }) => { - const { afterAction, apiContext } = await performAdminLogin(browser); - await adminUser.delete(apiContext); - await persona.delete(apiContext); - await persona2.delete(apiContext); - await afterAction(); -}); - test.describe( 'Customize Landing Page Flow', PLAYWRIGHT_BASIC_TEST_TAG_OBJ, @@ -72,11 +116,11 @@ test.describe( test('Add, Remove and Reset widget should work properly', async ({ adminPage, + persona, }) => { test.slow(true); await redirectToHomePage(adminPage); - await setUserDefaultPersona(adminPage, persona.responseData.displayName); await test.step('Remove widget', async () => { test.slow(true); @@ -267,7 +311,7 @@ test.describe( }); }); - test('Widget drag and drop reordering', async ({ adminPage }) => { + test('Widget drag and drop reordering', async ({ adminPage, persona }) => { test.slow(true); await navigateToCustomizeLandingPage(adminPage, { @@ -309,6 +353,7 @@ test.describe( // Discard twice. A single Discard click must exit the customize page. test('Cancel button should show a single confirmation modal and Discard should exit the customize landing page', async ({ adminPage, + persona, }) => { test.slow(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataInsight.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataInsight.spec.ts index dac8c6cf6c5c..55bc4a5f76f7 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataInsight.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataInsight.spec.ts @@ -268,7 +268,15 @@ test.describe('Data Insight Page', { tag: '@data-insight' }, () => { await redirectToHomePage(page); - await waitForLandingPageWidget(page, 'kpi-widget'); + // `kpi-widget` is a child of the KPI widget, not its layout key. The landing-page helper + // keys off the layout key — the testid the widget renders on its wrapper and the one its + // DeferredWidget slot is named after — so an inner testid reveals nothing. + const kpiWidget = await waitForLandingPageWidget( + page, + 'KnowledgePanel.KPI' + ); + + await expect(kpiWidget.getByTestId('kpi-widget')).toBeVisible(); }); test('Delete Kpi', async ({ page }) => { diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/customizeLandingPage.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/customizeLandingPage.ts index 1d9f626d4728..0db748a1c445 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/customizeLandingPage.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/customizeLandingPage.ts @@ -28,8 +28,6 @@ const DEFAULT_LANDING_PAGE_WIDGETS = [ 'KnowledgePanel.Following', ]; -const LANDING_PAGE_WIDGET_SCROLL_ATTEMPTS = 8; -const LANDING_PAGE_WIDGET_SCROLL_OFFSET = 600; export const CURATED_ASSETS_WIDGET_KEY = 'KnowledgePanel.CuratedAssets'; export type NameableEntityResponse = { @@ -37,30 +35,46 @@ export type NameableEntityResponse = { displayName?: string; }; -const waitForNextAnimationFrame = async (page: Page) => - page.evaluate( - () => - new Promise((resolve) => { - requestAnimationFrame(resolve); - }) - ); +// Landing-page widgets render inside `DeferredWidget` +// (src/components/common/DeferredWidget): the slot div mounts eagerly carrying a +// `deferred-widget-` testid, while the widget itself mounts only once that slot +// intersects the viewport. A below-the-fold widget therefore has no DOM node at all. +// +// The slot is keyed by the *layout* key, which is not always the widget key: widgets added +// through the "Add widget" modal get a lodash `uniqueId` suffix (`getAddWidgetHandler` in +// CustomizableLandingPagePureUtils), e.g. `KnowledgePanel.MyData-211`, whereas the widget +// always renders the un-suffixed key as its own testid. So the slot has to be matched by +// prefix — the trailing `-` keeps it unambiguous, as no widget key is a `-`-suffixed +// extension of another. +const getLandingPageWidgetSlot = (page: Page, widgetKey: string) => + page + .locator( + `[data-testid="deferred-widget-${widgetKey}"], [data-testid^="deferred-widget-${widgetKey}-"]` + ) + .first(); -const scrollLandingPageContent = async (page: Page) => { - await page - .getByTestId('page-layout-v1') - .hover() - .catch(() => undefined); - await page.mouse.wheel(0, LANDING_PAGE_WIDGET_SCROLL_OFFSET); - await page.evaluate((scrollOffset) => { - const scrollContainer = document.querySelector( - '.page-layout-v1-center.page-layout-v1-vertical-scroll' - ); - - scrollContainer?.scrollBy({ - top: scrollOffset, - }); - }, LANDING_PAGE_WIDGET_SCROLL_OFFSET); - await waitForNextAnimationFrame(page); +const revealLandingPageWidget = async (page: Page, widgetKey: string) => { + const slot = getLandingPageWidgetSlot(page, widgetKey); + + // Scroll failures are tolerated on both branches: `isLandingPageWidgetVisible` runs inside + // `expect.poll` callbacks, and Playwright's `pollMatcher` invokes the callback outside its + // try/catch — a throw here aborts the poll with no retry instead of riding out a transient + // detach. The `count()` guards are what prevent a stall; the caller's visibility assertion, + // not the scroll, is what decides whether the widget is really there. + if ((await slot.count()) > 0) { + await slot.scrollIntoViewIfNeeded().catch(() => undefined); + + return; + } + + // The customize-page edit view renders widgets without a deferred slot. Only scroll a + // widget that is already attached — scrolling a locator that resolves to nothing stalls + // for the full action timeout and starves the caller's own waiting. + const widget = page.getByTestId(widgetKey); + + if ((await widget.count()) > 0) { + await widget.scrollIntoViewIfNeeded().catch(() => undefined); + } }; // Entity types mapping from CURATED_ASSETS_LIST @@ -213,29 +227,19 @@ export const removeAndCheckWidget = async ( await expect(page.getByTestId(`${widgetKey}`)).not.toBeVisible(); }; +// Callers poll this across navigations, and each iteration starts from a fresh page load, so +// the widget needs a chance to mount inside the iteration — an instant `isVisible()` would +// never observe it. The assertion inherits the project's expect timeout. const isLandingPageWidgetVisible = async ( page: Page, widgetKey: string ): Promise => { - const widget = page.getByTestId(widgetKey); - - for (let index = 0; index < LANDING_PAGE_WIDGET_SCROLL_ATTEMPTS; index++) { - if (await widget.isVisible().catch(() => false)) { - return true; - } - - if ((await widget.count()) > 0) { - await widget.scrollIntoViewIfNeeded().catch(() => undefined); + await revealLandingPageWidget(page, widgetKey); - if (await widget.isVisible().catch(() => false)) { - return true; - } - } - - await scrollLandingPageContent(page); - } - - return false; + return expect(page.getByTestId(widgetKey)) + .toBeVisible() + .then(() => true) + .catch(() => false); }; const isLandingPageWidgetLoading = async (widget: Locator) => @@ -244,19 +248,29 @@ const isLandingPageWidgetLoading = async (widget: Locator) => .isVisible() .catch(() => false); +// Single gate every widget assertion goes through: reveal the deferred slot, prove the +// widget mounted, and let its own fetch settle. The skeleton wait belongs here rather than +// in the callers because a widget only starts loading once the slot reveals it — a caller +// that ran `waitForAllLoadersToDisappear(page, 'entity-list-skeleton')` beforehand saw no +// skeleton at all and then raced the fetch. +// +// `widgetKey` must be the widget's *layout* key — the `KnowledgePanel.*` value the widget +// renders as its own testid and that its DeferredWidget slot is named after. An inner testid +// (e.g. `kpi-widget`) matches neither, so nothing gets scrolled, the widget never mounts, and +// the assertion below fails on a widget that was simply never revealed. Assert inner testids +// against the returned locator instead. export const waitForLandingPageWidget = async ( page: Page, widgetKey: string ): Promise => { const widget = page.getByTestId(widgetKey); - const isVisible = await isLandingPageWidgetVisible(page, widgetKey); - if (isVisible) { - return widget; - } + await revealLandingPageWidget(page, widgetKey); await expect(widget).toBeVisible(); + await expect(widget.getByTestId('entity-list-skeleton')).toBeHidden(); + return widget; }; @@ -366,7 +380,10 @@ export const removeAndVerifyWidget = async ( await waitForAllLoadersToDisappear(page); - await expect(page.getByTestId(widgetKey)).not.toBeVisible(); + // Assert on the deferred slot rather than the widget: the slot renders for every layout + // entry, whereas the widget stays unmounted while below the fold — so + // `not.toBeVisible()` on the widget would pass whether it was removed or merely deferred. + await expect(getLandingPageWidgetSlot(page, widgetKey)).toHaveCount(0); }; export const addAndVerifyWidget = async ( @@ -402,18 +419,10 @@ export const addAndVerifyWidget = async ( await waitForAllLoadersToDisappear(page).catch(() => undefined); await removeLandingBanner(page); - await expect - .poll( - async () => { - await redirectToHomePage(page, false); - await removeLandingBanner(page); - await waitForAllLoadersToDisappear(page).catch(() => undefined); - - return isLandingPageWidgetVisible(page, widgetKey); - }, - { timeout: 30_000, intervals: [1_000, 2_000, 5_000] } - ) - .toBe(true); + // The save response is awaited and its toast asserted above, and `redirectToHomePage` + // disables ETag conditional reads, so the first read-back is authoritative — the widget + // helper's own web-first assertions do the waiting from here. + await waitForLandingPageWidget(page, widgetKey); }; export const addCuratedAssetPlaceholder = async ({ diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/widgetFilters.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/widgetFilters.ts index 6dabd85905cc..8f2232c8ab72 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/widgetFilters.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/widgetFilters.ts @@ -111,7 +111,7 @@ export const verifyDataFilters = async ( ); await page.getByRole('menuitem', { name: 'A to Z' }).click(); await aToZFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -125,7 +125,7 @@ export const verifyDataFilters = async ( ); await page.getByRole('menuitem', { name: 'Z to A' }).click(); await zToAFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -139,7 +139,7 @@ export const verifyDataFilters = async ( ); await page.getByRole('menuitem', { name: 'Latest' }).click(); await latestFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); }; @@ -163,7 +163,7 @@ export const verifyTotalDataAssetsFilters = async ( ); await page.getByRole('menuitem', { name: 'Last 14 days' }).click(); await last14DaysFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -180,7 +180,7 @@ export const verifyTotalDataAssetsFilters = async ( ); await page.getByRole('menuitem', { name: 'Last 7 days' }).click(); await last7DaysFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); }; @@ -203,7 +203,7 @@ export const verifyDataProductsFilters = async ( ); await page.getByRole('menuitem', { name: 'A to Z' }).click(); await aToZFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -217,7 +217,7 @@ export const verifyDataProductsFilters = async ( ); await page.getByRole('menuitem', { name: 'Z to A' }).click(); await zToAFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -231,7 +231,7 @@ export const verifyDataProductsFilters = async ( ); await page.getByRole('menuitem', { name: 'Latest' }).click(); await latestFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); }; @@ -303,7 +303,7 @@ export const verifyTaskFilters = async (page: Page, widgetKey: string) => { const mentionsTaskFilter = waitForTaskFilterResponse('MENTIONS'); await page.getByRole('menuitem', { name: 'Mentions' }).click(); await mentionsTaskFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -311,7 +311,7 @@ export const verifyTaskFilters = async (page: Page, widgetKey: string) => { const assignedTasksFilter = waitForTaskFilterResponse('ASSIGNED_TO'); await page.getByRole('menuitem', { name: 'Assigned' }).click(); await assignedTasksFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -319,7 +319,7 @@ export const verifyTaskFilters = async (page: Page, widgetKey: string) => { const allTasksFilter = waitForTaskFilterResponse('OWNER_OR_FOLLOWS'); await page.getByRole('menuitem', { name: 'All' }).click(); await allTasksFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); }; @@ -341,7 +341,7 @@ export const verifyDataAssetsFilters = async ( ); await page.getByRole('menuitem', { name: 'A to Z' }).click(); await aToZFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -354,7 +354,7 @@ export const verifyDataAssetsFilters = async ( ); await page.getByRole('menuitem', { name: 'Z to A' }).click(); await zToAFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -367,7 +367,7 @@ export const verifyDataAssetsFilters = async ( ); await page.getByRole('menuitem', { name: 'High to Low' }).click(); await highToLowFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); @@ -381,7 +381,7 @@ export const verifyDataAssetsFilters = async ( await page.getByRole('menuitem', { name: 'Low to High' }).click(); await lowToHighFilter; - await widget.locator('entity-list-skeleton').waitFor({ + await widget.getByTestId('entity-list-skeleton').waitFor({ state: 'detached', }); };