Allowed to overwrite resource id in serializer

[axieum]

Fixes #155
Fixes #722
Fixes #1126

Description of the Change

Currently, per default, the id is always retrieved by calling pk property on the instance. This change allows the id to be overwritten in the serializer by defining a id field.

Checklist

• PR only contains one change (considered splitting up PR)
• unit-test added
• documentation updated
• CHANGELOG.md updated (only for user relevant changes)
• author name in AUTHORS

[sliverc]

Thanks for looking into our oldest open issue. It is great to tackle this.

I think it is a great idea to allow a DJA user to overwrite an id by configuring an id field on the serializer, as this is most intuitive. I think your code needs to be adjusted though, as in its current states it expects an id on the instance, and not on the serialized data.

A check would need to do the following:

1. check if there is an id in the serialized data (in the method build_json_resource_obj the serialized data is called resource).
2. if not fall back to the pk on the resource instance

That should be enough in my opinion and no more if/else are needed. It will also be important to document this behavior in our documentation, that setting an id field on the serializer is the way to have a custom id or when using generic serializers. To fully resolve #155 it also needs to be possible to configure a custom pk for a relation. When digging into the code, this is actually already possible by setting pk_field on the ResourceRelatedField, so we would only need to document and add a test.

You think you could work on those things?

[sliverc]

@axieum Do you think you can still work on this PR (open issues see comment above)? I very much like the implementation and would love to get it integrated.

[axieum]

Hi, sorry about that @sliverc - got heavily distracted elsewhere.

I've gone ahead and quickly added some commits to do what you proposed, first it'll try the resource and then the resource_instance. In both scenarios, the resource (and resource_instance) may be a dict or object.

I couldn't think of how to write a test for the case where resource is preferred over resource_instance, could you assist here please?

Thanks,
Jonathan.

[sliverc]

def get_resource_id(resource_instance, resource):
   if resource and 'id' in resource:
      return force_str(resource['id'])
   return resource_instance and force_str(resource_instance.pk) or None

As we need to add a utility method anyway to get the resource id, force_str, checking for None etc. should all be in there. Above is how the method could look like (not tested, but as an idea). As passing on resource_instance and resource separately, it is actually much simpler.

For writing a test, you need to write a test where the serializer has a field called id. Easiest would then be to add a method get_id(self) to the serializer which returns a different id to see whether that id is actually returned. Then test it as you already do in the test_views.py file.

Hope this makes things clearer.

[axieum]

I think we're going to need to explicitly add code to check for and execute a get_id method for that piece to work.

[sliverc]

Thanks for continue working on this. See my inline comments + the tests do not seem to pass, therefore haven't reviewed those in detail yet.

[sliverc]

This is a very important feature and as we are soon releasing a new version I would like to get this ready. So I am currently working on it to be able to merge it before our next release.

[sliverc]

I now added some documentation as well. For this feature to be complete, it is also necessary to be able to overwrite resource id for resource related fields. Added this now as well. So this is ready to merge. In case there are any comments of anything I missed, let me know.