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 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
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 { useAppState } from "../../store/appState";
|
||||||
import type { AppSettings, CatalogItem, MarketplaceSnapshot } from "../../lib/types";
|
import type { AppSettings, CatalogItem, MarketplaceSnapshot } from "../../lib/types";
|
||||||
import type { MarketplaceApi } from "../../hooks/useMarketplace";
|
import type { MarketplaceApi } from "../../hooks/useMarketplace";
|
||||||
@@ -8,6 +8,10 @@ vi.mock("./InstallControls", () => ({
|
|||||||
default: ({ headCommit }: { headCommit: string | null }) => <div>install controls at {headCommit}</div>,
|
default: ({ headCommit }: { headCommit: string | null }) => <div>install controls at {headCommit}</div>,
|
||||||
}));
|
}));
|
||||||
vi.mock("./AddMarketplaceModal", () => ({ default: () => <div>add modal</div> }));
|
vi.mock("./AddMarketplaceModal", () => ({ default: () => <div>add modal</div> }));
|
||||||
|
const updateMarketplace = vi.fn();
|
||||||
|
vi.mock("../../lib/tauri-commands", () => ({
|
||||||
|
updateMarketplace: (m: unknown) => updateMarketplace(m),
|
||||||
|
}));
|
||||||
|
|
||||||
import BrowsePane from "./BrowsePane";
|
import BrowsePane from "./BrowsePane";
|
||||||
|
|
||||||
@@ -96,6 +100,43 @@ describe("BrowsePane", () => {
|
|||||||
expect(screen.getByText("SKILL.md missing")).toBeInTheDocument();
|
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(<BrowsePane mp={mp} />);
|
||||||
|
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(<BrowsePane mp={mp} />);
|
||||||
|
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", () => {
|
it("refreshes one marketplace", () => {
|
||||||
const mp = api();
|
const mp = api();
|
||||||
render(<BrowsePane mp={mp} />);
|
render(<BrowsePane mp={mp} />);
|
||||||
|
|||||||
@@ -15,6 +15,10 @@ type KindFilter = ItemKind | "all";
|
|||||||
|
|
||||||
const when = (iso: string | null) => (iso ? new Date(iso).toLocaleString() : "never");
|
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 }) {
|
export default function BrowsePane({ mp }: { mp: MarketplaceApi }) {
|
||||||
const marketplaces = useAppState((s) => s.appSettings?.marketplaces ?? []);
|
const marketplaces = useAppState((s) => s.appSettings?.marketplaces ?? []);
|
||||||
const accounts = useAppState((s) => s.appSettings?.marketplace_accounts ?? []);
|
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 projects = useAppState((s) => s.projects);
|
||||||
const filterId = useAppState((s) => s.marketplaceFilterProjectId);
|
const filterId = useAppState((s) => s.marketplaceFilterProjectId);
|
||||||
const setFilterId = useAppState((s) => s.setMarketplaceFilterProjectId);
|
const setFilterId = useAppState((s) => s.setMarketplaceFilterProjectId);
|
||||||
|
const pushToast = useAppState((s) => s.pushToast);
|
||||||
const [kind, setKind] = useState<KindFilter>("all");
|
const [kind, setKind] = useState<KindFilter>("all");
|
||||||
const [query, setQuery] = useState("");
|
const [query, setQuery] = useState("");
|
||||||
// The item is kept as it was read, with the head it was read at: an
|
// 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;
|
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) => {
|
const changeAccount = async (m: Marketplace, accountId: string | null) => {
|
||||||
await updateMarketplace({ ...m, account_id: accountId });
|
try {
|
||||||
await mp.reloadState();
|
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. */
|
/** Global + every project's installs of this marketplace, for the removal warning. */
|
||||||
|
|||||||
Reference in New Issue
Block a user