From 43146e2ecb55dc8434b4a194417f12af4de54253 Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Mon, 6 Jan 2020 15:33:54 -0500 Subject: [PATCH 1/6] Simplify test for statistics API --- app/api/tests/test_api.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py index 2c8b66bf..f806c222 100644 --- a/app/api/tests/test_api.py +++ b/app/api/tests/test_api.py @@ -1332,10 +1332,8 @@ class TestStatisticsAPI(APITestCase): 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_user_count(self): self.client.login(username=self.super_user_name, From 9f1bd5b2e5c0f33ba17cd7bfdfdaf0c2f4a77147 Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Mon, 6 Jan 2020 15:57:31 -0500 Subject: [PATCH 2/6] Add test for document filter --- app/api/tests/test_api.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py index f806c222..ed5cf843 100644 --- a/app/api/tests/test_api.py +++ b/app/api/tests/test_api.py @@ -383,6 +383,8 @@ class TestDocumentListAPI(APITestCase): email='fizz@buzz.com') 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 +401,29 @@ class TestDocumentListAPI(APITestCase): assign_user_to_role(project_member=project_member, project=cls.random_order_project, role_name=settings.ROLE_ANNOTATOR) + mommy.make('DocumentAnnotation', document=doc1, user=project_member) + mommy.make('DocumentAnnotation', document=doc2, user=project_member) + 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') self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.json().get('results')), 3) + + def test_returns_docs_to_project_member_filtered_to_active(self): + self.client.login(username=self.project_member_name, + password=self.project_member_pass) + response = self.client.get('{}?doc_annotations__isnull=true'.format(self.url), format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.json().get('results')), 1) + + def test_returns_docs_to_project_member_filtered_to_completed(self): + self.client.login(username=self.project_member_name, + password=self.project_member_pass) + response = self.client.get('{}?doc_annotations__isnull=false'.format(self.url), format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.json().get('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 +434,7 @@ class TestDocumentListAPI(APITestCase): 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) From 945d0b412ed893ac8e18b483a096a039d4af958f Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Mon, 6 Jan 2020 15:42:51 -0500 Subject: [PATCH 3/6] Extract _patch_project helper --- app/api/tests/test_api.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py index ed5cf843..dfb6403c 100644 --- a/app/api/tests/test_api.py +++ b/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): @@ -604,7 +617,7 @@ class TestApproveLabelsAPI(APITestCase): remove_all_role_mappings() -class TestAnnotationListAPI(APITestCase): +class TestAnnotationListAPI(APITestCase, TestUtilsMixin): @classmethod def setUpTestData(cls): From 70b63b68ee62d5c4815d321d4e1b2a8f1c5fed54 Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Mon, 6 Jan 2020 16:04:31 -0500 Subject: [PATCH 4/6] Extract list documents test helper --- app/api/tests/test_api.py | 41 +++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py index dfb6403c..c5a8f378 100644 --- a/app/api/tests/test_api.py +++ b/app/api/tests/test_api.py @@ -417,26 +417,29 @@ class TestDocumentListAPI(APITestCase): mommy.make('DocumentAnnotation', document=doc1, user=project_member) mommy.make('DocumentAnnotation', document=doc2, user=project_member) - 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') + 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')), 3) + 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.client.login(username=self.project_member_name, - password=self.project_member_pass) - response = self.client.get('{}?doc_annotations__isnull=true'.format(self.url), format='json') - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(len(response.json().get('results')), 1) + 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.client.login(username=self.project_member_name, - password=self.project_member_pass) - response = self.client.get('{}?doc_annotations__isnull=false'.format(self.url), format='json') - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(len(response.json().get('results')), 2) + 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_in_consistent_order_for_all_users(self): self.client.login(username=self.project_member_name, password=self.project_member_pass) @@ -472,10 +475,10 @@ class TestDocumentListAPI(APITestCase): 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, From e5d04947dfd8e443264ffda2a6409a87afb23789 Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Mon, 6 Jan 2020 15:27:03 -0500 Subject: [PATCH 5/6] Make document filter respect collab flag --- app/api/filters.py | 2 +- app/api/tests/test_api.py | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/api/filters.py b/app/api/filters.py index 1efbc448..76346f4d 100644 --- a/app/api/filters.py +++ b/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: diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py index c5a8f378..cd76d17a 100644 --- a/app/api/tests/test_api.py +++ b/app/api/tests/test_api.py @@ -376,7 +376,7 @@ class TestLabelDetailAPI(APITestCase): remove_all_role_mappings() -class TestDocumentListAPI(APITestCase): +class TestDocumentListAPI(APITestCase, TestUtilsMixin): @classmethod def setUpTestData(cls): @@ -441,6 +441,32 @@ class TestDocumentListAPI(APITestCase): 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) user1_documents = self.client.get(self.url, format='json').json().get('results') From 290e2cafd17e1f9945de9a529b4218023f705702 Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Mon, 6 Jan 2020 15:27:33 -0500 Subject: [PATCH 6/6] Make statistics respect collab flag --- app/api/tests/test_api.py | 35 +++++++++++++++++++++++++++++------ app/api/views.py | 10 ++++++---- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py index cd76d17a..0a77bb7c 100644 --- a/app/api/tests/test_api.py +++ b/app/api/tests/test_api.py @@ -1371,24 +1371,38 @@ class TestDownloader(APITestCase): 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='fizz@buzz.com') - 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='bar@buzz.com') + + 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, @@ -1397,6 +1411,15 @@ class TestStatisticsAPI(APITestCase): 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, password=self.super_user_pass) diff --git a/app/api/views.py b/app/api/views.py index 4b31bcd8..c0f694d3 100644 --- a/app/api/views.py +++ b/app/api/views.py @@ -2,7 +2,7 @@ from django.conf import settings 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 @@ class StatisticsAPI(APIView): 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}