diff --git a/.changeset/fn-7535-global-gitlab-setting-save.md b/.changeset/fn-7535-global-gitlab-setting-save.md new file mode 100644 index 0000000000..adf4d7d16c --- /dev/null +++ b/.changeset/fn-7535-global-gitlab-setting-save.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix the global GitLab integration setting not persisting when saved. +category: fix +dev: splitSettingsSave now diffs the five global GitLab keys (gitlabEnabled, gitlabInstanceUrl, gitlabApiBaseUrl, gitlabAuthToken, gitlabAuthTokenType) against scoped global initials only, never the project-effective merged initialValues, so a project override no longer suppresses a real global save. diff --git a/packages/dashboard/app/__tests__/settings-save-split.test.ts b/packages/dashboard/app/__tests__/settings-save-split.test.ts index 52b150783a..9ad7a921c1 100644 --- a/packages/dashboard/app/__tests__/settings-save-split.test.ts +++ b/packages/dashboard/app/__tests__/settings-save-split.test.ts @@ -268,6 +268,68 @@ describe("splitSettingsSave", () => { expect(projectPatch).toEqual({ gitlabEnabled: false, gitlabAuthToken: "project-token", gitlabAuthTokenType: "project" }); }); + /* + FNXC:GitLabEnablement 2026-07-04-00:00: + FN-7535 regression repro: scoped global initials omit `gitlabEnabled` (the operator has never + saved a global value before) while the merged/project-effective `initialValues` happens to equal + the new edited value. Before the fix, the changed-only comparison fell back to `initialValues` + when the scoped global object lacked the key, so this genuine global edit was misclassified as + "unchanged" and dropped from the global patch entirely. + */ + it("persists an explicit global GitLab edit when scoped global initials omit the key but merged initialValues matches the new value", () => { + const initialScopedValues = { + global: {}, // operator has never saved global GitLab settings before; key is absent, not `undefined` + project: { gitlabEnabled: false }, + } as never; + + const { globalPatch, projectPatch } = splitSettingsSave({ + payload: { gitlabEnabled: true }, + // The merged/project-effective initialValues happens to already be `true` + // (e.g. inherited default or a stale merge) — this must NOT suppress a + // genuine global edit. + initialValues: { gitlabEnabled: true } as never, + initialScopedValues, + activeSection: "global-general", + }); + + expect(globalPatch).toEqual({ gitlabEnabled: true }); + expect(projectPatch).toEqual({}); + }); + + it("does not emit a spurious global GitLab write when the scoped global initial already matches the unchanged value", () => { + const initialScopedValues = { + global: { gitlabEnabled: true }, + project: { gitlabEnabled: false }, + } as never; + + const { globalPatch } = splitSettingsSave({ + payload: { gitlabEnabled: true }, + initialValues: { gitlabEnabled: true } as never, + initialScopedValues, + activeSection: "global-general", + }); + + expect(globalPatch).toEqual({}); + }); + + it("treats a present-but-undefined scoped global GitLab key as unset, not as the merged fallback", () => { + const initialScopedValues = { + global: { gitlabEnabled: undefined }, + project: { gitlabEnabled: true }, + } as never; + + const { globalPatch } = splitSettingsSave({ + payload: { gitlabEnabled: true }, + // Merged initialValues also happens to be true, but the scoped global key + // is explicitly present-but-undefined (unset) — the edit must still land. + initialValues: { gitlabEnabled: true } as never, + initialScopedValues, + activeSection: "global-general", + }); + + expect(globalPatch).toEqual({ gitlabEnabled: true }); + }); + it("clears a project GitLab token with null-as-delete while preserving selected token type", () => { const initialScopedValues = { global: {}, diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx index 70c4aaade7..adf207625b 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx @@ -874,6 +874,42 @@ describe("SettingsModal", () => { } }); + /* + FNXC:GitLabEnablement 2026-07-04-00:00: + FN-7535 regression repro: the scoped `global` settings omit `gitlabEnabled` + entirely (the operator has never saved a global GitLab value before), while + the merged/project-effective `fetchSettings` value already happens to equal + the value the operator is about to set. Before the fix, `splitSettingsSave` + fell back to the merged `initialValues` for the changed-only comparison + when the scoped global object lacked the key, so this explicit global edit + was misclassified as "unchanged" and silently dropped from the global patch. + */ + it("saves an explicit global GitLab disable edit when scoped global omits the key but merged settings already match", async () => { + // Scoped global omits `gitlabEnabled` entirely (unset renders as checked/ + // enabled per the disclosure's documented "unset behaves as enabled" default). + // The merged/project-effective `fetchSettings` value already happens to be + // `false` — the same value the operator is about to explicitly set. + mockFetchSettings.mockResolvedValueOnce({ ...defaultSettings, gitlabEnabled: false }); + mockFetchSettingsByScope.mockResolvedValueOnce({ + global: { ...defaultSettings }, // no gitlabEnabled key at all + project: {}, + }); + + renderModal({ initialSection: "global-general" }); + await waitForSettingsModalReady(); + + const enableToggle = screen.getByLabelText("Enable GitLab integration") as HTMLInputElement; + expect(enableToggle).toBeChecked(); + + await settingsModalUser.click(enableToggle); + expect(enableToggle).not.toBeChecked(); + await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => { + expect(mockUpdateGlobalSettings).toHaveBeenCalledWith(expect.objectContaining({ gitlabEnabled: false })); + }); + }); + it("shows global tracking repo error hint and keeps custom entry when lookups fail", async () => { mockFetchProjects.mockRejectedValueOnce(new Error("no projects")); diff --git a/packages/dashboard/app/components/settings/save-split.ts b/packages/dashboard/app/components/settings/save-split.ts index 782afe84cc..81527ad5a4 100644 --- a/packages/dashboard/app/components/settings/save-split.ts +++ b/packages/dashboard/app/components/settings/save-split.ts @@ -45,6 +45,25 @@ export const MODEL_LANE_KEYS = [ const MODEL_LANE_KEY_SET = new Set(MODEL_LANE_KEYS); +/* +FNXC:GitLabEnablement 2026-07-04-00:00: +FN-7535: the five global GitLab keys must be diffed against the SCOPED global +initial only — never the merged, project-effective `initialValues` — because +SettingsModal already edits these keys through a dedicated `globalGitlabSettings` +state seeded from `scoped.global` (FN-7453). Falling back to merged initialValues +when the scoped global object lacks the key (e.g. the operator has never saved a +global value before) let a project override's effective value silently stand in +for "no change", so a genuine global edit that happened to match the merged value +was dropped from the global patch. These keys never fall back to `initialValues`. +*/ +const GLOBAL_GITLAB_SCOPED_ONLY_KEYS = new Set([ + "gitlabEnabled", + "gitlabInstanceUrl", + "gitlabApiBaseUrl", + "gitlabAuthToken", + "gitlabAuthTokenType", +]); + type RemoteAccessProvider = "tailscale" | "cloudflare"; type RemoteAccessPatch = NonNullable; @@ -366,11 +385,14 @@ export function splitSettingsSave({ continue; } + const scopedOnly = GLOBAL_GITLAB_SCOPED_ONLY_KEYS.has(key); const hasScopedInitial = hasOwn(initialScopedValues?.global, key); - const hasMergedInitial = hasOwn(initialValues, key); + const hasMergedInitial = !scopedOnly && hasOwn(initialValues, key); const initialValue = hasScopedInitial ? initialScopedValues?.global?.[key as keyof GlobalSettings] - : initialValues?.[key as keyof GlobalSettings]; + : scopedOnly + ? undefined + : initialValues?.[key as keyof GlobalSettings]; const hasInitialValue = hasScopedInitial || hasMergedInitial; if (settingsValueEquals(value, initialValue)) {