feat(transaction): add support for file attachments using Active Storage (#713)

* feat(transaction): add support for file attachments using Active Storage

* feat(attachments): implement transaction attachments with upload, show, and delete functionality

* feat(attachments): enhance attachment upload functionality to support multiple files and improved error handling

* feat(attachments): add attachment upload form and display functionality in transaction views

* feat(attachments): implement attachment validation for count, size, and content type; enhance upload form with validation hints

* fix(attachments): use correct UI components

* feat(attachments): Implement Turbo Stream responses for creating and deleting transaction attachments.

* fix(attachments): include auth in activestorage controller

* test(attachments): add test coverage for turbostream and auth

* feat(attachments): extract strings to i18n

* fix(attachments): ensure only newly added attachments are purged when transaction validation fails.

* fix(attachments): validate attachment params

* refactor(attachments): use stimulus declarative actions

* fix(attachments): add auth for other representations

* refactor(attachments): use Browse component for attachment uploads

* fix(attachments): reject empty values on attachment upload

* fix(attachments): hide the upload form if reached max uploads

* fix(attachments): correctly purge only newly added attachments on upload failure

* fix(attachments): ensure attachment count limit is respected within a transaction lock

* fix(attachments): update attachment parameter handling to avoid `ParameterMissing` errors.

* fix(components): adjust icon_only logic for buttonish

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>

test/controllers/transaction_attachments_controller_test.rb

@@ -0,0 +1,144 @@
+require "test_helper"
+
+class TransactionAttachmentsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in @user = users(:family_admin)
+    @entry = entries(:transaction)
+    @transaction = @entry.entryable
+  end
+
+  test "should upload attachment to transaction" do
+    file = fixture_file_upload("test.txt", "application/pdf")
+
+    assert_difference "@transaction.attachments.count", 1 do
+      post transaction_attachments_path(@transaction), params: { attachment: file }
+    end
+
+    assert_redirected_to transaction_path(@transaction)
+    assert_match "Attachment uploaded successfully", flash[:notice]
+  end
+
+  test "should upload multiple attachments to transaction" do
+    file1 = fixture_file_upload("test.txt", "application/pdf")
+    file2 = fixture_file_upload("test.txt", "image/jpeg")
+
+    assert_difference "@transaction.attachments.count", 2 do
+      post transaction_attachments_path(@transaction), params: { attachments: [ file1, file2 ] }
+    end
+
+    assert_redirected_to transaction_path(@transaction)
+    assert_match "2 attachments uploaded successfully", flash[:notice]
+  end
+
+  test "should ignore blank attachments in array" do
+    file = fixture_file_upload("test.txt", "application/pdf")
+
+    assert_difference "@transaction.attachments.count", 1 do
+      # Simulate Rails behavior where an empty string is often sent in the array
+      post transaction_attachments_path(@transaction), params: { attachments: [ file, "" ] }
+    end
+
+    assert_redirected_to transaction_path(@transaction)
+    assert_match "Attachment uploaded successfully", flash[:notice] # Should be singular
+  end
+
+  test "should handle upload with no files" do
+    assert_no_difference "@transaction.attachments.count" do
+      post transaction_attachments_path(@transaction), params: {}
+    end
+
+    assert_redirected_to transaction_path(@transaction)
+    assert_match "No files selected for upload", flash[:alert]
+  end
+
+  test "should reject unsupported file types" do
+    file = fixture_file_upload("test.txt", "text/plain")
+
+    assert_no_difference "@transaction.attachments.count" do
+      post transaction_attachments_path(@transaction), params: { attachment: file }
+    end
+
+    assert_redirected_to transaction_path(@transaction)
+    assert_match "unsupported format", flash[:alert]
+  end
+
+  test "should reject exceeding attachment count limit" do
+    # Fill up to the limit
+    (Transaction::MAX_ATTACHMENTS_PER_TRANSACTION).times do |i|
+      @transaction.attachments.attach(
+        io: StringIO.new("content #{i}"),
+        filename: "file#{i}.pdf",
+        content_type: "application/pdf"
+      )
+    end
+
+    file = fixture_file_upload("test.txt", "application/pdf")
+
+    assert_no_difference "@transaction.attachments.count" do
+      post transaction_attachments_path(@transaction), params: { attachment: file }
+    end
+
+    assert_redirected_to transaction_path(@transaction)
+    assert_match "Cannot exceed #{Transaction::MAX_ATTACHMENTS_PER_TRANSACTION} attachments", flash[:alert]
+  end
+
+  test "should show attachment for authorized user" do
+    @transaction.attachments.attach(
+      io: StringIO.new("test content"),
+      filename: "test.pdf",
+      content_type: "application/pdf"
+    )
+
+    attachment = @transaction.attachments.first
+    get transaction_attachment_path(@transaction, attachment)
+
+    assert_response :redirect
+  end
+
+  test "should upload attachment via turbo_stream" do
+    file = fixture_file_upload("test.txt", "application/pdf")
+
+    assert_difference "@transaction.attachments.count", 1 do
+      post transaction_attachments_path(@transaction), params: { attachment: file }, as: :turbo_stream
+    end
+
+    assert_response :success
+    assert_match(/turbo-stream action="replace" target="transaction_attachments_#{@transaction.id}"/, response.body)
+    assert_match(/turbo-stream action="append" target="notification-tray"/, response.body)
+    assert_match("Attachment uploaded successfully", response.body)
+  end
+
+  test "should show attachment inline" do
+    @transaction.attachments.attach(io: StringIO.new("test"), filename: "test.pdf", content_type: "application/pdf")
+    attachment = @transaction.attachments.first
+
+    get transaction_attachment_path(@transaction, attachment, disposition: :inline)
+
+    assert_response :redirect
+    assert_match(/disposition=inline/, response.redirect_url)
+  end
+
+  test "should show attachment as download" do
+    @transaction.attachments.attach(io: StringIO.new("test"), filename: "test.pdf", content_type: "application/pdf")
+    attachment = @transaction.attachments.first
+
+    get transaction_attachment_path(@transaction, attachment, disposition: :attachment)
+
+    assert_response :redirect
+    assert_match(/disposition=attachment/, response.redirect_url)
+  end
+
+  test "should delete attachment via turbo_stream" do
+    @transaction.attachments.attach(io: StringIO.new("test"), filename: "test.pdf", content_type: "application/pdf")
+    attachment = @transaction.attachments.first
+
+    assert_difference "@transaction.attachments.count", -1 do
+      delete transaction_attachment_path(@transaction, attachment), as: :turbo_stream
+    end
+
+    assert_response :success
+    assert_match(/turbo-stream action="replace" target="transaction_attachments_#{@transaction.id}"/, response.body)
+    assert_match(/turbo-stream action="append" target="notification-tray"/, response.body)
+    assert_match("Attachment deleted successfully", response.body)
+  end
+end

test/fixtures/files/test.txt

@@ -0,0 +1 @@
+This is a test file for attachment uploads.

test/integration/active_storage_authorization_test.rb

@@ -0,0 +1,58 @@
+require "test_helper"
+
+class ActiveStorageAuthorizationTest < ActionDispatch::IntegrationTest
+  setup do
+    @user_a = users(:family_admin) # In dylan_family
+    @user_b = users(:empty) # In empty family
+
+    @transaction_a = transactions(:one) # Assuming it belongs to dylan_family via its entry/account
+    @transaction_a.attachments.attach(
+      io: StringIO.new("Family A Secret Receipt"),
+      filename: "receipt.pdf",
+      content_type: "application/pdf"
+    )
+    @attachment_a = @transaction_a.attachments.first
+  end
+
+  test "user can access attachments within their own family" do
+    sign_in @user_a
+
+    # Get the redirect URL from our controller
+    get transaction_attachment_path(@transaction_a, @attachment_a)
+    assert_response :redirect
+
+    # Follow the redirect to ActiveStorage::Blobs::RedirectController
+    follow_redirect!
+
+    # In test/local environment, it will redirect again to a disk URL
+    assert_response :redirect
+    assert_match(/rails\/active_storage\/disk/, response.header["Location"])
+  end
+
+  test "user cannot access attachments from a different family" do
+    sign_in @user_b
+
+    # Even if they find the signed global ID (which is hard but possible),
+    # the monkey patch should block them at the blob controller level.
+    # We bypass our controller and go straight to the blob serving URL to test the security layer
+    get rails_blob_path(@attachment_a)
+
+    # The monkey patch raises ActiveRecord::RecordNotFound which rails converts to 404
+    assert_response :not_found
+  end
+
+  test "user cannot access variants from a different family" do
+    # Attach an image to test variants
+    file = File.open(Rails.root.join("test/fixtures/files/square-placeholder.png"))
+    @transaction_a.attachments.attach(io: file, filename: "test.png", content_type: "image/png")
+    attachment = @transaction_a.attachments.last
+    variant = attachment.variant(resize_to_limit: [ 100, 100 ]).processed
+
+    sign_in @user_b
+
+    # Straight to the representation URL
+    get rails_representation_path(variant)
+
+    assert_response :not_found
+  end
+end

test/models/transaction_attachment_validation_test.rb

@@ -0,0 +1,61 @@
+require "test_helper"
+
+class TransactionAttachmentValidationTest < ActiveSupport::TestCase
+  setup do
+    @transaction = transactions(:one)
+  end
+
+  test "should validate attachment content types" do
+    # Valid content type should pass
+    @transaction.attachments.attach(
+      io: StringIO.new("valid content"),
+      filename: "test.pdf",
+      content_type: "application/pdf"
+    )
+    assert @transaction.valid?
+
+    # Invalid content type should fail
+    @transaction.attachments.attach(
+      io: StringIO.new("invalid content"),
+      filename: "test.txt",
+      content_type: "text/plain"
+    )
+    assert_not @transaction.valid?
+    assert_includes @transaction.errors.full_messages_for(:attachments).join, "unsupported format"
+  end
+
+  test "should validate attachment count limit" do
+    # Fill up to the limit
+    Transaction::MAX_ATTACHMENTS_PER_TRANSACTION.times do |i|
+      @transaction.attachments.attach(
+        io: StringIO.new("content #{i}"),
+        filename: "file#{i}.pdf",
+        content_type: "application/pdf"
+      )
+    end
+    assert @transaction.valid?
+
+    # Exceeding the limit should fail
+    @transaction.attachments.attach(
+      io: StringIO.new("extra content"),
+      filename: "extra.pdf",
+      content_type: "application/pdf"
+    )
+    assert_not @transaction.valid?
+    assert_includes @transaction.errors.full_messages_for(:attachments).join, "cannot exceed"
+  end
+
+  test "should validate attachment file size" do
+    # Create a mock large attachment
+    large_content = "x" * (Transaction::MAX_ATTACHMENT_SIZE + 1)
+
+    @transaction.attachments.attach(
+      io: StringIO.new(large_content),
+      filename: "large.pdf",
+      content_type: "application/pdf"
+    )
+
+    assert_not @transaction.valid?
+    assert_includes @transaction.errors.full_messages_for(:attachments).join, "too large"
+  end
+end