[enhancement] Allow returning status explicitly with pagination

[johnbridstrup]

I've allowed the pagination decorator to still work if the method returns status_code, results. I understand it will almost always be 200 for paginated responses, but I like to explicitly set the codes anyway.

I've added this to _inject_pagination

# Check if function returns status code + iterable
code = None
is_two_tuple = isinstance(items, tuple) and len(items) == 2
if (
    is_two_tuple
    and isinstance(items[0], int)
    and isinstance(items[1], Iterable)
):
    code = items[0]
    items = items[1]
...
if code:
    return code, result
return result

I suppose I could see a case where someone returns a tuple instead of a list or queryset, and that it could possibly be exactly 2 entries, so if there is a more safe way I'm happy to implement that.

[johnbridstrup]

Relevant issue #1011

[pmdevita]

Just ran into this problem myself, would be nice to see this merged!

[vitalik]

I'm not sure about this one... my thoughts is actually returning tuples with included status was originally not best design idea

I'm actually leaning into some check that forces user output only querysets in paginated responses

[johnbridstrup]

I'm not sure about this one... my thoughts is actually returning tuples with included status was originally not best design idea

Yeah I understand my solution here was not particularly elegant. My reasoning for wanting this is that tuples with the code are allowed for the error responses, and I didn't like that some of the returns in my views were tuples and others were not.

I'm actually leaning into some check that forces user output only querysets in paginated responses

Does this mean if there is a caught error in the view that we just have to raise and let ninja handle adding the code to the response? Besides that there are a number of reasons I might want to return from a paginated view with codes other than 200, and it is a bit confusing that I HAVE to use tuples for those responses and then cannot for the nominal queryset response.

Perhaps using response classes like NinjaResponse and NinjaPaginatedResponse? Or is that too much of a change

[benjaoming]

I didn't notice this PR when I opened https://github.com/vitalik/django-ninja/pull/1148 because I went for fixing #940 :smile:

re @vitalik

I'm not sure about this one... my thoughts is actually returning tuples with included status was originally not best design idea

I'm actually leaning into some check that forces user output only querysets in paginated responses

I actually like the easiness of returning (status, ). However, I can see that it doesn't "scale", it's a very implicit data structure, which caused the issue here.

But as you can also see from the fix, it was very simple... but could definitely look more explicit.

I'm not sure how forcing only a queryset would make it impossible for list views that need pagination to benefit from differentiated status codes?

For my case, the reason that differentiated schemas+status codes are important: It enables entrypoints to implement authorization levels and return different schemas depending on access. There may be other similar cases.

My example could be a book_list that wants to return 200, BooksForAnonymousSchema and 222, BooksForRegisteredUserSchema. The BooksForRegisteredUserSchema would include annotations about what the user has rated the book, if they own a copy etc. I'm not sure if this is good API design, but this is currently where I've landed.