Skip to content

Commit 98977dd

Browse files
decentraland-botnearnshawclaude
authored
feat: add option to rename a project's folder from the Scenes view (#1357)
* feat: add option to rename a project's folder from the Scenes view * chore: fix prettier formatting * fix: address review feedback on folder rename - Make renameProject.pending a no-op so the rename modal survives the await and its inline error UI stays reachable (same pattern as duplicateProject) - Allow case-only renames on case-insensitive filesystems by skipping the collision check when paths differ only by case - Add missing rename_folder translation keys to es.json and zh.json - Disable Confirm when the name is empty/whitespace - Surface a distinct error message for folder-name collisions - Reject folder names ending with a dot (Windows strips them silently) - Undo the rename if the config write fails, keeping disk and config consistent - Rename shadowed config callback param to draft Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Signed-off-by: Nicolas Earnshaw <earnshaw.nico@gmail.com> Co-authored-by: Nicolas Earnshaw <earnshaw.nico@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 9231721 commit 98977dd

17 files changed

Lines changed: 475 additions & 2 deletions

File tree

packages/creator-hub/preload/src/modules/workspace.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { DEFAULT_DEPENDENCY_UPDATE_STRATEGY } from '/shared/types/settings';
1313
import type { GetProjectsOpts, Template, Workspace } from '/shared/types/workspace';
1414
import { FileSystemStorage } from '/shared/types/storage';
1515
import { fetch } from '/shared/fetch';
16+
import { isValidFolderName } from '/shared/utils';
1617
import { STUDIOS_ADMIN_URL } from '/shared/urls';
1718

1819
import type { Services } from '../services';
@@ -376,6 +377,55 @@ export function initializeWorkspace(services: Services) {
376377
return project;
377378
}
378379

380+
/**
381+
* Renames a project's folder on disk. The project keeps its identity (`.editor/` metadata,
382+
* `scene.json` title, etc.) since those live inside the folder and simply move along with it.
383+
*
384+
* @param path - The current path of the project directory to rename.
385+
* @param newName - The desired new folder name (not a full path).
386+
* @returns A Promise that resolves to the renamed Project.
387+
*/
388+
async function renameProject({
389+
path: _path,
390+
newName,
391+
}: {
392+
path: string;
393+
newName: string;
394+
}): Promise<Project> {
395+
const trimmedName = newName.trim();
396+
if (!isValidFolderName(trimmedName)) {
397+
throw new Error(`Invalid folder name: "${newName}"`);
398+
}
399+
400+
const newPath = path.join(path.dirname(_path), trimmedName);
401+
402+
if (newPath === _path) {
403+
return getProject({ path: _path });
404+
}
405+
406+
// On case-insensitive filesystems (APFS, NTFS) a case-only rename makes `fs.exists(newPath)`
407+
// match the project's own folder, so the collision check must be skipped for it —
408+
// `fs.rename` handles case-only renames fine on those filesystems.
409+
const isCaseOnlyRename = newPath.toLowerCase() === _path.toLowerCase();
410+
if (!isCaseOnlyRename && (await fs.exists(newPath))) {
411+
throw new Error(`A folder named "${trimmedName}" already exists`);
412+
}
413+
414+
await fs.rename(_path, newPath);
415+
416+
try {
417+
await config.setConfig(draft => {
418+
draft.workspace.paths = draft.workspace.paths.map($ => ($ === _path ? newPath : $));
419+
});
420+
} catch (error) {
421+
// Keep disk and config consistent: undo the rename if the config write fails.
422+
await fs.rename(newPath, _path);
423+
throw error;
424+
}
425+
426+
return getProject({ path: newPath });
427+
}
428+
379429
/**
380430
* Returns whether or not the provided directory is a valid base path to create new scenes/projects.
381431
* A valid base path is a writable directory.
@@ -522,6 +572,7 @@ export function initializeWorkspace(services: Services) {
522572
unlistProjects,
523573
deleteProject,
524574
duplicateProject,
575+
renameProject,
525576
reimportProject,
526577
saveThumbnail,
527578
openFolder,

packages/creator-hub/preload/src/services/fs.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ export async function rm(path: string, options?: { recursive?: boolean }) {
3131
await fs.rm(path, options);
3232
}
3333

34+
export async function rename(oldPath: string, newPath: string) {
35+
await fs.rename(oldPath, newPath);
36+
}
37+
3438
export async function readdir(path: string) {
3539
return fs.readdir(path);
3640
}

packages/creator-hub/preload/tests/modules/services.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ export const getMockServices = (): DeepMock<Services> => ({
2626
readdir: vi.fn(),
2727
isDirectory: vi.fn(),
2828
cp: vi.fn(),
29+
rename: vi.fn(),
2930
},
3031
ipc: {
3132
invoke: vi.fn(),
3233
},
3334
path: {
3435
join: vi.fn((...args) => args.join('/')),
36+
dirname: vi.fn(path => path.split('/').slice(0, -1).join('/')),
3537
} as any, // temp until we have a "path" service...
3638
npm: {
3739
install: vi.fn(),

packages/creator-hub/preload/tests/modules/workspace.spec.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,20 @@ import type { Scene } from '@dcl/schemas';
44
import { initializeWorkspace } from '../../src/modules/workspace';
55
import { getScenesPath } from '../../src/modules/settings';
66
import { getScene } from '../../src/modules/scene';
7+
import { getProjectId } from '../../src/modules/analytics';
78
import { NEW_SCENE_NAME, EMPTY_SCENE_TEMPLATE_REPO } from '../../src/modules/constants';
89

910
import { getMockServices } from './services';
1011

1112
vi.mock('../../src/modules/scene');
1213
vi.mock('../../src/modules/settings');
14+
// `getProjectId` goes through the real `services/ipc.ts` (backed by electron's `ipcRenderer`)
15+
// instead of the injected `ipc` service, so it can't be exercised in this Node test environment.
16+
vi.mock('../../src/modules/analytics');
17+
// `getProject` (used by `renameProject`) reads/writes a per-project metadata file via
18+
// `FileSystemStorage`, which uses `node:fs/promises` directly instead of the mocked `fs` service.
19+
// Auto-mock it so those tests never touch the real filesystem.
20+
vi.mock('node:fs/promises');
1321

1422
describe('initializeWorkspace', () => {
1523
const services = getMockServices();
@@ -27,6 +35,7 @@ describe('initializeWorkspace', () => {
2735

2836
vi.mocked(getScenesPath).mockResolvedValue(mockAppHome);
2937
vi.mocked(getScene).mockResolvedValue({ ...mockScene } as Scene);
38+
vi.mocked(getProjectId).mockResolvedValue('mock-id');
3039
});
3140

3241
describe('getPath', () => {
@@ -192,6 +201,100 @@ describe('initializeWorkspace', () => {
192201
});
193202
});
194203

204+
describe('renameProject', () => {
205+
const currentPath = `${mockAppHome}/My Scene`;
206+
207+
beforeEach(() => {
208+
services.fs.stat.mockResolvedValue({
209+
birthtime: new Date(0),
210+
mtime: new Date(0),
211+
size: 0,
212+
} as any);
213+
services.ipc.invoke.mockResolvedValue(undefined);
214+
vi.mocked(getScene).mockResolvedValue({
215+
...mockScene,
216+
scene: { parcels: [] },
217+
} as unknown as Scene);
218+
});
219+
220+
it('should reject an invalid folder name without touching the filesystem', async () => {
221+
const workspace = initializeWorkspace(services);
222+
223+
await expect(
224+
workspace.renameProject({ path: currentPath, newName: 'in/valid' }),
225+
).rejects.toThrow(/Invalid folder name/);
226+
expect(services.fs.rename).not.toHaveBeenCalled();
227+
expect(services.config.setConfig).not.toHaveBeenCalled();
228+
});
229+
230+
it('should reject a name that collides with an existing folder', async () => {
231+
services.fs.exists.mockResolvedValue(true);
232+
233+
const workspace = initializeWorkspace(services);
234+
235+
await expect(
236+
workspace.renameProject({ path: currentPath, newName: 'New Name' }),
237+
).rejects.toThrow(/already exists/);
238+
expect(services.fs.rename).not.toHaveBeenCalled();
239+
expect(services.config.setConfig).not.toHaveBeenCalled();
240+
});
241+
242+
it('should do nothing and return the current project if the name is unchanged', async () => {
243+
services.fs.exists.mockResolvedValue(false);
244+
245+
const workspace = initializeWorkspace(services);
246+
const result = await workspace.renameProject({ path: currentPath, newName: 'My Scene' });
247+
248+
expect(services.fs.rename).not.toHaveBeenCalled();
249+
expect(services.config.setConfig).not.toHaveBeenCalled();
250+
expect(result.path).toBe(currentPath);
251+
});
252+
253+
it('should rename the folder and update the workspace config with the new path', async () => {
254+
services.fs.exists.mockResolvedValue(false);
255+
const newPath = `${mockAppHome}/New Name`;
256+
257+
const workspace = initializeWorkspace(services);
258+
const result = await workspace.renameProject({ path: currentPath, newName: 'New Name' });
259+
260+
expect(services.fs.rename).toHaveBeenCalledWith(currentPath, newPath);
261+
expect(services.config.setConfig).toHaveBeenCalled();
262+
263+
const drafter = services.config.setConfig.mock.calls[0][0];
264+
const draftConfig = { workspace: { paths: [currentPath, '/other/project'] } };
265+
drafter(draftConfig);
266+
expect(draftConfig.workspace.paths).toEqual([newPath, '/other/project']);
267+
268+
expect(result.path).toBe(newPath);
269+
});
270+
271+
it('should allow a case-only rename even when the filesystem reports the target as existing', async () => {
272+
// On case-insensitive filesystems (APFS, NTFS) the target path matches the project's own folder.
273+
services.fs.exists.mockResolvedValue(true);
274+
const newPath = `${mockAppHome}/my scene`;
275+
276+
const workspace = initializeWorkspace(services);
277+
const result = await workspace.renameProject({ path: currentPath, newName: 'my scene' });
278+
279+
expect(services.fs.rename).toHaveBeenCalledWith(currentPath, newPath);
280+
expect(result.path).toBe(newPath);
281+
});
282+
283+
it('should undo the rename if updating the workspace config fails', async () => {
284+
services.fs.exists.mockResolvedValue(false);
285+
services.config.setConfig.mockRejectedValueOnce(new Error('disk write failed'));
286+
const newPath = `${mockAppHome}/New Name`;
287+
288+
const workspace = initializeWorkspace(services);
289+
290+
await expect(
291+
workspace.renameProject({ path: currentPath, newName: 'New Name' }),
292+
).rejects.toThrow(/disk write failed/);
293+
expect(services.fs.rename).toHaveBeenNthCalledWith(1, currentPath, newPath);
294+
expect(services.fs.rename).toHaveBeenNthCalledWith(2, newPath, currentPath);
295+
});
296+
});
297+
195298
describe('when getting the scene source file', () => {
196299
describe('and the file exists', () => {
197300
let projectPath: string;
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { useCallback, useEffect, useState } from 'react';
2+
import { OutlinedInput, Typography, FormGroup, CircularProgress as Loader } from 'decentraland-ui2';
3+
4+
import { t } from '/@/modules/store/translation/utils';
5+
import { getBaseName, isValidFolderName } from '/shared/utils';
6+
7+
import { Modal } from '..';
8+
import { Button } from '../../Button';
9+
10+
import type { Props } from './types';
11+
12+
import './styles.css';
13+
14+
export function RenameFolder({ open, project, onClose, onSubmit }: Props) {
15+
const currentName = getBaseName(project.path);
16+
const [name, setName] = useState(currentName);
17+
const [error, setError] = useState<string | null>(null);
18+
const [loading, setLoading] = useState(false);
19+
20+
// Reset the field whenever the modal is (re)opened for a project.
21+
useEffect(() => {
22+
if (open) {
23+
setName(currentName);
24+
setError(null);
25+
setLoading(false);
26+
}
27+
}, [open, currentName]);
28+
29+
const handleChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => {
30+
setError(null);
31+
setName(event.target.value);
32+
}, []);
33+
34+
const trimmedName = name.trim();
35+
const isEmpty = trimmedName.length === 0;
36+
const isUnchanged = trimmedName === currentName;
37+
const isInvalid = !isEmpty && !isValidFolderName(trimmedName);
38+
39+
const handleSubmit = useCallback(async () => {
40+
if (isEmpty || isUnchanged || isInvalid || loading) return;
41+
setLoading(true);
42+
setError(null);
43+
try {
44+
await onSubmit(project, trimmedName);
45+
onClose();
46+
} catch (error) {
47+
// `unwrap()` rejects with a SerializedError (plain object), not an Error instance.
48+
const message = (error as { message?: string } | null)?.message ?? '';
49+
const isCollision = message.includes('already exists');
50+
setError(
51+
isCollision
52+
? t('modal.rename_folder.errors.name_taken')
53+
: t('modal.rename_folder.errors.rename_failed'),
54+
);
55+
setLoading(false);
56+
}
57+
}, [onSubmit, onClose, project, trimmedName, isEmpty, isUnchanged, isInvalid, loading]);
58+
59+
return (
60+
<Modal
61+
open={open}
62+
title={t('modal.rename_folder.title')}
63+
size="tiny"
64+
className="RenameFolderModal"
65+
onClose={onClose}
66+
actions={
67+
<>
68+
<Button
69+
color="secondary"
70+
onClick={onClose}
71+
>
72+
{t('modal.cancel')}
73+
</Button>
74+
<Button
75+
onClick={handleSubmit}
76+
disabled={loading || isEmpty || isUnchanged || isInvalid}
77+
>
78+
{loading ? <Loader size={20} /> : t('modal.confirm')}
79+
</Button>
80+
</>
81+
}
82+
>
83+
<FormGroup className="RenameFolderFormControl">
84+
<Typography variant="body1">{t('modal.rename_folder.field_label')}</Typography>
85+
<OutlinedInput
86+
color="secondary"
87+
value={name}
88+
onChange={handleChange}
89+
autoFocus
90+
/>
91+
{isInvalid && (
92+
<Typography
93+
variant="body1"
94+
className="error"
95+
>
96+
{t('modal.rename_folder.errors.invalid_name')}
97+
</Typography>
98+
)}
99+
{error && (
100+
<Typography
101+
variant="body1"
102+
className="error"
103+
>
104+
{error}
105+
</Typography>
106+
)}
107+
</FormGroup>
108+
</Modal>
109+
);
110+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import { RenameFolder } from './component';
2+
export { RenameFolder };
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
.RenameFolderModal .MuiInputBase-root + .MuiTypography-root {
2+
margin-top: 16px;
3+
}
4+
5+
.RenameFolderModal .error {
6+
color: var(--error);
7+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import type { Project } from '/shared/types/projects';
2+
3+
export type Props = {
4+
open: boolean;
5+
project: Project;
6+
onClose: () => void;
7+
onSubmit: (project: Project, newName: string) => Promise<unknown>;
8+
};

0 commit comments

Comments
 (0)