Skip to content

Commit 80f7e87

Browse files
committed
fix(ui): restore the two rebrand legacy localStorage keys (#7782)
#5405 gave useLocalStorage a legacyKey param so a pre-rebrand gittensory_* value is read once and migrated forward to the loopover_* key. Two days later #5743's blanket gittensory->loopover substitution rewrote two string literals it should not have, leaving legacyKey identical to the current key: - onboarding-preview-card.tsx's LEGACY_DISMISS_KEY - notification-readiness-card.tsx's notification opt-in legacy key With both keys equal, useLocalStorage's fallback re-reads the same key it just missed, so a maintainer who dismissed the onboarding card or opted into notifications before the rebrand silently loses that preference. Restores both literals to their pre-rebrand gittensory_* values, matching the uncorrupted shape still present in api/try-it.tsx. Every other rebrand-migrated legacy key was already distinct and is left untouched. Adds a regression test per component asserting a value stored only under the legacy key is read AND written forward to the current key; both fail against the pre-fix literals and pass after.
1 parent 17ae3e6 commit 80f7e87

4 files changed

Lines changed: 48 additions & 3 deletions

File tree

apps/loopover-ui/src/components/site/app-panels/onboarding-preview-card.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,20 @@ describe("OnboardingPreviewCard", () => {
145145
expect(screen.queryByText(/Here's what LoopOver would have flagged/)).toBeNull();
146146
expect(apiFetch).not.toHaveBeenCalled();
147147
});
148+
149+
it("honors a pre-rebrand dismissal stored under the legacy key and migrates it forward (#7782)", () => {
150+
apiFetch.mockResolvedValue({ ok: true, data: preview() });
151+
const dismissed = JSON.stringify({ dismissed: true });
152+
window.localStorage.setItem("gittensory_maintainer_onboarding_preview_dismissed", dismissed);
153+
154+
render(<OnboardingPreviewCard reviewability={REVIEWABILITY} />);
155+
156+
// The card stays dismissed for a maintainer who dismissed it before the rebrand...
157+
expect(screen.queryByText(/Here's what LoopOver would have flagged/)).toBeNull();
158+
expect(apiFetch).not.toHaveBeenCalled();
159+
// ...and the legacy value is written forward so later reads hit the current key directly.
160+
expect(window.localStorage.getItem("loopover_maintainer_onboarding_preview_dismissed")).toBe(
161+
dismissed,
162+
);
163+
});
148164
});

apps/loopover-ui/src/components/site/app-panels/onboarding-preview-card.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type ReviewabilityRow = { pr: string; title: string; reason: string };
1919

2020
const DISMISS_KEY = "loopover_maintainer_onboarding_preview_dismissed";
2121
// One-time rebrand migration fallback -- see useLocalStorage's legacyKey param.
22-
const LEGACY_DISMISS_KEY = "loopover_maintainer_onboarding_preview_dismissed";
22+
const LEGACY_DISMISS_KEY = "gittensory_maintainer_onboarding_preview_dismissed";
2323

2424
/** Builds a settings-preview form from a REAL cached PR (title, and a linked-issue number scraped from
2525
* `reason` when present) — everything else (author identity, labels, body) isn't in the reviewability

apps/loopover-ui/src/components/site/notification-readiness-card.test.tsx

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { fireEvent, render, screen } from "@testing-library/react";
2-
import { describe, expect, it, vi } from "vitest";
2+
import { beforeEach, describe, expect, it, vi } from "vitest";
33

44
// #6985: a real fetch failure used to render the same generic text as "still loading" — these tests
55
// pin the three render paths (loading / error / success) now that LoadingState/ErrorState replace it.
@@ -95,3 +95,31 @@ describe("NotificationReadinessCard loading/error states (#6985)", () => {
9595
expect(screen.queryByText("Loading notification model…")).toBeNull();
9696
});
9797
});
98+
99+
describe("NotificationReadinessCard legacy opt-in migration (#7782)", () => {
100+
beforeEach(() => {
101+
window.localStorage.clear();
102+
useApiResource.mockReturnValue({
103+
status: "ready",
104+
data: notificationModelFixture,
105+
error: null,
106+
loadedAt: Date.now(),
107+
reload: () => {},
108+
});
109+
});
110+
111+
it("reads a pre-rebrand opt-in stored under the legacy key and migrates it forward", () => {
112+
window.localStorage.setItem("gittensory_notification_opt_in", "true");
113+
114+
render(<NotificationReadinessCard />);
115+
116+
expect(screen.getByText("opt-in enabled")).toBeTruthy();
117+
expect(window.localStorage.getItem("loopover_notification_opt_in")).toBe("true");
118+
});
119+
120+
it("stays opted out when neither the current nor the legacy key is set", () => {
121+
render(<NotificationReadinessCard />);
122+
123+
expect(screen.getByText("opt-in required")).toBeTruthy();
124+
});
125+
});

apps/loopover-ui/src/components/site/notification-readiness-card.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ export function NotificationReadinessCard() {
3333
const [optIn, setOptIn] = useLocalStorage<boolean>(
3434
"loopover_notification_opt_in",
3535
false,
36-
"loopover_notification_opt_in",
36+
// One-time rebrand migration fallback -- see useLocalStorage's legacyKey param.
37+
"gittensory_notification_opt_in",
3738
);
3839
const [busy, setBusy] = useState(false);
3940

0 commit comments

Comments
 (0)