Skip to content

Commit dcb022d

Browse files
committed
fix(config): prevents settings reset on partial update
1 parent 5a54903 commit dcb022d

File tree

2 files changed

+60
-8
lines changed

2 files changed

+60
-8
lines changed

src/app/stores/config.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,16 @@ export const [config, setConfig] = createStore<Config>(loadConfig());
7272

7373
export function updateConfig(partial: Partial<Config>): void {
7474
const validated = ConfigSchema.partial().safeParse(partial);
75-
if (!validated.success) return; // reject invalid updates
75+
if (!validated.success) return;
76+
// Only merge keys the caller actually provided. ConfigSchema.partial() marks
77+
// all fields optional but Zod still applies per-field .default() values for
78+
// absent keys, so validated.data contains defaults for every omitted field.
79+
// Filtering to keysProvided prevents those defaults from overwriting live state.
80+
const keysProvided = Object.keys(partial) as (keyof Config)[];
81+
const filtered = Object.fromEntries(keysProvided.map((k) => [k, validated.data[k]]));
7682
setConfig(
7783
produce((draft) => {
78-
Object.assign(draft, validated.data);
84+
Object.assign(draft, filtered);
7985
})
8086
);
8187
}

tests/stores/config.test.ts

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ describe("loadConfig", () => {
143143
});
144144
});
145145

146-
// Each updateConfig test creates its own isolated store to avoid shared state
147-
describe("updateConfig", () => {
146+
// Tests SolidJS store merge mechanics in isolation (does NOT exercise the real
147+
// updateConfig export — see "updateConfig (real export)" block below for that).
148+
describe("store merge behavior (produce + Object.assign)", () => {
148149
function makeStore() {
149150
const [cfg, setCfg] = createStore(ConfigSchema.parse({}));
150151
const update = (partial: Partial<ReturnType<typeof ConfigSchema.parse>>) => {
@@ -214,10 +215,7 @@ describe("updateConfig", () => {
214215

215216
describe("updateConfig (real export)", () => {
216217
beforeEach(() => {
217-
createRoot((dispose) => {
218-
resetConfig();
219-
dispose();
220-
});
218+
resetConfig();
221219
});
222220

223221
it("applies valid partial updates", () => {
@@ -243,4 +241,52 @@ describe("updateConfig (real export)", () => {
243241
dispose();
244242
});
245243
});
244+
245+
it("preserves non-default values when updating a different field", () => {
246+
createRoot((dispose) => {
247+
// Set several fields to non-default values
248+
updateConfig({ theme: "dark", refreshInterval: 120, itemsPerPage: 50 });
249+
// Now update a single unrelated field
250+
updateConfig({ hotPollInterval: 60 });
251+
// All previously-set fields must survive
252+
expect(config.hotPollInterval).toBe(60);
253+
expect(config.theme).toBe("dark");
254+
expect(config.refreshInterval).toBe(120);
255+
expect(config.itemsPerPage).toBe(50);
256+
dispose();
257+
});
258+
});
259+
260+
it("preserves onboardingComplete when updating refreshInterval", () => {
261+
createRoot((dispose) => {
262+
updateConfig({ onboardingComplete: true });
263+
updateConfig({ refreshInterval: 600 });
264+
expect(config.refreshInterval).toBe(600);
265+
expect(config.onboardingComplete).toBe(true);
266+
dispose();
267+
});
268+
});
269+
270+
it("preserves nested notifications when updating a top-level field", () => {
271+
createRoot((dispose) => {
272+
updateConfig({
273+
notifications: { enabled: true, issues: true, pullRequests: false, workflowRuns: true },
274+
});
275+
updateConfig({ viewDensity: "compact" });
276+
expect(config.viewDensity).toBe("compact");
277+
expect(config.notifications.enabled).toBe(true);
278+
expect(config.notifications.pullRequests).toBe(false);
279+
dispose();
280+
});
281+
});
282+
283+
it("preserves selectedOrgs when updating theme", () => {
284+
createRoot((dispose) => {
285+
updateConfig({ selectedOrgs: ["my-org", "other-org"] });
286+
updateConfig({ theme: "forest" });
287+
expect(config.theme).toBe("forest");
288+
expect(config.selectedOrgs).toEqual(["my-org", "other-org"]);
289+
dispose();
290+
});
291+
});
246292
});

0 commit comments

Comments
 (0)