Fix mediaDir being ignored when uploading new files#482
Conversation
commit: |
There was a problem hiding this comment.
Pull request overview
This PR addresses incorrect media file URL/path handling when a workspace’s mediaDir is nested under the configured publicDir, ensuring uploaded media entries store the correct public-facing location while still writing/removing files in the correct on-disk directory.
Changes:
- Add
workspaceMediaLocation/workspaceMediaFilehelpers to consistently translate between on-disk media file paths and public URL locations. - Update upload and file-removal flows to use the new helpers (fixing nested workspace mediaDir behavior).
- Add tests covering nested workspace
mediaDirscenarios for both upload behavior and filename utilities.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/uploads.test.ts | Adds an integration-style test for nested workspace mediaDir upload location behavior. |
| src/core/util/EntryFilenames.ts | Introduces helpers to map between stored public locations and filesystem media paths. |
| src/core/util/EntryFilenames.test.ts | Adds unit tests for the new media location/path helpers. |
| src/core/db/Operation.ts | Uses new helpers during upload to derive stored public location and filesystem target path. |
| src/core/db/EntryTransaction.ts | Uses new helper to compute filesystem paths when removing media files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function absolute(path: string): string { | ||
| const normalized = normalize(path) |
There was a problem hiding this comment.
absolute() currently turns an empty string into /. because normalize('') returns '.'. Since workspaceMediaDir() can return '' (mediaDir is optional), workspaceMediaLocation() / workspaceMediaFile() may compute bases like /., which can yield incorrect file locations when a workspace has a media root but no explicit mediaDir configured. Consider special-casing empty/undefined paths (and optionally trimming trailing slashes) so the computed base is either a real directory (e.g. config.publicDir) or stays empty, but never /..
| function absolute(path: string): string { | |
| const normalized = normalize(path) | |
| function absolute(path: string): string { | |
| if (!path) return '' | |
| const normalized = normalize(path) | |
| if (normalized === '.') return '' |
| }) | ||
| const db = new MultiWorkspaceDB(cms.config) | ||
|
|
||
| const uploadRegio= await db.upload({ |
There was a problem hiding this comment.
There’s a formatting issue here (const uploadRegio= ...) that likely won’t pass the repo’s formatter/linter (oxfmt/oxlint). Please add the missing space around = (or run the formatter on the file) to keep CI consistent.
| const uploadRegio= await db.upload({ | |
| const uploadRegio = await db.upload({ |
No description provided.