feat: use active storage for invoice attachments

until now invoice attachments were stored in the database directly,
this led to performance issues see #1037.
This commit migrates the invoice attachments to use active storage.
Additionally multiple attachments are now supported, a small preview
is rendered and the delete flow was adapted.

app/controllers/finance/invoices_controller.rb

@@ -79,11 +79,12 @@ def destroy
     redirect_to finance_invoices_url
   end
 
-  def attachment
-    @invoice = Invoice.find(params[:invoice_id])
-    type = MIME::Types[@invoice.attachment_mime].first
-    filename = "invoice_#{@invoice.id}_attachment.#{type.preferred_extension}"
-    send_data(@invoice.attachment_data, filename: filename, type: type)
+  def delete_attachment
+    attachment = ActiveStorage::Attachment.find(params[:attachment_id])
+    attachment.purge_later
+    respond_to do |format|
+      format.js
+    end
   end
 
   private

app/models/invoice.rb

@@ -7,10 +7,11 @@ class Invoice < ApplicationRecord
   belongs_to :financial_link, optional: true
   has_many :deliveries, dependent: :nullify
   has_many :orders, dependent: :nullify
+  has_many_attached :attachments
 
   validates :supplier_id, presence: true
   validates :amount, :deposit, :deposit_credit, numericality: true
-  validate :valid_attachment
+  validate :valid_attachments
 
   scope :unpaid, -> { where(paid_on: nil) }
   scope :without_financial_link, -> { where(financial_link: nil) }
@@ -20,19 +21,6 @@ class Invoice < ApplicationRecord
   # Replace numeric seperator with database format
   localize_input_of :amount, :deposit, :deposit_credit
 
-  def attachment=(incoming_file)
-    self.attachment_data = incoming_file.read
-    # allow to soft-fail when FileMagic isn't present and removed from Gemfile (e.g. Heroku)
-    self.attachment_mime = defined?(FileMagic) ? FileMagic.new(FileMagic::MAGIC_MIME).buffer(attachment_data) : 'application/octet-stream'
-  end
-
-  def delete_attachment=(value)
-    return unless value == '1'
-
-    self.attachment_data = nil
-    self.attachment_mime = nil
-  end
-
   def user_can_edit?(user)
     user.role_finance? || (user.role_invoices? && !paid_on && created_by.try(:id) == user.id)
   end
@@ -62,12 +50,10 @@ def expected_amount
 
   protected
 
-  def valid_attachment
-    return unless attachment_data
-
-    mime = MIME::Type.simplified(attachment_mime)
-    return if ['application/pdf', 'image/jpeg'].include? mime
-
-    errors.add :attachment, I18n.t('model.invoice.invalid_mime', mime: mime)
+  # validates that the attachments are jpeg, png or pdf
+  def valid_attachments
+    attachments.each do |attachment|
+      errors.add(:attachments, I18n.t('model.invoice.invalid_mime', mime: attachment.content_type)) unless attachment.content_type.in?(%w[image/jpeg image/png application/pdf])
+    end
   end
 end

app/views/finance/invoices/_form.html.haml

@@ -23,9 +23,20 @@
   = f.input :deposit, as: :string
   = f.input :deposit_credit, as: :string
   = render 'shared/custom_form_fields', f: f, type: :invoice
-  = f.input :attachment, as: :file, hint: t('.attachment_hint')
-  - if f.object.attachment_data.present?
-    = f.input :delete_attachment, as: :boolean
+  = f.input :attachments, as: :file, hint: t('.attachment_hint'), input_html: {multiple: true}, direct_upload: true
+  - if f.object.attachments.attached?
+    .control-group
+      %label.control-label
+        = t('ui.delete_attachment')
+      - f.object.attachments.reject(&:new_record?).each do |attachment|
+        .controls.control-text{id: "attachment_#{attachment.id}"}
+          = f.hidden_field :attachments, value: attachment.signed_id, multiple: true
+          = link_to url_for(attachment), target: "_blank" do
+            = image_tag attachment.preview(resize_to_limit: [100, 100]) if attachment.previewable?
+            = image_tag attachment.variant(resize_to_limit: [100, 100]) if attachment.variable?
+          = link_to 'Delete', delete_attachment_finance_invoice_path(f.object, attachment_id: attachment.id), method: :delete, remote: true, class: 'btn'
+          %label= attachment.filename
+          %hr
   = f.input :note
   .form-actions
     = f.submit class: 'btn'

app/views/finance/invoices/delete_attachment.js.erb

@@ -0,0 +1 @@
+$("#attachment_<%= params[:attachment_id] %>").remove();

app/views/finance/invoices/show.html.haml

@@ -63,10 +63,14 @@
         %dt= heading_helper(Invoice, :total) + ':'
         %dd= number_to_currency total
 
-      - if @invoice.attachment_data
+      - if @invoice.attachments.attached?
         %dt= heading_helper(Invoice, :attachment) + ':'
-        %dd= link_to t('ui.download'), finance_invoice_attachment_path(@invoice)
-
+        - for attachment in @invoice.attachments
+          %dd
+            =link_to url_for(attachment), target: "_blank" do
+              = image_tag attachment.preview(resize_to_limit: [100, 100]), style: 'margin: 0px 10px 10px 0px;' if attachment.previewable?
+              = image_tag attachment.variant(resize_to_limit: [100, 100]), style: 'margin: 0px 10px 10px 0px;' if attachment.variable?
+              = attachment.filename
       %dt= heading_helper(Invoice, :note) + ':'
       %dd= simple_format(@invoice.note)
 

app/views/finance/invoices/unpaid.html.haml

@@ -19,8 +19,9 @@
         %span{style: "color:#{invoice_amount_diff < 0 ? 'red' : 'green'}"}
           = invoice_amount_diff > 0 ? '+' : '-'
           = number_to_currency(invoice_amount_diff.abs)
-      - if invoice.attachment_data?
-        = link_to finance_invoice_attachment_path(invoice) do
+      - if invoice.attachments.attached?
+        - for attachment in invoice.attachments
+          = link_to attachment.filename, url_for(attachment)
           = glyph :download
       - if invoice.note?
         = '(' + invoice.note + ')'

config/locales/de.yml

@@ -972,7 +972,7 @@ de:
       edit:
         title: Rechnung bearbeiten
       form:
-        attachment_hint: Es sind nur JPEG und PDF erlaubt.
+        attachment_hint: Es sind nur JPEG, PNG und PDF erlaubt.
       index:
         action_new: Neue Rechnung anlegen
         show_unpaid: Unbezahlte Rechnungen anzeigen

config/locales/en.yml

@@ -972,7 +972,7 @@ en:
       edit:
         title: Edit invoice
       form:
-        attachment_hint: Only JPEG and PDF are allowed.
+        attachment_hint: Only JPEG, PNG and PDF are allowed.
       index:
         action_new: Create new invoice
         show_unpaid: Show unpaid invoices

config/locales/es.yml

@@ -972,7 +972,7 @@ es:
       edit:
         title: Edita factura
       form:
-        attachment_hint: Sólo se permiten los formatos JPEG y PDF.
+        attachment_hint: Sólo se permiten los formatos JPEG, PNG y PDF.
       index:
         action_new: Crea nueva factura
         show_unpaid: Mostrar facturas no pagadas

config/locales/nl.yml

@@ -972,7 +972,7 @@ nl:
       edit:
         title: Factuur bewerken
       form:
-        attachment_hint: Alleen JPEG en PDF zijn toegestaan.
+        attachment_hint: Alleen JPEG, PNG en PDF zijn toegestaan.
       index:
         action_new: Nieuwe factuur toevoegen
         show_unpaid: Toon onbetaalde facturen

config/locales/tr.yml

@@ -971,7 +971,7 @@ tr:
       edit:
         title: Faturayı düzenle
       form:
-        attachment_hint: Sadece JPEG ve PDF dosyaları kabul edilir.
+        attachment_hint: Sadece JPEG, PNG ve PDF dosyaları kabul edilir.
       index:
         action_new: Yeni fatura oluştur
         show_unpaid: Ödenmemiş faturaları göster

config/routes.rb

@@ -170,9 +170,9 @@
       end
 
       resources :invoices do
-        get :attachment
         get :form_on_supplier_id_change, on: :collection
         get :unpaid, on: :collection
+        delete 'delete_attachment/:attachment_id', to: 'invoices#delete_attachment', as: 'delete_attachment', on: :member
       end
 
       resources :links, controller: 'financial_links', only: %i[create show] do

db/migrate/20240126111615_move_attachment_to_active_storage.rb

@@ -0,0 +1,40 @@
+class MoveAttachmentToActiveStorage < ActiveRecord::Migration[7.0]
+  def up
+    Invoice.find_each do |invoice|
+      if invoice.attachment_data.present? && invoice.attachment_mime.present?
+        blob = ActiveStorage::Blob.create_and_upload!(
+          io: StringIO.new(invoice.attachment_data),
+          filename: 'attachment',
+          content_type: invoice.attachment_mime
+        )
+
+        invoice.attachments.attach(blob)
+      end
+    end
+
+    change_table :invoices, bulk: true do |t|
+      t.remove :attachment_data
+      t.remove :attachment_mime
+    end
+  end
+
+  def down
+    change_table :invoices, bulk: true do |t|
+      t.binary :attachment_data, limit: 16.megabyte
+      t.string :attachment_mime
+    end
+
+    Invoice.find_each do |invoice|
+      if invoice.attachments.attached?
+        attachment = invoice.attachments.first # Will only migrate the first attachment back, as multiple were not supported before
+        attachment_data = attachment.download
+        attachment_mime = attachment.blob.content_type
+
+        invoice.update(
+          attachment_data: attachment_data,
+          attachment_mime: attachment_mime
+        )
+      end
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_02_15_085312) do
+ActiveRecord::Schema[7.0].define(version: 2024_01_26_111615) do
   create_table "action_text_rich_texts", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t|
     t.string "name", null: false
     t.text "body", size: :long
@@ -251,8 +251,6 @@
     t.datetime "created_at", precision: nil
     t.datetime "updated_at", precision: nil
     t.integer "created_by_user_id"
-    t.string "attachment_mime"
-    t.binary "attachment_data", size: :medium
     t.integer "financial_link_id"
     t.index ["supplier_id"], name: "index_invoices_on_supplier_id"
   end