Skip to content

Commit 0398176

Browse files
author
Fabian
committed
fix(frontend): keep workspace settings on the workspace in the URL
MainApp renders a single WorkspaceSettings instance for every /workspaces/:id/settings/* view, so moving between two workspaces' settings changes the workspaceId prop under a mounted component. The component loaded its workspace only in onMount, so after such a switch the page kept rendering the previously opened workspace: its name, key, description and — on the danger tab — its "Active Workspace" toggle and delete confirmation, while saveWorkspace() and deleteWorkspace() already addressed the new id from the URL. Saving wrote one workspace's settings onto another, and the delete confirmation was validated against the old workspace's name while deleting the new one. Load from the workspaceId prop instead of from the mount, with a load version guard so a superseded response cannot repopulate the form, and clear workspace/form/delete-confirmation state on every switch. A workspace that cannot be loaded now falls through to the existing "Workspace not found" branch instead of showing another workspace. Fence the save and delete completions the same way: both pin their target id (and the payload / the confirmed name) before awaiting. What the server actually changed is applied unconditionally — the workspace list must drop a deleted workspace and pick up a renamed one wherever the user has moved to — while the view-scoped effects (form state, currentWorkspace, the toast, the redirect off the deleted workspace) only run while that target is still the one on screen. For the same reason, MainApp no longer leaves the previously hydrated workspace in currentWorkspace when the routed one fails to load: the shell (workspace header, avatar, gradient, command-palette scope) must not present a different workspace than the URL names.
1 parent ae1323d commit 0398176

2 files changed

Lines changed: 119 additions & 39 deletions

File tree

frontend/src/lib/pages/MainApp.svelte

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,15 @@
863863
async function hydrateCurrentWorkspaceFromSharedData(workspaceId) {
864864
await workspaceDataStore.initialize(workspaceId);
865865
const expectedId = Number.parseInt(String(workspaceId), 10);
866-
if (workspaceDataStore.workspaceId !== expectedId || !workspaceDataStore.workspace) return;
866+
// A newer route already owns the store — let its own hydration finish.
867+
if (workspaceDataStore.workspaceId !== expectedId) return;
868+
if (!workspaceDataStore.workspace) {
869+
// The routed workspace could not be loaded. Keeping the previously
870+
// hydrated one would leave the shell (workspace header, avatar, gradient,
871+
// command-palette scope) presenting a different workspace than the URL.
872+
currentWorkspace.clear();
873+
return;
874+
}
867875
currentWorkspace.hydrate(workspaceDataStore.workspace);
868876
}
869877

frontend/src/lib/workspaces/WorkspaceSettings.svelte

Lines changed: 110 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,19 @@
4747
let timeProjectCategories = $state([]);
4848
let selectedTimeProjectCategories = $state([]);
4949
50-
let formData = $state({
51-
name: '',
52-
key: '',
53-
description: '',
54-
active: true,
55-
time_project_id: null,
56-
default_view: 'board',
57-
internal_comments_enabled: false
58-
});
50+
function blankFormData() {
51+
return {
52+
name: '',
53+
key: '',
54+
description: '',
55+
active: true,
56+
time_project_id: null,
57+
default_view: 'board',
58+
internal_comments_enabled: false
59+
};
60+
}
61+
62+
let formData = $state(blankFormData());
5963
6064
// The active admin module (registry-driven), used to render the page header.
6165
const currentModule = $derived(
@@ -78,6 +82,16 @@
7882
// Permission check for workspace admin
7983
const canAdmin = $derived(workspacePermissions.canAdminWorkspace(workspaceId));
8084
85+
// MainApp renders one WorkspaceSettings instance for every
86+
// /workspaces/:id/settings/* view, so `workspaceId` changes under a mounted
87+
// component whenever the user moves between two workspaces' settings. The
88+
// load therefore has to follow the prop, not the mount: otherwise the form
89+
// keeps showing the previously opened workspace while saveWorkspace() and
90+
// deleteWorkspace() already act on the new id.
91+
let moduleSettingsReady = $state(false);
92+
let lastLoadedWorkspaceId = null;
93+
let workspaceLoadVersion = 0;
94+
8195
onMount(async () => {
8296
await moduleSettings.load();
8397
@@ -87,31 +101,67 @@
87101
// Don't return — still load data so the component isn't stuck in loading state
88102
}
89103
90-
const loadPromises = [loadWorkspace(), loadTimeProjectCategories()];
104+
moduleSettingsReady = true;
105+
});
106+
107+
$effect(() => {
108+
if (!moduleSettingsReady) return;
109+
const id = workspaceId ? String(workspaceId) : null;
110+
if (!id || id === lastLoadedWorkspaceId) return;
111+
lastLoadedWorkspaceId = id;
112+
void loadWorkspaceData();
113+
});
114+
115+
async function loadWorkspaceData() {
116+
const version = ++workspaceLoadVersion;
117+
loading = true;
118+
// Drop the previous workspace's state before rendering anything for the
119+
// new one — a populated form or a primed delete confirmation must never
120+
// outlive the workspace it was filled in for.
121+
workspace = null;
122+
formData = blankFormData();
123+
selectedTimeProjectCategories = [];
124+
showDeleteConfirm = false;
125+
deleteConfirmText = '';
126+
127+
const loadPromises = [loadWorkspace(version), loadTimeProjectCategories()];
91128
if ($moduleSettings.time_tracking_enabled) {
92129
loadPromises.push(loadTimeProjects());
93130
}
94131
95132
await Promise.all(loadPromises);
96-
loading = false;
97-
});
133+
if (version === workspaceLoadVersion) {
134+
loading = false;
135+
}
136+
}
137+
138+
// "Reset" discards local edits by re-reading the workspace currently shown.
139+
// It must pass the live load version, never be wired up as a bare handler —
140+
// loadWorkspace() would then receive the click event as its version.
141+
function resetWorkspaceForm() {
142+
void loadWorkspace(workspaceLoadVersion);
143+
}
98144
99-
async function loadWorkspace() {
145+
async function loadWorkspace(version) {
100146
try {
101-
workspace = await api.workspaces.get(workspaceId);
102-
if (workspace) {
147+
const loaded = await api.workspaces.get(workspaceId);
148+
// A newer workspace is already loading — its response owns the form.
149+
if (version !== workspaceLoadVersion) return;
150+
workspace = loaded;
151+
if (loaded) {
103152
formData = {
104-
name: workspace.name,
105-
key: workspace.key || '',
106-
description: workspace.description || '',
107-
active: workspace.active,
108-
time_project_id: workspace.time_project_id || null,
109-
default_view: workspace.default_view || 'board',
110-
internal_comments_enabled: workspace.internal_comments_enabled || false
153+
name: loaded.name,
154+
key: loaded.key || '',
155+
description: loaded.description || '',
156+
active: loaded.active,
157+
time_project_id: loaded.time_project_id || null,
158+
default_view: loaded.default_view || 'board',
159+
internal_comments_enabled: loaded.internal_comments_enabled || false
111160
};
112-
selectedTimeProjectCategories = workspace.time_project_categories || [];
161+
selectedTimeProjectCategories = loaded.time_project_categories || [];
113162
}
114163
} catch (error) {
164+
if (version !== workspaceLoadVersion) return;
115165
console.error('Failed to load workspace:', error);
116166
}
117167
}
@@ -145,20 +195,32 @@
145195
return;
146196
}
147197
198+
// The workspace can change under the component while the request is in
199+
// flight, so pin what this save is about before awaiting. Effects then
200+
// split: what the server actually changed is applied unconditionally,
201+
// what describes the view is applied only if we are still on that target.
202+
const targetId = workspaceId;
203+
const payload = {
204+
...formData,
205+
time_project_id: formData.time_project_id ? parseInt(formData.time_project_id, 10) : null,
206+
time_project_categories: selectedTimeProjectCategories
207+
};
208+
148209
try {
149210
saving = true;
150-
await api.workspaces.update(workspaceId, {
151-
...formData,
152-
time_project_id: formData.time_project_id ? parseInt(formData.time_project_id, 10) : null,
153-
time_project_categories: selectedTimeProjectCategories
211+
await api.workspaces.update(targetId, payload);
212+
213+
// Update stores so sidebar dropdown reflects name/description changes immediately
214+
workspacesStore.updateWorkspace(targetId, {
215+
name: payload.name,
216+
description: payload.description
154217
});
155218
156-
// Update local workspace object
157-
workspace = { ...workspace, ...formData };
219+
if (targetId !== workspaceId) return;
158220
159-
// Update stores so sidebar dropdown reflects name/description changes immediately
160-
workspacesStore.updateWorkspace(workspaceId, { name: formData.name, description: formData.description });
161-
currentWorkspace.patch({ name: formData.name, description: formData.description });
221+
// Update local workspace object
222+
workspace = { ...workspace, ...payload };
223+
currentWorkspace.patch({ name: payload.name, description: payload.description });
162224
163225
successToast(t('workspaceSettings.savedSuccessfully'));
164226
} catch (error) {
@@ -175,16 +237,26 @@
175237
}
176238
177239
async function deleteWorkspace() {
178-
if (deleteConfirmText !== workspace.name) {
240+
// `workspace` is null while a workspace switch is loading.
241+
if (!workspace || deleteConfirmText !== workspace.name) {
179242
errorToast(t('workspaceSettings.pleaseConfirmDeletion'));
180243
return;
181244
}
182245
246+
// Same fencing as saveWorkspace: the deletion is a fact about `targetId`
247+
// whatever the user is looking at afterwards, but leaving the page is only
248+
// right while that workspace is still the one on screen.
249+
const targetId = workspaceId;
250+
const targetName = workspace.name;
251+
183252
try {
184-
await api.workspaces.delete(workspaceId);
185-
workspacesStore.remove(workspaceId);
253+
await api.workspaces.delete(targetId);
254+
workspacesStore.remove(targetId);
255+
successToast(t('workspaceSettings.deletedSuccessfully', { name: targetName }));
256+
257+
if (targetId !== workspaceId) return;
258+
186259
currentWorkspace.clear();
187-
successToast(t('workspaceSettings.deletedSuccessfully', { name: workspace.name }));
188260
setTimeout(() => {
189261
navigate('/workspaces');
190262
}, 1000);
@@ -336,7 +408,7 @@
336408
<Button
337409
variant="secondary"
338410
size="medium"
339-
onclick={loadWorkspace}
411+
onclick={resetWorkspaceForm}
340412
>
341413
{t('workspaceSettings.reset')}
342414
</Button>
@@ -371,7 +443,7 @@
371443
<Button
372444
variant="secondary"
373445
size="medium"
374-
onclick={loadWorkspace}
446+
onclick={resetWorkspaceForm}
375447
>
376448
{t('workspaceSettings.reset')}
377449
</Button>

0 commit comments

Comments
 (0)