Skip to content

Commit

Permalink
Save Batch/Project permissions from admin forms even if custom_permis…
Browse files Browse the repository at this point in the history 
…sions flag is False (#143)
  • Loading branch information
@charman
charman committed Apr 20, 2021
1 parent 5f1df71 commit 98ea1dd
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 50 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Deprecate support for (EOL) Python 3.5, add support for
Python 3.9.
- On the Add/Change Batch and Add/Change Project forms, changes made
to Group and User permissions while editing the form are now saved
even if the "Restrict access" checkbox is unchecked (hiding the
form widgets for selecting Groups and Users) when the form is
submitted
- On the Add/Change User admin forms, the ability to assign superuser
status or grant model-level permission privileges to Users is now
restricted to superusers
Expand Down
98 changes: 48 additions & 50 deletions turkle/admin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,31 +779,30 @@ def save_model(self, request, obj, form, change):
super().save_model(request, obj, form, change)
logger.info("User(%i) updating Batch(%i) %s", request.user.id, obj.id, obj.name)

if 'custom_permissions' in form.data:
if 'can_work_on_groups' in form.data:
existing_groups = set(get_groups_with_perms(obj))
form_groups = set(form.cleaned_data['can_work_on_groups'])
groups_to_add = form_groups.difference(existing_groups)
groups_to_remove = existing_groups.difference(form_groups)
for group in groups_to_add:
assign_perm('can_work_on_batch', group, obj)
for group in groups_to_remove:
remove_perm('can_work_on_batch', group, obj)
else:
for group in get_groups_with_perms(obj):
remove_perm('can_work_on_batch', group, obj)
if 'can_work_on_users' in form.data:
existing_users = set(get_users_with_perms(obj, with_group_users=False))
form_users = set(form.cleaned_data['can_work_on_users'])
users_to_add = form_users.difference(existing_users)
users_to_remove = existing_users.difference(form_users)
for user in users_to_add:
assign_perm('can_work_on_batch', user, obj)
for user in users_to_remove:
remove_perm('can_work_on_batch', user, obj)
else:
for user in get_users_with_perms(obj, with_group_users=False):
remove_perm('can_work_on_batch', user, obj)
if 'can_work_on_groups' in form.data:
existing_groups = set(get_groups_with_perms(obj))
form_groups = set(form.cleaned_data['can_work_on_groups'])
groups_to_add = form_groups.difference(existing_groups)
groups_to_remove = existing_groups.difference(form_groups)
for group in groups_to_add:
assign_perm('can_work_on_batch', group, obj)
for group in groups_to_remove:
remove_perm('can_work_on_batch', group, obj)
else:
for group in get_groups_with_perms(obj):
remove_perm('can_work_on_batch', group, obj)
if 'can_work_on_users' in form.data:
existing_users = set(get_users_with_perms(obj, with_group_users=False))
form_users = set(form.cleaned_data['can_work_on_users'])
users_to_add = form_users.difference(existing_users)
users_to_remove = existing_users.difference(form_users)
for user in users_to_add:
assign_perm('can_work_on_batch', user, obj)
for user in users_to_remove:
remove_perm('can_work_on_batch', user, obj)
else:
for user in get_users_with_perms(obj, with_group_users=False):
remove_perm('can_work_on_batch', user, obj)

def stats(self, obj):
stats_url = reverse('turkle_admin:batch_stats', kwargs={'batch_id': obj.id})
Expand Down Expand Up @@ -1157,31 +1156,30 @@ def save_model(self, request, obj, form, change):
else:
logger.info("User(%i) updating Project(%i) %s", request.user.id, obj.id, obj.name)

if 'custom_permissions' in form.data:
if 'can_work_on_groups' in form.data:
existing_groups = set(get_groups_with_perms(obj))
form_groups = set(form.cleaned_data['can_work_on_groups'])
groups_to_add = form_groups.difference(existing_groups)
groups_to_remove = existing_groups.difference(form_groups)
for group in groups_to_add:
assign_perm('can_work_on', group, obj)
for group in groups_to_remove:
remove_perm('can_work_on', group, obj)
else:
for group in get_groups_with_perms(obj):
remove_perm('can_work_on', group, obj)
if 'can_work_on_users' in form.data:
existing_users = set(get_users_with_perms(obj, with_group_users=False))
form_users = set(form.cleaned_data['can_work_on_users'])
users_to_add = form_users.difference(existing_users)
users_to_remove = existing_users.difference(form_users)
for user in users_to_add:
assign_perm('can_work_on', user, obj)
for user in users_to_remove:
remove_perm('can_work_on', user, obj)
else:
for user in get_users_with_perms(obj, with_group_users=False):
remove_perm('can_work_on', user, obj)
if 'can_work_on_groups' in form.data:
existing_groups = set(get_groups_with_perms(obj))
form_groups = set(form.cleaned_data['can_work_on_groups'])
groups_to_add = form_groups.difference(existing_groups)
groups_to_remove = existing_groups.difference(form_groups)
for group in groups_to_add:
assign_perm('can_work_on', group, obj)
for group in groups_to_remove:
remove_perm('can_work_on', group, obj)
else:
for group in get_groups_with_perms(obj):
remove_perm('can_work_on', group, obj)
if 'can_work_on_users' in form.data:
existing_users = set(get_users_with_perms(obj, with_group_users=False))
form_users = set(form.cleaned_data['can_work_on_users'])
users_to_add = form_users.difference(existing_users)
users_to_remove = existing_users.difference(form_users)
for user in users_to_add:
assign_perm('can_work_on', user, obj)
for user in users_to_remove:
remove_perm('can_work_on', user, obj)
else:
for user in get_users_with_perms(obj, with_group_users=False):
remove_perm('can_work_on', user, obj)

def delete_model(self, request, obj):
logger.info("User(%i) deleting Project(%i) %s", request.user.id, obj.id, obj.name)
Expand Down
55 changes: 55 additions & 0 deletions turkle/tests/test_admin_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,32 @@ def test_batch_change_remove_and_add_user(self):
self.assertTrue(user_to_add.has_perm('can_work_on_batch', batch))
self.assertFalse(user_to_remove.has_perm('can_work_on', batch))

def test_batch_change_remove_and_add_user_with_custom_permissions_disabled(self):
project = Project.objects.create(name='testproject')
batch = Batch.objects.create(name='testbatch', project=project)
user_to_add = User.objects.create_user('user_to_add', password='secret')
user_to_remove = User.objects.create_user('user_to_remove', password='secret')
user_to_remove.add_obj_perm('can_work_on_batch', batch)
self.assertFalse(user_to_add.has_perm('can_work_on_batch', batch))
self.assertTrue(user_to_remove.has_perm('can_work_on_batch', batch))

client = django.test.Client()
client.login(username='admin', password='secret')
response = client.post(reverse('turkle_admin:turkle_batch_change',
args=(batch.id,)),
{
'assignments_per_task': 1,
'project': batch.project.id,
'name': 'newname',
'can_work_on_users': [user_to_add.id],
})
self.assertTrue(b'Please correct the error' not in response.content)
self.assertEqual(response.status_code, 302)
self.assertEqual(response['Location'], '/admin/turkle/batch/')
self.assertEqual(Batch.objects.filter(name='newname').count(), 1)
self.assertTrue(user_to_add.has_perm('can_work_on_batch', batch))
self.assertFalse(user_to_remove.has_perm('can_work_on', batch))

def test_batch_change_update_non_published(self):
self.test_batch_add()
batch = Batch.objects.get(name='batch_save')
Expand Down Expand Up @@ -645,6 +671,35 @@ def test_post_change_project_remove_and_add_group(self):
self.assertTrue(user_for_add.has_perm('can_work_on', project))
self.assertFalse(user_for_remove.has_perm('can_work_on', project))

def test_post_change_project_remove_and_add_group_with_custom_permissions_disabled(self):
project = Project.objects.create(name='testproject')
group_to_add = Group.objects.create(name='group_to_add')
group_to_remove = Group.objects.create(name='group_to_remove')
user_for_add = User.objects.create_user('user_for_add', password='secret')
user_for_remove = User.objects.create_user('user_for_remove', password='secret')
group_to_remove.add_obj_perm('can_work_on', project)
user_for_add.groups.add(group_to_add)
user_for_remove.groups.add(group_to_remove)
self.assertFalse(user_for_add.has_perm('can_work_on', project))
self.assertTrue(user_for_remove.has_perm('can_work_on', project))

client = django.test.Client()
client.login(username='admin', password='secret')
response = client.post(reverse('turkle_admin:turkle_project_change',
args=(project.id,)),
{
'assignments_per_task': 1,
'html_template': '<p>${foo}: ${bar}</p><textarea>',
'name': 'newname',
'can_work_on_groups': [group_to_add.id],
})
self.assertTrue(b'Please correct the error' not in response.content)
self.assertEqual(response.status_code, 302)
self.assertEqual(response['Location'], '/admin/turkle/project/')
self.assertEqual(Project.objects.filter(name='newname').count(), 1)
self.assertTrue(user_for_add.has_perm('can_work_on', project))
self.assertFalse(user_for_remove.has_perm('can_work_on', project))

def test_post_change_project_remove_and_add_user(self):
project = Project.objects.create(name='testproject')
user_to_add = User.objects.create_user('user_to_add', password='secret')
Expand Down

0 comments on commit 98ea1dd

Please sign in to comment.