From 7f477b5ff732ef572b07299528bc9e0ef42f14f1 Mon Sep 17 00:00:00 2001 From: Feri Date: Wed, 25 Nov 2020 13:02:51 +0000 Subject: [PATCH] Mar 731/organisations v2 (#233) MAR-731 - organisations improvement (#223): - add gov organisations to CSV export and dataset endpoint - add organisation filter to barrier list endpoint - add non ministerial departments --- api/barriers/filters.py | 1 + api/barriers/serializers/csv.py | 4 + api/barriers/serializers/data_workspace.py | 1 + api/barriers/views.py | 3 +- api/metadata/constants.py | 2 + .../0019_add_non_ministerial_departments.py | 112 ++++++++++++++++++ tests/barriers/factories.py | 17 +-- tests/barriers/test_csv_export.py | 15 +++ tests/barriers/test_list_barriers.py | 35 +++++- tests/dataset/test_views.py | 14 +++ tests/metadata/test_metadata.py | 2 +- 11 files changed, 189 insertions(+), 17 deletions(-) create mode 100644 api/metadata/migrations/0019_add_non_ministerial_departments.py diff --git a/api/barriers/filters.py b/api/barriers/filters.py index 169faf405..3cbf76133 100644 --- a/api/barriers/filters.py +++ b/api/barriers/filters.py @@ -54,6 +54,7 @@ class BarrierFilterSet(django_filters.FilterSet): tags = django_filters.BaseInFilter(method="tags_filter") trade_direction = django_filters.BaseInFilter("trade_direction") wto = django_filters.BaseInFilter(method="wto_filter") + organisation = django_filters.BaseInFilter("organisations", distinct=True) class Meta: model = Barrier diff --git a/api/barriers/serializers/csv.py b/api/barriers/serializers/csv.py index 50274853c..70f32309f 100644 --- a/api/barriers/serializers/csv.py +++ b/api/barriers/serializers/csv.py @@ -99,6 +99,7 @@ class BarrierCsvExportSerializer(serializers.Serializer): resolvability_assessment_time = serializers.SerializerMethodField() resolvability_assessment_effort = serializers.SerializerMethodField() strategic_assessment_scale = serializers.SerializerMethodField() + government_organisations = serializers.SerializerMethodField() class Meta: model = Barrier @@ -338,3 +339,6 @@ def get_resolvability_assessment_effort(self, obj): def get_strategic_assessment_scale(self, obj): if obj.current_strategic_assessment: return obj.current_strategic_assessment.scale + + def get_government_organisations(self, obj): + return [org.name for org in obj.government_organisations.all()] diff --git a/api/barriers/serializers/data_workspace.py b/api/barriers/serializers/data_workspace.py index 415347f19..d42ff7367 100644 --- a/api/barriers/serializers/data_workspace.py +++ b/api/barriers/serializers/data_workspace.py @@ -63,6 +63,7 @@ class Meta(BarrierSerializerBase.Meta): "unarchived_on", "unarchived_reason", "wto_profile", + "government_organisations", ) def get_status_history(self, obj): diff --git a/api/barriers/views.py b/api/barriers/views.py index 80573da17..a720d8ea1 100644 --- a/api/barriers/views.py +++ b/api/barriers/views.py @@ -252,7 +252,8 @@ class BarrierList(generics.ListAPIView): queryset = Barrier.barriers.all().select_related( "priority" ).prefetch_related( - "tags" + "tags", + "organisations", ) serializer_class = BarrierListSerializer filterset_class = BarrierFilterSet diff --git a/api/metadata/constants.py b/api/metadata/constants.py index 882e30071..264b15acf 100644 --- a/api/metadata/constants.py +++ b/api/metadata/constants.py @@ -215,6 +215,7 @@ class PublicBarrierStatus(StatusNameMixin): class OrganisationType(StatusNameMixin): MINISTERIAL_DEPARTMENTS = 1 DEVOLVED_ADMINISTRATIONS = 2 + NON_MINISTERIAL_DEPARTMENTS = 3 choices = Choices( (MINISTERIAL_DEPARTMENTS, "Ministerial departments"), @@ -225,4 +226,5 @@ class OrganisationType(StatusNameMixin): GOVERNMENT_ORGANISATION_TYPES = ( OrganisationType.MINISTERIAL_DEPARTMENTS, OrganisationType.DEVOLVED_ADMINISTRATIONS, + OrganisationType.NON_MINISTERIAL_DEPARTMENTS, ) diff --git a/api/metadata/migrations/0019_add_non_ministerial_departments.py b/api/metadata/migrations/0019_add_non_ministerial_departments.py new file mode 100644 index 000000000..f73965cc7 --- /dev/null +++ b/api/metadata/migrations/0019_add_non_ministerial_departments.py @@ -0,0 +1,112 @@ +from django.db import migrations + +from api.metadata.constants import OrganisationType + + +ORGANISATIONS = [ + { + "name": "The Charity Commission", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Competition and Markets Authority", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Crown Prosecution Service", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Food Standards Agency", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Forestry Commission", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Government Actuary's Department", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Government Legal Department", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "HM Land Registry", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "HM Revenue & Customs", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "NS&I", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "The National Archives", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "National Crime Agency", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Office of Rail and Road", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Ofgem", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Ofqual", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Ofsted", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Serious Fraud Office", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "Supreme Court of the United Kingdom", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "UK Statistics Authority", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + }, + { + "name": "The Water Services Regulation Authority", + "organisation_type": OrganisationType.NON_MINISTERIAL_DEPARTMENTS + } +] + + +def create_organisations(apps, schema_editor): + Organisation = apps.get_model("metadata", "Organisation") + + for item in ORGANISATIONS: + Organisation.objects.create(**item) + + +def delete_organisations(apps, schema_editor): + Organisation = apps.get_model("metadata", "Organisation") + + for item in ORGANISATIONS: + Organisation.objects.delete(**item) + + +class Migration(migrations.Migration): + + dependencies = [ + ("metadata", "0018_auto_20201118_1133"), + ] + + operations = [ + migrations.RunPython(create_organisations, reverse_code=delete_organisations), + ] diff --git a/tests/barriers/factories.py b/tests/barriers/factories.py index f6901c7db..608660822 100644 --- a/tests/barriers/factories.py +++ b/tests/barriers/factories.py @@ -19,17 +19,6 @@ def fuzzy_sector(): return FuzzyChoice(sectors).fuzz() -def fuzzy_country(): - countries = ( - "a25f66a0-5d95-e211-a939-e4115bead28a", # The Bahamas - "985f66a0-5d95-e211-a939-e4115bead28a", # Angola - "1f0be5c4-5d95-e211-a939-e4115bead28a", # Singapore - "a05f66a0-5d95-e211-a939-e4115bead28a", # Austria - "ad5f66a0-5d95-e211-a939-e4115bead28a", # Bosnia and Herzegovina - ) - return FuzzyChoice(countries).fuzz() - - def fuzzy_date(): return FuzzyDate( start_date=datetime.date.today() - datetime.timedelta(days=45), @@ -54,7 +43,7 @@ class PublicBarrierFactory(factory.django.DjangoModelFactory): class Meta: model = PublicBarrier - country = fuzzy_country() + country = "985f66a0-5d95-e211-a939-e4115bead28a" # Angola summary = "Some summary" title = "Some title" @@ -85,7 +74,7 @@ class Meta: term = 1 status = 1 - country = fuzzy_country() + country = "985f66a0-5d95-e211-a939-e4115bead28a" # Angola trade_direction = 1 sectors_affected = True sectors = [fuzzy_sector()] @@ -152,7 +141,7 @@ class Meta: archived = False term = 1 status = 1 - country = fuzzy_country() + country = "985f66a0-5d95-e211-a939-e4115bead28a" # Angola trade_direction = 1 sectors_affected = True sectors = [fuzzy_sector()] diff --git a/tests/barriers/test_csv_export.py b/tests/barriers/test_csv_export.py index efcce500c..3cf49db21 100644 --- a/tests/barriers/test_csv_export.py +++ b/tests/barriers/test_csv_export.py @@ -4,8 +4,10 @@ from api.barriers.serializers import BarrierCsvExportSerializer from api.core.test_utils import APITestMixin +from api.metadata.models import Organisation from tests.assessment.factories import AssessmentFactory from tests.barriers.factories import BarrierFactory +from tests.metadata.factories import OrganisationFactory class TestBarrierCsvExportSerializer(APITestMixin, APITestCase): @@ -62,3 +64,16 @@ def test_eu_overseas_region(self): barrier = BarrierFactory(trading_bloc="TB00016", country=None) serializer = BarrierCsvExportSerializer(barrier) assert ["Europe"] == serializer.data["overseas_region"] + + def test_government_organisations(self): + org1 = Organisation.objects.get(id=1) + org2 = OrganisationFactory(organisation_type=0) + barrier = BarrierFactory() + barrier.organisations.add(org1, org2) + + assert 2 == barrier.organisations.count() + assert 1 == barrier.government_organisations.count() + + serializer = BarrierCsvExportSerializer(barrier) + assert 1 == len(serializer.data["government_organisations"]) + assert [org1.name] == serializer.data["government_organisations"] diff --git a/tests/barriers/test_list_barriers.py b/tests/barriers/test_list_barriers.py index c3c6d9d00..edea91c52 100644 --- a/tests/barriers/test_list_barriers.py +++ b/tests/barriers/test_list_barriers.py @@ -9,7 +9,7 @@ from api.barriers.models import Barrier from api.history.models import CachedHistoryItem from api.metadata.constants import PublicBarrierStatus -from api.metadata.models import BarrierPriority +from api.metadata.models import BarrierPriority, Organisation from tests.barriers.factories import BarrierFactory, ReportFactory from tests.collaboration.factories import TeamMemberFactory from tests.metadata.factories import CategoryFactory, BarrierPriorityFactory @@ -623,6 +623,39 @@ def test_member_filter__distinct_records(self): assert status.HTTP_200_OK == response.status_code assert 1 == response.data["count"] + def test_organisation_filter(self): + org1 = Organisation.objects.get(id=1) + barrier1 = BarrierFactory() + barrier1.organisations.add(org1) + barrier2 = BarrierFactory() + + assert 2 == Barrier.objects.count() + assert 1 == Barrier.objects.filter(organisations__in=[org1]).count() + + url = f'{reverse("list-barriers")}?organisation={org1.id}' + response = self.api_client.get(url) + + assert status.HTTP_200_OK == response.status_code + assert 1 == response.data["count"] + assert str(barrier1.id) == response.data["results"][0]["id"] + + def test_organisation_filter__distinct_records(self): + org1 = Organisation.objects.get(id=1) + org2 = Organisation.objects.get(id=2) + barrier1 = BarrierFactory() + barrier1.organisations.add(org1, org2) + barrier2 = BarrierFactory() + + assert 2 == Barrier.objects.count() + assert 1 == Barrier.objects.filter(organisations__in=[org1]).count() + + url = f'{reverse("list-barriers")}?organisation={org1.id}&?organisation={org2.id}' + response = self.api_client.get(url) + + assert status.HTTP_200_OK == response.status_code + assert 1 == response.data["count"] + assert str(barrier1.id) == response.data["results"][0]["id"] + class PublicViewFilterTest(APITestMixin, APITestCase): def test_changed_filter(self): diff --git a/tests/dataset/test_views.py b/tests/dataset/test_views.py index cadf06f20..6030a89a8 100644 --- a/tests/dataset/test_views.py +++ b/tests/dataset/test_views.py @@ -7,6 +7,7 @@ from api.barriers.models import Barrier from api.core.test_utils import APITestMixin, create_test_user from api.collaboration.models import TeamMember +from api.metadata.models import Organisation from tests.barriers.factories import BarrierFactory @@ -87,3 +88,16 @@ def test_eu_barrier_overseas_region(self): overseas_regions = barrier_data["trading_bloc"]["overseas_regions"] assert len(overseas_regions) == 1 assert overseas_regions[0]["name"] == "Europe" + + def test_government_organisations(self): + org1 = Organisation.objects.get(id=1) + barrier = BarrierFactory() + barrier.organisations.add(org1) + + url = reverse("dataset:barrier-list") + response = self.api_client.get(url) + + assert status.HTTP_200_OK == response.status_code + data_item = response.data["results"][0] + assert "government_organisations" in data_item.keys() + assert org1.id == data_item["government_organisations"][0]["id"] diff --git a/tests/metadata/test_metadata.py b/tests/metadata/test_metadata.py index 4e5af268a..d7bedcb93 100644 --- a/tests/metadata/test_metadata.py +++ b/tests/metadata/test_metadata.py @@ -139,7 +139,7 @@ def test_government_organisations(self): assert status.HTTP_200_OK == response.status_code assert response.data[key] is not None - assert 26 == len(response.data[key]) + assert 46 == len(response.data[key]) org = response.data[key][0] assert "id" in org.keys()