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

Correctly serialize lists of URLs for nested relationships #195

Open
safts opened this issue May 15, 2017 · 3 comments
Open

Correctly serialize lists of URLs for nested relationships #195

safts opened this issue May 15, 2017 · 3 comments

Comments

@safts
Copy link

safts commented May 15, 2017

When reading the guides, I understood that related fields represented using nested URLs are understood by the drf serializer & the relationships were resolved automatically.

Ember Django Adapter has support for Django REST Framework's HyperlinkedRelatedField.
URLs in a json hash for hasMany and belongsTo relationship fields will
automatically be retrieved and added to the store.

However, this does not seem to be the case.

My API is structured as such:

GET /api/charginganalyses/
[
    {
        "id": 1,
        "date": 1494547200,
        "notes": "Some Notes",
        "billings": [
            2,
            3,
            4
        ],
        "extra_info": "Extra Info"
    },
...
]

On the Ember side, my charginganalysis model:

export default DS.Model.extend({

  date: DS.attr('string'),
  notes: DS.attr('string'),
  billings: DS.hasMany('servicebilling'),
  extra_info: DS.attr('string'),

});

When I am using the ModelSerializer and my billings are ids, I can correctly access the billings property on my charginganalysis instance (after using getEach on every charginganalysis in my route).

However, if I change my API to use the HyperlinkedModelSerializer as such:

GET /api/charginganalyses/
[
    {
        "id": 1,
        "date": 1494547200,
        "notes": "Some Notes",
        "billings": [
            "http://localhost:8000/api/servicebillings/2/",
            "http://localhost:8000/api/servicebillings/3/",
            "http://localhost:8000/api/servicebillings/4/"
        ],
        "extra_info": "Extra Info"
    },
]

the relationships cannot be identified. I took a look in the code and I feel that this should be the stanza responsible for handling this case.

When I stopped the execution there using a debugger, I found out that if payloadRel is anything else than a string the code does nothing and passes the instance on to the parent element. In this case, payloadRel is indeed not a string but an array.

This probably happens in the first case too, where I am using ids instead of URLs but I am guessing that somehow Ember Data is able to realize that those are IDs and make the association.

I know that the proposed approach is using a single URL for such relations and defining custom views in the back end using HyperlinkedIdentityFields, but this does not allow editing your models without defining extra API endpoints, so since my relationships are not that huge I feel like the performance loss would be minor.

Either way, I believe that since the documentation mentions that lists of nested URLs are supported this is an issue.

I could perhaps try to provide a PR fixing this, if this is something you are interested on.

@benkonrath
Copy link
Collaborator

Sorry for the late reply. I'm ok with supporting HyperlinkedModelSerializer out of the box but I would want to keep the current behavior as well. The documentation would need to be updated as well to mention both options with a short explanation of the pros & cons.

@dustinfarris What do you think?

@safts
Copy link
Author

safts commented Jun 6, 2017

I feel that, at least for now, the documentation should be updated to state that the only supported implementation is the HyperlinkedIdentityField one and that nested URLs are not supported and afterwards we could discuss how to implement this. It is quite misleading at the moment.

@benkonrath
Copy link
Collaborator

Sounds like a good plan. It would be much appreciated if you could make a pull request to improve the documentation about this. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants