Skip to content

Right way to represent relationships links.related view? #426

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

Closed
n2ygk opened this issue Apr 26, 2018 · 7 comments
Closed

Right way to represent relationships links.related view? #426

n2ygk opened this issue Apr 26, 2018 · 7 comments

Comments

@n2ygk
Copy link
Contributor

n2ygk commented Apr 26, 2018

I'm trying to correctly return a links.related view (not links.self) and haven't been able to find a good example. In looking at the example app it appears to have the same problem. Here's a fragment of my urlconf and view definitions:

router = routers.DefaultRouter()
router.register(r'courses', views.CourseViewSet)
router.register(r'course_terms', views.CourseTermViewSet)

urlpatterns = [
    url(r'^v1/', include(router.urls)),
    # http://127.0.0.1:8000/v1/courses/f009e671-b615-42c7-b35f-8500d7ef5d24/relationships/course_terms
    url(r'^v1/courses/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        views.CourseRelationshipView.as_view(),
        name='course-relationships'),
    # http://127.0.0.1:8000/v1/courses/f009e671-b615-42c7-b35f-8500d7ef5d24/course_terms/
    url(r'^v1/courses/(?P<fk>[^/.]+)/course_terms/',  # need fk (course_id) not pk (id)
        views.CourseTermViewSet.as_view({'get': 'list'}),
        name='course-course_terms'),
]
class CourseBaseViewSet(AuthnAuthzMixIn, SortMixin, FilterMixin, viewsets.ModelViewSet):
    pass


class CourseViewSet(CourseBaseViewSet):
    # API endpoint that allows course to be viewed or edited.
    queryset = Course.objects.all()
    serializer_class = CourseSerializer


class CourseTermViewSet(CourseBaseViewSet):
    # API endpoint that allows CourseTerm to be viewed or edited.
    queryset = CourseTerm.objects.all()
    serializer_class = CourseTermSerializer
#    queryset = queryset.filter(course_id='ec008c20-79a1-4ca7-931a-019d62c219c9')


class CourseRelationshipView(AuthnAuthzMixIn, RelationshipView):
    queryset = Course.objects
    self_link_view_name = 'course-relationships'

A get of a course looks like this:
GET http://127.0.0.1:8000/v1/courses/ec008c20-79a1-4ca7-931a-019d62c219c9/

{
    "data": {
        "type": "courses",
        "id": "ec008c20-79a1-4ca7-931a-019d62c219c9",
        "attributes": {
            "school_bulletin_prefix_code": "XCEFK9",
            "suffix_two": "00",
            "subject_area_code": "PSYB",
            "course_number": "00241",
            "course_identifier": "PSYC1138X",
            "course_name": "SOCIAL PSYCHOLOGY-LEC",
            "course_description": "SOCIAL PSYCHOLOGY-LEC",
            "effective_start_date": null,
            "effective_end_date": null,
            "last_mod_user_name": "loader",
            "last_mod_date": "2018-03-11"
        },
        "relationships": {
            "course_terms": {
                "data": [
                    {
                        "type": "course_terms",
                        "id": "3ce01e27-9a68-4970-b48a-e6a83166ca41"
                    }
                ],
                "links": {
                    "self": "http://127.0.0.1:8000/v1/courses/ec008c20-79a1-4ca7-931a-019d62c219c9/relationships/course_terms",
                    "related": "http://127.0.0.1:8000/v1/courses/ec008c20-79a1-4ca7-931a-019d62c219c9/course_terms/"
                },
                "meta": {
                    "count": 1
                }
            }
        },
        "links": {
            "self": "http://127.0.0.1:8000/v1/courses/ec008c20-79a1-4ca7-931a-019d62c219c9/"
        }
    }
}

But GET http://127.0.0.1:8000/v1/courses/ec008c20-79a1-4ca7-931a-019d62c219c9/course_terms/ returns all the course_terms, not just those with fk=ec008c20-79a1-4ca7-931a-019d62c219c9 because the foreign key is not being applied. (You can see where I tested and with queryset = queryset.filter(course_id='ec008c20-79a1-4ca7-931a-019d62c219c9')) so I'm wondering what the right way to do this is.

Am I missing something? I've reproduced this with the example app and it also does it "wrong". I think I need to extend the view function to check for some kwargs (e.g. fk) and filter the manager.

Thanks in advance for any help.

@n2ygk
Copy link
Contributor Author

n2ygk commented Apr 27, 2018

Here's my workaround but I suspect I'm just missing the obvious right way!
@sliverc -- Have you run into this?

  1. revert the URLs back to pk:
    url(r'^v1/courses/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        views.CourseRelationshipView.as_view(),
        name='course-relationships'),
    url(r'^v1/courses/(?P<pk>[^/.]+)/course_terms/',
        views.CourseTermViewSet.as_view({'get': 'list'}),
        name='course-course_terms'),
  1. extend list() to do the filtering when pk is present in kwargs.:
class CourseTermViewSet(CourseBaseViewSet):
    # API endpoint that allows CourseTerm to be viewed or edited.
    # if lookup_field (pk) is passed in to the view, then the query is filtered for matching foreign keys
    # TODO: there must be a correct way to do this as the RelationshipView does the right thing.
    queryset = CourseTerm.objects.all()
    serializer_class = CourseTermSerializer

    def list(self, request, *args, **kwargs):
        if __class__.lookup_field in kwargs:
            print("FK {}".format(kwargs[__class__.lookup_field]))
            self.queryset = self.get_queryset().filter(course_id=kwargs[__class__.lookup_field])
        return super(CourseTermViewSet, self).list(request, *args, **kwargs)

@sliverc
Copy link
Member

sliverc commented Apr 27, 2018

I haven't really used RelationshipVIew. But I assume you have followed the documentation on http://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html?highlight=RelationshipView#relationshipview

Potentially it is a bug. If you can reproduce it in the example app best write a test case and open a PR. Potentially a fix might be obvious then and can be added as well or someone else can have a look.

@n2ygk
Copy link
Contributor Author

n2ygk commented Apr 27, 2018 via email

@sliverc sliverc added the bug label Jun 12, 2018
@Anton-Shutik
Copy link
Contributor

@n2ygk I think you can do like it was done in example:

Configure url with parameter named entry_pk. It is important to give another name then pk

url(r'entries/(?P<entry_pk>[^/.]+)/comments',
        CommentViewSet.as_view({'get': 'list'}),
        name='entry-comments'),

Then in the view you can check kwargs dict for that parameter, like this:

class CommentViewSet(ModelViewSet):
    ...

    def get_queryset(self, *args, **kwargs):
        entry_pk = self.kwargs.get('entry_pk', None)
        if entry_pk is not None:
            return self.queryset.filter(entry_id=entry_pk)

        return super(CommentViewSet, self).get_queryset()

Thus, it will return only comments related to given entry.

@n2ygk
Copy link
Contributor Author

n2ygk commented Aug 10, 2018

@Anton-Shutik Thanks, yes, here's what I've ended up with that works basically the same way, just overriding list:

router = rest_framework.routers.DefaultRouter()
# ...
router.register(r'course_terms', views.CourseTermViewSet)

urlpatterns = [
    # ...
    url(r'^v1/', include(router.urls)),
    url(r'^v1/course_terms/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        views.CourseTermRelationshipView.as_view(),
        name='course_term-relationships'),
    url(r'^v1/course_terms/(?P<pk>[^/.]+)/course/',
        views.CourseViewSet.as_view({'get': 'list'}),
        name='course_terms-course'),
]

class CourseTermViewSet(CourseBaseViewSet):
    queryset = CourseTerm.objects.all()
    serializer_class = CourseTermSerializer

    def list(self, request, *args, **kwargs):
        if __class__.lookup_field in kwargs:
            self.queryset = self.get_queryset().filter(course=kwargs[__class__.lookup_field])
        return super(CourseTermViewSet, self).list(request, *args, **kwargs)

What would be really nice to have is a router = rest_framework_json_api.routers.JSONAPIRouter()
that builds all the necessary URLs to implement the normal resource along with relationships and related URLs so that I can just router.register(r'course_terms') instead of having to explicitly create routes for relationships and related URLs and a rest_framework_json_api.views.ModelViewSet that extends list (my example) or get_queryset (your example).

@n2ygk
Copy link
Contributor Author

n2ygk commented Aug 10, 2018

@Anton-Shutik Maybe I should take a closer look at #451

@Anton-Shutik
Copy link
Contributor

@n2ygk yep, probably @sliverc will merge it soon.

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

No branches or pull requests

3 participants