Singularize and capitalize polymorphic types #1615
Conversation
| @@ -125,7 +125,7 @@ def test_polymorphic | |||
| src: 'http://example.com/images/productivity.png', | |||
| author_id: nil, | |||
| photographer_id: '9', | |||
| photographer_type: 'people', | |||
There was a problem hiding this comment.
So, this shouldn't change, as this is the serialization
There was a problem hiding this comment.
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.
|
@dpdawson Awesome! Would you mind adding a changelog entry at the top of the fixes section? Also, a test of an ActiveRecord object using this result would be fantastic, if you don't mind the extra effort. |
|
As I looked into it more, I realized the error only gets thrown when you validate the relationship is present. The fix doesn't cause any of the other tests to fail, so I assume it's ok. I added a test, but I'm sure it's not the best implementation because I don't use Minitest and the organization of the tests is a bit difficult to discern at first glance. |
dpdawson
force-pushed
the
master
branch
3 times, most recently
from
8960b9c
to
ce1236c
Compare
Singularizing polymorphic association types allows you to use plural or singular type values in your requests as allowed by JSON API. Because ActiveRecord calls #constantize on the polymorphic association type when you use the hash to create a new model if you validate the presence of the related model, you must capitalize the type to avoid a NameError.
| @@ -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 | |||
There was a problem hiding this comment.
we probably want to encapsulate this singular.capitalize logic in a transformer cc @remear
There was a problem hiding this comment.
could we camelize instead of capitalize? that would add support for snake cased multi word model names, right?
There was a problem hiding this comment.
assoc_data.fetch('type').underscore.classify
Using [] is dangerous.
|
With a rebase and a brief look at the Transformer with @remear this should be ready to mege |
|
I'd be happy to take a look if pointed in the right direction. |
|
@dpdawson Could you rebase this off master and take a look at adjusting |
|
Any updates on this? Or has the problem been solved in a different way? I'm currently running into this issue. |
|
One note: I don't think the current implementation will work for multi-word classes, given that the standard wordbreak in jsonapi is a '-' "blog-posts".singularize.capitalize |
|
I get this situation here, by now, I solve with hardcoded fix on controller. I made a SO because I had not seen this. Please consider merge it. |
|
@brunowego @bartiaco I'm reading the comments above and looks like the PR didn't get finished by anyone. Unless @dpdawson wants to pick it up again, it'll need to be reworked against master, and also related #1928 cc @NullVoxPopuli |
Singularizing polymorphic association types allows you to use plural or singular
type values in your requests as allowed by JSON API.
Because ActiveRecord calls #constantize on the polymorphic association type
when you use the hash to create a new model, you must capitalize the type to
avoid a NameError.