Generate getter and setter methods in mixin

Generated attachment getter and setter methods are created within the model's `GeneratedAssociationMethods` module to allow overriding and composition using `super`. Includes tests for new functionality. Co-authored-by: Josh Susser <josh@hasmanythrough.com> Co-authored-by: Jamon Douglas <terrildouglas@gmail.com>
joshsusser · May 17, 2018 · fd0bd1b · fd0bd1b
1 parent 6c05728
commit fd0bd1b
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 2 deletions.
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Generated attachment getter and setter methods are created
+    within the model's `GeneratedAssociationMethods` module to
+    allow overriding and composition using `super`.
+
+    *Josh Susser*, *Jamon Douglas*
+
 *   Add `ActiveStorage::Blob#open`, which downloads a blob to a tempfile on disk
     and yields the tempfile. Deprecate `ActiveStorage::Downloading`.
 

diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb
@@ -28,7 +28,7 @@ module Attached::Macros
     # If the +:dependent+ option isn't set, the attachment will be purged
     # (i.e. destroyed) whenever the record is destroyed.
     def has_one_attached(name, dependent: :purge_later)
-      class_eval <<-CODE, __FILE__, __LINE__ + 1
+      generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}
           @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"})
         end
@@ -75,7 +75,7 @@ def #{name}=(attachable)
     # If the +:dependent+ option isn't set, all the attachments will be purged
     # (i.e. destroyed) whenever the record is destroyed.
     def has_many_attached(name, dependent: :purge_later)
-      class_eval <<-CODE, __FILE__, __LINE__ + 1
+      generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}
           @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"})
         end

diff --git a/activestorage/test/models/attached_test.rb b/activestorage/test/models/attached_test.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "database/setup"
+
+class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
+  include ActiveJob::TestHelper
+
+  setup do
+    @user = User.create!(name: "Josh")
+  end
+
+  teardown { ActiveStorage::Blob.all.each(&:purge) }
+
+  test "overriding has_one_attached methods works" do
+    # attach blob before messing with getter, which breaks `#attach`
+    @user.avatar.attach create_blob(filename: "funky.jpg")
+
+    # inherited only
+    assert_equal "funky.jpg", @user.avatar.filename.to_s
+
+    User.class_eval do
+      def avatar
+        super.filename.to_s.reverse
+      end
+    end
+
+    # override with super
+    assert_equal "funky.jpg".reverse, @user.avatar
+
+    User.send(:remove_method, :avatar)
+  end
+
+  test "overriding has_many_attached methods works" do
+    # attach blobs before messing with getter, which breaks `#attach`
+    @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
+
+    # inherited only
+    assert_equal "funky.jpg", @user.highlights.first.filename.to_s
+    assert_equal "wonky.jpg", @user.highlights.second.filename.to_s
+
+    User.class_eval do
+      def highlights
+        super.reverse
+      end
+    end
+
+    # override with super
+    assert_equal "wonky.jpg", @user.highlights.first.filename.to_s
+    assert_equal "funky.jpg", @user.highlights.second.filename.to_s
+
+    User.send(:remove_method, :highlights)
+  end
+end