From f453022ec16f320aa40f3c0057c7ec55dce6e11e Mon Sep 17 00:00:00 2001 From: Eduardo Fungairino Date: Wed, 3 Jul 2024 12:49:23 -0400 Subject: [PATCH 1/2] #8740 fix welcome starter mods not loading in welcome page (#8747) * fix welcome starter bricks not loading in welcome page * fix unused import * fix test * fix test --- end-to-end-tests/setup/utils.ts | 29 ++++------- .../regressions/welcomeStarterBricks.spec.ts | 52 +++++++++++++++++++ src/background/starterMods.test.ts | 2 +- src/background/starterMods.ts | 7 +-- 4 files changed, 68 insertions(+), 22 deletions(-) create mode 100644 end-to-end-tests/tests/regressions/welcomeStarterBricks.spec.ts diff --git a/end-to-end-tests/setup/utils.ts b/end-to-end-tests/setup/utils.ts index d543970978..883f6a762b 100644 --- a/end-to-end-tests/setup/utils.ts +++ b/end-to-end-tests/setup/utils.ts @@ -16,7 +16,6 @@ */ import { type BrowserContext, type Page, expect } from "@playwright/test"; -import { ensureVisibility } from "../utils"; export const openExtensionConsoleFromAdmin = async ( adminPage: Page, @@ -29,31 +28,25 @@ export const openExtensionConsoleFromAdmin = async ( // initialized to be able to receive messages via the external messenger api, which happens when the Extension // reloads after linking. Thus, we wrap the following with an `expect.toPass` retry. await expect(async () => { - // Ensure the extension console loads with authenticated user - const extensionConsolePagePromise = context.waitForEvent("page", { - timeout: 2000, - }); await adminPage .locator("button") .filter({ hasText: "Open Extension Console" }) .click(); - extensionConsolePage = await extensionConsolePagePromise; + extensionConsolePage = context + .pages() + .find((page) => page.url().endsWith("/options.html#/")); + + if (!extensionConsolePage) { + throw new Error("Extension console page not found"); + } await expect(extensionConsolePage.locator("#container")).toContainText( "Extension Console", ); - }).toPass({ timeout: 10_000 }); - - if (!extensionConsolePage) { - throw new Error("Extension console page not found"); - } + await expect(extensionConsolePage.getByText(userName)).toBeVisible(); + }).toPass({ timeout: 15_000 }); - await ensureVisibility( - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion,@typescript-eslint/no-unnecessary-type-assertion -- checked above - extensionConsolePage!.getByText(userName), - // The first time the extension console is opened after logging in, it sometimes takes a while to load the extension console - { timeout: 16_000 }, - ); - return extensionConsolePage; + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- needed for strict null check + return extensionConsolePage as Page; }; diff --git a/end-to-end-tests/tests/regressions/welcomeStarterBricks.spec.ts b/end-to-end-tests/tests/regressions/welcomeStarterBricks.spec.ts new file mode 100644 index 0000000000..1a8b432750 --- /dev/null +++ b/end-to-end-tests/tests/regressions/welcomeStarterBricks.spec.ts @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { test, expect } from "../../fixtures/testBase"; +// @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only +import { test as base } from "@playwright/test"; +import { ActivateModPage } from "../../pageObjects/extensionConsole/modsPage"; +import { getSidebarPage, runModViaQuickBar } from "../../utils"; + +test("#8740: can view the starter mods on the pixiebrix.com/welcome page", async ({ + page, + extensionId, +}) => { + const modId = "@e2e-testing/test-sidebar-navigation"; + const modActivationPage = new ActivateModPage(page, extensionId, modId); + await modActivationPage.goto(); + await modActivationPage.clickActivateAndWaitForModsPageRedirect(); + + await page.goto("https://pixiebrix.com/welcome"); + await runModViaQuickBar(page, "Open Sidebar"); + + const sideBarPage = await getSidebarPage(page, extensionId); + await expect( + sideBarPage.getByRole("heading", { name: "Announcements" }), + ).toBeVisible(); + await expect( + sideBarPage.getByRole("heading", { name: "Decision Tree" }), + ).toBeVisible(); + await expect( + sideBarPage.getByRole("heading", { name: "Search" }), + ).toBeVisible(); + await expect( + sideBarPage.getByRole("heading", { name: "Snippet Manager" }), + ).toBeVisible(); + await expect( + sideBarPage.getByRole("heading", { name: "Writing Assist" }), + ).toBeVisible(); +}); diff --git a/src/background/starterMods.test.ts b/src/background/starterMods.test.ts index c63ecb6513..20f7be80c9 100644 --- a/src/background/starterMods.test.ts +++ b/src/background/starterMods.test.ts @@ -67,7 +67,7 @@ jest.mock("@/auth/authStorage", () => ({ addListener: jest.fn(), })); -jest.mock("@/utils/extensionUtils"); +jest.mock("@/contentScript/messenger/api"); jest.mock("./refreshRegistries"); const isLinkedMock = jest.mocked(isLinked); diff --git a/src/background/starterMods.ts b/src/background/starterMods.ts index b0a9742315..669cdb8f2a 100644 --- a/src/background/starterMods.ts +++ b/src/background/starterMods.ts @@ -26,8 +26,7 @@ import { saveModComponentState, } from "@/store/extensionsStorage"; import { type ModDefinition } from "@/types/modDefinitionTypes"; -import { forEachTab } from "@/utils/extensionUtils"; -import { queueReloadFrameMods } from "@/contentScript/messenger/api"; +import { reloadModsEveryTab } from "@/contentScript/messenger/api"; import { type ModComponentState } from "@/store/extensionsTypes"; import reportError from "@/telemetry/reportError"; import { debounce } from "lodash"; @@ -261,7 +260,9 @@ async function activateMods(modDefinitions: ModDefinition[]): Promise { saveModComponentState(optionsState), saveSidebarState(sidebarState), ]); - await forEachTab(queueReloadFrameMods); + + reloadModsEveryTab(); + return newModConfigs.length > 0; } From 7f0595e443b9c969375666d27ac3b01d7f94e6ec Mon Sep 17 00:00:00 2001 From: Graham Langford <30706330+grahamlangford@users.noreply.github.com> Date: Wed, 3 Jul 2024 13:07:06 -0500 Subject: [PATCH 2/2] documentBuilderBranch -> documentBuilderSubtree (#8746) --- src/bricks/renderers/documentView/DocumentView.tsx | 8 ++++---- .../documentBuilder/documentBuilderTypes.ts | 2 +- src/pageEditor/documentBuilder/documentTree.test.tsx | 4 ++-- src/pageEditor/documentBuilder/documentTree.tsx | 12 ++++++------ .../documentBuilder/render/ListElement.tsx | 8 ++++---- src/pageEditor/exampleStarterBrickConfigs.ts | 10 +++++----- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/bricks/renderers/documentView/DocumentView.tsx b/src/bricks/renderers/documentView/DocumentView.tsx index c9705a5a75..a680a692d0 100644 --- a/src/bricks/renderers/documentView/DocumentView.tsx +++ b/src/bricks/renderers/documentView/DocumentView.tsx @@ -17,7 +17,7 @@ import "@/vendors/bootstrapWithoutRem.css"; import "@/sidebar/sidebarBootstrapOverrides.scss"; -import { buildDocumentBuilderBranch } from "@/pageEditor/documentBuilder/documentTree"; +import { buildDocumentBuilderSubtree } from "@/pageEditor/documentBuilder/documentTree"; import React from "react"; import { type DocumentViewProps } from "./DocumentViewProps"; import DocumentContext from "@/pageEditor/documentBuilder/render/DocumentContext"; @@ -56,7 +56,7 @@ const DocumentView: React.FC = ({ {body.map((documentElement, index) => { - const documentBuilderBranch = buildDocumentBuilderBranch( + const documentBuilderSubtree = buildDocumentBuilderSubtree( documentElement, { staticId: joinPathParts("body", "children"), @@ -65,11 +65,11 @@ const DocumentView: React.FC = ({ }, ); - if (documentBuilderBranch == null) { + if (documentBuilderSubtree == null) { return null; } - const { Component, props } = documentBuilderBranch; + const { Component, props } = documentBuilderSubtree; // eslint-disable-next-line react/no-array-index-key -- They have no other unique identifier return ; })} diff --git a/src/pageEditor/documentBuilder/documentBuilderTypes.ts b/src/pageEditor/documentBuilder/documentBuilderTypes.ts index c45cc47a7c..a2e0ca27ab 100644 --- a/src/pageEditor/documentBuilder/documentBuilderTypes.ts +++ b/src/pageEditor/documentBuilder/documentBuilderTypes.ts @@ -145,7 +145,7 @@ export type DynamicPath = { }>; }; -export type BuildDocumentBuilderBranch = ( +export type BuildDocumentBuilderSubtree = ( root: DocumentBuilderElement, tracePath: DynamicPath, ) => DocumentBuilderComponent | null; diff --git a/src/pageEditor/documentBuilder/documentTree.test.tsx b/src/pageEditor/documentBuilder/documentTree.test.tsx index f6aa26a75f..e777246843 100644 --- a/src/pageEditor/documentBuilder/documentTree.test.tsx +++ b/src/pageEditor/documentBuilder/documentTree.test.tsx @@ -23,7 +23,7 @@ import blockRegistry from "@/bricks/registry"; import MarkdownRenderer from "@/bricks/renderers/MarkdownRenderer"; import * as contentScriptAPI from "@/contentScript/messenger/api"; import { uuidv4 } from "@/types/helpers"; -import { buildDocumentBuilderBranch } from "./documentTree"; +import { buildDocumentBuilderSubtree } from "./documentTree"; import { type DocumentBuilderElement, type DocumentBuilderElementType, @@ -46,7 +46,7 @@ describe("When rendered in panel", () => { }); const renderDocument = (config: DocumentBuilderElement) => { - const branch = buildDocumentBuilderBranch(config, { + const branch = buildDocumentBuilderSubtree(config, { staticId: "body", branches: [], }); diff --git a/src/pageEditor/documentBuilder/documentTree.tsx b/src/pageEditor/documentBuilder/documentTree.tsx index d327b357fa..bc167765c4 100644 --- a/src/pageEditor/documentBuilder/documentTree.tsx +++ b/src/pageEditor/documentBuilder/documentTree.tsx @@ -20,7 +20,7 @@ import BlockElement from "@/pageEditor/documentBuilder/render/BlockElement"; import { get } from "lodash"; import { Col, Container, Row, Image } from "react-bootstrap"; import { - type BuildDocumentBuilderBranch, + type BuildDocumentBuilderSubtree, type ButtonElementConfig, type DocumentBuilderComponent, type DocumentBuilderElement, @@ -59,7 +59,7 @@ const UnknownType: React.FC<{ componentType: string }> = ({
Unknown component type: {componentType}
); -export const buildDocumentBuilderBranch: BuildDocumentBuilderBranch = ( +export const buildDocumentBuilderSubtree: BuildDocumentBuilderSubtree = ( root, tracePath, ) => { @@ -84,16 +84,16 @@ export const buildDocumentBuilderBranch: BuildDocumentBuilderBranch = ( ) { documentBuilderComponent.props.children = root.children.map( (child, index) => { - const branch = buildDocumentBuilderBranch(child, { + const subtree = buildDocumentBuilderSubtree(child, { staticId: joinPathParts(staticId, root.type, "children"), branches: [...branches, { staticId, index }], }); - if (branch == null) { + if (subtree == null) { return null; } - const { Component, props } = branch; + const { Component, props } = subtree; // eslint-disable-next-line react/no-array-index-key -- They have no other unique identifier return ; }, @@ -265,7 +265,7 @@ export function getDocumentBuilderComponent( elementKey: config.elementKey, config: config.element, tracePath, - buildDocumentBuilderBranch, + buildDocumentBuilderSubtree, }; return { diff --git a/src/pageEditor/documentBuilder/render/ListElement.tsx b/src/pageEditor/documentBuilder/render/ListElement.tsx index 5828cb9749..913d6182f8 100644 --- a/src/pageEditor/documentBuilder/render/ListElement.tsx +++ b/src/pageEditor/documentBuilder/render/ListElement.tsx @@ -20,7 +20,7 @@ import DocumentContext from "./DocumentContext"; import { type Args } from "@/runtime/mapArgs"; import Loader from "@/components/Loader"; import { - type BuildDocumentBuilderBranch, + type BuildDocumentBuilderSubtree, type DocumentBuilderElement, type DynamicPath, } from "@/pageEditor/documentBuilder/documentBuilderTypes"; @@ -41,7 +41,7 @@ type DocumentListProps = { array: UnknownObject[]; elementKey?: string; config: Args; - buildDocumentBuilderBranch: BuildDocumentBuilderBranch; + buildDocumentBuilderSubtree: BuildDocumentBuilderSubtree; tracePath: DynamicPath; }; @@ -51,7 +51,7 @@ const ListElementInternal: React.FC = ({ array = DEFAULT_ARRAY, elementKey, config, - buildDocumentBuilderBranch, + buildDocumentBuilderSubtree, tracePath, }) => { const { staticId, branches } = tracePath; @@ -138,7 +138,7 @@ const ListElementInternal: React.FC = ({ <> {rootDefinitions?.map(({ documentElement, elementContext }, index) => { const { Component, props } = - buildDocumentBuilderBranch( + buildDocumentBuilderSubtree( documentElement as DocumentBuilderElement, { staticId: joinPathParts(staticId, "list", "children"), diff --git a/src/pageEditor/exampleStarterBrickConfigs.ts b/src/pageEditor/exampleStarterBrickConfigs.ts index ae4476e35f..a73a3742c2 100644 --- a/src/pageEditor/exampleStarterBrickConfigs.ts +++ b/src/pageEditor/exampleStarterBrickConfigs.ts @@ -26,17 +26,17 @@ const quickbarActionId = validateRegistryId("@pixiebrix/quickbar/add"); export function getExampleBrickPipeline(type: StarterBrickType): BrickPipeline { if (type === "actionPanel") { - const documentBuilderBlock = createNewConfiguredBrick(documentBrickId); - return [documentBuilderBlock]; + const documentBuilderBrick = createNewConfiguredBrick(documentBrickId); + return [documentBuilderBrick]; } if (type === "quickBarProvider") { - const quickbarActionBlock = createNewConfiguredBrick(quickbarActionId); - quickbarActionBlock.config = { + const quickbarActionBrick = createNewConfiguredBrick(quickbarActionId); + quickbarActionBrick.config = { title: "Example Action", action: toExpression("pipeline", []), }; - return [quickbarActionBlock]; + return [quickbarActionBrick]; } return [];