[34446] Fix direct upload with backend-generated resources

https://community.openproject.com/wp/34446

frontend/src/app/modules/hal/resources/hal-resource.ts

@@ -161,7 +161,7 @@ export class HalResource {
   }
 
   public get isNew():boolean {
-    return this.id === 'new';
+    return !this.id || this.id === 'new';
   }
 
   public get persisted() {

frontend/src/app/modules/hal/resources/mixins/attachable-mixin.ts

@@ -172,7 +172,7 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
     }
 
     private performUpload(files:UploadFile[]) {
-      let href: string = this.directUploadURL || '';
+      let href:string = this.directUploadURL || '';
 
       if (href) {
         return this.opDirectFileUpload.uploadAndMapResponse(href, files);
@@ -191,7 +191,7 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
       }
 
       if (this.isNew) {
-        return this.config.prepareAttachmentURL
+        return this.config.prepareAttachmentURL;
       } else {
         return null;
       }

lib/api/v3/attachments/attachable_representer_mixin.rb

@@ -46,6 +46,9 @@ module API
           link :prepareAttachment do
             next unless OpenProject::Configuration.direct_uploads?
 
+            # We may not generate this link for new resources
+            next if represented.new_record?
+
             {
               href: attachments_by_resource + '/prepare',
               method: :post

modules/documents/spec/features/attachment_upload_spec.rb

@@ -48,40 +48,53 @@ describe 'Upload attachment to documents', js: true do
     login_as(user)
   end
 
-  it 'can upload an image' do
+  shared_examples 'can upload an image' do
-    visit project_documents_path(project)
+    it 'can upload an image' do
+      visit project_documents_path(project)
 
-    within '.toolbar-items' do
+      within '.toolbar-items' do
-      click_on 'Document'
+        click_on 'Document'
-    end
+      end
+
+      select(category.name, from: 'Category')
+      fill_in "Title", with: 'New documentation'
+
+      # adding an image
+      editor.drag_attachment image_fixture, 'Image uploaded on creation'
+      expect(page).to have_selector('attachment-list-item', text: 'image.png')
 
-    select(category.name, from: 'Category')
+      click_on 'Create'
-    fill_in "Title", with: 'New documentation'
 
-    # adding an image
+      expect(page).to have_selector('#content img', count: 1)
-    editor.drag_attachment image_fixture, 'Image uploaded on creation'
+      expect(page).to have_content('Image uploaded on creation')
 
-    expect(page).to have_selector('attachment-list-item', text: 'image.png')
+      click_on 'New documentation'
 
-    click_on 'Create'
+      within '.toolbar-items' do
+        click_on 'Edit'
+      end
 
-    expect(page).to have_selector('#content img', count: 1)
+      editor.drag_attachment image_fixture, 'Image uploaded the second time'
-    expect(page).to have_content('Image uploaded on creation')
+      expect(page).to have_selector('attachment-list-item', text: 'image.png', count: 2)
 
-    click_on 'New documentation'
+      click_on 'Save'
 
-    within '.toolbar-items' do
+      expect(page).to have_selector('#content img', count: 2)
-      click_on 'Edit'
+      expect(page).to have_content('Image uploaded on creation')
+      expect(page).to have_content('Image uploaded the second time')
+      expect(page).to have_selector('attachment-list-item', text: 'image.png', count: 2)
     end
+  end
 
-    editor.drag_attachment image_fixture, 'Image uploaded the second time'
+  context 'with direct uploads (Regression #34285)', with_direct_uploads: true do
-    expect(page).to have_selector('attachment-list-item', text: 'image.png', count: 2)
+    before do
+      allow_any_instance_of(Attachment).to receive(:diskfile).and_return Struct.new(:path).new(image_fixture.to_s)
+    end
 
-    click_on 'Save'
+    it_behaves_like 'can upload an image'
+  end
 
-    expect(page).to have_selector('#content img', count: 2)
+  context 'internal upload', with_direct_uploads: false do
-    expect(page).to have_content('Image uploaded on creation')
+    it_behaves_like 'can upload an image'
-    expect(page).to have_content('Image uploaded the second time')
-    expect(page).to have_selector('attachment-list-item', text: 'image.png', count: 2)
   end
 end

spec/features/admin/attribute_help_texts_spec.rb

@@ -28,7 +28,7 @@
 
 require 'spec_helper'
 
-describe 'Attribute help texts' do
+describe 'Attribute help texts', js: true do
   using_shared_fixtures :admin
 
   let(:instance) { AttributeHelpText.last }
@@ -46,8 +46,28 @@ describe 'Attribute help texts' do
       visit attribute_help_texts_path
     end
 
+    context 'with direct uploads (Regression #34285)', with_direct_uploads: true do
+      before do
+        allow_any_instance_of(Attachment).to receive(:diskfile).and_return Struct.new(:path).new(image_fixture.to_s)
+      end
+
+      it 'can upload an image' do
+        page.find('.attribute-help-texts--create-button').click
+        select 'Status', from: 'attribute_help_text_attribute_name'
+
+        editor.set_markdown('My attribute help text')
+        editor.drag_attachment image_fixture, 'Image uploaded on creation'
+
+        expect(page).to have_selector('attachment-list-item', text: 'image.png')
+        click_button 'Save'
+
+        expect(instance.help_text).to include 'My attribute help text'
+        expect(instance.help_text).to match /\/api\/v3\/attachments\/\d+\/content/
+      end
+    end
+
     context 'with help texts allowed by the enterprise token' do
-      it 'allows CRUD to attribute help texts', js: true do
+      it 'allows CRUD to attribute help texts' do
         expect(page).to have_selector('.generic-table--no-results-container')
 
         # Create help text

spec/support/shared/with_direct_uploads.rb

@@ -66,7 +66,7 @@ class WithDirectUploads
     example.metadata[:driver] = :headless_firefox_billy
 
     csp_config = SecureHeaders::Configuration.instance_variable_get("@default_config").csp
-    csp_config.connect_src = ["'self'", "my-bucket.s3.amazonaws.com"]
+    csp_config.connect_src = ["'self'", "test-bucket.s3.amazonaws.com"]
 
     begin
       example.run