diff --git a/api/src/changeRequests/documentProcessing/processImageDto.ts b/api/src/changeRequests/documentProcessing/processImageDto.ts index 9dddc765c3..90175deb43 100644 --- a/api/src/changeRequests/documentProcessing/processImageDto.ts +++ b/api/src/changeRequests/documentProcessing/processImageDto.ts @@ -187,6 +187,8 @@ async function migrateImagesBetweenBuckets( * Processes an embedded image upload by resizing the image and uploading to S3 * Requires bucket-specific credentials configured at the post/tag level * Bucket ID is passed from the parent post/tag document for consistency + * `sourceBucketId` is where a duplicated image's files are read from, resolved from the source + * document rather than the incoming one. * Returns object with migration failure status and warnings */ export async function processImage( @@ -195,13 +197,14 @@ export async function processImage( db: DbService, parentBucketId?: string, prevParentBucketId?: string, + sourceBucketId?: string, ): Promise<{ migrationFailed: boolean; warnings: string[] }> { const warnings: string[] = []; let migrationFailed = false; let duplicatedNow = false; try { - if (image.duplicate && image.fileCollections.length > 0) { + if ((image.duplicateFrom || image.duplicate) && image.fileCollections.length > 0) { const prevHasImageFiles = !!prevImage?.fileCollections?.some( (collection) => collection.imageFiles?.length > 0, ); @@ -209,6 +212,9 @@ export async function processImage( if (prevHasImageFiles) { image.fileCollections = prevImage.fileCollections; } else { + // Copy out of the bucket the source files actually live in. It falls back to the + // target bucket for older clients, which only send `duplicate` and no source id. + const copyFromBucketId = sourceBucketId || parentBucketId; if (!parentBucketId) { warnings.push("Parent bucket ID is required for duplicated image copy."); return { migrationFailed, warnings }; @@ -217,7 +223,7 @@ export async function processImage( const duplicateResult = await duplicateImageFilesWithoutReencoding( image, db, - parentBucketId, + copyFromBucketId, parentBucketId, ); warnings.push(...duplicateResult.warnings); @@ -226,6 +232,7 @@ export async function processImage( // Avoid persisting stale source filenames when copy fails. image.fileCollections = []; delete image.duplicate; + delete image.duplicateFrom; return { migrationFailed, warnings }; } duplicatedNow = true; @@ -323,6 +330,7 @@ export async function processImage( } delete image.duplicate; + delete image.duplicateFrom; } catch (error) { warnings.push(`Image processing failed: ${error.message}`); } diff --git a/api/src/changeRequests/documentProcessing/processPostTagDto.spec.ts b/api/src/changeRequests/documentProcessing/processPostTagDto.spec.ts index 8c43e0cb12..9cf84ccd39 100644 --- a/api/src/changeRequests/documentProcessing/processPostTagDto.spec.ts +++ b/api/src/changeRequests/documentProcessing/processPostTagDto.spec.ts @@ -95,7 +95,6 @@ describe("processPostTagDto", () => { await processChangeRequest("test-user", postCr, ["group-super-admins"], db); await processChangeRequest("test-user", contentCr, ["group-super-admins"], db); - let contentRes = await db.getDoc(contentCr.doc._id); expect(contentRes.docs[0].parentAlwaysOffline).toBeUndefined(); @@ -282,6 +281,7 @@ describe("processPostTagDto", () => { db, (changeRequest.doc as PostDto).imageBucketId, undefined, + undefined, // no duplication source bucket ); }); @@ -441,6 +441,7 @@ describe("processPostTagDto", () => { db, undefined, // no bucketId undefined, // no prevBucketId + undefined, // no duplication source bucket ); expect(result.warnings).toContain("Bucket is not specified for image processing."); }); @@ -638,4 +639,88 @@ describe("processPostTagDto", () => { (changeRequest.doc as PostDto).mediaBucketId, ); }); + it("resolves the source bucket and files from the document named by duplicateFrom", async () => { + const sourceRequest = changeRequest_post(); + sourceRequest.doc._id = "test-duplicate-source-post"; + (sourceRequest.doc as any).imageBucketId = "storage-bucket-1"; + (sourceRequest.doc as any).imageData = { + fileCollections: [ + { + aspectRatio: 1.5, + imageFiles: [{ width: 100, height: 66, filename: "src.webp" }], + }, + ], + }; + await db.upsertDoc(sourceRequest.doc); + + // The duplicate carries neither a bucket nor the source files — the API supplies both. + const duplicateRequest = changeRequest_post(); + duplicateRequest.doc._id = "test-duplicate-target-post"; + delete (duplicateRequest.doc as any).imageBucketId; + (duplicateRequest.doc as any).imageData = { + fileCollections: [], + duplicateFrom: "test-duplicate-source-post", + }; + + await processChangeRequest( + "test-user", + duplicateRequest as ChangeReqDto, + ["group-super-admins"], + db, + ); + + const stored = (await db.getDoc("test-duplicate-target-post")).docs[0] as PostDto; + expect(stored.imageBucketId).toBe("storage-bucket-1"); + + expect(processImage).toHaveBeenCalledWith( + expect.objectContaining({ + fileCollections: (sourceRequest.doc as any).imageData.fileCollections, + }), + undefined, + db, + "storage-bucket-1", + undefined, + "storage-bucket-1", + ); + }); + + it("keeps the duplicate's own bucket as the copy target when it has one", async () => { + const sourceRequest = changeRequest_post(); + sourceRequest.doc._id = "test-duplicate-source-post-2"; + (sourceRequest.doc as any).imageBucketId = "storage-bucket-1"; + (sourceRequest.doc as any).imageData = { + fileCollections: [ + { + aspectRatio: 1.5, + imageFiles: [{ width: 100, height: 66, filename: "src.webp" }], + }, + ], + }; + await db.upsertDoc(sourceRequest.doc); + + const duplicateRequest = changeRequest_post(); + duplicateRequest.doc._id = "test-duplicate-target-post-2"; + (duplicateRequest.doc as any).imageBucketId = "storage-bucket-2"; + (duplicateRequest.doc as any).imageData = { + fileCollections: [], + duplicateFrom: "test-duplicate-source-post-2", + }; + + await processChangeRequest( + "test-user", + duplicateRequest as ChangeReqDto, + ["group-super-admins"], + db, + ); + + // Source bucket is read from, the duplicate's own bucket is written to. + expect(processImage).toHaveBeenCalledWith( + expect.anything(), + undefined, + db, + "storage-bucket-2", + undefined, + "storage-bucket-1", + ); + }); }); diff --git a/api/src/changeRequests/documentProcessing/processPostTagDto.ts b/api/src/changeRequests/documentProcessing/processPostTagDto.ts index dfbd06fd45..4f9ec2d5b6 100644 --- a/api/src/changeRequests/documentProcessing/processPostTagDto.ts +++ b/api/src/changeRequests/documentProcessing/processPostTagDto.ts @@ -55,19 +55,37 @@ export default async function processPostTagDto( if (doc.imageData) { let imageWarnings: string[] = []; + // The source document is authoritative for where a duplicated image's files live and what + // they are, so a client that has lost or never synced its own bucket reference still + // produces a correct copy. + let sourceBucketId: Uuid | undefined; + if (doc.imageData.duplicateFrom) { + const sourceDoc = (await db.getDoc(doc.imageData.duplicateFrom)).docs[0] as + | PostDto + | TagDto + | undefined; + sourceBucketId = sourceDoc?.imageBucketId; + if (sourceDoc?.imageData?.fileCollections?.length) { + doc.imageData.fileCollections = sourceDoc.imageData.fileCollections; + } + // The copy stays in the source bucket unless the duplicate names its own. + if (!doc.imageBucketId) doc.imageBucketId = sourceBucketId; + } + if (!doc.imageBucketId) { imageWarnings.push("Bucket is not specified for image processing."); } // prevDoc is undefined on first upsert. A duplication request must include // existing file references and a source bucket on the parent document. - if (!prevDoc && doc.imageData.duplicate) { + if (!prevDoc && (doc.imageData.duplicateFrom || doc.imageData.duplicate)) { const hasSourceFiles = doc.imageData.fileCollections?.some( (collection) => collection.imageFiles?.length > 0, ); if (!doc.imageBucketId || !hasSourceFiles) { imageWarnings.push("Image duplication request is invalid."); delete doc.imageData.duplicate; + delete doc.imageData.duplicateFrom; doc.imageData.fileCollections = []; } } @@ -80,6 +98,7 @@ export default async function processPostTagDto( db, doc.imageBucketId, prevDoc?.imageBucketId, // Pass previous bucket ID for migration + sourceBucketId, ); imageWarnings = result.warnings; diff --git a/api/src/changeRequests/validateChangeRequestAccess.spec.ts b/api/src/changeRequests/validateChangeRequestAccess.spec.ts index 83948e1d9f..e71c4891a9 100644 --- a/api/src/changeRequests/validateChangeRequestAccess.spec.ts +++ b/api/src/changeRequests/validateChangeRequestAccess.spec.ts @@ -809,4 +809,51 @@ describe("validateChangeRequestAccess", () => { expect(res.validated).toBe(true); }); }); + describe("image duplication source access", () => { + // An existing public post is saved so the request clears the edit and group assign checks, + // leaving the duplication source as the only difference between these tests. + const duplicateChangeReq = (duplicateFrom: string) => + plainToClass(ChangeReqDto, { + doc: { + _id: "post-page1", + type: "post", + memberOf: ["group-public-content"], + image: "", + tags: [], + imageData: { fileCollections: [], duplicateFrom }, + }, + }); + + it("rejects a duplicate naming a source the user cannot view", async () => { + const res = await validateChangeRequestAccess( + duplicateChangeReq("post-blog2"), + ["group-public-editors"], + db, + ); + + expect(res.validated).toBe(false); + expect(res.error).toBe("No 'View' access to the document the image is duplicated from"); + }); + + it("rejects a duplicate naming a source that does not exist", async () => { + const res = await validateChangeRequestAccess( + duplicateChangeReq("post-does-not-exist"), + ["group-public-editors"], + db, + ); + + expect(res.validated).toBe(false); + expect(res.error).toBe("No 'View' access to the document the image is duplicated from"); + }); + + it("accepts a duplicate naming a source the user can view", async () => { + const res = await validateChangeRequestAccess( + duplicateChangeReq("post-blog1"), + ["group-public-editors"], + db, + ); + + expect(res.validated).toBe(true); + }); + }); }); diff --git a/api/src/changeRequests/validateChangeRequestAccess.ts b/api/src/changeRequests/validateChangeRequestAccess.ts index b6e73b0491..19da8c785d 100644 --- a/api/src/changeRequests/validateChangeRequestAccess.ts +++ b/api/src/changeRequests/validateChangeRequestAccess.ts @@ -114,6 +114,32 @@ export async function validateChangeRequestAccess( }; } + // A duplication names the document its image is copied from, and the server reads that + // document's files and bucket. The id is client-supplied, so it needs the same view access + // as reading the source document directly would. + if (doc.type === DocType.Post || doc.type === DocType.Tag) { + const duplicateFrom = (doc as _contentParentDto).imageData?.duplicateFrom; + if (duplicateFrom) { + const sourceDoc = (await dbService.getDoc(duplicateFrom)).docs[0]; + if ( + !sourceDoc || + sourceDoc.type !== doc.type || + !PermissionSystem.verifyAccess( + (sourceDoc as _contentBaseDto).memberOf, + sourceDoc.type, + AclPermission.View, + groupMembership, + "all", + ) + ) { + return { + validated: false, + error: "No 'View' access to the document the image is duplicated from", + }; + } + } + } + // Validate edit, translate and group ACL assign access // ==================================================== diff --git a/api/src/dto/ImageDto.ts b/api/src/dto/ImageDto.ts index ce9e8473a3..672c62aa5f 100644 --- a/api/src/dto/ImageDto.ts +++ b/api/src/dto/ImageDto.ts @@ -1,8 +1,9 @@ import "reflect-metadata"; // https://stackoverflow.com/questions/72009995/typeerror-reflect-getmetadata-is-not-a-function -import { IsArray, IsBoolean, IsOptional, ValidateNested } from "class-validator"; +import { IsArray, IsBoolean, IsOptional, IsString, ValidateNested } from "class-validator"; import { Expose, Type } from "class-transformer"; import { ImageUploadDto } from "./ImageUploadDto"; import { ImageFileCollectionDto } from "./ImageFileCollectionDto"; +import { Uuid } from "../enums"; /** * Database structured Image object @@ -20,6 +21,16 @@ export class ImageDto { @Expose() uploadData?: ImageUploadDto[]; + /** + * Id of the parent this image is being copied from. The source bucket and files are resolved + * from that document server-side, so the copy does not depend on the client's own state. + */ + @IsOptional() + @IsString() + @Expose() + duplicateFrom?: Uuid; + + /** @deprecated Superseded by `duplicateFrom`; still accepted from older clients. */ @IsOptional() @IsBoolean() @Expose() diff --git a/cms/src/components/content/EditContent.vue b/cms/src/components/content/EditContent.vue index d1c03c68a6..4ac2d212dc 100644 --- a/cms/src/components/content/EditContent.vue +++ b/cms/src/components/content/EditContent.vue @@ -106,8 +106,11 @@ const acceptServerVersion = () => window.location.reload(); const icon = props.docType === DocType.Tag ? TagIcon : DocumentIcon; -const notify = (state: "success" | "error" | "info", title: string, description: string) => - addNotification({ title, description, state }); +const notify = ( + state: "success" | "error" | "info" | "warning", + title: string, + description: string, +) => addNotification({ title, description, state }); // Which translation is being edited (driven by the route), + the language lists. const { diff --git a/cms/src/components/content/EditContentDuplication.spec.ts b/cms/src/components/content/EditContentDuplication.spec.ts index 89b166ee98..d49d5aee56 100644 --- a/cms/src/components/content/EditContentDuplication.spec.ts +++ b/cms/src/components/content/EditContentDuplication.spec.ts @@ -1,7 +1,15 @@ import { describe, it, afterEach, beforeEach, expect, vi } from "vitest"; import { mount } from "@vue/test-utils"; import { createTestingPinia } from "@pinia/testing"; -import { db, DocType, accessMap, PostType, TagType, type TagDto, PublishStatus } from "luminary-shared"; +import { + db, + DocType, + accessMap, + PostType, + TagType, + type TagDto, + PublishStatus, +} from "luminary-shared"; import * as mockData from "@/tests/mockdata"; import { setActivePinia } from "pinia"; import EditContent from "./EditContent.vue"; @@ -508,9 +516,124 @@ describe("EditContent.vue - Duplication", () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const vm: any = wrapper.vm; - // Image fileCollections should be preserved by default and duplicate intent should be set + // Image fileCollections should be preserved by default, and the clone should name the + // document the API copies the image from. expect(vm.editableParent.imageData.fileCollections.length).toBeGreaterThan(0); - expect(vm.editableParent.imageData.duplicate).toBe(true); + expect(vm.editableParent.imageData.duplicateFrom).toBe(mockData.mockPostDto._id); + expect(vm.editableParent.imageData.duplicate).toBeUndefined(); + }, 15000); + + it("carries the image across when the source has no local storage bucket", async () => { + const mockNotification = vi.fn(); + const notificationStore = useNotificationStore(); + notificationStore.addNotification = mockNotification; + + // mockPostDto carries fileCollections but no imageBucketId — the legacy shape. The API + // resolves the bucket from the source document, so the copy no longer depends on it. + expect(mockData.mockPostDto.imageData?.fileCollections.length).toBeGreaterThan(0); + expect((mockData.mockPostDto as any).imageBucketId).toBeUndefined(); + + const wrapper = mount(EditContent, { + props: { + docType: DocType.Post, + id: mockData.mockPostDto._id, + languageCode: "eng", + tagOrPostType: PostType.Blog, + }, + }); + + await waitForExpect(() => { + expect(wrapper.text()).toContain("English"); + }); + + const dropdownTrigger = wrapper.find('[data-test="dropdown-trigger"]'); + await dropdownTrigger.trigger("click"); + await nextTick(); + + let duplicateBtn; + await waitForExpect(() => { + duplicateBtn = wrapper.find("[data-test='duplicate-button']"); + expect(duplicateBtn.exists()).toBe(true); + }); + + let confirmBtn; + await waitForExpect(async () => { + duplicateBtn!.trigger("click"); + confirmBtn = wrapper.find('[data-test="modal-primary-button"]'); + expect(confirmBtn.exists()).toBe(true); + }); + await confirmBtn!.trigger("click"); + + await waitForExpect(() => { + expect(mockNotification).toHaveBeenCalledWith( + expect.objectContaining({ title: "Successfully duplicated" }), + ); + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const vm: any = wrapper.vm; + expect(vm.editableParent.imageData.fileCollections.length).toBeGreaterThan(0); + expect(vm.editableParent.imageData.duplicateFrom).toBe(mockData.mockPostDto._id); + expect(mockNotification).not.toHaveBeenCalledWith( + expect.objectContaining({ title: "Image not copied" }), + ); + }, 15000); + + it("does not warn about the image when the source has a storage bucket", async () => { + await db.docs.put({ + ...mockData.mockPostDto, + imageBucketId: "storage-image-bucket", + } as any); + + const mockNotification = vi.fn(); + const notificationStore = useNotificationStore(); + notificationStore.addNotification = mockNotification; + + const wrapper = mount(EditContent, { + props: { + docType: DocType.Post, + id: mockData.mockPostDto._id, + languageCode: "eng", + tagOrPostType: PostType.Blog, + }, + }); + + await waitForExpect(() => { + expect(wrapper.text()).toContain("English"); + }); + + await waitForExpect(() => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const vm: any = wrapper.vm; + expect(vm.editableParent.imageBucketId).toBe("storage-image-bucket"); + }); + + const dropdownTrigger = wrapper.find('[data-test="dropdown-trigger"]'); + await dropdownTrigger.trigger("click"); + await nextTick(); + + let duplicateBtn; + await waitForExpect(() => { + duplicateBtn = wrapper.find("[data-test='duplicate-button']"); + expect(duplicateBtn.exists()).toBe(true); + }); + + let confirmBtn; + await waitForExpect(async () => { + duplicateBtn!.trigger("click"); + confirmBtn = wrapper.find('[data-test="modal-primary-button"]'); + expect(confirmBtn.exists()).toBe(true); + }); + await confirmBtn!.trigger("click"); + + await waitForExpect(() => { + expect(mockNotification).toHaveBeenCalledWith( + expect.objectContaining({ title: "Successfully duplicated" }), + ); + }); + expect(mockNotification).not.toHaveBeenCalledWith( + expect.objectContaining({ title: "Image not copied" }), + ); }, 15000); it("clears image fileCollections when duplicate image is unchecked", async () => { diff --git a/cms/src/components/content/util/buildContentDuplicate.ts b/cms/src/components/content/util/buildContentDuplicate.ts index 9ea383dbcc..4c0ebadccf 100644 --- a/cms/src/components/content/util/buildContentDuplicate.ts +++ b/cms/src/components/content/util/buildContentDuplicate.ts @@ -8,33 +8,41 @@ import { } from "luminary-shared"; import * as _ from "lodash"; +/** What happened to the source image when building the duplicate. */ +export type DuplicateImageOutcome = "copied" | "skipped" | "none"; + /** * Build unsaved duplicate clones of a content parent and its translations: fresh ids, * stripped `_rev`, drafted + "(Copy)"/"-copy"-suffixed children. Returns new objects; * the inputs are not mutated. * - * `duplicateImage` carries over a copyable image collection — only possible when the - * source actually has an image bucket; otherwise the collection is cleared. + * `duplicateImage` names the source on the clone so the API can copy its image across. The + * source bucket and files are resolved server-side, so a stale or missing local bucket + * reference cannot lose the image. */ export function buildContentDuplicate( parent: ContentParentDto, content: ContentDto[], options: { duplicateImage: boolean }, -): { parent: ContentParentDto; content: ContentDto[] } { +): { parent: ContentParentDto; content: ContentDto[]; imageOutcome: DuplicateImageOutcome } { const clonedParent = _.cloneDeep(parent); clonedParent._id = db.uuid(); delete (clonedParent as any)._rev; if (clonedParent.type === DocType.Tag) (clonedParent as TagDto).taggedDocs = []; + let imageOutcome: DuplicateImageOutcome = "none"; + if (clonedParent.imageData) { - const imageData = clonedParent.imageData as typeof clonedParent.imageData & { - duplicate?: boolean; - }; - delete clonedParent.imageData.uploadData; + const imageData = clonedParent.imageData; + delete imageData.uploadData; delete imageData.duplicate; - if (options.duplicateImage && imageData.fileCollections?.length > 0) { - if (parent.imageBucketId) imageData.duplicate = true; - else imageData.fileCollections = []; + delete imageData.duplicateFrom; + if (imageData.fileCollections?.length > 0 && options.duplicateImage) { + imageData.duplicateFrom = parent._id; + imageOutcome = "copied"; + } else if (imageData.fileCollections?.length > 0) { + imageData.fileCollections = []; + imageOutcome = "skipped"; } else if (imageData.fileCollections) { imageData.fileCollections = []; } @@ -55,5 +63,5 @@ export function buildContentDuplicate( return newContent; }); - return { parent: clonedParent, content: clonedContent }; + return { parent: clonedParent, content: clonedContent, imageOutcome }; } diff --git a/cms/src/components/images/ImageEditor.spec.ts b/cms/src/components/images/ImageEditor.spec.ts index caf7b1bce5..37b8cf3fd4 100644 --- a/cms/src/components/images/ImageEditor.spec.ts +++ b/cms/src/components/images/ImageEditor.spec.ts @@ -181,7 +181,13 @@ describe("ImageEditor", () => { const origBuckets = [...mockImageBuckets.value]; mockImageBuckets.value = [ ...origBuckets, - { _id: "bucket-2", name: "Second", publicUrl: "http://test2.com", storageType: "image", mimeTypes: ["image/*"] }, + { + _id: "bucket-2", + name: "Second", + publicUrl: "http://test2.com", + storageType: "image", + mimeTypes: ["image/*"], + }, ]; const parent: ContentParentDto = { @@ -195,7 +201,11 @@ describe("ImageEditor", () => { const component = wrapper.vm as any; const mockFile = new File(["img"], "test.jpg", { type: "image/jpeg" }); - const fileList = { 0: mockFile, length: 1, item: (i: number) => (i === 0 ? mockFile : null) }; + const fileList = { + 0: mockFile, + length: 1, + item: (i: number) => (i === 0 ? mockFile : null), + }; component.handleFiles(fileList); await wrapper.vm.$nextTick(); @@ -227,6 +237,47 @@ describe("ImageEditor", () => { mockImageBuckets.value = origBuckets; }); + it("keeps an imageBucketId the user cannot resolve, and reports it", async () => { + const parent: ContentParentDto = { + ...JSON.parse(JSON.stringify(mockPostDto)), + imageBucketId: "bucket-in-a-group-this-user-cannot-view", + }; + + const wrapper = mount(ImageEditor, { + props: { parent, disabled: false }, + }); + await wrapper.vm.$nextTick(); + + expect(parent.imageBucketId).toBe("bucket-in-a-group-this-user-cannot-view"); + expect(parent.imageData!.fileCollections.length).toBeGreaterThan(0); + expect(wrapper.text()).toContain("bucket you don't have access to"); + }); + + it("refuses an upload to an unresolved bucket without discarding the reference", async () => { + const parent: ContentParentDto = { + ...JSON.parse(JSON.stringify(mockPostDto)), + imageBucketId: "bucket-in-a-group-this-user-cannot-view", + }; + + const wrapper = mount(ImageEditor, { + props: { parent, disabled: false }, + }); + const component = wrapper.vm as any; + const mockFile = new File(["img"], "test.jpg", { type: "image/jpeg" }); + const fileList = { + 0: mockFile, + length: 1, + item: (i: number) => (i === 0 ? mockFile : null), + }; + + component.handleFiles(fileList); + await wrapper.vm.$nextTick(); + + expect(parent.imageBucketId).toBe("bucket-in-a-group-this-user-cannot-view"); + expect(parent.imageData!.uploadData).toBeUndefined(); + expect(wrapper.text()).toContain("bucket you don't have access to"); + }); + it("processFiles adds upload data to parent", async () => { const parent: ContentParentDto = { ...mockPostDto, @@ -241,7 +292,11 @@ describe("ImageEditor", () => { const mockFile = new File(["image-data"], "test.jpg", { type: "image/jpeg" }); Object.defineProperty(mockFile, "size", { value: 1024 }); // Small file - const fileList = { 0: mockFile, length: 1, item: (i: number) => (i === 0 ? mockFile : null) }; + const fileList = { + 0: mockFile, + length: 1, + item: (i: number) => (i === 0 ? mockFile : null), + }; const component = wrapper.vm as any; component.handleFiles(fileList); @@ -268,7 +323,11 @@ describe("ImageEditor", () => { const mockFile = new File(["x"], "huge.jpg", { type: "image/jpeg" }); Object.defineProperty(mockFile, "size", { value: largeSize }); - const fileList = { 0: mockFile, length: 1, item: (i: number) => (i === 0 ? mockFile : null) }; + const fileList = { + 0: mockFile, + length: 1, + item: (i: number) => (i === 0 ? mockFile : null), + }; const component = wrapper.vm as any; component.handleFiles(fileList); @@ -294,7 +353,11 @@ describe("ImageEditor", () => { // An actual upload is a real user edit and should persist the effective bucket. const mockFile = new File(["x"], "img.jpg", { type: "image/jpeg" }); Object.defineProperty(mockFile, "size", { value: 1024 }); - const fileList = { 0: mockFile, length: 1, item: (i: number) => (i === 0 ? mockFile : null) }; + const fileList = { + 0: mockFile, + length: 1, + item: (i: number) => (i === 0 ? mockFile : null), + }; (wrapper.vm as any).handleFiles(fileList); await wrapper.vm.$nextTick(); diff --git a/cms/src/components/images/ImageEditor.vue b/cms/src/components/images/ImageEditor.vue index 24bac8d320..1db600633f 100644 --- a/cms/src/components/images/ImageEditor.vue +++ b/cms/src/components/images/ImageEditor.vue @@ -26,6 +26,9 @@ const emit = defineEmits<{ const parent = defineModel("parent"); const maxUploadFileSizeMb = computed(() => maxUploadFileSize.value / 1000000); +const UNRESOLVED_BUCKET_MESSAGE = + "This image is stored in a bucket you don't have access to. The image is unaffected, but you cannot upload a new one until a bucket you can access is selected."; + // Bucket selection (simplified approach using existing database data) const bucketSelection = storageSelection(); @@ -112,31 +115,26 @@ const dragCounter = ref(0); const showFailureMessage = ref(false); const failureMessage = ref(undefined); -// Validate that selected bucket still exists and auto-select if only one available -watchEffect(() => { - // Check if the currently selected bucket still exists in the database - if (parent.value?.imageBucketId) { - // Only validate if buckets have loaded (array is not empty) - // This prevents clearing imageBucketId while buckets are still loading from IndexedDB - if (bucketSelection.imageBuckets.value.length > 0) { - const currentBucketExists = bucketSelection.imageBuckets.value.some( - (b) => b._id === parent.value?.imageBucketId, - ); - - // If the bucket no longer exists, clear it - if (!currentBucketExists) { - parent.value.imageBucketId = undefined; - } - } - } +// A bucket absent from the list is either deleted or in a group this user cannot view — +// the two are indistinguishable here, so the reference is reported and never discarded. +// Dropping it would strip the image from the document on the next save. +const bucketIsUnresolved = computed( + () => + !!parent.value?.imageBucketId && + bucketSelection.imageBuckets.value.length > 0 && + !bucketSelection.imageBuckets.value.some((b) => b._id === parent.value?.imageBucketId), +); - // Proactively show error messages for bucket configuration issues - // This ensures users see the error immediately, not just when they try to upload +// Surface bucket configuration issues on load rather than only on upload. +watchEffect(() => { if (!bucketSelection.hasImageBuckets.value) { // No buckets configured at all failureMessage.value = "No storage buckets configured. Please configure at least one S3 bucket in the Storage settings before uploading images."; showFailureMessage.value = true; + } else if (bucketIsUnresolved.value) { + failureMessage.value = UNRESOLVED_BUCKET_MESSAGE; + showFailureMessage.value = true; } else if (!effectiveImageBucketId.value && bucketSelection.imageBuckets.value.length > 1) { // Multiple buckets available but none selected failureMessage.value = "Please select a storage bucket before uploading images."; @@ -147,7 +145,7 @@ watchEffect(() => { const bucketRelatedErrors = [ "No storage buckets configured. Please configure at least one S3 bucket in the Storage settings before uploading images.", "Please select a storage bucket before uploading images.", - "The selected storage bucket no longer exists. Please select another bucket.", + UNRESOLVED_BUCKET_MESSAGE, ]; if (failureMessage.value && bucketRelatedErrors.includes(failureMessage.value)) { failureMessage.value = undefined; @@ -174,15 +172,10 @@ const handleFiles = (files: FileList | null) => { emit("bucketSelected", effectiveImageBucketId.value); } - // Check if the currently selected bucket still exists in the database - const currentBucketExists = bucketSelection.imageBuckets.value.some( - (b) => b._id === parent.value?.imageBucketId, - ); - - if (!currentBucketExists) { - if (parent.value) parent.value.imageBucketId = undefined; - failureMessage.value = - "The selected storage bucket no longer exists. Please select another bucket."; + // An upload needs a bucket this user can write to, so it is refused — but the existing + // reference is left alone; selecting another bucket is what replaces it. + if (bucketIsUnresolved.value) { + failureMessage.value = UNRESOLVED_BUCKET_MESSAGE; showFailureMessage.value = true; return; } diff --git a/shared/src/types/dto.ts b/shared/src/types/dto.ts index bf5e4f0f7b..66984b1383 100644 --- a/shared/src/types/dto.ts +++ b/shared/src/types/dto.ts @@ -183,6 +183,12 @@ export type GroupDto = BaseDocumentDto & { export type ImageDto = { fileCollections: ImageFileCollectionDto[]; uploadData?: ImageUploadDto[]; + /** + * Id of the parent this image is being copied from. The API resolves the source bucket and + * files from that document, so the copy does not depend on the client's own state. + */ + duplicateFrom?: Uuid; + /** @deprecated Superseded by `duplicateFrom`; still accepted from older clients. */ duplicate?: boolean; };