From fd0bd1bf682622f064ac437ceee4e1b2a6b6d3b9 Mon Sep 17 00:00:00 2001
From: Josh Susser <josh@hasmanythrough.com>
Date: Thu, 17 May 2018 14:22:00 -0700
Subject: [PATCH] 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>
---
 activestorage/CHANGELOG.md                    |  6 +++
 .../lib/active_storage/attached/macros.rb     |  4 +-
 activestorage/test/models/attached_test.rb    | 54 +++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 activestorage/test/models/attached_test.rb

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index 761d8f50219cd..7b724b7b81b79 100644
--- 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
index 819f00cc06df0..f99cf35680304 100644
--- 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
new file mode 100644
index 0000000000000..14395e12dfa5f
--- /dev/null
+++ 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