fix geography filter on family search

app/repository/family.py

@@ -22,7 +22,7 @@
 from db_client.models.organisation.users import Organisation
 from sqlalchemy import Column, and_
 from sqlalchemy import delete as db_delete
-from sqlalchemy import desc, func, or_
+from sqlalchemy import desc, or_
 from sqlalchemy import update as db_update
 from sqlalchemy.exc import NoResultFound, OperationalError
 from sqlalchemy.orm import Query, Session
@@ -34,45 +34,39 @@
 
 _LOGGER = logging.getLogger(__name__)
 
-FamilyGeoMetaOrg = Tuple[Family, str, FamilyMetadata, Corpus, Organisation]
+FamilyGeoMetaOrg = Tuple[Family, Geography, FamilyMetadata, Corpus, Organisation]
 
 
 def _get_query(db: Session) -> Query:
     # NOTE: SqlAlchemy will make a complete hash of query generation
     #       if columns are used in the query() call. Therefore, entire
     #       objects are returned.
-    geo_subquery = (
-        db.query(
-            func.min(Geography.value).label("value"),
-            FamilyGeography.family_import_id,
-        )
-        .join(FamilyGeography, FamilyGeography.geography_id == Geography.id)
-        .filter(FamilyGeography.family_import_id == Family.import_id)
-        .group_by(Geography.value, FamilyGeography.family_import_id)
-    ).subquery("geo_subquery")
-
     return (
-        db.query(Family, geo_subquery.c.value, FamilyMetadata, Corpus, Organisation)  # type: ignore
+        db.query(Family, Geography, FamilyMetadata, Corpus, Organisation)
+        .join(FamilyGeography, FamilyGeography.family_import_id == Family.import_id)
+        .join(
+            Geography,
+            Geography.id == FamilyGeography.geography_id,
+        )
         .join(FamilyMetadata, FamilyMetadata.family_import_id == Family.import_id)
         .join(FamilyCorpus, FamilyCorpus.family_import_id == Family.import_id)
         .join(Corpus, Corpus.import_id == FamilyCorpus.corpus_import_id)
         .join(Organisation, Corpus.organisation_id == Organisation.id)
-        .filter(geo_subquery.c.family_import_id == Family.import_id)  # type: ignore
     )
 
 
 def _family_to_dto(
     db: Session, fam_geo_meta_corp_org: FamilyGeoMetaOrg
 ) -> FamilyReadDTO:
     fam, geo_value, meta, corpus, org = fam_geo_meta_corp_org
-
     metadata = cast(dict, meta.value)
     org = cast(str, org.name)
+
     return FamilyReadDTO(
         import_id=str(fam.import_id),
         title=str(fam.title),
         summary=str(fam.description),
-        geography=geo_value,
+        geography=str(geo_value.value),
         category=str(fam.family_category),
         status=str(fam.family_status),
         metadata=metadata,

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "admin_backend"
-version = "2.17.16"
+version = "2.17.17"
 description = ""
 authors = ["CPR-dev-team <[email protected]>"]
 packages = [{ include = "app" }, { include = "tests" }]

tests/integration_tests/family/test_search.py

@@ -7,6 +7,36 @@
 from tests.integration_tests.setup_db import setup_db
 
 
+def test_search_geographies(
+    client: TestClient, data_db: Session, superuser_header_token
+):
+    setup_db(data_db)
+
+    # search for where we know there is 2 families with a geopraphy
+    response = client.get(
+        "/api/v1/families/?geography=afghanistan",
+        headers=superuser_header_token,
+    )
+    assert response.status_code == status.HTTP_200_OK
+    data = response.json()
+    assert isinstance(data, list)
+
+    ids_found = set([f["import_id"] for f in data])
+    assert len(ids_found) == 2
+
+    # search for where we know there is 1 families with a geopraphy
+    response = client.get(
+        "/api/v1/families/?geography=zimbabwe",
+        headers=superuser_header_token,
+    )
+    assert response.status_code == status.HTTP_200_OK
+    data = response.json()
+    assert isinstance(data, list)
+
+    ids_found = set([f["import_id"] for f in data])
+    assert len(ids_found) == 1
+
+
 def test_search_family_super(
     client: TestClient, data_db: Session, superuser_header_token
 ):

tests/integration_tests/family/test_update.py

@@ -232,7 +232,7 @@ def test_update_family_collections_to_one_that_does_not_exist(
     assert db_family.description == ""
 
     expected_geo = (
-        data_db.query(Geography).filter(Geography.display_value == "Other").one()
+        data_db.query(Geography).filter(Geography.display_value == "Afghanistan").one()
     )
     assert expected_geo.id in [g.id for g in db_family.geographies]
     assert db_family.family_category == "UNFCCC"
@@ -279,7 +279,11 @@ def test_update_fails_family_when_user_org_different_to_family_org(
     assert db_family.description == "apple"
     assert db_family.family_category == "UNFCCC"
 
-    geo_id = data_db.query(Geography.id).filter(Geography.value == "Other").scalar()
+    geo_id = (
+        data_db.query(Geography.id)
+        # TODO: PDCT-1406: Properly implement multi-geography support
+        .filter(Geography.value == db_family.geographies[0].value).scalar()
+    )
     assert geo_id in [g.id for g in db_family.geographies]
 
 
@@ -355,7 +359,11 @@ def test_update_family_when_collection_org_different_to_family_org(
     assert db_family.description == ""
     assert db_family.family_category == "UNFCCC"
 
-    geo_id = data_db.query(Geography.id).filter(Geography.value == "Other").scalar()
+    geo_id = (
+        data_db.query(Geography.id)
+        # TODO: PDCT-1406: Properly implement multi-geography support
+        .filter(Geography.value == db_family.geographies[0].value).scalar()
+    )
     assert geo_id in [g.id for g in db_family.geographies]
 
 
@@ -375,28 +383,32 @@ def test_update_family_idempotent_when_ok(
     client: TestClient, data_db: Session, user_header_token
 ):
     setup_db(data_db)
-    family = EXPECTED_FAMILIES[1]
+    expected_family = EXPECTED_FAMILIES[1]
     response = client.put(
-        f"/api/v1/families/{family['import_id']}",
-        json=family,
+        f"/api/v1/families/{expected_family['import_id']}",
+        json=expected_family,
         headers=user_header_token,
     )
     assert response.status_code == status.HTTP_200_OK
     data = response.json()
-    assert data["title"] == EXPECTED_FAMILIES[1]["title"]
-    assert data["summary"] == EXPECTED_FAMILIES[1]["summary"]
-    assert data["geography"] == EXPECTED_FAMILIES[1]["geography"]
-    assert data["category"] == EXPECTED_FAMILIES[1]["category"]
+    assert data["title"] == expected_family["title"]
+    assert data["summary"] == expected_family["summary"]
+    assert data["geography"] == expected_family["geography"]
+    assert data["category"] == expected_family["category"]
     db_family: Family = (
         data_db.query(Family)
-        .filter(Family.import_id == EXPECTED_FAMILIES[1]["import_id"])
+        .filter(Family.import_id == expected_family["import_id"])
         .one()
     )
-    assert db_family.title == EXPECTED_FAMILIES[1]["title"]
-    assert db_family.description == EXPECTED_FAMILIES[1]["summary"]
-    assert db_family.family_category == EXPECTED_FAMILIES[1]["category"]
+    assert db_family.title == expected_family["title"]
+    assert db_family.description == expected_family["summary"]
+    assert db_family.family_category == expected_family["category"]
 
-    geo_id = data_db.query(Geography.id).filter(Geography.value == "Other").scalar()
+    geo_id = (
+        data_db.query(Geography.id)
+        .filter(Geography.value == expected_family["geography"])
+        .scalar()
+    )
     assert geo_id in [g.id for g in db_family.geographies]
 
 

tests/integration_tests/setup_db.py

@@ -31,7 +31,7 @@
         "import_id": "A.0.0.1",
         "title": "apple",
         "summary": "",
-        "geography": "Other",
+        "geography": "AFG",
         "category": "UNFCCC",
         "status": "Created",
         "metadata": {
@@ -57,7 +57,7 @@
         "import_id": "A.0.0.2",
         "title": "apple orange banana",
         "summary": "apple",
-        "geography": "Other",
+        "geography": "ZWE",
         "category": "UNFCCC",
         "status": "Created",
         "metadata": {
@@ -83,7 +83,7 @@
         "import_id": "A.0.0.3",
         "title": "title",
         "summary": "orange peas",
-        "geography": "Other",
+        "geography": "AFG",
         "category": "UNFCCC",
         "status": "Created",
         "metadata": {"author": "CPR", "author_type": "Party"},