From 90aaa591b5323f7e07b85111d2d6c3022dc9fd9b Mon Sep 17 00:00:00 2001 From: Katy Baulch <46493669+katybaulch@users.noreply.github.com> Date: Mon, 8 Apr 2024 14:05:01 +0100 Subject: [PATCH] Feature/pdct 937 change the schema to remove unused link tables (#122) * Bump dbclient version to 3.4.0 * Remove all references to MetadataTax, FamilyOrg & MetadataOrg * Bump package version to 2.3.0 * Add more options for type of change * Fix assertion to look at corpus.import_id * Check for one or none and then not none --- .github/pull_request_template.md | 3 ++ app/repository/family.py | 40 ++++--------------- poetry.lock | 9 ++--- pyproject.toml | 4 +- tests/integration_tests/family/test_create.py | 20 ++++------ tests/integration_tests/setup_db.py | 32 +-------------- 6 files changed, 25 insertions(+), 83 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 75b171d2..c5a8266c 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -36,6 +36,9 @@ Please select the option(s) below that are most relevant: - [ ] Bug fix - [ ] New feature - [ ] Breaking change +- [ ] GitHub workflow update +- [ ] Documentation update +- [ ] Remove legacy code ## How Has This Been Tested? diff --git a/app/repository/family.py b/app/repository/family.py index fe586448..00a9f3f4 100644 --- a/app/repository/family.py +++ b/app/repository/family.py @@ -10,12 +10,11 @@ Family, FamilyCorpus, FamilyDocument, - FamilyOrganisation, FamilyStatus, Slug, ) from db_client.models.dfce.geography import Geography -from db_client.models.dfce.metadata import FamilyMetadata, MetadataOrganisation +from db_client.models.dfce.metadata import FamilyMetadata from db_client.models.organisation.corpus import Corpus from db_client.models.organisation.counters import CountedEntity from db_client.models.organisation.users import Organisation @@ -50,22 +49,6 @@ def _get_query(db: Session) -> Query: ) -def _family_org_from_dto( - dto: FamilyCreateDTO, geo_id: int, org_id: int -) -> Tuple[Family, Organisation]: - return ( - Family( - import_id="", - title=dto.title, - description=dto.summary, - geography_id=geo_id, - family_category=dto.category, - ), - # TODO: Remove use of FamilyOrganisation - FamilyOrganisation(family_import_id="", organisation_id=org_id), - ) - - def _family_to_dto(db: Session, fam_geo_meta_org: FamilyGeoMetaOrg) -> FamilyReadDTO: f = fam_geo_meta_org[0] geo_value = cast(str, fam_geo_meta_org[1].value) @@ -343,16 +326,16 @@ def create(db: Session, family: FamilyCreateDTO, geo_id: int, org_id: int) -> st :return bool: True if new Family was created otherwise False. """ try: - new_family, new_fam_org = _family_org_from_dto(family, geo_id, org_id) - new_family.import_id = cast( - Column, generate_import_id(db, CountedEntity.Family, org_id) + import_id = cast(Column, generate_import_id(db, CountedEntity.Family, org_id)) + new_family = Family( + import_id=import_id, + title=family.title, + description=family.summary, + geography_id=geo_id, + family_category=family.category, ) db.add(new_family) - # Old schema (to be removed in PDCT-937). - new_fam_org.family_import_id = new_family.import_id - db.add(new_fam_org) - # New schema. new_fam_corpus = db.query(Corpus).filter(Corpus.organisation_id == org_id).one() db.add( @@ -380,16 +363,9 @@ def create(db: Session, family: FamilyCreateDTO, geo_id: int, org_id: int) -> st # TODO Validate that the metadata being added conforms to corpus type. PDCT-945 # Add the metadata - # tax to be removed in PDCT-937. - tax = ( - db.query(MetadataOrganisation) - .filter(MetadataOrganisation.organisation_id == org_id) # TODO: Remove - .one() - ) db.add( FamilyMetadata( family_import_id=new_family.import_id, - taxonomy_id=tax.taxonomy_id, # TODO Remove as part PDCT-937 value=family.metadata, ) ) diff --git a/poetry.lock b/poetry.lock index ee21dd8c..c88f9903 100644 --- a/poetry.lock +++ b/poetry.lock @@ -518,7 +518,7 @@ test-randomorder = ["pytest-randomly"] [[package]] name = "db-client" -version = "3.3.0" +version = "3.4.0" description = "All things to do with the datamodel and its storage. Including alembic migrations and datamodel code." optional = false python-versions = "^3.9" @@ -538,8 +538,8 @@ SQLAlchemy-Utils = "^0.38.2" [package.source] type = "git" url = "https://github.com/climatepolicyradar/navigator-db-client.git" -reference = "v3.3.0" -resolved_reference = "26311c40257ae4e85acc6d7d4d64e73400328b8e" +reference = "v3.4.0" +resolved_reference = "82502bf4663c5650dd02962533dbd9328c155be7" [[package]] name = "dill" @@ -2077,7 +2077,6 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, @@ -3047,4 +3046,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "ba05614a58b7fcc6fda781a007ddbcc9d7b2961f7b39cd82d355d046b58e2b18" +content-hash = "92e22adbccb82f449d427df856adf0581867f0a678f7f17afe058e900a505f21" diff --git a/pyproject.toml b/pyproject.toml index 261ce695..f0daaa8b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "admin_backend" -version = "2.2.1" +version = "2.3.0" description = "" authors = ["CPR-dev-team "] packages = [{ include = "app" }, { include = "tests" }] @@ -29,7 +29,7 @@ boto3 = "^1.28.46" moto = "^4.2.2" types-sqlalchemy = "^1.4.53.38" urllib3 = "^1.26.17" -db-client = { git = "https://github.com/climatepolicyradar/navigator-db-client.git", tag = "v3.3.0" } +db-client = { git = "https://github.com/climatepolicyradar/navigator-db-client.git", tag = "v3.4.0" } [tool.poetry.dev-dependencies] pre-commit = "^2.17.0" diff --git a/tests/integration_tests/family/test_create.py b/tests/integration_tests/family/test_create.py index 07432c86..769bc51f 100644 --- a/tests/integration_tests/family/test_create.py +++ b/tests/integration_tests/family/test_create.py @@ -1,7 +1,7 @@ from typing import Optional from db_client.models.dfce.collection import CollectionFamily -from db_client.models.dfce.family import Family, FamilyCorpus, FamilyOrganisation, Slug +from db_client.models.dfce.family import Family, FamilyCorpus, Slug from db_client.models.dfce.metadata import FamilyMetadata from db_client.models.organisation.corpus import Corpus from fastapi import status @@ -51,16 +51,6 @@ def test_create_family(client: TestClient, data_db: Session, user_header_token): assert len(db_collection) == 1 assert db_collection[0].collection_import_id == "C.0.0.3" - # Old schema test. - org_id = ( - data_db.query(FamilyOrganisation.organisation_id) # To be removed - .filter( - FamilyOrganisation.family_import_id == expected_import_id - ) # To be removed - .scalar() - ) - assert org_id is not None - # New schema tests. fc = ( data_db.query(FamilyCorpus) @@ -69,8 +59,12 @@ def test_create_family(client: TestClient, data_db: Session, user_header_token): ) assert len(fc) == 1 assert fc[-1].corpus_import_id is not None - corpus = data_db.query(Corpus).filter(Corpus.organisation_id == org_id).one() - assert fc[-1].corpus_import_id == corpus.import_id + corpus = ( + data_db.query(Corpus) + .filter(Corpus.import_id == fc[-1].corpus_import_id) + .one_or_none() + ) + assert corpus is not None def test_create_family_when_not_authorised(client: TestClient, data_db: Session): diff --git a/tests/integration_tests/setup_db.py b/tests/integration_tests/setup_db.py index 96ce92f7..104bfaae 100644 --- a/tests/integration_tests/setup_db.py +++ b/tests/integration_tests/setup_db.py @@ -13,11 +13,7 @@ FamilyEvent, Slug, ) -from db_client.models.dfce.metadata import ( - FamilyMetadata, - MetadataOrganisation, - MetadataTaxonomy, -) +from db_client.models.dfce.metadata import FamilyMetadata from db_client.models.document.physical_document import ( LanguageSource, PhysicalDocument, @@ -245,15 +241,6 @@ def _setup_organisation(test_db: Session) -> tuple[int, int]: # Now an organisation org = test_db.query(Organisation).filter(Organisation.name == "CCLW").one() - # Remove default taxonomy from CCLW - old schema - # TODO: Remove this deletion in Milestone 4 - mo = ( - test_db.query(MetadataOrganisation) # remove - .filter(MetadataOrganisation.organisation_id == org.id) # remove - .one() - ) - test_db.delete(mo) - another_org = Organisation( name="Another org", description="because we will have more than one org", @@ -367,22 +354,6 @@ def _setup_family_data( "allowed_values": [], }, } - dummy_tax = MetadataTaxonomy( - id=99, description="to go", valid_metadata=valid_metadata - ) - - test_db.add(dummy_tax) - test_db.flush() - - # Old Schema modification (to be removed) - # MetadataOrganisation - mo = MetadataOrganisation(taxonomy_id=dummy_tax.id, organisation_id=default_org_id) - test_db.add(mo) - test_db.flush() - - omo = MetadataOrganisation(taxonomy_id=dummy_tax.id, organisation_id=other_org_id) - test_db.add(omo) - # End of "to be removed" # New Schema modification # CorpusType @@ -446,7 +417,6 @@ def _setup_family_data( test_db.add( FamilyMetadata( family_import_id=data["import_id"], - taxonomy_id=dummy_tax.id, # soon no longer needed value=data["metadata"], ) )