Skip to content

Conversation

@donewell
Copy link

No description provided.

change detail to message and update text
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we simplify this to self.permission_denied(request, message=getattr(permission, 'message', None))?

@donewell
Copy link
Author

Thanks. Both suggestions worked and helped to simplify the code. I also made a small change to the custom message used in the tests.

@jpadilla
Copy link
Contributor

@tomchristie This seems like a good and simple addition, but perhaps it should go on 3.1. Thoughts?

@lovelydinosaur
Copy link
Contributor

@jpadilla : Still pending a decision, but it doesn't matter that the pull request is against master - we're not planning on any further 3.0 releases so we'd be fine to just merge both this and the 3.1 branch to master.

@lovelydinosaur
Copy link
Contributor

Given that we don't do this uniformly throughout other exception classes let's not just do this in a single place.

@donewell
Copy link
Author

donewell commented Mar 3, 2015

The validator API uses the same convention. At the moment it's very brittle to create permissions with custom messages, so it would be worth thinking about other ways we could address this. The most natural, elegant solutions involve some kind of metaprogramming.

@donewell
Copy link
Author

donewell commented Mar 3, 2015

Have you considered an approach where an exception takes a permission as a parameter? At the moment default_details are implemented on both the permission and exception.

@lovelydinosaur lovelydinosaur reopened this Mar 3, 2015
@lovelydinosaur
Copy link
Contributor

Reopening for later review.

@lovelydinosaur lovelydinosaur added this to the 3.1.4 Release milestone Jun 24, 2015
@lovelydinosaur
Copy link
Contributor

Sure.

lovelydinosaur added a commit that referenced this pull request Jun 24, 2015
@lovelydinosaur lovelydinosaur merged commit 8329411 into encode:master Jun 24, 2015
@lovelydinosaur lovelydinosaur changed the title add message to custom permission Allow permission classes to customize the failure message. Jun 24, 2015
@lovelydinosaur lovelydinosaur modified the milestones: 3.1.4 Release, 3.2.0 Release Jul 30, 2015
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.

3 participants