[BUGFIX] AMS polymorphic type for namespaced models #3026
Conversation
|
I am not sure about |
| }, | ||
|
|
||
| typeForRoot: function(key) { | ||
|
|
|
I think this looks good. |
| @@ -166,7 +166,10 @@ var ActiveModelSerializer = RESTSerializer.extend({ | |||
| if (Ember.isNone(belongsTo)) { | |||
| json[jsonKey] = null; | |||
| } else { | |||
| json[jsonKey] = capitalize(camelize(belongsTo.typeKey)); | |||
| json[jsonKey] = belongsTo.typeKey.split("/").map(function(part) { | |||
There was a problem hiding this comment.
We can't use map here due to IE8 support (I'm not sure when we're dropping it). You can use Ember.EnumerableUtils.map instead.
|
This also looks good to me but I think we should call this out in CHANGELOG.md and the release blog post. Can you add an entry to the CHANGELOG ? |
|
In fact there is no need for Ember.String.classify("evilMinions/yellowMinion");
//=> unexpected "EvilMinions/yellowMinion"
// expected "EvilMinions/YellowMinion"
Ember.String.camelize("YellowMinions/MiniPig");
//=> unexpected "yellowMinions/MiniPig"
// expected "yellowMinions/miniPig"
Ember.String.camelize("yellow_minions/mini_pig");
//=> "yellowMinions/miniPig" as expectedI will post new issue if you agree. |
|
Looks like you already added an issue to Ember Inflector. Looks like a bug to me and @stefanpenner emberjs/ember-inflector#77 |
artych
changed the title
|
@fivetanley CHANGELOG entry added. Is it good now or I should do something more? |
|
Ember.String issue is moved in ember.js repo just to make this conversation clear. |
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| ### Master | |||
|
|
|||
| * Fix ActiveModelSerializer serializePolymorphicType and typeForRoot methods for properly serializing and extracting namespaced model names in polymorphic type attributes. | |||
There was a problem hiding this comment.
Can you mention the issue name, and also just quickly say it's now accepting :: instead of /
|
Looks, good can you just cleanup the changelog a bit, and I'll merge asap |
…racting namespaced model names in polymorphic type attributes. Now AMS accepts '::' istead of '/'. Fixes #3019.
|
Thank you, @igorT! Changelog entry is changed. This patch should be simplified after fixing emberjs/ember.js#11000. |
| @@ -166,7 +166,9 @@ var ActiveModelSerializer = RESTSerializer.extend({ | |||
| if (Ember.isNone(belongsTo)) { | |||
| json[jsonKey] = null; | |||
| } else { | |||
| json[jsonKey] = capitalize(camelize(belongsTo.typeKey)); | |||
| json[jsonKey] = classify(belongsTo.typeKey).replace(/(\/)([a-z])/g, function(match, separator, chr) { | |||
There was a problem hiding this comment.
Perhaps this could be rewritten as classify(belongsTo.typeKey).split('/').map(classify).join('::') ?
There was a problem hiding this comment.
Seems fine to me overall though.
…mespaced-models [BUGFIX] AMS polymorphic type for namespaced models
Tests and patch fixes #3019