From e343d6a02a59e8990ac429b5fe24deb7434c6d0c Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Sun, 30 Jun 2024 10:19:35 -0400 Subject: [PATCH 1/3] Prevent retired starter bricks from crashing mods screen --- src/mods/useMods.test.ts | 73 ++++++++++++++++-- src/mods/useMods.ts | 160 ++++++++++++++++++++++++--------------- 2 files changed, 165 insertions(+), 68 deletions(-) diff --git a/src/mods/useMods.test.ts b/src/mods/useMods.test.ts index c9035d0b33..f3ad25e505 100644 --- a/src/mods/useMods.test.ts +++ b/src/mods/useMods.test.ts @@ -26,11 +26,15 @@ import { standaloneModDefinitionFactory, activatedModComponentFactory, } from "@/testUtils/factories/modComponentFactories"; -import { defaultModDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories"; +import { + defaultModDefinitionFactory, + innerStarterBrickModDefinitionFactory, + starterBrickInnerDefinitionFactory, +} from "@/testUtils/factories/modDefinitionFactories"; import { type ModDefinition } from "@/types/modDefinitionTypes"; import { type UseCachedQueryResult } from "@/types/sliceTypes"; import { metadataFactory } from "@/testUtils/factories/metadataFactory"; -import { DefinitionKinds } from "@/types/registryTypes"; +import { DefinitionKinds, InnerDefinitionRef } from "@/types/registryTypes"; jest.mock("@/modDefinitions/modDefinitionHooks"); @@ -120,7 +124,7 @@ describe("useMods", () => { }); }); - it("handles known recipe", async () => { + it("handles known mod definition", async () => { const metadata = metadataFactory(); useAllModDefinitionsMock.mockReturnValue({ @@ -158,7 +162,7 @@ describe("useMods", () => { expect(wrapper.result.current.mods[0]).not.toHaveProperty("isStub"); }); - it("handles inactive cloud extension", async () => { + it("handles inactive standalone mod component", async () => { appApiMock .onGet("/api/extensions/") .reply(200, [standaloneModDefinitionFactory()]); @@ -178,7 +182,7 @@ describe("useMods", () => { }); }); - it("handles active cloud extension", async () => { + it("handles active standalone mod component", async () => { appApiMock.reset(); const standaloneModDefinition = standaloneModDefinitionFactory(); @@ -207,4 +211,63 @@ describe("useMods", () => { error: undefined, }); }); + + it("handles retired starter brick type with no valid mods", async () => { + appApiMock.reset(); + + const standaloneModDefinition = standaloneModDefinitionFactory(); + standaloneModDefinition.definitions = { + extensionPoint: starterBrickInnerDefinitionFactory(), + }; + ( + standaloneModDefinition.definitions.extensionPoint.definition as any + ).type = "retired"; + standaloneModDefinition.extensionPointId = + "extensionPoint" as InnerDefinitionRef; + + appApiMock.onGet("/api/extensions/").reply(200, [standaloneModDefinition]); + + const wrapper = renderHook(() => useMods(), { + setupRedux(dispatch) {}, + }); + + await wrapper.waitForEffect(); + + expect(wrapper.result.current.mods).toEqual([]); + expect(wrapper.result.current.error).toBeInstanceOf(Error); + }); + + it("handles ignores retired mods", async () => { + appApiMock.reset(); + + const retiredDefinition = standaloneModDefinitionFactory(); + retiredDefinition.definitions = { + extensionPoint: starterBrickInnerDefinitionFactory(), + }; + (retiredDefinition.definitions.extensionPoint.definition as any).type = + "retired"; + retiredDefinition.extensionPointId = "extensionPoint" as InnerDefinitionRef; + + const validDefinition = standaloneModDefinitionFactory(); + + appApiMock + .onGet("/api/extensions/") + .reply(200, [retiredDefinition, validDefinition]); + + const wrapper = renderHook(() => useMods(), { + setupRedux(dispatch) {}, + }); + + await wrapper.waitForEffect(); + + expect(wrapper.result.current).toEqual({ + mods: [ + expect.objectContaining({ + active: false, + extensionPointId: validDefinition.extensionPointId, + }), + ], + error: undefined, + }); + }); }); diff --git a/src/mods/useMods.ts b/src/mods/useMods.ts index 382f79ae94..e36eac48c8 100644 --- a/src/mods/useMods.ts +++ b/src/mods/useMods.ts @@ -28,6 +28,7 @@ import useAsyncState from "@/hooks/useAsyncState"; import { type ModComponentBase } from "@/types/modComponentTypes"; import type { Mod, UnavailableMod } from "@/types/modTypes"; import { DefinitionKinds } from "@/types/registryTypes"; +import { allSettled } from "@/utils/promiseUtils"; type ModsState = { /** @@ -41,7 +42,7 @@ type ModsState = { error: unknown; }; -export function unavailableModFactory( +export function mapModComponentToUnavailableMod( modComponent: ModComponentBase, ): UnavailableMod { return { @@ -59,99 +60,132 @@ export function unavailableModFactory( */ function useMods(): ModsState { const scope = useSelector(selectScope); - const unresolvedExtensions = useSelector(selectActivatedModComponents); + const activatedModComponents = useSelector(selectActivatedModComponents); - const { data: knownRecipes, ...recipesState } = useAllModDefinitions(); + const { data: knownModDefinitions = [], ...modDefinitionsState } = + useAllModDefinitions(); const standaloneModDefinitions = useGetAllStandaloneModDefinitionsQuery(); - const { installedExtensionIds, installedRecipeIds } = useMemo( - () => ({ - installedExtensionIds: new Set( - unresolvedExtensions.map((extension) => extension.id), - ), - installedRecipeIds: new Set( - unresolvedExtensions.map((extension) => extension._recipe?.id), - ), - }), - [unresolvedExtensions], - ); + const { activatedStandaloneModComponentIds, activatedModDefinitionIds } = + useMemo( + () => ({ + activatedStandaloneModComponentIds: new Set( + activatedModComponents.map((x) => x.id), + ), + activatedModDefinitionIds: new Set( + activatedModComponents.map((x) => x._recipe?.id), + ), + }), + [activatedModComponents], + ); - const knownPersonalOrTeamRecipes = useMemo( + const knownPersonalOrTeamModDefinitions = useMemo( () => - (knownRecipes ?? []).filter( - (recipe) => - // Is personal blueprint - recipe.metadata.id.includes(scope) || - // Is blueprint shared with user - recipe.sharing.organizations.length > 0 || - // Is blueprint active, e.g. installed via marketplace - installedRecipeIds.has(recipe.metadata.id), + (knownModDefinitions ?? []).filter( + (modDefinition) => + // Is personal mod + modDefinition.metadata.id.includes(scope) || + // Is mod shared with the current user + modDefinition.sharing.organizations.length > 0 || + // Is mod active, e.g. activated via marketplace + activatedModDefinitionIds.has(modDefinition.metadata.id), ), - [installedRecipeIds, knownRecipes, scope], + [activatedModDefinitionIds, knownModDefinitions, scope], ); - const allExtensions = useMemo(() => { - const inactiveExtensions = + // All known mod components, including activated mods and standalone mod components retrieved from the server. + const knownModComponents = useMemo(() => { + const unactivatedStandaloneModComponents = standaloneModDefinitions.data - ?.filter((x) => !installedExtensionIds.has(x.id)) + ?.filter((x) => !activatedStandaloneModComponentIds.has(x.id)) .map((x) => ({ ...x, active: false })) ?? []; - return [...unresolvedExtensions, ...inactiveExtensions]; + return [...activatedModComponents, ...unactivatedStandaloneModComponents]; }, [ standaloneModDefinitions.data, - installedExtensionIds, - unresolvedExtensions, + activatedStandaloneModComponentIds, + activatedModComponents, ]); - const { data: resolvedExtensions, error: resolveError } = useAsyncState( - async () => - Promise.all( - allExtensions.map(async (extension) => - hydrateModComponentInnerDefinitions(extension), - ), - ), - [allExtensions], - { initialValue: [] }, - ); + const { data: hydratedModComponents = [], error: hydrationError } = + useAsyncState( + async () => { + // Hydration can fail if we've dropped support for a starter brick type (e.g., tour, inline panel) + const hydrationPromises = await allSettled( + knownModComponents.map(async (x) => { + try { + return await hydrateModComponentInnerDefinitions(x); + } catch (error) { + // Enrich the error with the mod component id to support resolving the issue. E.g., in the case of + // an unsupported starter brick, deleting the standalone mod component from the server + throw new Error(`Error hydrating mod component: ${x.id}`, { + cause: error, + }); + } + }), + { + catch(errors) { + console.warn( + `Failed to hydrate mod ${errors.length} component(s)`, + errors, + ); + }, + }, + ); + + const { fulfilled: hydratedModComponents } = hydrationPromises; + + if ( + knownModComponents.length > 0 && + hydratedModComponents.length === 0 + ) { + throw new Error("Failed to hydrate any mod components"); + } + + return hydratedModComponents; + }, + [knownModComponents], + { initialValue: [] }, + ); - const extensionsWithoutRecipe = useMemo( + const standaloneModComponents = useMemo( () => - // `resolvedExtensions` can be undefined if resolveDefinitions errors above - (resolvedExtensions ?? []).filter((extension) => - extension._recipe?.id - ? !installedRecipeIds.has(extension._recipe?.id) - : true, + hydratedModComponents.filter((x) => + x._recipe?.id ? !activatedModDefinitionIds.has(x._recipe?.id) : true, ), - [installedRecipeIds, resolvedExtensions], + [activatedModDefinitionIds, hydratedModComponents], ); - // Find extensions that were installed by a recipe that's no longer available to the user, e.g., because it was - // deleted, or because the user no longer has access to it. - const unavailableRecipes: UnavailableMod[] = useMemo(() => { - const knownRecipeIds = new Set( - (knownRecipes ?? []).map((x) => x.metadata.id), + // Find mod components that were activated by a mod definitions that's no longer available to the user, e.g., + // because it was deleted, or because the user no longer has access to it. + const unavailableMods: UnavailableMod[] = useMemo(() => { + const knownModDefinitionIds = new Set( + knownModDefinitions.map((x) => x.metadata.id), ); - // `resolvedExtensions` can be undefined if resolveDefinitions errors above - const unavailable = (resolvedExtensions ?? []).filter( - (extension) => - extension._recipe?.id && !knownRecipeIds.has(extension._recipe?.id), + const unavailable = hydratedModComponents.filter( + (modComponent) => + modComponent._recipe?.id && + !knownModDefinitionIds.has(modComponent._recipe?.id), ); - // Show one entry per missing recipe + // Show one entry per missing mod id return uniqBy( - unavailable.map((x) => unavailableModFactory(x)), + unavailable.map((x) => mapModComponentToUnavailableMod(x)), (x) => x.metadata.id, ); - }, [knownRecipes, resolvedExtensions]); + }, [knownModDefinitions, hydratedModComponents]); return { mods: [ - ...extensionsWithoutRecipe, - ...knownPersonalOrTeamRecipes, - ...unavailableRecipes, + ...standaloneModComponents, + ...knownPersonalOrTeamModDefinitions, + ...unavailableMods, ], - error: standaloneModDefinitions.error ?? recipesState.error ?? resolveError, + error: + standaloneModDefinitions.error ?? + modDefinitionsState.error ?? + hydrationError, }; } From ab800bcb2c3e94200af4806228fdf09e207bd1a1 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Sun, 30 Jun 2024 10:48:31 -0400 Subject: [PATCH 2/3] Fix broken import --- src/mods/useModViewItems.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mods/useModViewItems.test.tsx b/src/mods/useModViewItems.test.tsx index 8db3974fa8..90de7560a2 100644 --- a/src/mods/useModViewItems.test.tsx +++ b/src/mods/useModViewItems.test.tsx @@ -24,7 +24,7 @@ import extensionsSlice from "@/store/extensionsSlice"; import MockAdapter from "axios-mock-adapter"; import axios from "axios"; import { type UnavailableMod } from "@/types/modTypes"; -import { unavailableModFactory } from "@/mods/useMods"; +import { mapModComponentToUnavailableMod } from "@/mods/useMods"; import { renderHook } from "@/extensionConsole/testHelpers"; import { modComponentFactory, @@ -65,7 +65,7 @@ describe("useModViewItems", () => { }); }); - it("creates entry for recipe", async () => { + it("creates entry for an undefined mod", async () => { const recipe = defaultModDefinitionFactory(); const activatedModComponent = activatedModComponentFactory({ _recipe: pickModDefinitionMetadata(recipe), @@ -93,17 +93,17 @@ describe("useModViewItems", () => { }); it("creates for unavailable recipe", async () => { - const recipe = defaultModDefinitionFactory(); + const modDefinition = defaultModDefinitionFactory(); const activatedModComponent = activatedModComponentFactory({ - _recipe: pickModDefinitionMetadata(recipe), + _recipe: pickModDefinitionMetadata(modDefinition), }); - const unavailableRecipe: UnavailableMod = unavailableModFactory( + const unavailableMod: UnavailableMod = mapModComponentToUnavailableMod( activatedModComponent, ); const { waitForEffect, result } = renderHook( - () => useModViewItems([unavailableRecipe]), + () => useModViewItems([unavailableMod]), { setupRedux(dispatch) { dispatch( From 4016272df5e49b7624540ca874ada40a34f3a042 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Sun, 30 Jun 2024 10:54:52 -0400 Subject: [PATCH 3/3] Fix lint --- src/mods/useMods.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mods/useMods.test.ts b/src/mods/useMods.test.ts index f3ad25e505..ad7ae69cac 100644 --- a/src/mods/useMods.test.ts +++ b/src/mods/useMods.test.ts @@ -28,13 +28,15 @@ import { } from "@/testUtils/factories/modComponentFactories"; import { defaultModDefinitionFactory, - innerStarterBrickModDefinitionFactory, starterBrickInnerDefinitionFactory, } from "@/testUtils/factories/modDefinitionFactories"; import { type ModDefinition } from "@/types/modDefinitionTypes"; import { type UseCachedQueryResult } from "@/types/sliceTypes"; import { metadataFactory } from "@/testUtils/factories/metadataFactory"; -import { DefinitionKinds, InnerDefinitionRef } from "@/types/registryTypes"; +import { + DefinitionKinds, + type InnerDefinitionRef, +} from "@/types/registryTypes"; jest.mock("@/modDefinitions/modDefinitionHooks");