Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Manage soil intervals on backend #1100

Merged
merged 18 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions terraso_backend/apps/graphql/schema/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,7 @@ input SiteAddMutationInput {
longitude: Float!
privacy: ProjectManagementSitePrivacyChoices
projectId: ID
createSoilData: Boolean
clientMutationId: String
}

Expand Down
26 changes: 23 additions & 3 deletions terraso_backend/apps/graphql/schema/sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from apps.audit_logs import api as audit_log_api
from apps.project_management.graphql.projects import ProjectNode
from apps.project_management.models import Project, Site, sites
from apps.soil_id.models.soil_data import SoilData
from apps.soil_id.models import SoilData

from .commons import (
BaseAuthenticatedMutation,
Expand Down Expand Up @@ -108,9 +108,10 @@ class Input:
longitude = graphene.Float(required=True)
privacy = SiteNode.privacy_enum()
project_id = graphene.ID()
create_soil_data = graphene.Boolean()

@classmethod
def mutate_and_get_payload(cls, root, info, **kwargs):
def mutate_and_get_payload(cls, root, info, create_soil_data=True, **kwargs):
log = cls.get_logger()
user = info.context.user

Expand All @@ -134,6 +135,14 @@ def mutate_and_get_payload(cls, root, info, **kwargs):
if result.errors:
return result

if create_soil_data:
SoilData.objects.create(site=result.site)
if adding_to_project:
if hasattr(project, "soil_settings"):
project.soil_settings.convert_site_intervals_to_preset(
new_preset=None, sites=[result.site]
)

site = result.site
site.mark_seen_by(user)
metadata = {
Expand Down Expand Up @@ -181,6 +190,7 @@ class Input:
project_id = graphene.ID()

@classmethod
@transaction.atomic
def mutate_and_get_payload(cls, root, info, **kwargs):
log = cls.get_logger()
user = info.context.user
Expand Down Expand Up @@ -215,7 +225,14 @@ def mutate_and_get_payload(cls, root, info, **kwargs):
continue
metadata[key] = value
if project_id:
metadata["project_id"] = str(project.id)
if hasattr(project, "soil_settings") and hasattr(site, "soil_data"):
if project_id is not None:
project.soil_settings.convert_site_intervals_to_preset(sites=[site])
metadata["project_id"] = str(project.id)
else:
if hasattr(site, "soil_data"):
# Delete existing intervals if removed from project
site.soil_data.remove_from_project()

log.log(
user=user,
Expand Down Expand Up @@ -301,6 +318,9 @@ def mutate_and_get_payload(cls, root, info, **kwargs):
old_projects.append(site.project)

Site.bulk_change_project(to_change, project)
# update site depth intervals where necessary
david-code marked this conversation as resolved.
Show resolved Hide resolved
if hasattr(project, "soil_settings"):
project.soil_settings.convert_site_intervals_to_preset(sites=to_change)

log.log(
user=user,
Expand Down
49 changes: 47 additions & 2 deletions terraso_backend/apps/soil_id/models/project_soil_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@
#
# 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/.
from typing import Optional

from dirtyfields import DirtyFieldsMixin
from django.db import models, transaction

from apps.core.models.commons import BaseModel
from apps.project_management.models.projects import Project
from apps.project_management.models import Project, Site
from apps.soil_id import permission_rules
from apps.soil_id.models.depth_dependent_soil_data import DepthDependentSoilData
from apps.soil_id.models.depth_interval import BaseDepthInterval
from apps.soil_id.models.soil_data import SoilDataDepthInterval


class DepthIntervalPreset(models.TextChoices):
Expand Down Expand Up @@ -90,20 +92,63 @@ class MeasurementUnit(models.TextChoices):
def is_custom_preset(self):
return self.depth_interval_preset == DepthIntervalPreset.CUSTOM

@property
def methods(self):
field_names = [
field.name.removesuffix("_enabled")
for field in SoilDataDepthInterval._meta.fields
if field.name.endswith("_enabled")
]
return {
field_name + "_enabled": getattr(self, field_name + "_required")
david-code marked this conversation as resolved.
Show resolved Hide resolved
for field_name in field_names
}

def save(self, *args, **kwargs):
dirty_fields = self.get_dirty_fields()
with transaction.atomic():
result = super().save(*args, **kwargs)
if (
"depth_interval_preset" in dirty_fields
and dirty_fields.get("depth_interval_preset") != self.depth_interval_preset
and (new_preset := dirty_fields.get("depth_interval_preset"))
!= self.depth_interval_preset
):
# delete project intervals
ProjectDepthInterval.objects.filter(project=self).delete()
# delete related soil data
DepthDependentSoilData.delete_in_project(self.project.id)
# create site intervals
self.convert_site_intervals_to_preset(new_preset)
return result

def convert_site_intervals_to_preset(
self, new_preset: Optional[DepthIntervalPreset] = None, sites: Optional[list[Site]] = None
):
if not sites:
sites = Site.objects.filter(project=self.project, soil_data__isnull=False)
david-code marked this conversation as resolved.
Show resolved Hide resolved
intervals = SoilDataDepthInterval.objects.filter(soil_data__site__in=sites)

intervals.delete()
# create new site intervals
david-code marked this conversation as resolved.
Show resolved Hide resolved
options = {
DepthIntervalPreset.LANDPKS: LandPKSIntervalDefaults,
DepthIntervalPreset.NRCS: NRCSIntervalDefaults,
}
preset = new_preset or self.depth_interval_preset
interval_bounds = options.get(preset)
if interval_bounds:
intervals = []
for site in sites:
if not hasattr(site, "soil_data"):
# Not going to create an interval if soil_data doesn't exist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Not going to create an interval if soil_data doesn't exist
# Do not create an interval if soil_data doesn't exist

continue
interval_objects = [
SoilDataDepthInterval(soil_data=site.soil_data, **interval, **self.methods)
for interval in interval_bounds
]
intervals.extend(interval_objects)
return SoilDataDepthInterval.objects.bulk_create(intervals)


class ProjectDepthInterval(BaseModel, BaseDepthInterval):
project = models.ForeignKey(
Expand Down
7 changes: 7 additions & 0 deletions terraso_backend/apps/soil_id/models/soil_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ class SoilDataDepthIntervalPreset(models.TextChoices):
choices=SoilDataDepthIntervalPreset.choices, blank=True, null=True
)

def remove_from_project(self):
SoilDataDepthInterval.objects.filter(soil_data=self).delete()
# Default is to set the site interval to custom for now
# At some point, user will be able to set a default preset
david-code marked this conversation as resolved.
Show resolved Hide resolved
self.depth_interval_preset = self.SoilDataDepthIntervalPreset.CUSTOM
self.save()


class SoilDataDepthInterval(BaseModel, BaseDepthInterval):
soil_data = models.ForeignKey(
Expand Down
32 changes: 18 additions & 14 deletions terraso_backend/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,13 @@
from django.test.client import Client
from mixer.backend.django import mixer
from pyproj import CRS, Transformer
from tests.utils import add_soil_data_to_site

from apps.auth.services import JWTService
from apps.collaboration.models import Membership
from apps.core.gis.utils import DEFAULT_CRS
from apps.core.models import User
from apps.project_management.models import Project, Site
from apps.soil_id.models import (
DepthDependentSoilData,
LandPKSIntervalDefaults,
ProjectSoilSettings,
SoilData,
)

pytestmark = pytest.mark.django_db

Expand Down Expand Up @@ -141,12 +136,21 @@ def project_user_w_role(request, project: Project):

@pytest.fixture
def site_with_soil_data(request, project_manager: User, project: Project, project_site: Site):
ProjectSoilSettings.objects.create(project=project)
SoilData.objects.create(site=project_site)
for interval in LandPKSIntervalDefaults:
DepthDependentSoilData.objects.create(
soil_data=project_site.soil_data,
depth_interval_start=interval["depth_interval_start"],
depth_interval_end=interval["depth_interval_end"],
)
add_soil_data_to_site(project_site)
return project_site


@pytest.fixture
def site_with_soil_data_or_not(request, project_site, site_with_soil_data):
"""This fixture is for indirect parametrization, when we want to test with both a
site that has soil data, and one that does not.

Usage:

@pytest.mark.parametrize("site_with_soil_data_or_not", [False, True], indirect=True)
def test_sites(site_with_soil_data_or_not):
...
"""
if not request:
return (False, project_site)
return (True, site_with_soil_data)
47 changes: 43 additions & 4 deletions terraso_backend/tests/graphql/mutations/test_sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,21 @@
import structlog
from graphene_django.utils.testing import graphql_query
from mixer.backend.django import mixer
from tests.utils import match_json
from tests.utils import (
add_soil_data_to_site,
match_json,
site_intervals_match_project_preset,
)

from apps.audit_logs.api import CHANGE, CREATE, DELETE
from apps.audit_logs.models import Log
from apps.core.models import User
from apps.project_management.models import Project, Site
from apps.soil_id.models import (
DepthIntervalPreset,
ProjectSoilSettings,
SoilDataDepthInterval,
)

pytestmark = pytest.mark.django_db

Expand Down Expand Up @@ -70,6 +79,9 @@ def test_site_creation(client_query, user):

@pytest.mark.parametrize("project_user_w_role", ["manager", "contributor"], indirect=True)
def test_site_creation_in_project(client, project_user_w_role, project):
project.soil_settings = ProjectSoilSettings()
project.soil_settings.depth_interval_preset = DepthIntervalPreset.LANDPKS
project.soil_settings.save()
kwargs = site_creation_keywords()
kwargs["projectId"] = str(project.id)
client.force_login(project_user_w_role)
Expand All @@ -89,6 +101,8 @@ def test_site_creation_in_project(client, project_user_w_role, project):
assert log_result.metadata["latitude"] == expected_metadata["latitude"]
assert log_result.metadata["longitude"] == expected_metadata["longitude"]

assert site_intervals_match_project_preset(site)


UPDATE_SITE_QUERY = """
mutation SiteUpdateMutation($input: SiteUpdateMutationInput!) {
Expand All @@ -106,12 +120,17 @@ def test_site_creation_in_project(client, project_user_w_role, project):
"""


def test_update_site_in_project(client, project, project_manager, site):
@pytest.mark.parametrize("site_with_soil_data_or_not", [False, True], indirect=True)
def test_update_site_in_project(client, project, project_manager, site_with_soil_data_or_not):
original_project = mixer.blend(Project)
original_project.add_manager(project_manager)
has_soil_data, site = site_with_soil_data_or_not
site.add_to_project(project)

client.force_login(project_manager)
if has_soil_data:
project.soil_settings.depth_interval_preset = DepthIntervalPreset.NRCS
project.save()
response = graphql_query(
UPDATE_SITE_QUERY,
variables={
Expand All @@ -137,6 +156,10 @@ def test_update_site_in_project(client, project, project_manager, site):
assert log_result.resource_object == site
assert log_result.metadata["project_id"] == str(project.id)

if has_soil_data:
# test that the soil intervals match the new project's preset
assert site_intervals_match_project_preset(site)


def test_adding_site_to_project_user_not_manager(client, project, site, project_user):
site_creator = project_user
Expand Down Expand Up @@ -171,7 +194,11 @@ def test_adding_site_owned_by_user_to_project(client, project, site, project_man
assert site.owned_by(project)


def test_removing_site_from_project(client, project, project_site, project_manager):
@pytest.mark.parametrize("site_with_soil_data_or_not", [False, True], indirect=True)
def test_removing_site_from_project(site_with_soil_data_or_not, client, project, project_manager):
has_soil_data, project_site = site_with_soil_data_or_not
if has_soil_data:
assert SoilDataDepthInterval.objects.filter(soil_data=project_site.soil_data).exists()
client.force_login(project_manager)
response = graphql_query(
UPDATE_SITE_QUERY, input_data={"id": str(project_site.id), "projectId": None}, client=client
Expand All @@ -180,6 +207,8 @@ def test_removing_site_from_project(client, project, project_site, project_manag
assert match_json("*..site.project", response) == [None]
project_site.refresh_from_db()
assert project_site.owner == project_manager
if has_soil_data:
assert not SoilDataDepthInterval.objects.filter(soil_data=project_site.soil_data).exists()


def test_not_providing_project_id_does_not_change_project(
Expand Down Expand Up @@ -315,7 +344,14 @@ def test_delete_linked_site(client, linked_site, project_manager):


@pytest.mark.parametrize("linked_site", ["linked", "manager"], indirect=True)
def test_site_transfer_success(linked_site, client, project, project_manager):
@pytest.mark.parametrize("has_project_data", [True, False])
def test_site_transfer_success(linked_site, has_project_data, client, project, project_manager):
if has_project_data:
ProjectSoilSettings.objects.create(
project=project, depth_interval_preset=DepthIntervalPreset.NRCS
)
if getattr(linked_site, "project", None):
add_soil_data_to_site(linked_site, preset=DepthIntervalPreset.LANDPKS)
input_data = {"siteIds": [str(linked_site.id)], "projectId": str(project.id)}
client.force_login(project_manager)
old_projects = [str(linked_site.project.id)] if linked_site.project else []
Expand All @@ -336,6 +372,9 @@ def test_site_transfer_success(linked_site, client, project, project_manager):
assert log_result.metadata["project_id"] == str(project.id)
assert log_result.metadata["transfered_sites"] == [str(linked_site.id)]

if has_project_data and getattr(linked_site, "project", None):
assert site_intervals_match_project_preset(linked_site)


def test_site_transfer_unlinked_site_user_contributor_success(client, user, site, project):
project.add_user_with_role(user, "contributor")
Expand Down
Loading