Fix model attribute accessors #2022
Conversation
|
@bf4, thanks for your PR! By analyzing the history of the files in this pull request, we identified @domitian, @NullVoxPopuli and @Rodrigora to be potential reviewers. |
| # Silence redefinition of methods warnings | ||
| ActiveModelSerializers.silence_warnings do | ||
| attr_accessor(*names) | ||
| end | ||
| return if included_modules.include?(DeriveAttributesFromNamesAndFixAccessors) |
There was a problem hiding this comment.
warning code needs some thought. might be better in a separate pr
| end | ||
|
|
||
| # Opt-in to breaking change | ||
| def self.derive_attributes_from_names_and_fix_accessors |
There was a problem hiding this comment.
Anyone think of a better name for this?
| base.attributes :id | ||
| end | ||
|
|
||
| # Override the initailize method so that attributes aren't processed. |
| @bio = Bio.new(id: 1, content: 'AMS Contributor') | ||
| @author = Author.new(name: 'Joao M. D. Moura') | ||
| @author = Author.new(id: 'author', name: 'Joao M. D. Moura') | ||
| @blog = Blog.new(id: 999, name: 'Custom blog', writer: @author, articles: []) | ||
| @role = Role.new(name: 'Great Author') | ||
| @location = Location.new(lat: '-23.550520', lng: '-46.633309') |
There was a problem hiding this comment.
A bunch of the associations here aren't used and could probably be removed both in this file and in the poro fixtures file
| @@ -330,7 +374,7 @@ def test_uses_file_digest_in_cache_key | |||
| end | |||
|
|
|||
| def test_cache_digest_definition | |||
| assert_equal(::Model::FILE_DIGEST, @post_serializer.class._cache_digest) | |||
| assert_equal(FILE_DIGEST, @post_serializer.class._cache_digest) | |||
There was a problem hiding this comment.
not sure how I feel about this assertion.. maybe this should just be Digest::MD5.hexdigest(File.open(__FILE__).read)
| @@ -1,5 +1,6 @@ | |||
| class Model < ActiveModelSerializers::Model | |||
| FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) | |||
| rand(2).zero? && derive_attributes_from_names_and_fix_accessors | |||
There was a problem hiding this comment.
make sure the test suite works the same with and without the accessors fix
bf4
force-pushed
the
fix_model_attribute_accessors
branch
from
fe51b31 to
ee6bc30
Compare
bf4
force-pushed
the
fix_model_attribute_accessors
branch
from
07f24ed to
1570437
Compare
Purpose
Replaces #1998 and #1984 with an opt-in change, rather that breaking change with backwards compatibility.
Changes
Add new opt-in behavior for fixing the attributes change via accessors bug.
Caveats
It will still need to be reconciled with master, which I'll take care of once this is reviewed.