Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

proposal: DJA DefaultRouter #467

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 Aug 28, 2018 · 11 comments
Closed

proposal: DJA DefaultRouter #467

n2ygk opened this issue Aug 28, 2018 · 11 comments

Comments

@n2ygk
Copy link
Contributor

n2ygk commented Aug 28, 2018

Proposal: rest_framework_json_api.routers.DefaultRouter

Current method of declaring URLPatterns

The current typical usage example is like this:

from rest_framework import routers

router = routers.DefaultRouter(trailing_slash=False)

router.register(r'blogs', BlogViewSet)
router.register(r'entries', EntryViewSet)

urlpatterns = [
    # basic collection and item URLs:
    url(r'^', include(router.urls)),
    # relationships URLs:
    url(r'^entries/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        EntryRelationshipView.as_view(),
        name='entry-relationships'),
    url(r'^blogs/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        BlogRelationshipView.as_view(),
        name='blog-relationships'),
   # related URLs:
    url(r'entries/(?P<entry_pk>[^/.]+)/blog',
        BlogViewSet.as_view({'get': 'retrieve'}),
        name='entry-blog'),
]

URLPatterns created by the Default Router

The above creates the following URLPatterns for router.register(r'blogs', BlogViewSet):

 00 = {URLPattern} <URLPattern '^blogs$' [name='blog-list']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-list'
  pattern = {RegexPattern} ^blogs$
   _is_endpoint = {bool} True
   _regex = {str} '^blogs$'
   _regex_dict = {dict} {}
   converters = {dict} {}
   name = {str} 'blog-list'
   regex = {SRE_Pattern} re.compile('^blogs$')
 01 = {URLPattern} <URLPattern '^blogs\.(?P<format>[a-z0-9]+)/?$' [name='blog-list']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-list'
  pattern = {RegexPattern} ^blogs\.(?P<format>[a-z0-9]+)/?$
   _is_endpoint = {bool} True
   _regex = {str} '^blogs\\.(?P<format>[a-z0-9]+)/?$'
   _regex_dict = {dict} {}
   converters = {dict} {}
   name = {str} 'blog-list'
   regex = {SRE_Pattern} re.compile('^blogs\\.(?P<format>[a-z0-9]+)/?$')
 02 = {URLPattern} <URLPattern '^blogs/(?P<pk>[^/.]+)$' [name='blog-detail']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-detail'
  pattern = {RegexPattern} ^blogs/(?P<pk>[^/.]+)$
   _is_endpoint = {bool} True
   _regex = {str} '^blogs/(?P<pk>[^/.]+)$'
   _regex_dict = {dict} {}
   converters = {dict} {}
   name = {str} 'blog-detail'
   regex = {SRE_Pattern} re.compile('^blogs/(?P<pk>[^/.]+)$')
 03 = {URLPattern} <URLPattern '^blogs/(?P<pk>[^/.]+)\.(?P<format>[a-z0-9]+)/?$' [name='blog-detail']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-detail'
  pattern = {RegexPattern} ^blogs/(?P<pk>[^/.]+)\.(?P<format>[a-z0-9]+)/?$

along with an api-root view that lists the available registry entries. This is a nice feature.

Drawbacks of the current method

  • Relationships & related URLs follow a standard pattern yet have to be "manually" declared separately. This is hard to do, error prone and just plain annoying.
  • We might not need the .<format> flavor of each ViewSet. (Not a huge concern.)

Proposed: rest_framework_json_api.DefaultRouter

Create a DJA DefaultRouter that extends DRF's DefaultRouter:

  1. Add URLPatterns for relationships and their RelationshipView. This seems pretty straightforward. Just add another argument to DefaultRouter.register() (or apply a default naming pattern) to add the RelationshipView.
  2. Add URLPatterns for related links. This is a bit harder as each related link has to be enumerated (link name and ViewSet).

The goal would be to have a URLPatterns that looks like this:

from rest_framework_json_api import routers

router = routers.DefaultRouter(trailing_slash=False)

router.register(r'blogs', BlogViewSet, BlogRelationshipView)
router.register(r'entries', EntryViewSet, EntryRelationshipView, ??related stuff here??)

urlpatterns = [
    url(r'^', include(router.urls)),
]

For how to do the "related stuff here" take a look at @Anton-Shutik's approach in #451

@sliverc
Copy link
Member

sliverc commented Aug 29, 2018

I like the idea 👍

I think to register related urls as implemented in #451 will be fairly straight forward. With RelationshipView is going to be a bit harder but playing around with implementation might lead to a good solution. Only thing which also need to be considered is to implement a json api specific SimpleRouter and DefaultRouter as both might be used.

@n2ygk

This comment has been minimized.

@sliverc

This comment has been minimized.

@n2ygk

This comment has been minimized.

@n2ygk n2ygk changed the title proposal: JSONAPIRouter proposal: DJA DefaultRouter Aug 30, 2018
n2ygk added a commit to n2ygk/django-rest-framework-json-api that referenced this issue Aug 30, 2018
Per discussion about naming, the idea is that it should be easy to updgrade from DRF to DJA
by simply changing some imports, retaining the same DRF (or in this case, django-filter) class
names that are extended by DJA.
see django-json-api#467 (comment)
n2ygk added a commit to n2ygk/django-rest-framework-json-api that referenced this issue Aug 30, 2018
- see django-json-api#467 (comment)
- JSONAPIPageNumberPagination goes back to PageNumberPagination
- JSONAPILimitOffsetPagination goas back to LimitOffsetPagination
- requires new setting: `JSON_API_STANDARD_PAGINATION=True` to get `page[number]` and `page[size]` instead of
  `page` and `page_size`. This is a breaking change no matter which is the default value.
n2ygk added a commit to n2ygk/django-rest-framework-json-api that referenced this issue Aug 30, 2018
- see django-json-api#467 (comment)
- JSONAPIPageNumberPagination goes back to PageNumberPagination
- JSONAPILimitOffsetPagination goas back to LimitOffsetPagination
- requires new setting: `JSON_API_STANDARD_PAGINATION=True` to get `page[number]` and `page[size]` instead of
  `page` and `page_size`. This is a breaking change no matter which is the default value.
@sliverc

This comment has been minimized.

@n2ygk
Copy link
Contributor Author

n2ygk commented Aug 31, 2018

I've "moved" this comment thread about naming to it's own issue: #471

@auvipy
Copy link
Contributor

auvipy commented Nov 13, 2019

might be related encode/django-rest-framework#6789

@platinumazure
Copy link
Contributor

Is this something the project is still interested in? When I started using DJA, I found it a little frustrating and unintuitive that we couldn't just "automatically" include the relationship and related resource routes.

If a PR is likely to be accepted, I could look into this within the next 1-2 weeks.

@auvipy
Copy link
Contributor

auvipy commented Dec 28, 2020

Is this something the project is still interested in? When I started using DJA, I found it a little frustrating and unintuitive that we couldn't just "automatically" include the relationship and related resource routes.

If a PR is likely to be accepted, I could look into this within the next 1-2 weeks.

A PR is certainly welcome in this regard

@sliverc
Copy link
Member

sliverc commented Dec 28, 2020

@platinumazure I don't think this will be straight forward but I would love to see this implemented as well.

@sliverc
Copy link
Member

sliverc commented Sep 23, 2021

This is a great idea which we should investigate further. Especially thinking about how the RelatedMixin could be replaced with a router as the current implementation is confusing especially when it comes to permissions and lacks feature such as filter support. See also #916

I do think though we should talk this through more about what a proper design of a DJA router would be and of such a discussion or proof of concept in that regard extract specific issues. Therefore converting this issue to a discussion.

@django-json-api django-json-api locked and limited conversation to collaborators Sep 23, 2021
@sliverc sliverc closed this as completed Sep 23, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

4 participants