Added some API tests & some fixes

cvat/apps/authentication/auth.py

@@ -113,6 +113,16 @@ def is_job_owner(db_user, db_job):
 
 @rules.predicate
 def is_job_annotator(db_user, db_job):
+    db_task = db_job.segment.task
+    # A job can be annotated by any user if the task's assignee is None.
+    has_rights = (db_task.assignee is None and not settings.RESTRICTIONS['reduce_task_visibility']) or is_task_assignee(db_user, db_task)
+    if db_job.assignee is not None:
+        has_rights |= db_user == db_job.assignee
+
+    return has_rights
+
+@rules.predicate
+def has_change_permissions(db_user, db_job):
     db_task = db_job.segment.task
     # A job can be annotated by any user if the task's assignee is None.
     has_rights = (db_task.assignee is None and not settings.RESTRICTIONS['reduce_task_visibility']) or is_task_assignee(db_user, db_task)
@@ -125,7 +135,7 @@ def is_job_annotator(db_user, db_job):
 
 @rules.predicate
 def is_job_reviewer(db_user, db_job):
-    has_rights = (db_job.reviewer == db_user) and (db_job.status == 'validation')
+    has_rights = db_job.reviewer == db_user
     return has_rights
 
 @rules.predicate
@@ -159,10 +169,10 @@ def is_comment_owner(db_user, db_comment):
 rules.add_perm('engine.task.delete', has_admin_role | is_task_owner)
 
 rules.add_perm('engine.job.access', has_admin_role | has_observer_role |
-    is_job_owner | is_job_annotator)
+    is_job_owner | is_job_annotator | is_job_reviewer)
 rules.add_perm('engine.job.change', has_admin_role | is_job_owner |
-    is_job_annotator)
-rules.add_perm('engine.job.review', has_admin_role | is_job_reviewer)
+    has_change_permissions)
+rules.add_perm('engine.job.review', has_admin_role | (is_job_reviewer & has_change_permissions))
 
 rules.add_perm('engine.issue.destroy', has_admin_role | is_issue_owner)
 

cvat/apps/engine/models.py

@@ -444,7 +444,7 @@ class Issue(models.Model):
     owner = models.ForeignKey(User, null=True, blank=True, related_name='issues', on_delete=models.SET_NULL)
     resolver = models.ForeignKey(User, null=True, blank=True, related_name='resolved_issues', on_delete=models.SET_NULL)
     created_date = models.DateTimeField(auto_now_add=True)
-    resolved_date = models.DateTimeField(auto_now=True, null=True, blank=True)
+    resolved_date = models.DateTimeField(null=True, blank=True)
 
 class Comment(models.Model):
     issue = models.ForeignKey(Issue, on_delete=models.CASCADE)

cvat/apps/engine/serializers.py

@@ -6,6 +6,7 @@
 import re
 import shutil
 
+from functools import reduce
 from rest_framework import serializers
 from django.contrib.auth.models import User, Group
 
@@ -509,24 +510,25 @@ class Meta:
 class ReviewSummarySerializer(serializers.Serializer):
     reviews = serializers.IntegerField(read_only=True)
     average_estimated_quality = serializers.FloatField(read_only=True)
-    comments_unresolved = serializers.IntegerField(read_only=True)
-    comments_resolved = serializers.IntegerField(read_only=True)
-    annotators = serializers.ListField(allow_empty=True, read_only=True)
+    issues_unresolved = serializers.IntegerField(read_only=True)
+    issues_resolved = serializers.IntegerField(read_only=True)
+    assignees = serializers.ListField(allow_empty=True, read_only=True)
     reviewers = serializers.ListField(allow_empty=True, read_only=True)
 
     def to_representation(self, instance):
-        db_reviews = instance.review_set.all()
+        db_reviews = instance.review_set.prefetch_related('issue_set').all()
         qualities = list(map(lambda db_review: db_review.estimated_quality, db_reviews))
-        reviewers = list(map(lambda db_review: db_review.reviewer, db_reviews))
-        annotators = list(map(lambda db_review: db_review.annotator, db_reviews))
+        reviewers = list(map(lambda db_review: db_review.reviewer.username if db_review.reviewer else None, db_reviews))
+        assignees = list(map(lambda db_review: db_review.assignee.username if db_review.assignee else None, db_reviews))
+        db_issues = list(reduce(lambda acc, db_review: acc + list(db_review.issue_set.all()), db_reviews, []))
 
         return {
             'reviews': len(db_reviews),
             'average_estimated_quality': sum(qualities) / len(qualities) if qualities else 0,
-            'comments_unresolved': 0,
-            'comments_resolved': 0,
-            'annotators': list(set(annotators)),
-            'reviewers': list(set(reviewers)),
+            'issues_unresolved': len(list(filter(lambda db_issue: db_issue.resolved_date is None, db_issues))),
+            'issues_resolved': len(list(filter(lambda db_issue: db_issue.resolved_date is not None, db_issues))),
+            'assignees': list(set(filter(lambda username: username is not None, assignees))),
+            'reviewers': list(set(filter(lambda username: username is not None, reviewers))),
         }
 
 class IssueSerializer(serializers.ModelSerializer):

cvat/apps/engine/tests/test_rest_api.py

@@ -360,6 +360,138 @@ def test_api_v1_jobs_id_admin_partial(self):
         response = self._run_api_v1_jobs_id(self.job.id, self.owner, data)
         self._check_request(response, data)
 
+class JobReview(APITestCase):
+    def setUp(self):
+        self.client = APIClient()
+
+    @classmethod
+    def setUpTestData(cls):
+        create_db_users(cls)
+        cls.task = create_dummy_db_tasks(cls)[0]
+        cls.job = Job.objects.filter(segment__task_id=cls.task.id).first()
+        cls.reviewer = cls.annotator
+        cls.job.reviewer = cls.reviewer
+        cls.job.assignee = cls.assignee
+        cls.job.save()
+        cls.reject_review_data = {
+            "issue_set": [
+                {
+                "roi": [
+                    50, 50, 100, 100
+                ],
+                "comment_set": [
+                    {
+                    "message": "This is wrong!"
+                    }, {
+                    "message": "This is wrong 2!"
+                    }
+                ],
+                "frame": 0
+                }
+            ],
+            "estimated_quality": 3,
+            "status": "rejected"
+        }
+
+        cls.accept_review_data = {
+            "issue_set": [],
+            "estimated_quality": 5,
+            "status": "accepted"
+        }
+
+        cls.review_further_data = {
+            "issue_set": [],
+            "estimated_quality": 4,
+            "status": "review_further"
+        }
+
+    def _post_request(self, jid, user, data, path=''):
+        with ForceLogin(user, self.client):
+            response = self.client.post('/api/v1/jobs/{}{}'.format(jid, path), data=data, format='json')\
+
+        return response
+
+    def _patch_request(self, jid, user, data,  path=''):
+        with ForceLogin(user, self.client):
+            response = self.client.patch('/api/v1/jobs/{}{}'.format(jid, path), data=data, format='json')
+
+        return response
+
+    def _get_request(self, jid, user,  path=''):
+        with ForceLogin(user, self.client):
+            response = self.client.get('/api/v1/jobs/{}{}'.format(jid, path))
+
+        return response
+
+    def _fetch_job_from_db(self):
+        self.job = Job.objects.prefetch_related(
+            'review_set',
+            'review_set__issue_set',
+            'review_set__issue_set__comment_set').filter(segment__task_id=self.task.id).first()
+
+    def _set_annotation_status(self):
+        self._patch_request(self.job.id, self.admin, {'status': 'annotation'})
+
+    def _set_validation_status(self):
+        self._patch_request(self.job.id, self.admin, {'status': 'validation'})
+
+    def test_api_v1_job_annotation_review(self):
+        self._set_annotation_status()
+        response = self._post_request(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        response = self._post_request(self.job.id, self.assignee, self.accept_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_api_v1_job_validation_review_create(self):
+        self._set_validation_status()
+        response = self._post_request(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self._fetch_job_from_db()
+        self.assertEqual(self.job.status, 'completed')
+        response = self._post_request(self.job.id, self.assignee, self.accept_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.job.review_set.first().delete()
+
+    def test_api_v1_job_reject_review(self):
+        self._set_validation_status()
+        response = self._post_request(self.job.id, self.reviewer, self.reject_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self._fetch_job_from_db()
+        self.assertEqual(self.job.status, 'annotation')
+        self.job.review_set.first().delete()
+
+    def test_api_v1_job_review_further(self):
+        self._set_validation_status()
+        response = self._post_request(self.job.id, self.reviewer, self.review_further_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self._fetch_job_from_db()
+        self.assertEqual(self.job.status, 'validation')
+        self.job.review_set.first().delete()
+
+    def test_api_v1_job_review_summary(self):
+        self._set_validation_status()
+        response = self._post_request(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self._set_validation_status()
+        response = self._post_request(self.job.id, self.reviewer, self.review_further_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        response = self._post_request(self.job.id, self.reviewer, self.reject_review_data, path='/reviews/create')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        response = self._get_request(self.job.id, self.reviewer, path='/reviews/summary')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data.get('reviews', None), 3)
+        self.assertEqual(response.data.get('average_estimated_quality', None), 4)
+        self.assertEqual(response.data.get('issues_unresolved', None), 1)
+        self.assertEqual(response.data.get('issues_resolved', None), 0)
+        self.assertSequenceEqual(response.data.get('assignees', None), ['user2'])
+        self.assertSequenceEqual(response.data.get('reviewers', None), ['user3'])
+
+        # todo: left comment
+        # todo: edit comment (+ with wrong permissions)
+        # todo: remove comment (+ with wrong permissions)
+        # todo: resolve issue (+ with wrong permissions)
+        # todo: unresolve issue (+ with wrong permissions)
+
 class ServerAboutAPITestCase(APITestCase):
     def setUp(self):
         self.client = APIClient()

cvat/apps/engine/views.py

@@ -38,7 +38,7 @@
 from cvat.apps.engine.frame_provider import FrameProvider
 from cvat.apps.engine.models import (
     Job, StatusChoice, Task, Review, Issue,
-    Comment, StorageMethodChoice
+    Comment, StorageMethodChoice, ReviewStatus
 )
 from cvat.apps.engine.serializers import (
     AboutSerializer, AnnotationFileSerializer, BasicUserSerializer,
@@ -729,7 +729,7 @@ def create_review(self, request, pk):
         request.data.update({
             'job': db_job.id,
             'reviewer': request.user.id,
-            'assignee': db_job.assignee,
+            'assignee': db_job.assignee.id,
         })
 
         issue_set = request.data['issue_set']
@@ -743,8 +743,13 @@ def create_review(self, request, pk):
         serializer = CombinedReviewSerializer(data=request.data, partial=True)
         if serializer.is_valid(raise_exception=True):
             instance = serializer.save()
-            return Response(CombinedReviewSerializer(instance).data)
-
+            if instance.status == ReviewStatus.ACCEPTED:
+                db_job.status = StatusChoice.COMPLETED
+                db_job.save()
+            elif instance.status == ReviewStatus.REJECTED:
+                db_job.status = StatusChoice.ANNOTATION
+                db_job.save()
+            return Response(CombinedReviewSerializer(instance).data, status=status.HTTP_201_CREATED)
 
     @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews')
     @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer)