From 0d5af4116a024a7ea8364d4093f154b77a647dbb Mon Sep 17 00:00:00 2001 From: Adam <41971533+jcadam14@users.noreply.github.com> Date: Tue, 12 Mar 2024 13:31:28 -0400 Subject: [PATCH] Updated DAO, added migration, model_validator, and pytests --- ...0c01fb42_update_tax_id_to_10_characters.py | 32 +++++++++++++ src/entities/models/dao.py | 2 +- src/entities/models/dto.py | 12 +++++ tests/api/conftest.py | 2 +- tests/api/routers/test_institutions_api.py | 46 ++++++++++++++++--- .../entities/repos/test_institutions_repo.py | 8 ++-- tests/entities/test_listeners.py | 2 +- tests/migrations/test_migrations.py | 11 +++++ 8 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 db_revisions/versions/0e520c01fb42_update_tax_id_to_10_characters.py diff --git a/db_revisions/versions/0e520c01fb42_update_tax_id_to_10_characters.py b/db_revisions/versions/0e520c01fb42_update_tax_id_to_10_characters.py new file mode 100644 index 0000000..e4ed26f --- /dev/null +++ b/db_revisions/versions/0e520c01fb42_update_tax_id_to_10_characters.py @@ -0,0 +1,32 @@ +"""update tax-id to 10 characters + +Revision ID: 0e520c01fb42 +Revises: 8106d83ff594 +Create Date: 2024-03-12 11:40:25.790658 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = "0e520c01fb42" +down_revision: Union[str, None] = "8106d83ff594" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + with op.batch_alter_table("financial_institutions") as batch_op: + batch_op.alter_column("tax_id", existing_type=sa.String(9), type_=sa.String(10)) + with op.batch_alter_table("financial_institutions_history") as batch_op: + batch_op.alter_column("tax_id", existing_type=sa.String(9), type_=sa.String(10)) + + +def downgrade() -> None: + with op.batch_alter_table("financial_institutions") as batch_op: + batch_op.alter_column("tax_id", existing_type=sa.String(10), type_=sa.String(9)) + with op.batch_alter_table("financial_institutions_history") as batch_op: + batch_op.alter_column("tax_id", existing_type=sa.String(10), type_=sa.String(9)) diff --git a/src/entities/models/dao.py b/src/entities/models/dao.py index 57502c6..597ae7d 100644 --- a/src/entities/models/dao.py +++ b/src/entities/models/dao.py @@ -46,7 +46,7 @@ class FinancialInstitutionDao(AuditMixin, Base): domains: Mapped[List["FinancialInstitutionDomainDao"]] = relationship( "FinancialInstitutionDomainDao", back_populates="fi", lazy="selectin" ) - tax_id: Mapped[str] = mapped_column(String(9), unique=True, nullable=True) + tax_id: Mapped[str] = mapped_column(String(10), unique=True, nullable=True) rssd_id: Mapped[int] = mapped_column(unique=True, nullable=True) primary_federal_regulator_id: Mapped[str] = mapped_column(ForeignKey("federal_regulator.id"), nullable=True) primary_federal_regulator: Mapped["FederalRegulatorDao"] = relationship(lazy="selectin") diff --git a/src/entities/models/dto.py b/src/entities/models/dto.py index 94b5dc1..55f3dea 100644 --- a/src/entities/models/dto.py +++ b/src/entities/models/dto.py @@ -1,3 +1,5 @@ +import re + from typing import Generic, List, Set, Sequence from pydantic import BaseModel, model_validator from typing import TypeVar @@ -76,6 +78,16 @@ class FinancialInstitutionDto(FinancialInstitutionBase): top_holder_rssd_id: int | None = None version: int | None = None + @model_validator(mode="after") + def validate_fi(self) -> "FinancialInstitutionDto": + if self.tax_id: + match = re.match(r"^([0-9]{2}-[0-9]{7})", self.tax_id) + if not match: + raise ValueError( + f"Invalid tax_id {self.tax_id}. FinancialInstitution tax_id must conform to XX-XXXXXXX pattern." + ) + return self + class Config: from_attributes = True diff --git a/tests/api/conftest.py b/tests/api/conftest.py index 8e35aea..9a2b984 100644 --- a/tests/api/conftest.py +++ b/tests/api/conftest.py @@ -64,7 +64,7 @@ def get_institutions_mock(mocker: MockerFixture, authed_user_mock: Mock) -> Mock lei="TESTBANK123", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test.bank", lei="TESTBANK123")], - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI1", primary_federal_regulator=FederalRegulatorDao(id="FRI1", name="FRI1"), diff --git a/tests/api/routers/test_institutions_api.py b/tests/api/routers/test_institutions_api.py index 0fd11d6..81e7ecc 100644 --- a/tests/api/routers/test_institutions_api.py +++ b/tests/api/routers/test_institutions_api.py @@ -37,6 +37,38 @@ def test_create_institution_unauthed(self, app_fixture: FastAPI, unauthed_user_m res = client.post("/v1/institutions/", json={"name": "testName", "lei": "testLei"}) assert res.status_code == 403 + def test_invalid_tax_id(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock): + client = TestClient(app_fixture) + res = client.post( + "/v1/institutions/", + json={ + "name": "testName", + "lei": "testLei", + "is_active": True, + "tax_id": "123456789", + "rssd_id": 12344, + "primary_federal_regulator_id": "FRI2", + "hmda_institution_type_id": "HIT2", + "sbl_institution_type_ids": ["SIT2"], + "hq_address_street_1": "Test Address Street 1", + "hq_address_street_2": "", + "hq_address_city": "Test City 1", + "hq_address_state_code": "VA", + "hq_address_zip": "00000", + "parent_lei": "PARENTTESTBANK123", + "parent_legal_name": "PARENT TEST BANK 123", + "parent_rssd_id": 12345, + "top_holder_lei": "TOPHOLDERLEI123", + "top_holder_legal_name": "TOP HOLDER LEI 123", + "top_holder_rssd_id": 123456, + }, + ) + assert ( + res.json()["detail"][0]["msg"] + == "Value error, Invalid tax_id 123456789. FinancialInstitution tax_id must conform to XX-XXXXXXX pattern." + ) + assert res.status_code == 422 + def test_create_institution_authed(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock): upsert_institution_mock = mocker.patch("entities.repos.institutions_repo.upsert_institution") upsert_institution_mock.return_value = FinancialInstitutionDao( @@ -44,7 +76,7 @@ def test_create_institution_authed(self, mocker: MockerFixture, app_fixture: Fas lei="testLei", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test.bank", lei="TESTBANK123")], - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI2", primary_federal_regulator=FederalRegulatorDao(id="FRI2", name="FRI2"), @@ -73,7 +105,7 @@ def test_create_institution_authed(self, mocker: MockerFixture, app_fixture: Fas "name": "testName", "lei": "testLei", "is_active": True, - "tax_id": "123456789", + "tax_id": "12-3456789", "rssd_id": 12344, "primary_federal_regulator_id": "FRI2", "hmda_institution_type_id": "HIT2", @@ -181,7 +213,7 @@ def test_create_institution_authed_no_permission(self, app_fixture: FastAPI, aut "name": "testName", "lei": "testLei", "is_active": True, - "tax_id": "123456789", + "tax_id": "12-3456789", "rssd_id": 12344, "primary_federal_regulator_id": "FIR2", "hmda_institution_type_id": "HIT2", @@ -214,7 +246,7 @@ def test_get_institution_authed(self, mocker: MockerFixture, app_fixture: FastAP lei="TESTBANK123", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test.bank", lei="TESTBANK123")], - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI1", primary_federal_regulator=FederalRegulatorDao(id="FRI1", name="FRI1"), @@ -311,7 +343,7 @@ def test_get_associated_institutions( lei="TESTBANK123", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test123.bank", lei="TESTBANK123")], - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI1", primary_federal_regulator=FederalRegulatorDao(id="FRI1", name="FRI1"), @@ -336,7 +368,7 @@ def test_get_associated_institutions( lei="TESTBANK234", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test234.bank", lei="TESTBANK234")], - tax_id="123456879", + tax_id="12-3456879", rssd_id=6879, primary_federal_regulator_id="FRI1", primary_federal_regulator=FederalRegulatorDao(id="FRI1", name="FRI1"), @@ -437,7 +469,7 @@ def test_get_sbl_types(self, mocker: MockerFixture, app_fixture: FastAPI, authed lei="TESTBANK123", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test.bank", lei="TESTBANK123")], - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI1", primary_federal_regulator=FederalRegulatorDao(id="FRI1", name="FRI1"), diff --git a/tests/entities/repos/test_institutions_repo.py b/tests/entities/repos/test_institutions_repo.py index 152b383..1d1a6fc 100644 --- a/tests/entities/repos/test_institutions_repo.py +++ b/tests/entities/repos/test_institutions_repo.py @@ -55,7 +55,7 @@ async def setup( lei="TESTBANK123", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test.bank.1", lei="TESTBANK123")], - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI1", hmda_institution_type_id="HIT1", @@ -78,7 +78,7 @@ async def setup( lei="TESTBANK456", is_active=True, domains=[FinancialInstitutionDomainDao(domain="test.bank.2", lei="TESTBANK456")], - tax_id="987654321", + tax_id="98-7654321", rssd_id=4321, primary_federal_regulator_id="FRI2", hmda_institution_type_id="HIT2", @@ -101,7 +101,7 @@ async def setup( lei="TESTSUBBANK456", is_active=True, domains=[FinancialInstitutionDomainDao(domain="sub.test.bank.2", lei="TESTSUBBANK456")], - tax_id="765432198", + tax_id="76-5432198", rssd_id=2134, primary_federal_regulator_id="FRI3", hmda_institution_type_id="HIT3", @@ -206,7 +206,7 @@ async def test_add_institution(self, transaction_session: AsyncSession): name="New Bank 123", lei="NEWBANK123", is_active=True, - tax_id="654321987", + tax_id="65-4321987", rssd_id=6543, primary_federal_regulator_id="FRI3", hmda_institution_type_id="HIT3", diff --git a/tests/entities/test_listeners.py b/tests/entities/test_listeners.py index 9c8f908..f7d2ece 100644 --- a/tests/entities/test_listeners.py +++ b/tests/entities/test_listeners.py @@ -20,7 +20,7 @@ class TestListeners: name="Test Bank 123", lei="TESTBANK123", is_active=True, - tax_id="123456789", + tax_id="12-3456789", rssd_id=1234, primary_federal_regulator_id="FRI1", hmda_institution_type_id="HIT1", diff --git a/tests/migrations/test_migrations.py b/tests/migrations/test_migrations.py index 1c1b66d..d92ffbf 100644 --- a/tests/migrations/test_migrations.py +++ b/tests/migrations/test_migrations.py @@ -47,3 +47,14 @@ def test_fi_history_tables_8106d83ff594(alembic_runner: MigrationContext, alembi tables = inspector.get_table_names() assert "financial_institutions_history" in tables assert "fi_to_type_mapping_history" in tables + + +def test_fi_history_tables_0e520c01fb42(alembic_runner: MigrationContext, alembic_engine: Engine): + alembic_runner.migrate_up_to("0e520c01fb42") + inspector = sqlalchemy.inspect(alembic_engine) + + tax_column = [c for c in inspector.get_columns("financial_institutions") if c["name"] == "tax_id"][0] + assert tax_column["type"].length == 10 + + tax_column = [c for c in inspector.get_columns("financial_institutions_history") if c["name"] == "tax_id"][0] + assert tax_column["type"].length == 10