From 06a385415960c449aa1296d2e1f83befa5ad7fba Mon Sep 17 00:00:00 2001 From: Will Keeling Date: Fri, 24 Sep 2021 14:05:47 +0100 Subject: [PATCH] Introduce is_ogd flag --- api/data_workspace/tests/test_views.py | 2 +- api/flags/tests/test_create_flags.py | 34 ++++++------------- .../migrations/0003_auto_20210927_1429.py | 23 +++++++++++++ api/teams/models.py | 6 +++- api/teams/serializers.py | 9 ++++- api/teams/tests/test_create.py | 16 ++++++--- api/teams/tests/test_edit.py | 24 ++++++++++--- 7 files changed, 79 insertions(+), 35 deletions(-) create mode 100644 api/teams/migrations/0003_auto_20210927_1429.py diff --git a/api/data_workspace/tests/test_views.py b/api/data_workspace/tests/test_views.py index 239ae8d5d..b13de23a7 100644 --- a/api/data_workspace/tests/test_views.py +++ b/api/data_workspace/tests/test_views.py @@ -92,7 +92,7 @@ def test_queues(self): def test_teams(self): url = reverse("data_workspace:dw-teams-list") - expected_fields = ("id", "name", "part_of_ecju") + expected_fields = ("id", "name", "part_of_ecju", "is_ogd") response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) results = response.json()["results"] diff --git a/api/flags/tests/test_create_flags.py b/api/flags/tests/test_create_flags.py index 4b6cbd8ef..b029c9d97 100644 --- a/api/flags/tests/test_create_flags.py +++ b/api/flags/tests/test_create_flags.py @@ -30,7 +30,13 @@ def test_gov_user_can_create_flags(self): self.assertEqual(response_data["label"], "This is label") self.assertEqual( response_data["team"], - {"id": str(self.team.id), "name": self.team.name, "part_of_ecju": None, "department": None}, + { + "id": str(self.team.id), + "name": self.team.name, + "part_of_ecju": None, + "department": None, + "is_ogd": False, + }, ) @parameterized.expand( @@ -50,11 +56,7 @@ def test_create_flag_failure(self, name): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) def test_cannot_set_priority_to_less_than_0(self): - data = { - "name": "new flag", - "level": "Organisation", - "priority": -1, - } + data = {"name": "new flag", "level": "Organisation", "priority": -1} response = self.client.post(self.url, data, **self.gov_headers) response_data = response.json() @@ -63,11 +65,7 @@ def test_cannot_set_priority_to_less_than_0(self): self.assertIn(strings.Flags.ValidationErrors.PRIORITY_NEGATIVE, response_data["errors"]["priority"]) def test_cannot_set_priority_to_greater_than_100(self): - data = { - "name": "new flag", - "level": "Organisation", - "priority": 101, - } + data = {"name": "new flag", "level": "Organisation", "priority": 101} response = self.client.post(self.url, data, **self.gov_headers) response_data = response.json() @@ -76,12 +74,7 @@ def test_cannot_set_priority_to_greater_than_100(self): self.assertIn(strings.Flags.ValidationErrors.PRIORITY_TOO_LARGE, response_data["errors"]["priority"]) def test_cannot_create_flag_with_colour_and_no_label(self): - data = { - "name": "new flag", - "level": "Organisation", - "colour": FlagColours.ORANGE, - "label": "", - } + data = {"name": "new flag", "level": "Organisation", "colour": FlagColours.ORANGE, "label": ""} response = self.client.post(self.url, data, **self.gov_headers) @@ -89,12 +82,7 @@ def test_cannot_create_flag_with_colour_and_no_label(self): self.assertIn(strings.Flags.ValidationErrors.LABEL_MISSING, response.json()["errors"]["label"]) def test_cannot_create_flag_without_blocks_finalising(self): - data = { - "name": "new flag", - "level": "Organisation", - "colour": FlagColours.ORANGE, - "label": "This is label", - } + data = {"name": "new flag", "level": "Organisation", "colour": FlagColours.ORANGE, "label": "This is label"} response = self.client.post(self.url, data, **self.gov_headers) diff --git a/api/teams/migrations/0003_auto_20210927_1429.py b/api/teams/migrations/0003_auto_20210927_1429.py new file mode 100644 index 000000000..7e29fe0a0 --- /dev/null +++ b/api/teams/migrations/0003_auto_20210927_1429.py @@ -0,0 +1,23 @@ +# Generated by Django 3.1.12 on 2021-09-27 13:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [("teams", "0002_auto_20210924_1241")] + + operations = [ + migrations.AddField( + model_name="team", + name="is_ogd", + field=models.BooleanField(default=False, help_text="Whether the team is an Other Government Department"), + ), + migrations.AlterField( + model_name="team", + name="part_of_ecju", + field=models.BooleanField( + default=None, help_text="Whether the team is part of Export Control Joint Unit", null=True + ), + ), + ] diff --git a/api/teams/models.py b/api/teams/models.py index b5d3a1496..72758c8a4 100644 --- a/api/teams/models.py +++ b/api/teams/models.py @@ -16,10 +16,14 @@ def get_by_natural_key(self, name): class Team(models.Model): id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) name = models.TextField(default=None, unique=True) - part_of_ecju = models.BooleanField(default=None, null=True) department = models.ForeignKey( Department, null=True, blank=True, default=None, on_delete=models.SET_NULL, related_name="teams" ) + part_of_ecju = models.BooleanField( + default=None, null=True, help_text="Whether the team is part of Export Control Joint Unit" + ) + # Note that certain teams can be OGDs *and* be part of ECJU, for example: FCDO + is_ogd = models.BooleanField(default=False, help_text="Whether the team is an Other Government Department") objects = TeamManager() diff --git a/api/teams/serializers.py b/api/teams/serializers.py index be78312dc..fbe1285c4 100644 --- a/api/teams/serializers.py +++ b/api/teams/serializers.py @@ -13,13 +13,14 @@ class TeamReadOnlySerializer(serializers.Serializer): id = serializers.UUIDField(read_only=True) name = serializers.CharField(read_only=True) part_of_ecju = serializers.BooleanField(read_only=True) + is_ogd = serializers.BooleanField(read_only=True) class TeamSerializer(serializers.ModelSerializer): name = serializers.CharField( max_length=50, validators=[ - UniqueValidator(queryset=Team.objects.all(), lookup="iexact", message=strings.Teams.NOT_UNIQUE_NAME,) + UniqueValidator(queryset=Team.objects.all(), lookup="iexact", message=strings.Teams.NOT_UNIQUE_NAME) ], error_messages={"blank": strings.Teams.BLANK_NAME}, ) @@ -29,6 +30,9 @@ class TeamSerializer(serializers.ModelSerializer): "required": "Select yes if the team is part of ECJU", } ) + is_ogd = serializers.BooleanField( + error_messages={"null": "Select yes if the team is an OGD", "required": "Select yes if the team is an OGD"} + ) class Meta: model = Team @@ -40,4 +44,7 @@ def validate(self, attrs): if "part_of_ecju" not in validated_data: raise serializers.ValidationError({"part_of_ecju": "Select yes if the team is part of ECJU"}) + if "is_ogd" not in validated_data: + raise serializers.ValidationError({"is_ogd": "Select yes if the team is an OGD"}) + return validated_data diff --git a/api/teams/tests/test_create.py b/api/teams/tests/test_create.py index aba1e1631..a40e2153c 100644 --- a/api/teams/tests/test_create.py +++ b/api/teams/tests/test_create.py @@ -14,24 +14,32 @@ def setUp(self): super().setUp() self.team_preexisting_count = Team.objects.all().count() - def test_create_team_failure(self): - data = {"name": "new team"} + def test_create_team_missing_part_of_ecju(self): + data = {"name": "new team", "is_ogd": False} response = self.client.post(self.url, data, **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) response = response.json()["errors"] self.assertEqual(response["part_of_ecju"][0], "Select yes if the team is part of ECJU") + def test_create_team_missing_is_ogd(self): + data = {"name": "new team", "part_of_ecju": False} + response = self.client.post(self.url, data, **self.gov_headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + response = response.json()["errors"] + self.assertEqual(response["is_ogd"][0], "Select yes if the team is an OGD") + def test_create_team(self): - data = {"name": "new team", "part_of_ecju": True} + data = {"name": "new team", "part_of_ecju": True, "is_ogd": True} response = self.client.post(self.url, data, **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(Team.objects.filter(name="new team").count(), 1) self.assertEqual(Team.objects.get(name="new team").name, "new team") self.assertEqual(Team.objects.get(name="new team").part_of_ecju, True) + self.assertEqual(Team.objects.get(name="new team").is_ogd, True) @parameterized.expand( - [[{"name": "this is a name"}], [{"name": "ThIs iS A NaMe"}], [{"name": " this is a name "}],] + [[{"name": "this is a name"}], [{"name": "ThIs iS A NaMe"}], [{"name": " this is a name "}]] ) def test_team_name_must_be_unique(self, data): Team(name="this is a name", part_of_ecju=True).save() diff --git a/api/teams/tests/test_edit.py b/api/teams/tests/test_edit.py index 6bba36498..6c97360ce 100644 --- a/api/teams/tests/test_edit.py +++ b/api/teams/tests/test_edit.py @@ -15,7 +15,7 @@ def test_edit_team_name_successful(self): team = Team(name="Team", part_of_ecju=True) team.save() - data = {"name": "Renamed Team", "part_of_ecju": False} + data = {"name": "Renamed Team", "part_of_ecju": False, "is_ogd": False} url = reverse("teams:team", kwargs={"pk": team.id}) response = self.client.put(url, data, **self.gov_headers) @@ -24,12 +24,13 @@ def test_edit_team_name_successful(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(team.name, data["name"]) self.assertEqual(team.part_of_ecju, data["part_of_ecju"]) + self.assertEqual(team.part_of_ecju, data["is_ogd"]) - def test_edit_team_failure(self): + def test_edit_team_missing_part_of_ecju(self): team = Team(name="Team") team.save() - data = {"name": "Renamed Team"} + data = {"name": "Renamed Team", "is_ogd": False} url = reverse("teams:team", kwargs={"pk": team.id}) response = self.client.put(url, data, **self.gov_headers) @@ -38,12 +39,25 @@ def test_edit_team_failure(self): response = response.json()["errors"] self.assertEqual(response["part_of_ecju"][0], "Select yes if the team is part of ECJU") + def test_edit_team_missing_is_ogd(self): + team = Team(name="Team") + team.save() + + data = {"name": "Renamed Team", "part_of_ecju": False} + + url = reverse("teams:team", kwargs={"pk": team.id}) + response = self.client.put(url, data, **self.gov_headers) + team.refresh_from_db() + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + response = response.json()["errors"] + self.assertEqual(response["is_ogd"][0], "Select yes if the team is an OGD") + def test_cannot_rename_to_an_already_used_name_case_insensitive(self): """ Test that a valid gov user cannot edit their team name to the same as another teams """ - team = Team(name="name", part_of_ecju=False) + team = Team(name="name", part_of_ecju=False, is_ogd=False) team.save() Team(name="test").save() @@ -59,7 +73,7 @@ def test_cannot_rename_to_an_already_used_name_case_insensitive(self): def test_cannot_edit_admin_team(self): team = get_team_by_pk(pk=Teams.ADMIN_TEAM_ID) - data = {"name": "Renamed Team", "part_of_ecju": True} + data = {"name": "Renamed Team", "part_of_ecju": True, "is_ogd": False} url = reverse("teams:team", kwargs={"pk": team.id}) response = self.client.put(url, data, **self.gov_headers)