-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Fix schemas for extra actions #5992
Conversation
rest_framework/views.py
Outdated
# copy class-level schema to prevent instances using the same object | ||
if 'schema' not in self.__dict__: | ||
self.schema = deepcopy(self.schema) | ||
self.schema.view = self |
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.
Can we do this in ViewInspector.__get__()
? (The goal being to keep the api out out APIView
, bar the minimal schema
property.)
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.
Let me take a second look at this. It might be possible to do this with __set__
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.
Alright, updated the descriptor with a setter. This seems to work fine.
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. This looks good. Let me have a play with it and I'll get back to you. 👍
17d42c6
to
df1d4b7
Compare
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, again, great stuff.
I think we need to add something about the schema
argument to @action()
to the docs.
d9b4f63
to
f3dd14e
Compare
@carltongibson - rebased/updated the PR. I've opted for a note in the shema docs instead of the viewset docs, given that the latter don't currently mention schemas. |
* Add failing test for extra action schemas * Add ViewInspector setter to store instances * Fix schema disabling for extra actions * Add docs note about disabling schemas for actions
Ensures that view instances are operating on their own
schema
instance. Previously, all instances of the class were using a shared, class-level schema instance, and the descriptor was mutating the schema to ensure the most recently accessed view instance was set on the schema.