django-json-api / django-rest-framework-json-api

JSON:API support for Django REST framework
https://django-rest-framework-json-api.readthedocs.org/
BSD 2-Clause "Simplified" License
1.18k stars 294 forks source link

Unable to add links to action endpoints #885

Open BlackVoid opened 3 years ago

BlackVoid commented 3 years ago

Hi

I'm trying to figure out how to put links on objects to actions with this extension. With DRF without any other extensions I'm able to use HyperlinkedIdentityField in the serializer to add the link to a view as can be seen in the following code and screenshot of output json.

from rest_framework.viewsets import ModelViewSet
from rest_framework.relations import HyperlinkedIdentityField
from rest_framework.serializers import HyperlinkedModelSerializer

class ActivityListSet(ModelViewSet):
    queryset = Activity.objects.all()
    serializer_class = ActivitySerializer

    @action(detail=True)
    def geojson(self, request, pk=None):
        return Response({"a": "test"})

class ActivitySerializer(HyperlinkedModelSerializer):

    geo_json = HyperlinkedIdentityField(
        view_name="activity-geojson"
    )

    class Meta:
        model = Activity
        fields = [
            "self",
            "status",
            "error",
            "started_at",
            "completed_at",
            "geo_json"
        ]

image

However once switching over to use the DRF Json API equivalent (HyperlinkedIndentityField does not seem to exist) I'm getting the following error: type object 'Activity' has no attribute 'geo_json'

I've searched through the issues in the project as well as the documentation, but I'm unable to find out why this does not work with this extension. DRF also has an example with a similar use case as mine, so it seems like I'm using HyperlinkedIdentityField as intended

Would greatly appreciate if anyone can help me figure out how to solve this issue

sliverc commented 3 years ago

Thanks for your report. It could be that the support for HyperlinkedIdentityField is not complete. Could you provide an exception stacktrace?

If you have more time at hand best actually would be if you could provide a test which reproduces this issue within the test framework of DJA. There are tests for plain DRF classes support and there is a test for HyperlinkedIdentityField already. Somehow your use case seems to be different though.

sliverc commented 3 months ago

Some more info to this issue as I have discovered when working on #1228

HyperlinkedIdentityField is currently rendered as a relationship see code here which is not correct. It should actually be a link in the links object. This is already done for the url field which is added to links as self but other HyperlinkedIdentityField should be added there with the given field name.

fazeelghafoor commented 1 month ago

Hi @sliverc Is this issue still open, I would like to work on it

sliverc commented 1 month ago

@fazeelghafoor Thanks for volunteering. The thing is, before we start working on this, we should discuss what the best way forward is.

After my comment above, there was another discussion #1239 talking about where links can all appear. Even though initially I thought HyperlinkedIdentityField should be in resource links, I did some more investigation. It turns out that it is derived from HyperlinkedRelatedField which is clearly a relationship link and I think it is most natural to render it as a relationship with only links.

An exception is the HyperlinkedIdentiyField which is defined as URL_FIELD_NAME which is pointing to itself and is therefore represented in the resource links as technically not a relationship.

After the initial description of this bug report, I think we should first figure out what the current behavior is when using HyperlinkedIdentityField best with a test. There is code in the renderer handling HyperlinkedIdentityField, but it seems it might not be working, as this bug report suggests. Once we know what it currently does, we can decide how to move forward, also considering backwards compatibility if it actually renders.