Merge pull request doccano#527 from CatalystCode/bugfix/document-filter-collaborative-annotation

Bugfix/Fix collaborative annotation for stats and filter

Author: Hironsan

app/api/filters.py

@@ -12,7 +12,7 @@ class DocumentFilter(FilterSet):
     def filter_annotations(self, queryset, field_name, value):
         queryset = queryset.annotate(num_annotations=
             Count(field_name, filter=
-                Q(**{ f"{field_name}__user": self.request.user})))
+                Q(**{ f"{field_name}__user": self.request.user}) | Q(project__collaborative_annotation=True)))
 
         should_have_annotations = not value
         if should_have_annotations:

app/api/tests/test_api.py

@@ -29,6 +29,19 @@ def remove_all_role_mappings():
     RoleMapping.objects.all().delete()
 
 
+class TestUtilsMixin:
+    def _patch_project(self, project, attribute, value):
+        old_value = getattr(project, attribute, None)
+        setattr(project, attribute, value)
+        project.save()
+
+        def cleanup_project():
+            setattr(project, attribute, old_value)
+            project.save()
+
+        self.addCleanup(cleanup_project)
+
+
 @override_settings(STATICFILES_STORAGE='django.contrib.staticfiles.storage.StaticFilesStorage')
 class TestProjectListAPI(APITestCase):
 
@@ -363,7 +376,7 @@ def doCleanups(cls):
         remove_all_role_mappings()
 
 
-class TestDocumentListAPI(APITestCase):
+class TestDocumentListAPI(APITestCase, TestUtilsMixin):
 
     @classmethod
     def setUpTestData(cls):
@@ -383,6 +396,8 @@ def setUpTestData(cls):
                                                    email='[email protected]')
 
         cls.main_project = mommy.make('TextClassificationProject', users=[project_member, super_user])
+        doc1 = mommy.make('Document', project=cls.main_project)
+        doc2 = mommy.make('Document', project=cls.main_project)
         mommy.make('Document', project=cls.main_project)
 
         cls.random_order_project = mommy.make('TextClassificationProject', users=[project_member, super_user],
@@ -399,11 +414,58 @@ def setUpTestData(cls):
         assign_user_to_role(project_member=project_member, project=cls.random_order_project,
                             role_name=settings.ROLE_ANNOTATOR)
 
-    def test_returns_docs_to_project_member(self):
-        self.client.login(username=self.project_member_name,
-                          password=self.project_member_pass)
-        response = self.client.get(self.url, format='json')
+        mommy.make('DocumentAnnotation', document=doc1, user=project_member)
+        mommy.make('DocumentAnnotation', document=doc2, user=project_member)
+
+    def _test_list(self, url, username, password, expected_num_results):
+        self.client.login(username=username, password=password)
+        response = self.client.get(url, format='json')
         self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(len(response.json().get('results')), expected_num_results)
+
+    def test_returns_docs_to_project_member(self):
+        self._test_list(self.url,
+                        username=self.project_member_name,
+                        password=self.project_member_pass,
+                        expected_num_results=3)
+
+    def test_returns_docs_to_project_member_filtered_to_active(self):
+        self._test_list('{}?doc_annotations__isnull=true'.format(self.url),
+                        username=self.project_member_name,
+                        password=self.project_member_pass,
+                        expected_num_results=1)
+
+    def test_returns_docs_to_project_member_filtered_to_completed(self):
+        self._test_list('{}?doc_annotations__isnull=false'.format(self.url),
+                        username=self.project_member_name,
+                        password=self.project_member_pass,
+                        expected_num_results=2)
+
+    def test_returns_docs_to_project_member_filtered_to_active_with_collaborative_annotation(self):
+        self._test_list('{}?doc_annotations__isnull=true'.format(self.url),
+                        username=self.super_user_name,
+                        password=self.super_user_pass,
+                        expected_num_results=3)
+
+        self._patch_project(self.main_project, 'collaborative_annotation', True)
+
+        self._test_list('{}?doc_annotations__isnull=true'.format(self.url),
+                        username=self.super_user_name,
+                        password=self.super_user_pass,
+                        expected_num_results=1)
+
+    def test_returns_docs_to_project_member_filtered_to_completed_with_collaborative_annotation(self):
+        self._test_list('{}?doc_annotations__isnull=false'.format(self.url),
+                        username=self.super_user_name,
+                        password=self.super_user_pass,
+                        expected_num_results=0)
+
+        self._patch_project(self.main_project, 'collaborative_annotation', True)
+
+        self._test_list('{}?doc_annotations__isnull=false'.format(self.url),
+                        username=self.super_user_name,
+                        password=self.super_user_pass,
+                        expected_num_results=2)
 
     def test_returns_docs_in_consistent_order_for_all_users(self):
         self.client.login(username=self.project_member_name, password=self.project_member_pass)
@@ -414,7 +476,7 @@ def test_returns_docs_in_consistent_order_for_all_users(self):
         user2_documents = self.client.get(self.url, format='json').json().get('results')
         self.client.logout()
 
-        self.assertEqual(user1_documents, user2_documents)
+        self.assertEqual([doc['id'] for doc in user1_documents], [doc['id'] for doc in user2_documents])
 
     def test_can_return_docs_in_consistent_random_order(self):
         self.client.login(username=self.project_member_name, password=self.project_member_pass)
@@ -439,10 +501,10 @@ def test_do_not_return_docs_to_non_project_member(self):
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_do_not_return_docs_of_other_projects(self):
-        self.client.login(username=self.project_member_name,
-                          password=self.project_member_pass)
-        response = self.client.get(self.url, format='json')
-        self.assertEqual(response.data['count'], self.main_project.documents.count())
+        self._test_list(self.url,
+                        username=self.project_member_name,
+                        password=self.project_member_pass,
+                        expected_num_results=self.main_project.documents.count())
 
     def test_allows_superuser_to_create_doc(self):
         self.client.login(username=self.super_user_name,
@@ -584,7 +646,7 @@ def doCleanups(cls):
         remove_all_role_mappings()
 
 
-class TestAnnotationListAPI(APITestCase):
+class TestAnnotationListAPI(APITestCase, TestUtilsMixin):
 
     @classmethod
     def setUpTestData(cls):
@@ -1309,33 +1371,54 @@ def test_can_download_plain_text(self):
                                   expected_status=status.HTTP_400_BAD_REQUEST)
 
 
-class TestStatisticsAPI(APITestCase):
+class TestStatisticsAPI(APITestCase, TestUtilsMixin):
 
     @classmethod
     def setUpTestData(cls):
         cls.super_user_name = 'super_user_name'
         cls.super_user_pass = 'super_user_pass'
+        cls.other_user_name = 'other_user_name'
+        cls.other_user_pass = 'other_user_pass'
         create_default_roles()
         # Todo: change super_user to project_admin.
         super_user = User.objects.create_superuser(username=cls.super_user_name,
                                                    password=cls.super_user_pass,
                                                    email='[email protected]')
 
-        main_project = mommy.make('TextClassificationProject', users=[super_user])
-        doc1 = mommy.make('Document', project=main_project)
-        mommy.make('Document', project=main_project)
+        other_user = User.objects.create_user(username=cls.other_user_name,
+                                              password=cls.other_user_pass,
+                                              email='[email protected]')
+
+        cls.project = mommy.make('TextClassificationProject', users=[super_user, other_user])
+        doc1 = mommy.make('Document', project=cls.project)
+        doc2 = mommy.make('Document', project=cls.project)
         mommy.make('DocumentAnnotation', document=doc1, user=super_user)
-        cls.url = reverse(viewname='statistics', args=[main_project.id])
-        cls.doc = Document.objects.filter(project=main_project)
+        mommy.make('DocumentAnnotation', document=doc2, user=other_user)
+        cls.url = reverse(viewname='statistics', args=[cls.project.id])
+        cls.doc = Document.objects.filter(project=cls.project)
+
+        assign_user_to_role(project_member=other_user, project=cls.project,
+                            role_name=settings.ROLE_ANNOTATOR)
+
+    @classmethod
+    def doCleanups(cls):
+        remove_all_role_mappings()
 
     def test_returns_exact_progress(self):
         self.client.login(username=self.super_user_name,
                           password=self.super_user_pass)
         response = self.client.get(self.url, format='json')
-        total = self.doc.count()
-        remaining = self.doc.filter(doc_annotations__isnull=True).count()
-        self.assertEqual(response.data['total'], total)
-        self.assertEqual(response.data['remaining'], remaining)
+        self.assertEqual(response.data['total'], 2)
+        self.assertEqual(response.data['remaining'], 1)
+
+    def test_returns_exact_progress_with_collaborative_annotation(self):
+        self._patch_project(self.project, 'collaborative_annotation', True)
+
+        self.client.login(username=self.other_user_name,
+                          password=self.other_user_pass)
+        response = self.client.get(self.url, format='json')
+        self.assertEqual(response.data['total'], 2)
+        self.assertEqual(response.data['remaining'], 0)
 
     def test_returns_user_count(self):
         self.client.login(username=self.super_user_name,

app/api/views.py

@@ -2,7 +2,7 @@
 from django.contrib.auth.models import User
 from django.shortcuts import get_object_or_404, redirect
 from django_filters.rest_framework import DjangoFilterBackend
-from django.db.models import Count, F
+from django.db.models import Count, F, Q
 from libcloud.base import DriverType, get_driver
 from libcloud.storage.types import ContainerDoesNotExistError, ObjectDoesNotExistError
 from rest_framework import generics, filters, status
@@ -90,9 +90,11 @@ def progress(self, project):
         docs = project.documents
         annotation_class = project.get_annotation_class()
         total = docs.count()
-        done = annotation_class.objects.filter(document_id__in=docs.all(),
-            user_id=self.request.user).\
-            aggregate(Count('document', distinct=True))['document__count']
+        annotation_filter = Q(document_id__in=docs.all())
+        if not project.collaborative_annotation:
+            annotation_filter &= Q(user_id=self.request.user)
+        done = annotation_class.objects.filter(annotation_filter)\
+            .aggregate(Count('document', distinct=True))['document__count']
         remaining = total - done
         return {'total': total, 'remaining': remaining}