RDISCROWD-5627: Allow editing of taskruns (#804)

* allow editing of taskruns

* update tests

* code review

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':

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:

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:

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)
 

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)

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