inflection ignored in lookup by RelationshipView

[sdhawley]

I did a quick search and didn't find a similar issue, but I apologize if I missed one on this topic. And, maybe this is an enhancement request rather than a bug report. It is an inconsistency in type names between the url and rendered document.

For an api view of a model I have eg http://localhost/api/customer-accounts/ where the relationship type (lets just call it relationship-name) is inflected correctly according to the setting JSON_API_FORMAT_TYPES (dasherized, in this case)

{
    "links": {
        "first": "http://localhost/api/customer-accounts/?page%5Bnumber%5D=1",
        "last": "http://localhost/api/customer-accounts/?page%5Bnumber%5D=1",
        "next": null,
        "prev": null
    },
    "data": [
        {
            "type": "customer-accounts",
            "id": "1",
            "attributes": {
                "account-id": 1,
                "account": "1234",
                "insert-ts": "2020-05-20T20:42:52.895506Z",
                "update-ts": "2020-05-20T20:42:52.895643Z"
            },
            "relationships": {
                "relationship-name": {
                    "data": {
                        "type": "relationship-name",
                        "id": "1"
                    }
                }
            }
        }
    ],
    "meta": {
        "pagination": {
            "page": 1,
            "pages": 1,
            "count": 1
        }
    }
}

Then I go to eg http://localhost/api/customer-accounts/1/relationships/relationship-name/ and get a 404. Instead, I find the relationship at http://localhost/api/customer-accounts/1/relationships/relationship_name/ which looks like:

{
    "data": {
        "type": "relationship-name",
        "id": "1"
    }
}

The rendered document has the correctly inflected type, which doesn't match the url.

I am working around this for now using field name mapping in RelationshipView

field_name_mapping = {
        'relationship-name': 'relationship_name'
    }

Could RelationshipView pass relationship-name through the inflector first to get it back to the internal python (underscore) convention?

[sliverc]

Thanks for reporting. I agree the link of RelationshipView should adhere to the inflection setting and the current behavior is inconsistent. However it is actually the setting JSON_API_FORMAT_FIELD_NAMES it should follow as it is the name of the relationship and not the type.

PR is welcome. This change will be breaking though so I think best would be to still allow underscored values for now + the correct inflected value url. When the underscored verison is accessed a deprecation warning should be raised pointing to the right url.

[platinumazure]

Hi @sliverc, I'm interested in seeing this fixed and I am willing to take a shot at this.

However, this will be my first contribution to a Python codebase, let alone anything related to Django or DRF or DJA. So, it could be a while before I have a PR up. (If anyone else wants to jump on this first, no worries, please just comment here so I'm aware-- thanks!)

I'm definitely willing to put in the hard work and figure out where the necessary changes would need to go. However, I would appreciate if you or someone else could point to an example or documentation around raising a deprecation warning. Is that a standard Python exception, or something more specific to Django, DRF, or DJA? Once I understand what a deprecation warning would look like, I think I should be able to work everything else out. Thanks in advance for your help!

[sliverc]

Thanks @platinumazure that you are willing to work on this issue.

I think inflection for relationship view should be the default as I consider this issue a bug. As this is not a minor change it is certainly good to implement it backwards compatible. I think easiest would be to implement a setting where inflection of relationship view can be turned on. However when the setting is off and someone uses the relationship view a deprecation warning can be raised.

In PR #776 a similar approach has been taken so you can find how to introduce a new setting and how to raise a deprecation warning.

Let me know if you need any more hinters.

[platinumazure]

Thanks for the reply!

I'm about 40% through on a local branch. I am successfully inflecting the related and relationship URLs in the links objects and have modified tests, and added new ones. (By the way, I've been adding new tests to the tests package, outside of example. Is that where tests should be going from now on? Or should they be in the example app?)

I have realized that the changes to the relationship and related views will be more difficult, because those views need to reverse the inflection from the URL related_field kwarg to find the field for retrieving the related objects queryset.

The approach I currently have in mind is to create a dict in the RelatedMixin sometime after the serializer fields are initialized, with the keys composed of the inflected field names and the values being either the fields themselves or the default field names. In order to do that, though, I might need a hook around field object initialization. At any rate, I'm not 100% sure it's the best approach; do you have any other suggestions?

Also, I'm thinking it might be worth creating a new setting for this inflection, which may also allow this to be backward compatible more easily. For example, I like using "camelize" for JSON_API_FORMAT_FIELD_NAMES, but "dasherize" for JSON_API_FORMAT_TYPES and how I would prefer to inflect the related field kwarg (i.e., I prefer different formatting for URL vs JSON payloads). So, if I just throw this into a new setting, that might remove the need to issue a warning... What do you think?

I'll take a look at #776 as well per your recommendation. Thanks!

[sliverc]

By the way, I've been adding new tests to the tests package, outside of example. Is that where tests should be going from now on? Or should they be in the example app?

We are migrating our tests to the tests module. New tests should be added there.

So, if I just throw this into a new setting, that might remove the need to issue a warning... What do you think?

At first I have considered this issue a bug as I assumed that how the relationship name is formatted the link should be formatted as well. But when I read through the specification there is no hint that this actually needs to be the case. And when thinking about it again it might be confusing that JSON_API_FORMAT_FIELD_NAMES formats the url as well. So I think it is fine when we introduce an option like JSON_API_FORMAT_LINKS and make this a feature instead of a bugfix.

I have realized that the changes to the relationship and related views will be more difficult, because those views need to reverse the inflection from the URL related_field kwarg to find the field for retrieving the related objects queryset.

I only had a quick glance at the code but can you not adjust the method get_related_field_name so it returns the reversed field name?

[platinumazure]

I only had a quick glance at the code but can you not adjust the method get_related_field_name so it returns the reversed field name?

Yes, that's where the lookup ultimately needs to happen, but the difficulty is "un-inflecting" the inflected field name.

As an example, let's say I'm using a "dasherize" setting and I have a relationship URL ending with created-by. Both createdBy and created_by are valid field names and would be inflected to created-by in the URL (along with a few other possibilities).

So the get_related_field_name method needs to take the inflected name as input, and get the correct un-inflected name as output. One option is simply to do a linear search over all the fields and find the one that inflected to the same value as the input. But I think there could be performance impact on larger serializers. So I think it makes more sense to create a lookup on serializer or view initialization if possible. But if you're not worried about performance impact from a linear search of fields, I could do it that way too.

Let me know if I can further clarify anything.

[sliverc]

When the feature of formatting is being used DJA assumes that the field names are named with the Python convention of underscoring. This is currently done in the parser so you can do it here too.

[platinumazure]

Got it, then I can do the same thing here for now. Thanks!