From c25f5cdba5bcfa2aa220eeecaca3fc15b1f88125 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 27 Mar 2024 15:22:47 -0700 Subject: [PATCH 1/6] fix: move measurement units to the right model (d'oh!) --- .../apps/project_management/graphql/projects.py | 7 +++++++ .../apps/project_management/models/projects.py | 8 ++++++++ terraso_backend/apps/soil_id/graphql/soil_data.py | 5 ----- .../apps/soil_id/models/project_soil_settings.py | 6 ------ 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/terraso_backend/apps/project_management/graphql/projects.py b/terraso_backend/apps/project_management/graphql/projects.py index 83a939974..c97716a9f 100644 --- a/terraso_backend/apps/project_management/graphql/projects.py +++ b/terraso_backend/apps/project_management/graphql/projects.py @@ -118,6 +118,7 @@ class Meta: filterset_class = ProjectFilterSet fields = ( "name", + "measurement_units", "privacy", "description", "updated_at", @@ -136,6 +137,10 @@ def resolve_seen(self, info): return True return self.seen_by.filter(id=user.id).exists() + @classmethod + def measurement_units_enum(cls): + return cls._meta.fields["measurement_units"].type.of_type() + @classmethod def privacy_enum(cls): return cls._meta.fields["privacy"].type.of_type() @@ -158,6 +163,7 @@ class ProjectAddMutation(BaseWriteMutation): class Input: name = graphene.String(required=True) + measurement_units = ProjectNode.measurement_units_enum() privacy = ProjectNode.privacy_enum() description = graphene.String() site_instructions = graphene.String() @@ -270,6 +276,7 @@ class ProjectUpdateMutation(BaseWriteMutation): class Input: id = graphene.ID(required=True) name = graphene.String() + measurement_units = ProjectNode.measurement_units_enum() privacy = ProjectNode.privacy_enum() description = graphene.String() site_instructions = graphene.String() diff --git a/terraso_backend/apps/project_management/models/projects.py b/terraso_backend/apps/project_management/models/projects.py index cdf4d3a53..2ed169552 100644 --- a/terraso_backend/apps/project_management/models/projects.py +++ b/terraso_backend/apps/project_management/models/projects.py @@ -61,6 +61,14 @@ class Meta(BaseModel.Meta): description = models.CharField(max_length=512, default="", blank=True) membership_list = models.OneToOneField(ProjectMembershipList, on_delete=models.CASCADE) + class MeasurementUnit(models.TextChoices): + IMPERIAL = "IMPERIAL" + METRIC = "METRIC" + + measurement_units = models.CharField( + default=MeasurementUnit.METRIC, choices=MeasurementUnit.choices + ) + class Privacy(models.TextChoices): PRIVATE = "PRIVATE" PUBLIC = "PUBLIC" diff --git a/terraso_backend/apps/soil_id/graphql/soil_data.py b/terraso_backend/apps/soil_id/graphql/soil_data.py index d31623a9a..3f7636b4e 100644 --- a/terraso_backend/apps/soil_id/graphql/soil_data.py +++ b/terraso_backend/apps/soil_id/graphql/soil_data.py @@ -129,10 +129,6 @@ class Meta: "updated_at", ] - @classmethod - def measurement_units_enum(cls): - return cls._meta.fields["measurement_units"].type() - @classmethod def depth_interval_preset_enum(cls): return cls._meta.fields["depth_interval_preset"].type.of_type() @@ -432,7 +428,6 @@ class ProjectSoilSettingsUpdateMutation(BaseWriteMutation): class Input: project_id = graphene.ID(required=True) - measurement_units = ProjectSoilSettingsNode.measurement_units_enum() depth_interval_preset = ProjectSoilSettingsNode.depth_interval_preset_enum() soil_pit_required = graphene.Boolean() slope_required = graphene.Boolean() diff --git a/terraso_backend/apps/soil_id/models/project_soil_settings.py b/terraso_backend/apps/soil_id/models/project_soil_settings.py index 2bafbe607..367e3b07e 100644 --- a/terraso_backend/apps/soil_id/models/project_soil_settings.py +++ b/terraso_backend/apps/soil_id/models/project_soil_settings.py @@ -56,12 +56,6 @@ class Meta(BaseModel.Meta): project = models.OneToOneField(Project, on_delete=models.CASCADE, related_name="soil_settings") - class MeasurementUnit(models.TextChoices): - IMPERIAL = "IMPERIAL" - METRIC = "METRIC" - - measurement_units = models.CharField(blank=True, null=True, choices=MeasurementUnit.choices) - depth_interval_preset = models.CharField( default=DepthIntervalPreset.LANDPKS.value, choices=DepthIntervalPreset.choices, From fb4a3776b9a0e1a98d39a94a5ffc4fc7dfa90c0d Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 27 Mar 2024 14:37:41 -0700 Subject: [PATCH 2/6] refactor: rename IMPERIAL -> ENGLISH --- terraso_backend/apps/project_management/models/projects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/project_management/models/projects.py b/terraso_backend/apps/project_management/models/projects.py index 2ed169552..23d83a852 100644 --- a/terraso_backend/apps/project_management/models/projects.py +++ b/terraso_backend/apps/project_management/models/projects.py @@ -62,7 +62,7 @@ class Meta(BaseModel.Meta): membership_list = models.OneToOneField(ProjectMembershipList, on_delete=models.CASCADE) class MeasurementUnit(models.TextChoices): - IMPERIAL = "IMPERIAL" + ENGLISH = "ENGLISH" METRIC = "METRIC" measurement_units = models.CharField( From 20cde35c79718c182d45fb7d904b3901cfdcbd79 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 27 Mar 2024 14:37:58 -0700 Subject: [PATCH 3/6] chore: generate migrations & schema --- .../apps/graphql/schema/schema.graphql | 23 ++++++------ .../0027_project_measurement_units.py | 35 +++++++++++++++++++ ...e_projectsoilsettings_measurement_units.py | 32 +++++++++++++++++ 3 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 terraso_backend/apps/project_management/migrations/0027_project_measurement_units.py create mode 100644 terraso_backend/apps/soil_id/migrations/0013_remove_projectsoilsettings_measurement_units.py diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 429cb85c7..b674b7013 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -640,6 +640,7 @@ type ProjectNode implements Node { name: String! description: String! membershipList: ProjectMembershipListNode! + measurementUnits: ProjectManagementProjectMeasurementUnitsChoices! privacy: ProjectManagementProjectPrivacyChoices! archived: Boolean! siteInstructions: String @@ -706,6 +707,15 @@ enum ProjectMembershipProjectRoleChoices { MANAGER } +"""An enumeration.""" +enum ProjectManagementProjectMeasurementUnitsChoices { + """English""" + ENGLISH + + """Metric""" + METRIC +} + """An enumeration.""" enum ProjectManagementProjectPrivacyChoices { """Private""" @@ -1407,7 +1417,6 @@ enum SoilIdDepthDependentSoilDataCarbonatesChoices { type ProjectSoilSettingsNode { project: ProjectNode! - measurementUnits: SoilIdProjectSoilSettingsMeasurementUnitsChoices depthIntervalPreset: SoilIdProjectSoilSettingsDepthIntervalPresetChoices! soilPitRequired: Boolean! slopeRequired: Boolean! @@ -1427,15 +1436,6 @@ type ProjectSoilSettingsNode { depthIntervals: [ProjectDepthIntervalNode!]! } -"""An enumeration.""" -enum SoilIdProjectSoilSettingsMeasurementUnitsChoices { - """Imperial""" - IMPERIAL - - """Metric""" - METRIC -} - """An enumeration.""" enum SoilIdProjectSoilSettingsDepthIntervalPresetChoices { """Landpks""" @@ -2030,6 +2030,7 @@ type ProjectAddMutationPayload { input ProjectAddMutationInput { name: String! + measurementUnits: ProjectManagementProjectMeasurementUnitsChoices privacy: ProjectManagementProjectPrivacyChoices description: String siteInstructions: String @@ -2046,6 +2047,7 @@ type ProjectUpdateMutationPayload { input ProjectUpdateMutationInput { id: ID! name: String + measurementUnits: ProjectManagementProjectMeasurementUnitsChoices privacy: ProjectManagementProjectPrivacyChoices description: String siteInstructions: String @@ -2238,7 +2240,6 @@ type ProjectSoilSettingsUpdateMutationPayload { input ProjectSoilSettingsUpdateMutationInput { projectId: ID! - measurementUnits: SoilIdProjectSoilSettingsMeasurementUnitsChoices depthIntervalPreset: SoilIdProjectSoilSettingsDepthIntervalPresetChoices soilPitRequired: Boolean slopeRequired: Boolean diff --git a/terraso_backend/apps/project_management/migrations/0027_project_measurement_units.py b/terraso_backend/apps/project_management/migrations/0027_project_measurement_units.py new file mode 100644 index 000000000..0cb908f1e --- /dev/null +++ b/terraso_backend/apps/project_management/migrations/0027_project_measurement_units.py @@ -0,0 +1,35 @@ +# Copyright © 2024 Technology Matters +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see https://www.gnu.org/licenses/. + +# Generated by Django 5.0.2 on 2024-03-27 21:33 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("project_management", "0026_alter_project_privacy"), + ] + + operations = [ + migrations.AddField( + model_name="project", + name="measurement_units", + field=models.CharField( + choices=[("ENGLISH", "English"), ("METRIC", "Metric")], default="METRIC" + ), + ), + ] diff --git a/terraso_backend/apps/soil_id/migrations/0013_remove_projectsoilsettings_measurement_units.py b/terraso_backend/apps/soil_id/migrations/0013_remove_projectsoilsettings_measurement_units.py new file mode 100644 index 000000000..296520efd --- /dev/null +++ b/terraso_backend/apps/soil_id/migrations/0013_remove_projectsoilsettings_measurement_units.py @@ -0,0 +1,32 @@ +# Copyright © 2024 Technology Matters +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see https://www.gnu.org/licenses/. + +# Generated by Django 5.0.2 on 2024-03-27 21:33 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("soil_id", "0012_remove_depthdependentsoildata_color_hue_substep_and_more"), + ] + + operations = [ + migrations.RemoveField( + model_name="projectsoilsettings", + name="measurement_units", + ), + ] From 2a5bcbe3b32d86241b2916675ec064fb80cfe98c Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 27 Mar 2024 15:20:48 -0700 Subject: [PATCH 4/6] chore: remove unnecessary enum value unwrapping --- terraso_backend/apps/graphql/schema/sites.py | 6 ------ .../apps/project_management/graphql/projects.py | 7 +++---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/sites.py b/terraso_backend/apps/graphql/schema/sites.py index a7f31665d..b510c2f85 100644 --- a/terraso_backend/apps/graphql/schema/sites.py +++ b/terraso_backend/apps/graphql/schema/sites.py @@ -115,9 +115,6 @@ def mutate_and_get_payload(cls, root, info, create_soil_data=True, **kwargs): log = cls.get_logger() user = info.context.user - if "privacy" in kwargs: - kwargs["privacy"] = kwargs["privacy"].value - client_time = kwargs.pop("client_time", None) if not client_time: client_time = datetime.now() @@ -194,9 +191,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs): raise cls.not_allowed(MutationTypes.UPDATE) project_id = kwargs.pop("project_id", False) - if "privacy" in kwargs: - kwargs["privacy"] = kwargs["privacy"].value - result = super().mutate_and_get_payload(root, info, **kwargs) if project_id is False: # no project id included diff --git a/terraso_backend/apps/project_management/graphql/projects.py b/terraso_backend/apps/project_management/graphql/projects.py index c97716a9f..acccf8e33 100644 --- a/terraso_backend/apps/project_management/graphql/projects.py +++ b/terraso_backend/apps/project_management/graphql/projects.py @@ -174,7 +174,6 @@ def mutate_and_get_payload(cls, root, info, create_soil_settings=True, **kwargs) logger = cls.get_logger() user = info.context.user with transaction.atomic(): - kwargs["privacy"] = kwargs["privacy"].value result = super().mutate_and_get_payload(root, info, **kwargs) result.project.add_manager(user) @@ -187,9 +186,9 @@ def mutate_and_get_payload(cls, root, info, create_soil_settings=True, **kwargs) client_time = datetime.now() action = log_api.CREATE metadata = { - "name": kwargs["name"], - "privacy": kwargs["privacy"], - "description": kwargs.get("description"), + "name": result.project.name, + "privacy": result.project.privacy, + "description": result.project.description, } logger.log( user=user, From 6dc97aa7b0e464948f14e949125457159e11f30c Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 27 Mar 2024 15:21:13 -0700 Subject: [PATCH 5/6] tests: fix tests, test default case, fix surfaced bug --- .../project_management/models/projects.py | 6 +- .../tests/graphql/mutations/test_projects.py | 72 ++++++++++++++++--- .../tests/graphql/mutations/test_soil_data.py | 2 - 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/terraso_backend/apps/project_management/models/projects.py b/terraso_backend/apps/project_management/models/projects.py index 23d83a852..df6eba9dd 100644 --- a/terraso_backend/apps/project_management/models/projects.py +++ b/terraso_backend/apps/project_management/models/projects.py @@ -66,14 +66,16 @@ class MeasurementUnit(models.TextChoices): METRIC = "METRIC" measurement_units = models.CharField( - default=MeasurementUnit.METRIC, choices=MeasurementUnit.choices + choices=MeasurementUnit.choices, default=MeasurementUnit.METRIC.value ) class Privacy(models.TextChoices): PRIVATE = "PRIVATE" PUBLIC = "PUBLIC" - privacy = models.CharField(max_length=32, choices=Privacy.choices, default=Privacy.PRIVATE) + privacy = models.CharField( + max_length=32, choices=Privacy.choices, default=Privacy.PRIVATE.value + ) seen_by = models.ManyToManyField(User, related_name="+") archived = models.BooleanField( diff --git a/terraso_backend/tests/graphql/mutations/test_projects.py b/terraso_backend/tests/graphql/mutations/test_projects.py index 58bc4c8de..9aedfa558 100644 --- a/terraso_backend/tests/graphql/mutations/test_projects.py +++ b/terraso_backend/tests/graphql/mutations/test_projects.py @@ -32,6 +32,8 @@ addProject(input: $input) { project { id + privacy + measurementUnits seen membershipList { id @@ -42,12 +44,15 @@ """ -def test_create_project(client, user): +def test_create_project_default_values(client, user): client.force_login(user) response = graphql_query( CREATE_PROJECT_QUERY, variables={ - "input": {"name": "testProject", "privacy": "PRIVATE", "description": "A test project"} + "input": { + "name": "testProject", + "description": "A test project", + } }, client=client, ) @@ -56,6 +61,8 @@ def test_create_project(client, user): id = content["data"]["addProject"]["project"]["id"] project = Project.objects.get(pk=id) assert list([mb.user for mb in project.manager_memberships.all()]) == [user] + assert project.measurement_units == Project.MeasurementUnit.METRIC + assert project.privacy == Project.Privacy.PRIVATE assert project.description == "A test project" assert project.soil_settings is not None @@ -72,6 +79,43 @@ def test_create_project(client, user): assert log_result.metadata == expected_metadata +def test_create_project_values(client, user): + client.force_login(user) + response = graphql_query( + CREATE_PROJECT_QUERY, + variables={ + "input": { + "name": "testProject", + "description": "A test project", + "privacy": "PUBLIC", + "measurementUnits": "ENGLISH", + } + }, + client=client, + ) + content = json.loads(response.content) + assert "errors" not in content + id = content["data"]["addProject"]["project"]["id"] + project = Project.objects.get(pk=id) + assert list([mb.user for mb in project.manager_memberships.all()]) == [user] + assert project.measurement_units == Project.MeasurementUnit.ENGLISH + assert project.privacy == Project.Privacy.PUBLIC + assert project.description == "A test project" + assert project.soil_settings is not None + + logs = Log.objects.all() + assert len(logs) == 1 + log_result = logs[0] + assert log_result.event == CREATE.value + assert log_result.resource_object == project + expected_metadata = { + "name": "testProject", + "privacy": "PUBLIC", + "description": "A test project", + } + assert log_result.metadata == expected_metadata + + DELETE_PROJECT_GRAPHQL = """ mutation($input: ProjectDeleteMutationInput!) { deleteProject(input: $input) { @@ -164,27 +208,35 @@ def test_archive_project_user_not_manager(project, client, project_user): UPDATE_PROJECT_GRAPHQL = """ mutation($input: ProjectUpdateMutationInput!) { - updateProject(input: $input) { - project{ - id, - name, - privacy + updateProject(input: $input) { + project{ + id + name + privacy + measurementUnits + } + errors } - errors - } } """ def test_update_project_user_is_manager(project, client, project_manager): - input = {"id": str(project.id), "name": "test_name", "privacy": "PRIVATE"} + input = { + "id": str(project.id), + "name": "test_name", + "privacy": "PRIVATE", + "measurementUnits": "ENGLISH", + } client.force_login(project_manager) response = graphql_query(UPDATE_PROJECT_GRAPHQL, input_data=input, client=client) content = json.loads(response.content) + assert "errors" not in content assert content["data"]["updateProject"]["errors"] is None assert content["data"]["updateProject"]["project"]["id"] == str(project.id) assert content["data"]["updateProject"]["project"]["name"] == "test_name" assert content["data"]["updateProject"]["project"]["privacy"] == "PRIVATE" + assert content["data"]["updateProject"]["project"]["measurementUnits"] == "ENGLISH" @pytest.mark.parametrize( diff --git a/terraso_backend/tests/graphql/mutations/test_soil_data.py b/terraso_backend/tests/graphql/mutations/test_soil_data.py index 7a9ef0621..76b418f10 100644 --- a/terraso_backend/tests/graphql/mutations/test_soil_data.py +++ b/terraso_backend/tests/graphql/mutations/test_soil_data.py @@ -583,7 +583,6 @@ def test_updating_project_interval_not_allowed_with_preset( ) { updateProjectSoilSettings(input: $input) { projectSoilSettings { - measurementUnits depthIntervalPreset soilPitRequired slopeRequired @@ -629,7 +628,6 @@ def test_update_project_soil_settings(client, user, project_manager, project): new_data = { "projectId": str(project.id), - "measurementUnits": "METRIC", "depthIntervalPreset": "NRCS", "soilPitRequired": True, "slopeRequired": False, From e3d87f1517ce0c2434d5830745b432800da0a520 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 27 Mar 2024 16:38:47 -0700 Subject: [PATCH 6/6] fix: tests --- terraso_backend/apps/graphql/schema/sites.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/sites.py b/terraso_backend/apps/graphql/schema/sites.py index b510c2f85..bdd84eb8b 100644 --- a/terraso_backend/apps/graphql/schema/sites.py +++ b/terraso_backend/apps/graphql/schema/sites.py @@ -138,9 +138,9 @@ def mutate_and_get_payload(cls, root, info, create_soil_data=True, **kwargs): site = result.site site.mark_seen_by(user) metadata = { - "latitude": kwargs["latitude"], - "longitude": kwargs["longitude"], - "name": kwargs["name"], + "latitude": site.latitude, + "longitude": site.longitude, + "name": site.name, } if kwargs.get("project_id", None): metadata["project_id"] = kwargs["project_id"] @@ -209,10 +209,10 @@ def mutate_and_get_payload(cls, root, info, **kwargs): client_time = datetime.now() metadata = {} - for key, value in kwargs.items(): + for key in kwargs.keys(): if key == "id": continue - metadata[key] = value + metadata[key] = getattr(site, key) if project_id: if hasattr(project, "soil_settings") and hasattr(site, "soil_data"): if project_id is not None: