Skip to content

Commit

Permalink
feature/pdct 1553 fix query for muli geography (#366)
Browse files Browse the repository at this point in the history
* Add a test for get_family_document_and_context

* Add tests for multi geog

* Turn test green

* bump version

* update geo subquery

* make test more data driven

* use a different geo id
  • Loading branch information
diversemix authored Oct 7, 2024
1 parent c0c28f7 commit a78a4c0
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 5 deletions.
6 changes: 4 additions & 2 deletions app/db/crud/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ def get_slugged_objects(db: Session, slug: str) -> tuple[Optional[str], Optional
def get_family_document_and_context(
db: Session, family_document_import_id: str
) -> FamilyDocumentWithContextResponse:
geo_subquery = get_geo_subquery(db)
geo_subquery = get_geo_subquery(
db, family_document_import_id=family_document_import_id
)
db_objects = (
db.query(
Family, FamilyDocument, PhysicalDocument, geo_subquery.c.value, FamilyCorpus # type: ignore
Expand All @@ -71,7 +73,7 @@ def get_family_document_and_context(
.filter(FamilyDocument.physical_document_id == PhysicalDocument.id)
.filter(FamilyCorpus.family_import_id == Family.import_id)
.filter(geo_subquery.c.family_import_id == Family.import_id) # type: ignore
).one_or_none()
).first() # TODO: Fix when handling multiple geographies /PDCT-1510

if not db_objects:
_LOGGER.warning(
Expand Down
18 changes: 16 additions & 2 deletions app/db/crud/geography.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

import logging

from db_client.models.dfce.family import Family, FamilyGeography, FamilyStatus
from db_client.models.dfce.family import (
Family,
FamilyDocument,
FamilyGeography,
FamilyStatus,
)
from db_client.models.dfce.geography import Geography
from sqlalchemy import func
from sqlalchemy.exc import OperationalError
Expand All @@ -15,7 +20,10 @@


def get_geo_subquery(
db: Session, allowed_geo_slugs=None, allowed_geo_values=None
db: Session,
allowed_geo_slugs=None,
allowed_geo_values=None,
family_document_import_id=None,
) -> Query:

geo_subquery = (
Expand Down Expand Up @@ -46,6 +54,12 @@ def get_geo_subquery(
if allowed_geo_values is not None:
geo_subquery = geo_subquery.filter(Geography.value.in_(allowed_geo_values))

if family_document_import_id is not None:
geo_subquery = geo_subquery.join(
FamilyDocument,
FamilyDocument.family_import_id == FamilyGeography.family_import_id,
).filter(FamilyDocument.import_id == family_document_import_id)

return geo_subquery.subquery("geo_subquery")


Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "navigator_backend"
version = "1.17.2"
version = "1.17.3"
description = ""
authors = ["CPR-dev-team <[email protected]>"]
packages = [{ include = "app" }, { include = "tests" }]
Expand Down
53 changes: 53 additions & 0 deletions tests/unit/app/db/crud/test_get_family_document_and_context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from typing import Dict

from db_client.models.dfce import FamilyDocument, FamilyGeography
from sqlalchemy.orm import Session

from app.api.api_v1.schemas.document import FamilyDocumentWithContextResponse
from app.db.crud.document import get_family_document_and_context
from tests.non_search.setup_helpers import setup_with_documents_large_with_families


def test_get_family_document_and_context(
documents_large: list[Dict],
data_db: Session,
):
setup_with_documents_large_with_families(documents_large, data_db)
doc = (
data_db.query(FamilyDocument)
.filter(FamilyDocument.import_id == "CCLW.document.i00000192.n0000")
.one()
)

response: FamilyDocumentWithContextResponse = get_family_document_and_context(
data_db, doc.import_id
)

assert response.family.import_id == doc.family_import_id
assert response.document.import_id == doc.import_id


def test_get_family_document_and_context_extra_geog(
documents_large: list[Dict],
data_db: Session,
):
setup_with_documents_large_with_families(documents_large, data_db)
doc = (
data_db.query(FamilyDocument)
.filter(FamilyDocument.import_id == "CCLW.document.i00000192.n0000")
.one()
)
# add an extra geography
data_db.add(
FamilyGeography(
family_import_id=doc.family_import_id,
geography_id=17,
)
)
data_db.commit()
response: FamilyDocumentWithContextResponse = get_family_document_and_context(
data_db, doc.import_id
)

assert response.family.import_id == doc.family_import_id
assert response.document.import_id == doc.import_id
1 change: 1 addition & 0 deletions tests/unit/fixtures/CCLW.document.i00000192.n0000.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"url": "https://www.ndrc.gov.cn/xxgk/zcfb/ghxwj/202304/P020230407401908613786.pdf",
"content_type": "application/pdf",
"import_id": "CCLW.document.i00000192.n0000",
"geography_id": "CHN",
"language_variant": "Original Language",
"status": "PUBLISHED",
"metadata": {
Expand Down

0 comments on commit a78a4c0

Please sign in to comment.