Skip to content

Update get_object() example in permissions.md #5401

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

Merged
merged 2 commits into from
Sep 8, 2017
Merged

Update get_object() example in permissions.md #5401

merged 2 commits into from
Sep 8, 2017

Conversation

sanjuroj
Copy link
Contributor

@sanjuroj sanjuroj commented Sep 7, 2017

I'm a bit confused about the example that's provided in the 'Object level permissions' section. Other examples (e.g. Tutorial 3 - Class Based Views) provided a pk to get_object(). It doesn't seem like this example has any way of identifying a specific object.

Just in case I'm correct, I've prepared this pull request. But if I'm wrong, would it be possible for you to explain the example I modified?

Many Thanks...

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Please describe your pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue. When linking to an issue, please use refs #... in the description of the pull request.

I'm a bit confused about the example that's provided in the 'Object level permissions' section.  Other examples (e.g. Tutorial 3 - Class Based Views) provided a pk to get_object().  It doesn't seem like this example has any way of identifying a specific object.  

Just in case I'm correct, I've prepared this pull request. But if I'm wrong, would it be possible for you to explain the example I modified?  

Many Thanks...
Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid issue. The fix isn't quite right.

Hopefully me comments are clear enough?

Thanks!

@@ -43,8 +43,8 @@ This will either raise a `PermissionDenied` or `NotAuthenticated` exception, or

For example:

def get_object(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add the parameter here.

def get_object(self):
obj = get_object_or_404(self.get_queryset())
def get_object(self, pk):
obj = get_object_or_404(self.get_queryset(), pk=pk)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default get_object() implementation does this:

        filter_kwargs = {self.lookup_field: self.kwargs[lookup_url_kwarg]}
        obj = get_object_or_404(queryset, **filter_kwargs)

That's a bit generic for the point being made here so I'd be happy with just:

    obj = get_object_or_404(queryset, pk=self.kwargs["pk"])

Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the adjustment

@sanjuroj
Copy link
Contributor Author

sanjuroj commented Sep 8, 2017

@carltongibson Ack, sorry, didn't quite get to it fast enough. Thanks for making the adjustment.

@carltongibson carltongibson merged commit 0e341c2 into encode:master Sep 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants