From 4e1bbd3bcc1ca83e375660e7d7c97e49d7ff8112 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 3 Sep 2026 16:23:40 +0300 Subject: [PATCH 1/4] Claim attachments referenced in the description on work package create A new work package created with `_links.attachments: []` but a description embedding an uploaded image left that attachment uncontainered, so the cleanup job deleted it. Uncontainered attachments of the current user that the description references are now claimed alongside any explicit list. --- .../attachments/claimable_ids_from_text.rb | 44 ++++++ .../work_packages/set_attributes_service.rb | 11 ++ .../v3/work_packages/create_resource_spec.rb | 84 +++++++++++ .../claimable_ids_from_text_spec.rb | 99 +++++++++++++ ...vice_description_attachment_claims_spec.rb | 131 ++++++++++++++++++ 5 files changed, 369 insertions(+) create mode 100644 app/services/attachments/claimable_ids_from_text.rb create mode 100644 spec/services/attachments/claimable_ids_from_text_spec.rb create mode 100644 spec/services/work_packages/set_attributes_service_description_attachment_claims_spec.rb diff --git a/app/services/attachments/claimable_ids_from_text.rb b/app/services/attachments/claimable_ids_from_text.rb new file mode 100644 index 000000000000..97dd35198c7e --- /dev/null +++ b/app/services/attachments/claimable_ids_from_text.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Attachments + module ClaimableIdsFromText + REFERENCE_REGEX = %r{/attachments/(\d+)/content} + + module_function + + def call(text, user:) + ids = text.to_s.scan(REFERENCE_REGEX).flatten.map(&:to_i).uniq + return [] if ids.empty? + + Attachment.where(id: ids, container: nil, author: user).pluck(:id) + end + end +end diff --git a/app/services/work_packages/set_attributes_service.rb b/app/services/work_packages/set_attributes_service.rb index faecb1066926..74ecbcba7dd8 100644 --- a/app/services/work_packages/set_attributes_service.rb +++ b/app/services/work_packages/set_attributes_service.rb @@ -44,6 +44,7 @@ def set_attributes(attributes) validate_custom_fields = attributes.delete(:validate_custom_fields) set_attachments_attributes(attributes) + claim_attachments_referenced_in_description(attributes) set_versions_attributes(attributes) set_static_attributes(attributes) @@ -65,6 +66,16 @@ def set_custom_values_to_validate(attributes, validate_custom_fields = nil) end end + def claim_attachments_referenced_in_description(attributes) + return unless model.new_record? && attributes.key?(:description) + + claimable_ids = Attachments::ClaimableIdsFromText.call(attributes[:description], user:) + return if claimable_ids.empty? + + explicit_ids = model.attachments_replacements&.ids || [] + model.attachments_replacements = Attachment.where(id: explicit_ids | claimable_ids) + end + def set_versions_attributes(attributes) target_ids = attributes.delete(:target_version_ids) observed_in_ids = attributes.delete(:observed_in_version_ids) diff --git a/spec/requests/api/v3/work_packages/create_resource_spec.rb b/spec/requests/api/v3/work_packages/create_resource_spec.rb index b0eea3735b7b..6acd44e9587c 100644 --- a/spec/requests/api/v3/work_packages/create_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/create_resource_spec.rb @@ -528,6 +528,90 @@ end end + context "when attachments are referenced in the description" do + let(:attachment) { create(:attachment, container: nil, author: current_user) } + let(:parameters) do + { + subject: "subject", + description: { + raw: %() + }, + _links: { + type: { + href: api_v3_paths.type(project.enabled_types.first.id) + }, + project: { + href: api_v3_paths.project(project.id) + }, + attachments: [] + } + } + end + + it "creates the work package, claims the attachment and journals it" do + expect(last_response).to have_http_status(:created) + + work_package = WorkPackage.last + expect(work_package.attachments).to match_array(attachment) + expect(attachment.reload.container).to eq(work_package) + expect(work_package.journals.first.attachable_journals.map(&:attachment_id)) + .to contain_exactly(attachment.id) + end + + context "and the referenced attachment belongs to another user" do + let(:attachment) { create(:attachment, container: nil, author: create(:user)) } + + it "creates the work package without claiming the attachment" do + expect(last_response).to have_http_status(:created) + + expect(WorkPackage.last.attachments).to be_empty + expect(attachment.reload.container).to be_nil + end + end + + context "and the referenced attachment is already containered in another work package" do + let(:attachment) do + create(:attachment, container: create(:work_package, project:), author: current_user) + end + + it "creates the work package without claiming the attachment" do + expect(last_response).to have_http_status(:created) + + expect(WorkPackage.last.attachments).to be_empty + expect(attachment.reload.container).not_to eq(WorkPackage.last) + end + end + + context "and attachment_ids explicitly names another attachment" do + let(:explicitly_claimed_attachment) { create(:attachment, container: nil, author: current_user) } + let(:parameters) do + { + subject: "subject", + description: { + raw: %() + }, + _links: { + type: { + href: api_v3_paths.type(project.enabled_types.first.id) + }, + project: { + href: api_v3_paths.project(project.id) + }, + attachments: [ + { href: api_v3_paths.attachment(explicitly_claimed_attachment.id) } + ] + } + } + end + + it "claims both the explicit and the description-referenced attachments" do + expect(last_response).to have_http_status(:created) + + expect(WorkPackage.last.attachments).to contain_exactly(attachment, explicitly_claimed_attachment) + end + end + end + context "when file links are being claimed" do let(:storage) { create(:nextcloud_storage) } let(:project_storage) { create(:project_storage, project:, storage:) } diff --git a/spec/services/attachments/claimable_ids_from_text_spec.rb b/spec/services/attachments/claimable_ids_from_text_spec.rb new file mode 100644 index 000000000000..fca9849fc587 --- /dev/null +++ b/spec/services/attachments/claimable_ids_from_text_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Attachments::ClaimableIdsFromText do + shared_let(:user) { create(:user) } + shared_let(:other_user) { create(:user) } + shared_let(:own_uncontainered) { create(:attachment, author: user, container: nil) } + + subject(:claimable_ids) { described_class.call(text, user:) } + + context "when the text is blank" do + let(:text) { "" } + + it "returns an empty array without querying" do + allow(Attachment).to receive(:where) + + expect(claimable_ids).to eq([]) + expect(Attachment).not_to have_received(:where) + end + end + + context "when the text is nil" do + let(:text) { nil } + + it "returns an empty array without querying" do + allow(Attachment).to receive(:where) + + expect(claimable_ids).to eq([]) + expect(Attachment).not_to have_received(:where) + end + end + + context "when the text references an inline image attachment" do + let(:text) { %() } + + it { is_expected.to contain_exactly(own_uncontainered.id) } + end + + context "when the text references an attachment via markdown" do + let(:text) { "![](/api/v3/attachments/#{own_uncontainered.id}/content)" } + + it { is_expected.to contain_exactly(own_uncontainered.id) } + end + + context "when the same attachment is referenced multiple times" do + let(:text) do + "#{own_uncontainered.id} " \ + "![](/api/v3/attachments/#{own_uncontainered.id}/content) " \ + "" + end + + it { is_expected.to contain_exactly(own_uncontainered.id) } + end + + context "when the attachment belongs to another user" do + shared_let(:other_users_attachment) { create(:attachment, author: other_user, container: nil) } + + let(:text) { "![](/api/v3/attachments/#{other_users_attachment.id}/content)" } + + it { is_expected.to eq([]) } + end + + context "when the attachment is already containered elsewhere" do + shared_let(:containered_attachment) { create(:attachment, author: user, container: create(:work_package)) } + + let(:text) { "![](/api/v3/attachments/#{containered_attachment.id}/content)" } + + it { is_expected.to eq([]) } + end +end diff --git a/spec/services/work_packages/set_attributes_service_description_attachment_claims_spec.rb b/spec/services/work_packages/set_attributes_service_description_attachment_claims_spec.rb new file mode 100644 index 000000000000..732eec379b30 --- /dev/null +++ b/spec/services/work_packages/set_attributes_service_description_attachment_claims_spec.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe WorkPackages::SetAttributesService do + shared_let(:user) { create(:user) } + shared_let(:other_user) { create(:user) } + + subject(:service_result) do + described_class + .new(user:, model: work_package, contract_class: EmptyContract) + .call(params) + end + + context "when the work package is new" do + let(:work_package) { WorkPackage.new } + + context "and the description references an own uncontainered attachment" do + shared_let(:attachment) { create(:attachment, author: user, container: nil) } + + let(:params) do + { description: %() } + end + + it "claims the attachment" do + expect(service_result.result.attachments_replacements).to contain_exactly(attachment) + end + end + + context "and attachment_ids is explicitly empty alongside a description reference" do + shared_let(:attachment) { create(:attachment, author: user, container: nil) } + + let(:params) do + { + attachment_ids: [], + description: "![](/api/v3/attachments/#{attachment.id}/content)" + } + end + + it "claims the attachment referenced in the description" do + expect(service_result.result.attachments_replacements).to contain_exactly(attachment) + end + end + + context "and attachment_ids already names another attachment alongside a description reference" do + shared_let(:explicit_attachment) { create(:attachment, author: user, container: nil) } + shared_let(:described_attachment) { create(:attachment, author: user, container: nil) } + + let(:params) do + { + attachment_ids: [explicit_attachment.id], + description: "![](/api/v3/attachments/#{described_attachment.id}/content)" + } + end + + it "unions the explicit and description-referenced attachments" do + expect(service_result.result.attachments_replacements) + .to contain_exactly(explicit_attachment, described_attachment) + end + end + + context "and the params carry no description key" do + let(:params) { { subject: "no description here" } } + + it "leaves attachments_replacements untouched" do + expect(service_result.result.attachments_replacements).to be_nil + end + end + + context "and the description references nothing" do + let(:params) { { description: "Lorem ipsum dolor sit amet" } } + + it "leaves attachments_replacements untouched" do + expect(service_result.result.attachments_replacements).to be_nil + end + end + + context "and the description references another user's uncontainered attachment" do + shared_let(:other_users_attachment) { create(:attachment, author: other_user, container: nil) } + + let(:params) do + { description: "![](/api/v3/attachments/#{other_users_attachment.id}/content)" } + end + + it "does not claim it" do + expect(service_result.result.attachments_replacements).to be_nil + end + end + end + + context "when the work package is persisted" do + shared_let(:work_package) { create(:work_package, author: user) } + shared_let(:attachment) { create(:attachment, author: user, container: nil) } + + let(:params) do + { description: "![](/api/v3/attachments/#{attachment.id}/content)" } + end + + it "does not claim attachments referenced in the description" do + expect(service_result.result.attachments_replacements).to be_nil + end + end +end From 243b5f7632144f4248503e83e9190e6e3196a4e7 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 3 Sep 2026 16:24:04 +0300 Subject: [PATCH 2/4] Mirror uploaded attachments into new resources without the attachment list The store-to-resource sync lived only in op-attachments, so with the attachments list hidden a CKEditor upload on a new work package never reached the create payload. --- .../attachments/attachments.service.spec.ts | 64 ++++++++++++++++++- .../state/attachments/attachments.service.ts | 16 +++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/core/state/attachments/attachments.service.spec.ts b/frontend/src/app/core/state/attachments/attachments.service.spec.ts index 3418a80d7e5b..14f2b5f95dca 100644 --- a/frontend/src/app/core/state/attachments/attachments.service.spec.ts +++ b/frontend/src/app/core/state/attachments/attachments.service.spec.ts @@ -27,28 +27,86 @@ //++ import { TestBed } from '@angular/core/testing'; -import { provideHttpClient, withInterceptorsFromDi, withXhr } from '@angular/common/http'; +import { + HttpResponse, + provideHttpClient, + withInterceptorsFromDi, + withXhr, +} from '@angular/common/http'; import { provideHttpClientTesting } from '@angular/common/http/testing'; +import { firstValueFrom, of } from 'rxjs'; import { States } from 'core-app/core/states/states.service'; import { ConfigurationService } from 'core-app/core/config/configuration.service'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; import { OpUploadService } from 'core-app/core/upload/upload.service'; +import { ToastService } from 'core-app/shared/components/toaster/toast.service'; +import { HalResource } from 'core-app/features/hal/resources/hal-resource'; +import { IAttachment } from 'core-app/core/state/attachments/attachment.model'; import { AttachmentsResourceService } from './attachments.service'; describe('AttachmentsResourceService', () => { + let service:AttachmentsResourceService; + + const attachment = { + id: '42', + fileName: 'a.png', + _links: { + self: { href: '/api/v3/attachments/42' }, + delete: { href: '/api/v3/attachments/42' }, + }, + } as unknown as IAttachment; + beforeEach(() => { TestBed.configureTestingModule({ providers: [ AttachmentsResourceService, { provide: States, useValue: new States() }, { provide: ConfigurationService, useValue: {} }, - { provide: OpUploadService, useValue: {} }, + { provide: I18nService, useValue: { t: () => '' } }, + { provide: ToastService, useValue: { addUpload: vi.fn() } }, + { + provide: OpUploadService, + useValue: { upload: vi.fn(() => [of(new HttpResponse({ body: attachment }))]) }, + }, provideHttpClient(withXhr(), withInterceptorsFromDi()), provideHttpClientTesting(), ], }); + + service = TestBed.inject(AttachmentsResourceService); }); it('initialises via dependency injection', () => { - expect(TestBed.inject(AttachmentsResourceService)).toBeTruthy(); + expect(service).toBeTruthy(); + }); + + describe('attachFiles', () => { + it('mirrors uploaded attachments into a new resource', async () => { + const resource = { + $source: { id: 'new' }, + id: 'new', + $links: {}, + attachments: { elements: [] }, + } as unknown as HalResource; + + await firstValueFrom(service.attachFiles(resource, [new File([''], 'a.png')])); + + expect(resource.attachments).toEqual({ elements: [{ href: '/api/v3/attachments/42' }] }); + }); + + it('leaves the attachments link of a persisted resource untouched', async () => { + const attachments = { href: '/api/v3/work_packages/5/attachments' }; + const resource = { + $source: { id: '5' }, + id: '5', + $links: {}, + attachments, + addAttachment: { href: '/api/v3/work_packages/5/attachments' }, + } as unknown as HalResource; + + await firstValueFrom(service.attachFiles(resource, [new File([''], 'a.png')])); + + expect(resource.attachments).toBe(attachments); + }); }); }); diff --git a/frontend/src/app/core/state/attachments/attachments.service.ts b/frontend/src/app/core/state/attachments/attachments.service.ts index b4102adc69fa..86516f85d6b8 100644 --- a/frontend/src/app/core/state/attachments/attachments.service.ts +++ b/frontend/src/app/core/state/attachments/attachments.service.ts @@ -104,6 +104,13 @@ export class AttachmentsResourceService extends ResourceStoreService { + if (isNewResource(resource)) { + this.syncNewResourceAttachments(resource); + } + }), ); } @@ -150,6 +157,15 @@ export class AttachmentsResourceService extends ResourceStoreService this.query.getEntity(id)) + .filter((attachment):attachment is IAttachment => !!attachment); + + resource.attachments = { elements: attachments.map((attachment) => attachment._links.self) }; + } + private uploadAttachments(href:string, files:IUploadFile[]):Observable { const observables = this.uploadService.upload(href, files); const uploads = files.map((f, i):[File, Observable>] => [f.file, observables[i]]); From 9ce79c2f245a25a6201e2c61718d2de498756da5 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 3 Sep 2026 16:48:29 +0300 Subject: [PATCH 3/4] Reuse the attachment reference parser for comment attachment claims Both work package descriptions and comments now resolve claimable attachments through Attachments::ClaimableIdsFromText, which also picks up markdown image references the CSS selector missed. --- .../attachments/claimable_ids_from_text.rb | 11 ++++- .../set_attributes_service.rb | 39 +-------------- .../claimable_ids_from_text_spec.rb | 49 +++++++++++++++++++ 3 files changed, 59 insertions(+), 40 deletions(-) diff --git a/app/services/attachments/claimable_ids_from_text.rb b/app/services/attachments/claimable_ids_from_text.rb index 97dd35198c7e..acc5ad2f10b2 100644 --- a/app/services/attachments/claimable_ids_from_text.rb +++ b/app/services/attachments/claimable_ids_from_text.rb @@ -34,11 +34,18 @@ module ClaimableIdsFromText module_function - def call(text, user:) + def call(text, user:, container: nil) ids = text.to_s.scan(REFERENCE_REGEX).flatten.map(&:to_i).uniq return [] if ids.empty? - Attachment.where(id: ids, container: nil, author: user).pluck(:id) + claimable_scope(container).where(id: ids, author: user).pluck(:id) + end + + def claimable_scope(container) + uncontainered = Attachment.where(container: nil) + return uncontainered if container.nil? || container.new_record? + + uncontainered.or(Attachment.where(container:)) end end end diff --git a/app/services/work_packages/activities_tab/comment_attachments_claims/set_attributes_service.rb b/app/services/work_packages/activities_tab/comment_attachments_claims/set_attributes_service.rb index f4a59ff12dbd..8bf7db74a47b 100644 --- a/app/services/work_packages/activities_tab/comment_attachments_claims/set_attributes_service.rb +++ b/app/services/work_packages/activities_tab/comment_attachments_claims/set_attributes_service.rb @@ -34,49 +34,12 @@ module CommentAttachmentsClaims class SetAttributesService < ::BaseServices::SetAttributes include ::Attachments::SetReplacements - ATTACHMENT_CSS_SELECTOR = "img.op-uc-image" - def perform - ids_from_notes = collect_attachment_ids_from_notes - claimable_ids = filter_claimable_attachment_ids(ids_from_notes) + claimable_ids = Attachments::ClaimableIdsFromText.call(model.notes, user: User.current, container: model) self.params = params.reverse_merge(attachment_ids: claimable_ids) super end - - private - - def collect_attachment_ids_from_notes - return [] if model.notes.blank? - - parser.css(ATTACHMENT_CSS_SELECTOR).filter_map do |img| - src = img["src"] - next if src.blank? - - # Extract the attachment ID from the src URL - # Example: "/api/v3/attachments/30381/content" -> "30381" - match = src.match(%r{/attachments/(\d+)/content}) - match[1] if match - end - end - - def filter_claimable_attachment_ids(ids) - return [] if ids.blank? - - # Only claim attachments that are actually claimable. We must not try to - # reassign attachments that are already attached to another container - # (e.g., the work package, or another comment), and we must only claim unattached files of - # the current user to satisfy validation rules. - Attachment - .where(container: nil) - .or(Attachment.where(container: model)) - .where(id: ids, author: User.current) - .pluck(:id) - end - - def parser - @parser ||= Nokogiri::HTML.fragment(model.notes) - end end end end diff --git a/spec/services/attachments/claimable_ids_from_text_spec.rb b/spec/services/attachments/claimable_ids_from_text_spec.rb index fca9849fc587..fa023af058f5 100644 --- a/spec/services/attachments/claimable_ids_from_text_spec.rb +++ b/spec/services/attachments/claimable_ids_from_text_spec.rb @@ -96,4 +96,53 @@ it { is_expected.to eq([]) } end + + context "when a container is given" do + subject(:claimable_ids) { described_class.call(text, user:, container:) } + + shared_let(:work_package) { create(:work_package) } + shared_let(:containered_in_given_container) { create(:attachment, author: user, container: work_package) } + shared_let(:containered_elsewhere) { create(:attachment, author: user, container: create(:work_package)) } + + let(:container) { work_package } + let(:text) do + "![](/api/v3/attachments/#{own_uncontainered.id}/content) " \ + "![](/api/v3/attachments/#{containered_in_given_container.id}/content) " \ + "![](/api/v3/attachments/#{containered_elsewhere.id}/content)" + end + + it "includes attachments already in that container alongside the still-uncontainered ones" do + expect(claimable_ids).to contain_exactly(own_uncontainered.id, containered_in_given_container.id) + end + + context "when the text also references another user's uncontainered attachment" do + shared_let(:other_users_attachment) { create(:attachment, author: other_user, container: nil) } + + let(:text) do + "![](/api/v3/attachments/#{own_uncontainered.id}/content) " \ + "![](/api/v3/attachments/#{containered_in_given_container.id}/content) " \ + "![](/api/v3/attachments/#{other_users_attachment.id}/content)" + end + + it "excludes the other user's attachment" do + expect(claimable_ids).to contain_exactly(own_uncontainered.id, containered_in_given_container.id) + end + end + + context "when the container is a new record" do + let(:container) { WorkPackage.new } + + it "behaves like no container was given" do + expect(claimable_ids).to contain_exactly(own_uncontainered.id) + end + end + + context "when the container is nil" do + let(:container) { nil } + + it "behaves like the container kwarg was omitted" do + expect(claimable_ids).to contain_exactly(own_uncontainered.id) + end + end + end end From b8aac34f0cc9657d8ea3b444d02eef733e5bea98 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 3 Sep 2026 17:10:04 +0300 Subject: [PATCH 4/4] Cover the description image upload with the attachments list hidden Feature spec for the ticket flow: attachments list deactivated, image uploaded through CKEditor on a new work package, attachment claimed on save. --- .../attachments/attachment_upload_spec.rb | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/spec/features/work_packages/attachments/attachment_upload_spec.rb b/spec/features/work_packages/attachments/attachment_upload_spec.rb index 9bb1984c13e9..cc27dec317fd 100644 --- a/spec/features/work_packages/attachments/attachment_upload_spec.rb +++ b/spec/features/work_packages/attachments/attachment_upload_spec.rb @@ -141,6 +141,49 @@ wp.reload expect(wp.attachments.count).to eq(1) end + + context "with the attachments list hidden" do + let!(:project) do + create(:project, types: [type], deactivate_work_package_attachments: true) + end + + it "claims the image uploaded in the description (Regression COMMS-890)" do + table.visit! + new_page = table.create_wp_by_button type + subject = new_page.edit_field :subject + subject.set_value "My subject" + + expect(page).to have_no_css("op-attachments") + + target = find(".ck-content") + attachments.drag_and_drop_file(target, image_fixture.path) + + sleep 2 unless using_cuprite? # rubocop:disable OpenProject/NoSleepInFeatureSpecs + editor.wait_until_upload_progress_toaster_cleared + + editor.in_editor do |_container, editable| + expect(editable).to have_css('img[src*="/api/v3/attachments/"]', wait: 20) + expect(editable).to have_no_css(".ck-upload-placeholder-loader") + end + + sleep 2 unless using_cuprite? # rubocop:disable OpenProject/NoSleepInFeatureSpecs + + scroll_to_and_click find_by_id("work-packages--edit-actions-save") + + new_page.expect_and_dismiss_toaster( + message: "Successful creation." + ) + + split_view = Pages::SplitWorkPackage.new(WorkPackage.last) + + field = split_view.edit_field :description + expect(field.display_element).to have_css("img") + + wp = WorkPackage.last + expect(wp.attachments.count).to eq(1) + expect(wp.attachments.first.container).to eq(wp) + end + end end context "when on a new page" do