Merge pull request #3526 from AlchemyCMS/fix-overlay-uploader

Fix overlay uploader

Author: tvdeyen

app/javascript/alchemy_admin/components/uploader.js

@@ -21,6 +21,32 @@ export class Uploader extends AlchemyHTMLElement {
     if (this.dropzone) {
       this._dragAndDropBehavior()
     }
+    this.addEventListener("Alchemy.upload.successful", this)
+  }
+
+  handleEvent(evt) {
+    switch (evt.type) {
+      case "Alchemy.upload.successful":
+        this._handleUploadComplete()
+        break
+    }
+  }
+
+  _handleUploadComplete() {
+    setTimeout(() => {
+      const url = this.redirectUrl
+      const turboFrame = this.closest("turbo-frame")
+      this.uploadProgress.visible = false
+
+      if (!url) return
+
+      if (turboFrame) {
+        turboFrame.setAttribute("src", url)
+        turboFrame.reload()
+      } else {
+        Turbo.visit(url)
+      }
+    }, 750)
   }
 
   /**
@@ -126,6 +152,10 @@ export class Uploader extends AlchemyHTMLElement {
   get fileInput() {
     return this.querySelector("input[type='file']")
   }
+
+  get redirectUrl() {
+    return this.getAttribute("redirect-url")
+  }
 }
 
 customElements.define("alchemy-uploader", Uploader)

app/views/alchemy/admin/attachments/_archive_overlay.html.erb

@@ -8,8 +8,16 @@
             dropzone: '#assign_file_list',
             file_attribute: 'file',
             in_dialog: true,
+            accept: Array(search_filter_params[:only]).map { |extension|
+              Marcel::MimeType.for(extension:)
+            }.join(",").presence,
             redirect_url: admin_attachments_path(
-              form_field_id: @form_field_id
+              form_field_id: @form_field_id,
+              only: search_filter_params[:only],
+              except: search_filter_params[:except],
+              q: search_filter_params[:q].merge(
+                last_upload: true
+              )
             ) %>
         </div>
       <% end %>

app/views/alchemy/admin/attachments/_replace_button.html.erb

@@ -1,19 +1,14 @@
-<% file_upload_id = "file_upload_#{dom_id(object)}" %>
+<% file_types = Alchemy.config.uploader.allowed_filetypes[object.class.model_name.collection] || ['*'] %>
+<% accept ||= file_types.to_a == ["*"] ? nil : file_types.map {|type| ".#{type}"}.join(", ") %>
 
-<alchemy-uploader id="<%= file_upload_id %>">
+<alchemy-uploader redirect-url="<%= redirect_url %>">
   <%= form_for [:admin, object], html: {multipart: true, class: 'upload-button'} do |f| %>
     <%= f.file_field file_attribute,
-      class: 'fileupload--field',
+      class: 'fileupload--field', accept: accept,
       name: "#{f.object_name}[#{file_attribute}]",
       id: "replace_#{dom_id(object)}" %>
     <%= label_tag "replace_#{dom_id(object)}", class: "icon_button" do %>
       <%= render_icon "file-upload" %>
     <% end %>
   <% end %>
 </alchemy-uploader>
-
-<script type="text/javascript">
-  document.getElementById("<%= file_upload_id %>").addEventListener("Alchemy.upload.successful", (event) => {
-    Turbo.visit('<%= redirect_url.html_safe %>');
-  })
-</script>

app/views/alchemy/admin/attachments/index.html.erb

@@ -3,7 +3,9 @@
     <% if can? :create, Alchemy::Attachment %>
       <div class="toolbar_button">
         <%= render 'alchemy/admin/uploader/button',
-          redirect_url: alchemy.admin_attachments_path,
+          redirect_url: alchemy.admin_attachments_path(q: {
+            last_upload: true
+          }),
           object: Alchemy::Attachment.new,
           file_attribute: 'file' %>
       </div>

app/views/alchemy/admin/uploader/_button.html.erb

@@ -1,7 +1,7 @@
 <% file_types = Alchemy.config.uploader.allowed_filetypes[object.class.model_name.collection] || ['*'] %>
-<% accept = file_types.to_a == ["*"] ? nil : file_types.map {|type| ".#{type}"}.join(", ") %>
+<% accept ||= file_types.to_a == ["*"] ? nil : file_types.map {|type| ".#{type}"}.join(", ") %>
 
-<alchemy-uploader dropzone="<%= local_assigns[:dropzone] || "#main_content" %>">
+<alchemy-uploader redirect-url="<%= redirect_url %>" dropzone="<%= local_assigns[:dropzone] || "#main_content" %>">
   <%= form_for [:admin, object], html: { multipart: true, class: 'upload-button' } do |f| %>
     <%= f.file_field file_attribute,
       class: 'fileupload fileupload--field', multiple: true, accept: accept,
@@ -15,17 +15,3 @@
     <% end %>
   <% end %>
 </alchemy-uploader>
-
-<script type="text/javascript">
-  document.querySelector("alchemy-uploader").addEventListener("Alchemy.upload.successful", (evt) => {
-    setTimeout(() => {
-      var url = '<%= redirect_url.html_safe %>';
-      evt.target.uploadProgress.visible = false;
-      <% if local_assigns[:in_dialog] %>
-      $.get(url, null, null, 'script');
-      <% else %>
-      Turbo.visit(url);
-      <% end %>
-    }, 1000)
-  })
-</script>

spec/javascript/alchemy_admin/components/uploader.spec.js

@@ -282,4 +282,89 @@ describe("alchemy-uploader", () => {
       })
     })
   })
+
+  describe("_handleUploadComplete", () => {
+    beforeEach(() => {
+      vi.useFakeTimers()
+      component._uploadFiles([firstFile])
+    })
+
+    afterEach(() => {
+      vi.useRealTimers()
+    })
+
+    describe("without redirect-url", () => {
+      beforeEach(() => {
+        global.Turbo = { visit: vi.fn() }
+        component.dispatchEvent(
+          new CustomEvent("Alchemy.upload.successful", { bubbles: true })
+        )
+      })
+
+      it("hides the progress after timeout", () => {
+        vi.advanceTimersByTime(750)
+        expect(component.uploadProgress.visible).toBeFalsy()
+      })
+
+      it("does not call Turbo.visit", () => {
+        vi.advanceTimersByTime(750)
+        expect(Turbo.visit).not.toHaveBeenCalled()
+      })
+    })
+
+    describe("without turbo-frame", () => {
+      beforeEach(() => {
+        global.Turbo = { visit: vi.fn() }
+        component.setAttribute("redirect-url", "/admin/pictures")
+        component.dispatchEvent(
+          new CustomEvent("Alchemy.upload.successful", { bubbles: true })
+        )
+      })
+
+      it("hides the progress after timeout", () => {
+        expect(component.uploadProgress.visible).toBeTruthy()
+        vi.advanceTimersByTime(750)
+        expect(component.uploadProgress.visible).toBeFalsy()
+      })
+
+      it("visits the redirect URL via Turbo", () => {
+        vi.advanceTimersByTime(750)
+        expect(Turbo.visit).toHaveBeenCalledWith("/admin/pictures")
+      })
+    })
+
+    describe("with turbo-frame", () => {
+      let turboFrame
+
+      beforeEach(() => {
+        // Wrap the component in a turbo-frame
+        turboFrame = document.createElement("turbo-frame")
+        turboFrame.id = "test-frame"
+        turboFrame.reload = vi.fn()
+        component.parentNode.insertBefore(turboFrame, component)
+        turboFrame.appendChild(component)
+
+        component.setAttribute("redirect-url", "/admin/pictures")
+        component.dispatchEvent(
+          new CustomEvent("Alchemy.upload.successful", { bubbles: true })
+        )
+      })
+
+      it("sets the turbo-frame src attribute", () => {
+        vi.advanceTimersByTime(750)
+        expect(turboFrame.getAttribute("src")).toBe("/admin/pictures")
+      })
+
+      it("reloads the turbo-frame", () => {
+        vi.advanceTimersByTime(750)
+        expect(turboFrame.reload).toHaveBeenCalled()
+      })
+
+      it("hides the progress after timeout", () => {
+        expect(component.uploadProgress.visible).toBeTruthy()
+        vi.advanceTimersByTime(750)
+        expect(component.uploadProgress.visible).toBeFalsy()
+      })
+    })
+  })
 })

spec/views/alchemy/admin/attachments/_replace_button.html.erb_spec.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe "alchemy/admin/attachments/_replace_button.html.erb" do
+  let(:object) { Alchemy::Attachment.new }
+  let(:file_attribute) { :file }
+  let(:redirect_url) { "/admin/attachments" }
+
+  before do
+    allow(view).to receive(:admin_attachments_path).and_return("/admin/attachments")
+    view.extend Alchemy::BaseHelper
+  end
+
+  it "renders a alchemy-uploader component" do
+    render partial: "alchemy/admin/attachments/replace_button",
+      locals: {object: object, file_attribute: file_attribute, redirect_url: redirect_url}
+    expect(rendered).to have_selector("alchemy-uploader[redirect-url='/admin/attachments']")
+  end
+
+  context "with allowed_filetypes configured as wildcard" do
+    before do
+      allow(Alchemy.config.uploader.allowed_filetypes).to receive(:alchemy_attachments) do
+        ["*"]
+      end
+    end
+
+    it "does not render the accept attribute" do
+      render partial: "alchemy/admin/attachments/replace_button",
+        locals: {object: object, file_attribute: file_attribute, redirect_url: redirect_url}
+
+      expect(rendered).not_to have_selector('input[type="file"][accept]')
+    end
+  end
+
+  context "with allowed_filetypes configured as specific file types" do
+    before do
+      allow(Alchemy.config.uploader.allowed_filetypes).to receive(:alchemy_attachments) do
+        ["pdf", "doc", "docx"]
+      end
+    end
+
+    it "renders the accept attribute with the correct file extensions" do
+      render partial: "alchemy/admin/attachments/replace_button",
+        locals: {object: object, file_attribute: file_attribute, redirect_url: redirect_url}
+
+      expect(rendered).to have_selector('input[type="file"][accept=".pdf, .doc, .docx"]')
+    end
+  end
+end

spec/views/alchemy/admin/uploader/_button.html.erb_spec.rb

@@ -8,12 +8,17 @@
   let(:redirect_url) { "/admin/pictures" }
 
   before do
-    allow(view).to receive(:can?).and_return(true)
     allow(view).to receive(:admin_pictures_path).and_return("/admin/pictures")
     allow(view).to receive(:admin_attachments_path).and_return("/admin/attachments")
     view.extend Alchemy::BaseHelper
   end
 
+  it "renders a alchemy-uploader component" do
+    render partial: "alchemy/admin/uploader/button",
+      locals: {object: object, file_attribute: file_attribute, redirect_url: redirect_url}
+    expect(rendered).to have_selector("alchemy-uploader[redirect-url='/admin/pictures']")
+  end
+
   context "when wildcard is configured (all file types allowed)" do
     before do
       allow(Alchemy.config.uploader.allowed_filetypes).to receive(:alchemy_pictures) do