Skip to content
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: don't break when user overrides ModelAdmin.has_change_permission #27

Merged
merged 1 commit into from
Jul 3, 2017

Conversation

PetrDlouhy
Copy link
Contributor

Current implementation is not compatible with projects and applications where is has_change_permission function is overriden. Such as django-advanced-filters. This fixes that.

@coveralls
Copy link

coveralls commented Jun 29, 2017

Coverage Status

Coverage decreased (-3.5%) to 96.471% when pulling 53dc325 on PetrDlouhy:master into 046048f on ctxis:master.

return change_permission
return change_permission

def has_change_permission_only_change(self, request, obj=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add some tests for this function? Also, could you make this function private as it is used internally only? I think has_change_permission_only it would be better name. The _change I think it is redundant.

@lefterisnik
Copy link
Contributor

lefterisnik commented Jun 29, 2017

Hi @PetrDlouhy,

Thanks you for the PR. It is very useful.

@lefterisnik lefterisnik merged commit 8e645b4 into ctxis:master Jul 3, 2017
@PetrDlouhy
Copy link
Contributor Author

@lefterisnik Thanks for merging. Are all requested changes already made?

@lefterisnik
Copy link
Contributor

Hi @PetrDlouhy, yes, they have been added 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants