-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Add validation for decorator order with @api_view #9821
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -87,64 +87,90 @@ def handler(self, *args, **kwargs): | |||||||||
| return decorator | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def _check_decorator_order(func, decorator_name): | ||||||||||
| """ | ||||||||||
| Check if an API policy decorator is being applied after @api_view. | ||||||||||
| """ | ||||||||||
| # Check if func is actually a view function (result of APIView.as_view()) | ||||||||||
| if hasattr(func, 'cls') and issubclass(func.cls, APIView): | ||||||||||
| raise TypeError( | ||||||||||
| f"@{decorator_name} must be applied before @api_view. " | ||||||||||
| f"The correct order is:\n\n" | ||||||||||
| f" @api_view(['GET'])\n" | ||||||||||
| f" @{decorator_name}(...)\n" | ||||||||||
|
||||||||||
| f" @api_view(['GET'])\n" | |
| f" @{decorator_name}(...)\n" | |
| f" @{decorator_name}(...)\n" | |
| f" @api_view(['GET'])\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The decorator needs to go below @api_view, just as the docs show—this way, DRF reads the attribute properly and permissions work.
Our tests confirm the documented order is correct; error is raised only if you swap them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will get back to you on saturday morning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, Copilot is wrong here, but that might be a symptom that "applied before" is a bit ambiguous in this case. The docs are actually using the term "after":
These must come after (below) the
@api_viewdecorator
The docs refer to the order the decorators appear in the code: as we read top to bottom the order is swapped vs how they are applied by the code (bottom to top).
Understanding how decorators are applied by the language is important, but less experience people might not know it, and therefore won't understand the error message.
I'd rather make the language of the error message closer to what we have in the docs.
Uh oh!
There was an error while loading. Please reload this page.