From fe15f541b9d19a46750f7cdd1941af2b9fc17383 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 27 Sep 2026 13:13:11 -0700 Subject: [PATCH] Marketplace: toast a refused account change and refresh after a saved one (PR review #7) changeAccount now catches a rejected updateMarketplace (e.g. an account for another host) and toasts it instead of leaving an unhandled rejection, and refreshes the marketplace after a successful change so the old fetch error is replaced. Co-Authored-By: Claude Opus 5.5 --- .../marketplace/BrowsePane.test.tsx | 43 ++++++++++++++++++- app/src/components/marketplace/BrowsePane.tsx | 21 ++++++++- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/app/src/components/marketplace/BrowsePane.test.tsx b/app/src/components/marketplace/BrowsePane.test.tsx index e52ce8f..1287166 100644 --- a/app/src/components/marketplace/BrowsePane.test.tsx +++ b/app/src/components/marketplace/BrowsePane.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { useAppState } from "../../store/appState"; import type { AppSettings, CatalogItem, MarketplaceSnapshot } from "../../lib/types"; import type { MarketplaceApi } from "../../hooks/useMarketplace"; @@ -8,6 +8,10 @@ vi.mock("./InstallControls", () => ({ default: ({ headCommit }: { headCommit: string | null }) =>
install controls at {headCommit}
, })); vi.mock("./AddMarketplaceModal", () => ({ default: () =>
add modal
})); +const updateMarketplace = vi.fn(); +vi.mock("../../lib/tauri-commands", () => ({ + updateMarketplace: (m: unknown) => updateMarketplace(m), +})); import BrowsePane from "./BrowsePane"; @@ -96,6 +100,43 @@ describe("BrowsePane", () => { expect(screen.getByText("SKILL.md missing")).toBeInTheDocument(); }); + describe("PR review #7: changing a marketplace's account", () => { + const withAccount = () => + useAppState.setState({ + toasts: [], + appSettings: { + marketplaces: [{ id: "m1", name: "Starter", url: "https://github.com/s/m.git", branch: null, account_id: null }], + marketplace_accounts: [{ id: "a1", label: "Work", host: "gitlab.com", method: "token", username: null }], + global_marketplace_installs: [], + } as unknown as AppSettings, + }); + + it("toasts a refused change instead of leaving it unhandled", async () => { + withAccount(); + updateMarketplace.mockRejectedValueOnce("The account \"Work\" is for gitlab.com, but this marketplace is on github.com."); + const mp = api(); + render(); + fireEvent.change(screen.getByLabelText("Account for Starter"), { target: { value: "a1" } }); + await waitFor(() => expect(useAppState.getState().toasts).toHaveLength(1)); + const toast = useAppState.getState().toasts[0]; + expect(toast.kind).toBe("error"); + expect(toast.detail).toContain("is for gitlab.com"); + expect(mp.refresh).not.toHaveBeenCalled(); + }); + + it("refreshes the marketplace after a successful change", async () => { + withAccount(); + updateMarketplace.mockResolvedValueOnce({}); + const mp = api(); + render(); + fireEvent.change(screen.getByLabelText("Account for Starter"), { target: { value: "a1" } }); + await waitFor(() => expect(mp.refresh).toHaveBeenCalledWith("m1")); + expect(updateMarketplace).toHaveBeenCalledWith(expect.objectContaining({ id: "m1", account_id: "a1" })); + expect(mp.reloadState).toHaveBeenCalled(); + expect(useAppState.getState().toasts).toHaveLength(0); + }); + }); + it("refreshes one marketplace", () => { const mp = api(); render(); diff --git a/app/src/components/marketplace/BrowsePane.tsx b/app/src/components/marketplace/BrowsePane.tsx index 3f1edef..9d76f80 100644 --- a/app/src/components/marketplace/BrowsePane.tsx +++ b/app/src/components/marketplace/BrowsePane.tsx @@ -15,6 +15,10 @@ type KindFilter = ItemKind | "all"; const when = (iso: string | null) => (iso ? new Date(iso).toLocaleString() : "never"); +function errorText(e: unknown): string { + return typeof e === "string" ? e : e instanceof Error ? e.message : String(e); +} + export default function BrowsePane({ mp }: { mp: MarketplaceApi }) { const marketplaces = useAppState((s) => s.appSettings?.marketplaces ?? []); const accounts = useAppState((s) => s.appSettings?.marketplace_accounts ?? []); @@ -22,6 +26,7 @@ export default function BrowsePane({ mp }: { mp: MarketplaceApi }) { const projects = useAppState((s) => s.projects); const filterId = useAppState((s) => s.marketplaceFilterProjectId); const setFilterId = useAppState((s) => s.setMarketplaceFilterProjectId); + const pushToast = useAppState((s) => s.pushToast); const [kind, setKind] = useState("all"); const [query, setQuery] = useState(""); // The item is kept as it was read, with the head it was read at: an @@ -47,9 +52,21 @@ export default function BrowsePane({ mp }: { mp: MarketplaceApi }) { const nameOf = (id: string) => marketplaces.find((m) => m.id === id)?.name ?? id; + /** A refused change (e.g. an account for another host) is toasted; a saved + * one is fetched with the new account so its old fetch error goes away. */ const changeAccount = async (m: Marketplace, accountId: string | null) => { - await updateMarketplace({ ...m, account_id: accountId }); - await mp.reloadState(); + try { + await updateMarketplace({ ...m, account_id: accountId }); + } catch (e) { + pushToast({ kind: "error", message: `Could not change the account for ${m.name}`, detail: errorText(e) }); + return; + } + try { + await mp.reloadState(); + } catch (e) { + console.error("Failed to reload after changing a marketplace account:", e); + } + await mp.refresh(m.id); }; /** Global + every project's installs of this marketplace, for the removal warning. */