Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Singularize and capitalize polymorphic types #1615

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ Features:
- [#1340](https://github.com/rails-api/active_model_serializers/pull/1340) Add support for resource-level meta. (@beauby)

Fixes:
- [#1615](https://github.com/rails-api/active_model_serializers/pull/1615) Allow plural polymorphic relationship types when validating
presence. (@dpdawson)
- [#1615](https://github.com/rails-api/active_model_serializers/pull/1615) Capitalize polymorphic association types to prevent error
when ActiveRecord constantizes them when validating presence. (@dpdawson)
- [#1478](https://github.com/rails-api/active_model_serializers/pull/1478) Cache store will now be correctly set when serializers are
loaded *before* Rails initializes. (@bf4)
- [#1570](https://github.com/rails-api/active_model_serializers/pull/1570) Fixed pagination issue with last page size. (@bmorrall)
Expand Down
2 changes: 1 addition & 1 deletion docs/general/deserialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ ActiveModelSerializers::Deserialization
# title: 'Title 1',
# published_at: '2015-12-20',
# author_id: '2',
# author_type: 'user'
# author_type: 'User'
# }
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ module Deserialization
# # title: 'Title 1',
# # published_at: '2015-12-20',
# # author_id: '2',
# # author_type: 'people'
# # author_type: 'People'
# # }
#
def parse!(document, options = {})
Expand Down Expand Up @@ -188,7 +188,7 @@ def parse_relationship(assoc_name, assoc_data, options)
end

polymorphic = (options[:polymorphic] || []).include?(assoc_name.to_sym)
hash["#{prefix_key}_type".to_sym] = assoc_data['type'] if polymorphic
hash["#{prefix_key}_type".to_sym] = assoc_data['type'].singularize.capitalize if polymorphic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably want to encapsulate this singular.capitalize logic in a transformer cc @remear

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we camelize instead of capitalize? that would add support for snake cased multi word model names, right?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assoc_data.fetch('type').underscore.classify

Using [] is dangerous.


hash
end
Expand Down
33 changes: 33 additions & 0 deletions test/active_record_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,38 @@ class ActiveRecordTest < ActiveSupport::TestCase

def setup
@resource = ARModels::Post.new
@image = Image.create
end

def test_active_model_record_with_validated_polymorphic_relationship_creation
picture = Picture.create!(picture_params)

assert_equal(@image, picture.imageable)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

end

private

def picture_params
params = ActionController::Parameters.new({
data: {
attributes: {
title: 'Picture Title'
},
relationships: {
imageable: {
data: {
type: 'images',
id: @image.id
}
},
},
type: 'pictures'
}
})

ActiveModelSerializers::Deserialization.jsonapi_parse!(
params.to_unsafe_h,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

polymorphic: [:imageable]
)
end
end
2 changes: 1 addition & 1 deletion test/adapter/json_api/parse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def test_polymorphic
src: 'http://example.com/images/productivity.png',
author_id: nil,
photographer_id: '9',
photographer_type: 'people',
photographer_type: 'Person',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this shouldn't change, as this is the serialization

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n/m I take that back. You're right. I misread that this was the input being changed, not the output. Rails needs this format as the deserialized attributes.

comment_ids: %w(1 2)
}
assert_equal(expected, parsed_hash)
Expand Down
6 changes: 4 additions & 2 deletions test/fixtures/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
end
create_table :pictures, force: true do |t|
t.string :title
t.string :imageable_type
t.string :imageable_id
t.references :imageable, polymorphic: true
t.timestamp null: false
end
create_table :images, force: true do |t|
t.timestamp null: false
end
end
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/poro.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class Employee < ActiveRecord::Base

class Picture < ActiveRecord::Base
belongs_to :imageable, polymorphic: true

validates :imageable, presence: true
end

class Image < ActiveRecord::Base
has_many :pictures, as: :imageable
end

module Spam; end
Expand Down