Merge pull request #90 from ricardogsilva/87-creating-a-new-coverage-…

…configuration-removes-possible-values-from-others

Fix coverage creation errors

arpav_ppcv/database.py

@@ -864,12 +864,18 @@ def create_coverage_configuration(
         db_conf_param_value = get_configuration_parameter_value(
             session, possible.configuration_parameter_value_id
         )
-        possible_value = coverages.ConfigurationParameterPossibleValue(
-            coverage_configuration=db_coverage_configuration,
-            configuration_parameter_value=db_conf_param_value,
-        )
-        session.add(possible_value)
-        to_refresh.append(possible_value)
+        if db_conf_param_value is not None:
+            possible_value = coverages.ConfigurationParameterPossibleValue(
+                coverage_configuration=db_coverage_configuration,
+                configuration_parameter_value_id=db_conf_param_value.id,
+            )
+            session.add(possible_value)
+            to_refresh.append(possible_value)
+        else:
+            raise ValueError(
+                f"Configuration parameter value with id "
+                f"{possible.configuration_parameter_value_id} does not exist"
+            )
     session.commit()
     for item in to_refresh:
         session.refresh(item)

arpav_ppcv/schemas/coverages.py

@@ -336,7 +336,9 @@ class CoverageConfigurationCreate(sqlmodel.SQLModel):
     ] = None
     uncertainty_lower_bounds_coverage_configuration_id: Optional[uuid.UUID] = None
     uncertainty_upper_bounds_coverage_configuration_id: Optional[uuid.UUID] = None
-    secondary_coverage_configurations_ids: Optional[list[uuid.UUID]] = None
+    secondary_coverage_configurations_ids: Annotated[
+        Optional[list[uuid.UUID]], pydantic.Field(default_factory=list)
+    ]
 
     @pydantic.field_validator("thredds_url_pattern")
     @classmethod

arpav_ppcv/webapp/admin/views.py

@@ -60,9 +60,12 @@ def related_observation_variable_choices_loader(
 def coverage_configurations_choices_loader(
     request: Request,
 ) -> Sequence[tuple[str, str]]:
-    main_cov_conf_id = uuid.UUID(request.path_params["pk"])
-    all_cov_confs = database.collect_all_coverage_configurations(request.state.session)
+    if (pk := request.path_params.get("pk")) is not None:
+        main_cov_conf_id = uuid.UUID(pk)
+    else:
+        main_cov_conf_id = None
     result = []
+    all_cov_confs = database.collect_all_coverage_configurations(request.state.session)
     for cov_conf in [cc for cc in all_cov_confs if cc.id != main_cov_conf_id]:
         result.append((cov_conf.name, cov_conf.name))
     return result

tests/conftest.py

@@ -18,7 +18,10 @@
     database,
     main,
 )
-from arpav_ppcv.schemas import observations
+from arpav_ppcv.schemas import (
+    coverages,
+    observations,
+)
 from arpav_ppcv.webapp import dependencies
 from arpav_ppcv.webapp.app import create_app_from_settings
 from arpav_ppcv.webapp.legacy.django_settings import get_custom_django_settings
@@ -188,6 +191,33 @@ def sample_monthly_measurements(
     return db_monthly_measurements
 
 
+@pytest.fixture()
+def sample_configuration_parameters(arpav_db_session):
+    db_conf_params = []
+    for i in range(5):
+        allowed_values = []
+        for j in range(4):
+            allowed_values.append(
+                coverages.ConfigurationParameterValue(
+                    name=f"fake_parameter_value{j}",
+                    description=f"Description for fake param value {j}",
+                )
+            )
+        db_conf_params.append(
+            coverages.ConfigurationParameter(
+                name=f"fake_parameter_{i}",
+                description=f"Description for fake param {i}",
+                allowed_values=allowed_values,
+            )
+        )
+    for db_conf_param in db_conf_params:
+        arpav_db_session.add(db_conf_param)
+    arpav_db_session.commit()
+    for db_conf_param in db_conf_params:
+        arpav_db_session.refresh(db_conf_param)
+    return db_conf_params
+
+
 def _override_get_settings():
     standard_settings = config.get_settings()
     return standard_settings

tests/test_database.py

@@ -1,6 +1,11 @@
+import random
+from contextlib import nullcontext as does_not_raise
+
+import pydantic
 import pytest
 
 from arpav_ppcv import database
+from arpav_ppcv.schemas import coverages
 
 
 @pytest.mark.parametrize(
@@ -71,3 +76,164 @@ def test_list_monthly_measurements(
         assert total is None
     for index, db_measurement in enumerate(db_measurements):
         assert db_measurement.date == expected_dates[index]
+
+
+@pytest.mark.parametrize(
+    "name, thredds_url_pattern, expected_raise",
+    [
+        pytest.param("fake_cov_conf1", "fake-thredds-pattern1", does_not_raise()),
+        pytest.param(
+            "fake_cov_conf2",
+            " fake-thredds-pattern2 ",
+            does_not_raise(),
+            id="spaces in thredds url pattern edges",
+        ),
+        pytest.param(
+            "fake-cov_conf2",
+            "fake-thredds-pattern2",
+            pytest.raises(pydantic.ValidationError),
+            id="invalid_name",
+        ),
+    ],
+)
+def test_create_coverage_configuration_simple(
+    arpav_db_session,
+    name,
+    thredds_url_pattern,
+    expected_raise,
+):
+    with expected_raise:
+        cov_conf_create = coverages.CoverageConfigurationCreate(
+            name=name,
+            netcdf_main_dataset_name="fake_ds",
+            thredds_url_pattern=thredds_url_pattern,
+            unit="fake_unit",
+            palette="fake_palette",
+            color_scale_min=0.0,
+            color_scale_max=1.0,
+            possible_values=[],
+        )
+        created = database.create_coverage_configuration(
+            arpav_db_session, cov_conf_create
+        )
+        assert created.id is not None
+        assert created.name == cov_conf_create.name
+        assert (
+            created.thredds_url_pattern == cov_conf_create.thredds_url_pattern.strip()
+        )
+
+
+def test_create_coverage_configuration_with_possible_values(
+    arpav_db_session, sample_configuration_parameters
+):
+    used_params = random.sample(sample_configuration_parameters, k=3)
+    possible_values = []
+    for used_param in used_params:
+        possible_values.extend(random.sample(used_param.allowed_values, k=2))
+    print(f"possible_values: {[(type(pv), pv.model_dump()) for pv in possible_values]}")
+    cov_conf_create = coverages.CoverageConfigurationCreate(
+        name="fake_name",
+        netcdf_main_dataset_name="fake_ds",
+        thredds_url_pattern="fake_thredds_pattern",
+        unit="fake_unit",
+        palette="fake_palette",
+        color_scale_min=0.0,
+        color_scale_max=1.0,
+        possible_values=[
+            coverages.ConfigurationParameterPossibleValueCreate(
+                configuration_parameter_value_id=p.id
+            )
+            for p in possible_values
+        ],
+    )
+    created = database.create_coverage_configuration(arpav_db_session, cov_conf_create)
+    assert created.id is not None
+    for possible_value in possible_values:
+        assert possible_value.id in [
+            pv.configuration_parameter_value_id for pv in created.possible_values
+        ]
+
+
+def test_create_coverage_configuration_uses_new_possible_values(
+    arpav_db_session, sample_configuration_parameters
+):
+    """Ensure that when a new coverage configuration is created it does not
+    clear any possible values that may be repeated in other coverage configurations.
+    This is to ensure that
+
+    https://github.com/geobeyond/Arpav-PPCV-backend/issues/87
+
+    is fixed.
+    """
+    used_param = sample_configuration_parameters[0]
+    possible_value = used_param.allowed_values[0]
+    cov_conf_create1 = coverages.CoverageConfigurationCreate(
+        name="fake_name1",
+        netcdf_main_dataset_name="fake_ds1",
+        thredds_url_pattern="fake_thredds_pattern1",
+        unit="fake_unit",
+        palette="fake_palette",
+        color_scale_min=0.0,
+        color_scale_max=1.0,
+        possible_values=[
+            coverages.ConfigurationParameterPossibleValueCreate(
+                configuration_parameter_value_id=possible_value.id
+            )
+        ],
+    )
+    created1 = database.create_coverage_configuration(
+        arpav_db_session, cov_conf_create1
+    )
+
+    assert created1.id is not None
+    assert (
+        created1.possible_values[
+            0
+        ].configuration_parameter_value.configuration_parameter.name
+        == possible_value.configuration_parameter.name
+    )
+    assert (
+        created1.possible_values[0].configuration_parameter_value.name
+        == possible_value.name
+    )
+
+    cov_conf_create2 = coverages.CoverageConfigurationCreate(
+        name="fake_name2",
+        netcdf_main_dataset_name="fake_ds2",
+        thredds_url_pattern="fake_thredds_pattern2",
+        unit="fake_unit",
+        palette="fake_palette",
+        color_scale_min=0.0,
+        color_scale_max=1.0,
+        possible_values=[
+            coverages.ConfigurationParameterPossibleValueCreate(
+                configuration_parameter_value_id=possible_value.id
+            )
+        ],
+    )
+    created2 = database.create_coverage_configuration(
+        arpav_db_session, cov_conf_create2
+    )
+    assert created2.id is not None
+    assert (
+        created2.possible_values[
+            0
+        ].configuration_parameter_value.configuration_parameter.name
+        == possible_value.configuration_parameter.name
+    )
+    assert (
+        created2.possible_values[0].configuration_parameter_value.name
+        == possible_value.name
+    )
+
+    assert created1.id is not None
+    assert (
+        created1.possible_values[
+            0
+        ].configuration_parameter_value.configuration_parameter.name
+        == possible_value.configuration_parameter.name
+    )
+    assert (
+        created1.possible_values[0].configuration_parameter_value.name
+        == possible_value.name
+    )