Skip to content

refactor(deployment): send deployment names to the api instead of localstorage #3928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stalniy wants to merge 1 commit into from
Open

refactor(deployment): send deployment names to the api instead of localstorage #3928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 8 additions & 29 deletions apps/deploy-web/src/components/LocalNoteManager/DeploymentNameModal.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { createStore, Provider as JotaiStoreProvider } from "jotai";
import { describe, expect, it, vi } from "vitest";
import type { MockProxy } from "vitest-mock-extended";
import { mock, mockDeep } from "vitest-mock-extended";

import { MAX_DEPLOYMENT_NAME_LENGTH } from "@src/config/deploy.config";
import type { AppDIContainer } from "@src/context/ServicesProvider/ServicesProvider";
import type { DeploymentStorageService } from "@src/services/deployment-storage/deployment-storage.service";
import { settingsIdAtom } from "@src/store/settingsStore";
import type { DEPENDENCIES } from "./DeploymentNameModal";
import { DeploymentNameModal } from "./DeploymentNameModal";

Expand All @@ -31,12 +28,13 @@ describe("DeploymentNameModal", () => {
await waitFor(() => expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: api.v1.getDeployment.getKey({ dseq: "12345" }) }));
});

it("records the new name in this browser too, which the deployments list still reads", async () => {
const { deploymentLocalStorage } = setup({});
it("records nothing in this browser, because the api is now where the name lives", async () => {
const { deploymentLocalStorage, onSaved } = setup({});

await rename("my-app");

await waitFor(() => expect(deploymentLocalStorage.update).toHaveBeenCalledWith("akash1abc", "12345", { name: "my-app" }));
await waitFor(() => expect(onSaved).toHaveBeenCalled());
expect(deploymentLocalStorage.update).not.toHaveBeenCalled();
});

it("reports the rename as saved once the api accepted it", async () => {
Expand Down Expand Up @@ -85,19 +83,6 @@ describe("DeploymentNameModal", () => {
expect(patchMutate).not.toHaveBeenCalled();
});

it("still refreshes and closes when this browser cannot record the new name", async () => {
const deploymentLocalStorage = mock<DeploymentStorageService>();
deploymentLocalStorage.update.mockImplementation(() => {
throw new Error("QuotaExceededError");
});
const { queryClient, onSaved } = setup({ deploymentLocalStorage });

await rename("my-app");

await waitFor(() => expect(onSaved).toHaveBeenCalled());
expect(queryClient.invalidateQueries).toHaveBeenCalled();
});

it.each(["", " "])("refuses a name of %p, which the api rejects rather than reading as unnamed", async typed => {
const { patchMutate, deploymentLocalStorage, onSaved } = setup({ resolvedName: "old-name" });

Expand Down Expand Up @@ -175,7 +160,6 @@ describe("DeploymentNameModal", () => {
resolvedName?: string;
patchMutate?: ReturnType<typeof vi.fn>;
isPending?: boolean;
deploymentLocalStorage?: MockProxy<DeploymentStorageService>;
}) {
const dseq = input.dseq === undefined ? "12345" : input.dseq;
const patchMutate = input.patchMutate ?? vi.fn((_variables, options) => options?.onSuccess?.());
Expand All @@ -186,7 +170,7 @@ describe("DeploymentNameModal", () => {
mock<ReturnType<typeof api.v1.patchDeployment.useMutation>>({ mutate: patchMutate as never, isPending: input.isPending ?? false })
);

const deploymentLocalStorage = input.deploymentLocalStorage ?? mock<DeploymentStorageService>();
const deploymentLocalStorage = mock<DeploymentStorageService>();
const queryClient = mock<ReturnType<typeof DEPENDENCIES.useQueryClient>>();
const enqueueSnackbar = vi.fn();
const onSaved = vi.fn();
Expand All @@ -198,15 +182,10 @@ describe("DeploymentNameModal", () => {
useResolvedDeploymentName: () => resolvedName
};

const store = createStore();
store.set(settingsIdAtom, "akash1abc");

const modalFor = (shownDseq: string | number | null) => (
<JotaiStoreProvider store={store}>
<TestContainerProvider services={{ api: () => api, deploymentLocalStorage: () => deploymentLocalStorage }}>
<DeploymentNameModal dseq={shownDseq} onClose={onClose} onSaved={onSaved} dependencies={dependencies} />
</TestContainerProvider>
</JotaiStoreProvider>
<TestContainerProvider services={{ api: () => api, deploymentLocalStorage: () => deploymentLocalStorage }}>
<DeploymentNameModal dseq={shownDseq} onClose={onClose} onSaved={onSaved} dependencies={dependencies} />
</TestContainerProvider>
);
const { rerender } = render(modalFor(dseq));

Expand Down
17 changes: 2 additions & 15 deletions apps/deploy-web/src/components/LocalNoteManager/DeploymentNameModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ import { useForm } from "react-hook-form";
import { Form, FormField, FormInput, Popup, Snackbar } from "@akashnetwork/ui/components";
import { zodResolver } from "@hookform/resolvers/zod";
import { useQueryClient } from "@tanstack/react-query";
import { useAtom } from "jotai";
import { useSnackbar } from "notistack";
import { z } from "zod";

import { MAX_DEPLOYMENT_NAME_LENGTH } from "@src/config/deploy.config";
import { useServices } from "@src/context/ServicesProvider";
import { useResolvedDeploymentName } from "@src/hooks/useResolvedDeploymentName/useResolvedDeploymentName";
import { settingsIdAtom } from "@src/store/settingsStore";

export const DEPENDENCIES = { useSnackbar, useQueryClient, useResolvedDeploymentName };

Expand All @@ -31,8 +29,7 @@ type Props = {
};

export const DeploymentNameModal: React.FC<Props> = ({ dseq, onClose, onSaved, dependencies: d = DEPENDENCIES }) => {
const { api, deploymentLocalStorage } = useServices();
const [address] = useAtom(settingsIdAtom);
const { api } = useServices();
const formRef = useRef<HTMLFormElement | null>(null);
const { enqueueSnackbar } = d.useSnackbar();
const queryClient = d.useQueryClient();
Expand Down Expand Up @@ -70,23 +67,13 @@ export const DeploymentNameModal: React.FC<Props> = ({ dseq, onClose, onSaved, d
formRef.current?.dispatchEvent(new Event("submit", { cancelable: true, bubbles: true }));
};

/** The deployments list still resolves names from this browser alone, so the record is kept in step until it reads the api too — and a full or blocked store must not strand a rename the api has already accepted. */
function recordNameInThisBrowser(name: string) {
try {
deploymentLocalStorage.update(address, dseq, { name });
} catch {
return;
}
}

function onSubmit({ name }: z.infer<typeof formSchema>) {
if (!dseq || renameDeployment.isPending) return;

renameDeployment.mutate(
{ dseq: String(dseq), data: { name } },
{
onSuccess: function recordRename() {
recordNameInThisBrowser(name);
onSuccess: function reportRenameSaved() {
queryClient.invalidateQueries({ queryKey: api.v1.getDeployment.getKey({ dseq: String(dseq) }) });
enqueueSnackbar(<Snackbar title="Success!" iconVariant="success" />, { variant: "success", autoHideDuration: 1000 });
onSaved();
Comment on lines +76 to 79

@claude claude Bot Sep 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Removing the localStorage mirror write on rename means any deployment that already has a name cached in this browser (named before this change, or by an earlier session) keeps its OLD name there forever after a rename — worse than the PR's documented 'shows no name' case, since the 5 surfaces reading only local storage (deployment list, home, alerts, billing, provider leases via useLocalNotes.getDeploymentName) now display a stale, incorrect name instead of nothing. Fix: either have those 5 surfaces read the API-resolved name (per the plan's slice 3) before this write is removed, or invalidate/clear the local record on successful rename so it falls back to null rather than a wrong value.

Extended reasoning...

User has an old deployment whose local record already has name:"old-name" (written by pre-slice-2 code). They open DeploymentNameModal, PATCH succeeds renaming it to "new-name" on the API; onSuccess (reportRenameSaved, lines 76-80) now only invalidates the getDeployment query and calls onSaved — it never touches deploymentLocalStorage. The local record's name field stays "old-name". DeploymentList.tsx, HomeContainer.tsx, AlertsListContainer.tsx, useAccountBalanceOverview.ts and LeaseRow.tsx all call useLocalNotes().getDeploymentName(dseq), which reads deploymentLocalStorage directly (useLocalNotes.ts lines 25-31) with no API fallback, so they keep showing "old-name" indefinitely even though the API and the rename dialog itself (via useResolvedDeploymentName) now show "new-name". The PR's own claim 'nothing here deletes them' undersells this: the record isn't deleted, but it silently diverges from the truth after every rename, which is a worse UX than the missing-name case the PR explicitly accepts.

Verification: normal — acknowledged in diff, but the note's claim does not hold for this subset. On the base branch, DeploymentNameModal's onSuccess ran recordNameInThisBrowser(name) -> deploymentLocalStorage.update(address, dseq, { name }), so renaming a deployment that already had a local record kept that record in sync; the five surfaces reading useLocalNotes.getDeploymentName (useLocalNotes.ts:27-28,…

Expand Down
10 changes: 10 additions & 0 deletions ...c/components/deployments/ConfigureDeployment/useDeploymentFlow/useDeploymentFlow.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,16 @@ describe(useDeploymentFlow.name, () => {
}
});

it("records no name in this browser, because the create request is what carries it", () => {
const createDeployment = mockMutation();
createDeployment.mutate.mockImplementation((_i, o) => o.onSuccess({ data: { dseq: "555", manifest: "M" } }));
const { result, deploymentLocalStorage } = renderFlow({ createDeployment });

act(() => result.current.actions.requestQuotes("SDL_AT_CREATE", "my-app"));

expect(deploymentLocalStorage.update).not.toHaveBeenCalledWith(expect.anything(), expect.anything(), expect.objectContaining({ name: expect.anything() }));
});

it("caches the created SDL by the settings id and dseq at create time so an in-progress deployment can be resumed after a reload", () => {
const createDeployment = mockMutation();
createDeployment.mutate.mockImplementation((_i, o) => o.onSuccess({ data: { dseq: "555", manifest: "M" } }));
Expand Down
56 changes: 4 additions & 52 deletions ...c/components/deployments/ConfigureDeployment/useDeploymentName/useDeploymentName.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import type { PropsWithChildren } from "react";
import { createStore, Provider as JotaiStoreProvider } from "jotai";
import { describe, expect, it } from "vitest";
import { mock } from "vitest-mock-extended";

import { MAX_DEPLOYMENT_NAME_LENGTH } from "@src/config/deploy.config";
import type { DeploymentStorageService } from "@src/services/deployment-storage/deployment-storage.service";
import { settingsIdAtom } from "@src/store/settingsStore";
import type { DEPENDENCIES } from "./useDeploymentName";
import { useDeploymentName } from "./useDeploymentName";

Expand Down Expand Up @@ -73,54 +68,11 @@ describe(useDeploymentName.name, () => {
expect(result.current.name).toBe("renamed");
});

it("writes the name to the settings-scoped record when a dseq is first assigned", () => {
const { rerender, deploymentLocalStorage } = setup({ initialName: "my-app", dseq: null, settingsId: "akash1abc" });

rerender({ initialName: "my-app", dseq: "12345" });

expect(deploymentLocalStorage.update).toHaveBeenCalledWith("akash1abc", "12345", { name: "my-app" });
});

it("does not write before a dseq exists", () => {
const { deploymentLocalStorage } = setup({ initialName: "my-app", dseq: null });

expect(deploymentLocalStorage.update).not.toHaveBeenCalled();
});

it("does not write when the session resumed already carrying a dseq", () => {
const { deploymentLocalStorage } = setup({ initialName: "my-app", dseq: "12345" });

expect(deploymentLocalStorage.update).not.toHaveBeenCalled();
});

it("defers the write until settingsId is available instead of dropping it", () => {
const { rerender, store, deploymentLocalStorage } = setup({ initialName: "my-app", dseq: null, settingsId: null });

rerender({ initialName: "my-app", dseq: "12345" });
expect(deploymentLocalStorage.update).not.toHaveBeenCalled();

act(() => store.set(settingsIdAtom, "akash1abc"));

expect(deploymentLocalStorage.update).toHaveBeenCalledWith("akash1abc", "12345", { name: "my-app" });
});

function setup(input: { initialName?: string; dseq?: string | null; settingsId?: string | null; apiName?: string }) {
const deploymentLocalStorage = mock<DeploymentStorageService>();
const useServices: typeof DEPENDENCIES.useServices = () => mock<ReturnType<typeof DEPENDENCIES.useServices>>({ deploymentLocalStorage });
function setup(input: { initialName?: string; dseq?: string | null; apiName?: string }) {
const useResolvedDeploymentName: typeof DEPENDENCIES.useResolvedDeploymentName = dseq => (dseq ? input.apiName : undefined);

const store = createStore();
store.set(settingsIdAtom, input.settingsId ?? null);
const wrapper = ({ children }: PropsWithChildren) => <JotaiStoreProvider store={store}>{children}</JotaiStoreProvider>;
const initialProps = { initialName: input.initialName, dseq: input.dseq ?? null };

return {
...renderHook((props: { initialName?: string; dseq: string | null }) => useDeploymentName(props, { useServices, useResolvedDeploymentName }), {
wrapper,
initialProps
}),
deploymentLocalStorage,
store
};
return renderHook((props: { initialName?: string; dseq: string | null }) => useDeploymentName(props, { useResolvedDeploymentName }), {
initialProps: { initialName: input.initialName, dseq: input.dseq ?? null }
});
}
});
24 changes: 3 additions & 21 deletions ...web/src/components/deployments/ConfigureDeployment/useDeploymentName/useDeploymentName.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { useEffect, useRef, useState } from "react";
import { useAtomValue } from "jotai";
import { useState } from "react";

import { MAX_DEPLOYMENT_NAME_LENGTH } from "@src/config/deploy.config";
import { useServices } from "@src/context/ServicesProvider";
import { useResolvedDeploymentName } from "@src/hooks/useResolvedDeploymentName/useResolvedDeploymentName";
import { settingsIdAtom } from "@src/store/settingsStore";

export const DEPENDENCIES = { useServices, useResolvedDeploymentName };
export const DEPENDENCIES = { useResolvedDeploymentName };

export interface DeploymentName {
/** The name to show: the one typed in this session, and the api's own only where this session has none. */
Expand All @@ -23,24 +20,9 @@ interface UseDeploymentNameInput {
dseq: string | null;
}

/** Owns the configure session's deployment name: the api's own once the deployment exists, the typed one before that, and the write of the typed one to the wallet-scoped local record `settingsId` keys. */
/** Owns the configure session's deployment name: the api's own once the deployment exists, and the typed one before that. */
export function useDeploymentName({ initialName, dseq }: UseDeploymentNameInput, dependencies = DEPENDENCIES): DeploymentName {
const { deploymentLocalStorage } = dependencies.useServices();
const settingsId = useAtomValue(settingsIdAtom);
const [typedName, setTypedName] = useState(() => (initialName ?? "").slice(0, MAX_DEPLOYMENT_NAME_LENGTH));
const nameRef = useRef(typedName);
nameRef.current = typedName;
const resolvedName = dependencies.useResolvedDeploymentName(dseq);
/** Seeded with the mounting `dseq`, so a session resumed already carrying one is treated as written and never clobbers a name edited since on the deployment page. */
const writtenDseqRef = useRef(dseq);
useEffect(
function persistNameOnCreate() {
if (dseq && settingsId && dseq !== writtenDseqRef.current) {
writtenDseqRef.current = dseq;
deploymentLocalStorage.update(settingsId, dseq, { name: nameRef.current });
}
},
[dseq, settingsId, deploymentLocalStorage]
);
return { name: typedName || resolvedName || "", typedName, setName: setTypedName };
}