From 4f6d416f5a5dd2bb10ac3a64466958d62d4cb241 Mon Sep 17 00:00:00 2001 From: Deepsingh Chhabda Date: Wed, 25 Jan 2023 11:08:59 -0500 Subject: [PATCH] RDISCROWD-5627: Allow editing of taskruns (#804) * allow editing of taskruns * update tests * code review --- pybossa/api/task.py | 3 +++ pybossa/api/task_run.py | 4 ++++ pybossa/auth/taskrun.py | 10 +++++++++- test/test_api/test_task_api.py | 8 ++++---- test/test_api/test_taskrun_api.py | 20 +++++++++++++------- test/test_authorization/test_taskrun_auth.py | 8 ++++---- 6 files changed, 37 insertions(+), 16 deletions(-) diff --git a/pybossa/api/task.py b/pybossa/api/task.py index 40928e704e..9d1645302b 100644 --- a/pybossa/api/task.py +++ b/pybossa/api/task.py @@ -62,6 +62,9 @@ def _forbidden_attributes(self, data): raise BadRequest("Reserved keys in payload") def _update_attribute(self, new, old): + for key, value in old.info.items(): + new.info.setdefault(key, value) + gold_task = bool(new.gold_answers) n_taskruns = len(new.task_runs) if new.state == 'completed': diff --git a/pybossa/api/task_run.py b/pybossa/api/task_run.py index 248cf4e6cd..61e82b2b3d 100644 --- a/pybossa/api/task_run.py +++ b/pybossa/api/task_run.py @@ -114,6 +114,10 @@ def _update_object(self, taskrun): self._add_user_info(taskrun) self._add_timestamps(taskrun, task, guard) + def _update_attribute(self, new, old): + for key, value in old.info.items(): + new.info.setdefault(key, value) + def _forbidden_attributes(self, data): for key in data.keys(): if key in self.reserved_keys: diff --git a/pybossa/auth/taskrun.py b/pybossa/auth/taskrun.py index 2c4b83d694..0a2659b948 100644 --- a/pybossa/auth/taskrun.py +++ b/pybossa/auth/taskrun.py @@ -67,7 +67,15 @@ def _read(self, user, taskrun=None): return user.is_authenticated def _update(self, user, taskrun): - return self._delete(user, taskrun) + if user.is_anonymous: + return False + + if taskrun.user_id is None: + return user.admin + project = self.project_repo.get(taskrun.project_id) + allow_taskrun_edit = project.info.get("allow_taskrun_edit") or False + return user.admin or (allow_taskrun_edit and taskrun.user_id == user.id) + def _delete(self, user, taskrun): if user.is_anonymous: diff --git a/test/test_api/test_task_api.py b/test/test_api/test_task_api.py index d777fd4632..10759255d5 100644 --- a/test/test_api/test_task_api.py +++ b/test/test_api/test_task_api.py @@ -680,7 +680,7 @@ def test_task_post(self): assert_equal(out.info, 'my root task data'), out assert_equal(out.project_id, project.id) - # test user_pref + # test user_pref root_data = dict(project_id=project.id, info='my root task data') root_data['user_pref'] = dict(languages=["Traditional Chinese"], locations=["United States"], assign_user=["email@domain.com"]) res = self.app.post('/api/task?api_key=' + admin.api_key, @@ -787,8 +787,8 @@ def test_task_update(self): make_subadmin(user) non_owner = UserFactory.create() project = ProjectFactory.create(owner=user) - task = TaskFactory.create(project=project) - root_task = TaskFactory.create(project=project) + task = TaskFactory.create(project=project, info=dict(x=1)) + root_task = TaskFactory.create(project=project, info=dict(x=1)) data = {'n_answers': 1} datajson = json.dumps(data) root_data = {'n_answers': 4} @@ -852,7 +852,7 @@ def test_task_update_state(self): user = UserFactory.create() project = ProjectFactory.create(owner=user) task = TaskFactory.create(project=project, n_answers=1, - state='ongoing') + state='ongoing', info=dict(x=1)) data = {'n_answers': 2} datajson = json.dumps(data) diff --git a/test/test_api/test_taskrun_api.py b/test/test_api/test_taskrun_api.py index e216c18c6d..6b2dfa5449 100644 --- a/test/test_api/test_taskrun_api.py +++ b/test/test_api/test_taskrun_api.py @@ -765,9 +765,9 @@ def test_taskrun_update_with_result(self): project = ProjectFactory.create(owner=owner) task = TaskFactory.create(project=project, n_answers=2) anonymous_taskrun = AnonymousTaskRunFactory.create(task=task, info='my task result') - user_taskrun = TaskRunFactory.create(task=task, user=owner, info='my task result') + user_taskrun = TaskRunFactory.create(task=task, user=owner, info=dict(x='my task result')) - task_run = dict(project_id=project.id, task_id=task.id, info='another result') + task_run = dict(project_id=project.id, task_id=task.id, info=dict(x='another result')) datajson = json.dumps(task_run) # anonymous user @@ -817,7 +817,7 @@ def test_taskrun_update_with_result(self): # root user url = '/api/taskrun/%s?api_key=%s' % (user_taskrun.id, admin.api_key) res = self.app.put(url, data=datajson) - assert_equal(res.status, '403 FORBIDDEN', error_msg) + assert_equal(res.status, '200 OK', error_msg) @with_context @@ -829,9 +829,9 @@ def test_taskrun_update(self): project = ProjectFactory.create(owner=owner) task = TaskFactory.create(project=project) anonymous_taskrun = AnonymousTaskRunFactory.create(task=task, info='my task result') - user_taskrun = TaskRunFactory.create(task=task, user=owner, info='my task result') + user_taskrun = TaskRunFactory.create(task=task, user=owner, info=dict(x='my task result')) - task_run = dict(project_id=project.id, task_id=task.id, info='another result') + task_run = dict(project_id=project.id, task_id=task.id, info=dict(x='another result')) datajson = json.dumps(task_run) # anonymous user @@ -860,10 +860,16 @@ def test_taskrun_update(self): datajson = json.dumps(datajson) url = '/api/taskrun/%s?api_key=%s' % (user_taskrun.id, owner.api_key) res = self.app.put(url, data=datajson) + assert_equal(res.status, '403 FORBIDDEN', res.data) + + # project configured to allow editing of taskruns + # admin and taskrun submitter can edit taskruns + project.info["allow_taskrun_edit"] = True + project_repo.save(project) + res = self.app.put(url, data=datajson) out = json.loads(res.data) assert res.status_code == 200 - assert out['info'] == 'another result' - # assert_equal(res.status, '403 FORBIDDEN', res.data) + assert out['info'] == {'x': 'another result'} # PUT with not JSON data res = self.app.put(url, data=task_run) diff --git a/test/test_authorization/test_taskrun_auth.py b/test/test_authorization/test_taskrun_auth.py index 6d07fe9beb..18a45069fb 100644 --- a/test/test_authorization/test_taskrun_auth.py +++ b/test/test_authorization/test_taskrun_auth.py @@ -210,12 +210,12 @@ def test_authenticated_user_update_anonymous_taskrun(self): @with_context @patch('pybossa.auth.current_user', new=mock_admin) def test_admin_update_anonymous_taskrun_result(self): - """Test admins cannot update anonymously posted taskruns + """Test admins can update anonymously posted taskruns when there is a result associated.""" task = TaskFactory.create(n_answers=1) anonymous_taskrun = AnonymousTaskRunFactory.create(task=task) - assert_raises(Forbidden, + assert_not_raises(Forbidden, ensure_authorized_to, 'update', anonymous_taskrun) @with_context @@ -247,7 +247,7 @@ def test_authenticated_user_update_other_users_taskrun(self): assert self.mock_authenticated.id == own_taskrun.user.id assert self.mock_authenticated.id != other_users_taskrun.user.id - assert_not_raises(Exception, ensure_authorized_to, + assert_raises(Exception, ensure_authorized_to, 'update', own_taskrun) assert_raises(Forbidden, ensure_authorized_to, 'update', other_users_taskrun) @@ -288,7 +288,7 @@ def test_admin_update_user_taskrun_with_result(self): user_taskrun = TaskRunFactory.create(task=task) assert self.mock_admin.id != user_taskrun.user.id - assert_raises(Forbidden, ensure_authorized_to, + assert_not_raises(Forbidden, ensure_authorized_to, 'update', user_taskrun) @with_context