Conversation
|
@enricofer Thanks for the patch. Could you please add some tests demonstrating the new function and fix the linting error? |
massadmin/massadmin.py
Outdated
| class MassEditMixin(object): | ||
|
|
||
| def get_actions(self, request): | ||
| actions = super(MassEditMixin, self).get_actions(request) |
There was a problem hiding this comment.
Couldn't we the Python 3+ style super().get_actions(request)?
There was a problem hiding this comment.
basically the suggested code keep the mass edit action hidden from admin action list. Core app code is untouched.
I don't know how to test this. Suggestions are wellcome.
massadmin/massadmin.py
Outdated
| def get_actions(self, request): | ||
| actions = super(MassEditMixin, self).get_actions(request) | ||
|
|
||
| if settings.MASS_USERS_GROUP and settings.MASS_USERS_GROUP in [g.name for g in request.user.groups.all()]: |
There was a problem hiding this comment.
Why do you use a group for this rather than checking permission with has_perm()?
It seems to me, that using permission would be much more flexible - the permission can be given to more than one groups.
You can also add permission to non-managed models so this functionality can work entirely without additional settings: https://stackoverflow.com/questions/13932774/how-can-i-use-django-permissions-without-defining-a-content-type-or-model
There was a problem hiding this comment.
I implemented a new "can perform mass editing" permission with a new proxy model as from stackoverflow answer.
everything seem fine.
|
@enricofer I was also thinking why does we need to limit the access restriction only for cases when I am not sure from the docs if it is available also for the global actions, but can you please try if it works? |
|
Unfortunately I can't get user permission checking with |
|
documentation update will follow |
|
And finally there is a demo site missing migration in the master repo: 3e7243b |
PetrDlouhy
left a comment
There was a problem hiding this comment.
@enricofer Did you update the docs yet? The README seems to still mention the group settings and group behavior.
|
@enricofer Thank you very much for everything. It looks very nice now. There is one last thing. Can you please add following tests:
|
Hi, thanks for your handy app.
With the PR I'm suggesting to limit the availability of 'Mass edit' action only to users belonging to a specified group.
Mass Edit action could be very dangerous and sometimes should be perfomed only by conscious users.
Best Regards.
Enrico