From a6a3fbd34cf3d3c720f8d4e7dd694f6e0bf837cd Mon Sep 17 00:00:00 2001 From: Adam <41971533+jcadam14@users.noreply.github.com> Date: Mon, 28 Oct 2024 14:37:42 -0600 Subject: [PATCH 1/4] Changed is_voluntary to nullable --- ...f5cf036_change_is_voluntary_to_nullable.py | 38 +++++++++++++++++++ src/sbl_filing_api/entities/models/dao.py | 2 +- src/sbl_filing_api/entities/models/dto.py | 2 +- tests/api/conftest.py | 5 --- tests/api/routers/test_filing_api.py | 2 - tests/migrations/test_migrations.py | 8 ++++ 6 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py diff --git a/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py b/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py new file mode 100644 index 00000000..173dffc1 --- /dev/null +++ b/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py @@ -0,0 +1,38 @@ +"""change is_voluntary to nullable + +Revision ID: 63138f5cf036 +Revises: 26f11ac15b3c +Create Date: 2024-10-28 14:22:58.391354 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = "63138f5cf036" +down_revision: Union[str, None] = "26f11ac15b3c" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + with op.batch_alter_table("filing", schema=None) as batch_op: + batch_op.alter_column("is_voluntary", nullable=True) + + conn = op.get_bind() + conn.execute( + sa.text( + """ + UPDATE filing SET is_voluntary = null + """ + ) + ) + + +def downgrade() -> None: + with op.batch_alter_table("filing", schema=None) as batch_op: + batch_op.alter_column("is_voluntary", nullable=False) diff --git a/src/sbl_filing_api/entities/models/dao.py b/src/sbl_filing_api/entities/models/dao.py index 40ed0de8..17138e93 100644 --- a/src/sbl_filing_api/entities/models/dao.py +++ b/src/sbl_filing_api/entities/models/dao.py @@ -118,7 +118,7 @@ class FilingDAO(Base): confirmation_id: Mapped[str] = mapped_column(nullable=True) creator_id: Mapped[int] = mapped_column(ForeignKey("user_action.id")) creator: Mapped[UserActionDAO] = relationship(lazy="selectin", foreign_keys=[creator_id]) - is_voluntary: Mapped[bool] = mapped_column(default=False) + is_voluntary: Mapped[bool] = mapped_column(nullable=True) def __str__(self): return f"ID: {self.id}, Filing Period: {self.filing_period}, LEI: {self.lei}, Tasks: {self.tasks}, Institution Snapshot ID: {self.institution_snapshot_id}, Contact Info: {self.contact_info}" diff --git a/src/sbl_filing_api/entities/models/dto.py b/src/sbl_filing_api/entities/models/dto.py index 4b3ad9e3..e7ee3294 100644 --- a/src/sbl_filing_api/entities/models/dto.py +++ b/src/sbl_filing_api/entities/models/dto.py @@ -87,7 +87,7 @@ class FilingDTO(BaseModel): confirmation_id: str | None = None signatures: List[UserActionDTO] = [] creator: UserActionDTO - is_voluntary: bool + is_voluntary: bool | None = None class FilingPeriodDTO(BaseModel): diff --git a/tests/api/conftest.py b/tests/api/conftest.py index 148c03a7..482378d1 100644 --- a/tests/api/conftest.py +++ b/tests/api/conftest.py @@ -94,7 +94,6 @@ def get_filing_mock(mocker: MockerFixture) -> Mock: action_type=UserActionType.SUBMIT, timestamp=datetime.now(), ), - is_voluntary=False, ) return mock @@ -130,7 +129,6 @@ def get_filings_mock(mocker: MockerFixture) -> Mock: action_type=UserActionType.SUBMIT, timestamp=datetime.now(), ), - is_voluntary=False, ), FilingDAO( id=1, @@ -159,7 +157,6 @@ def get_filings_mock(mocker: MockerFixture) -> Mock: action_type=UserActionType.SUBMIT, timestamp=datetime.now(), ), - is_voluntary=False, ), FilingDAO( id=1, @@ -188,7 +185,6 @@ def get_filings_mock(mocker: MockerFixture) -> Mock: action_type=UserActionType.SUBMIT, timestamp=datetime.now(), ), - is_voluntary=False, ), ] return mock @@ -224,7 +220,6 @@ def post_filing_mock(mocker: MockerFixture) -> Mock: action_type=UserActionType.CREATE, timestamp=datetime.now(), ), - is_voluntary=False, ) return mock diff --git a/tests/api/routers/test_filing_api.py b/tests/api/routers/test_filing_api.py index 37be7904..6c71fb4e 100644 --- a/tests/api/routers/test_filing_api.py +++ b/tests/api/routers/test_filing_api.py @@ -713,7 +713,6 @@ def test_put_contact_info( action_type=UserActionType.CREATE, timestamp=datetime.datetime.now(), ), - is_voluntary=False, ) client = TestClient(app_fixture) @@ -807,7 +806,6 @@ def test_no_extension( action_type=UserActionType.CREATE, timestamp=datetime.datetime.now(), ), - is_voluntary=True, ) client = TestClient(app_fixture) diff --git a/tests/migrations/test_migrations.py b/tests/migrations/test_migrations.py index b87b8f1c..e7624563 100644 --- a/tests/migrations/test_migrations.py +++ b/tests/migrations/test_migrations.py @@ -442,3 +442,11 @@ def test_migrations_to_26f11ac15b3c(alembic_runner: MigrationContext, alembic_en inspector = sqlalchemy.inspect(alembic_engine) assert "is_voluntary" in set([c["name"] for c in inspector.get_columns("filing")]) + + +def test_migrations_to_63138f5cf036(alembic_runner: MigrationContext, alembic_engine: Engine): + alembic_runner.migrate_up_to("63138f5cf036") + + inspector = sqlalchemy.inspect(alembic_engine) + columns = inspector.get_columns("filing") + assert next(c for c in columns if c["name"] == "is_voluntary")["nullable"] From 526f6efdd00dd7f0a70e2b5f7dc67b8c1693c03e Mon Sep 17 00:00:00 2001 From: Adam <41971533+jcadam14@users.noreply.github.com> Date: Mon, 28 Oct 2024 14:51:41 -0600 Subject: [PATCH 2/4] Added check for is_voluntary in sign --- src/sbl_filing_api/routers/filing.py | 6 ++++++ tests/api/routers/test_filing_api.py | 9 ++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/sbl_filing_api/routers/filing.py b/src/sbl_filing_api/routers/filing.py index 1d6db621..e4de2e25 100644 --- a/src/sbl_filing_api/routers/filing.py +++ b/src/sbl_filing_api/routers/filing.py @@ -128,6 +128,12 @@ async def sign_filing(request: Request, lei: str, period_code: str): name="Filing Action Forbidden", detail=f"Cannot sign filing. Filing for {lei} for period {period_code} does not have a latest submission the SUBMISSION_ACCEPTED state.", ) + if filing.is_voluntary is None: + raise RegTechHttpException( + status_code=status.HTTP_403_FORBIDDEN, + name="Filing Action Forbidden", + detail=f"Cannot sign filing. Filing for {lei} for period {period_code} does not have a selection of is_voluntary defined.", + ) if not filing.contact_info: raise RegTechHttpException( status_code=status.HTTP_403_FORBIDDEN, diff --git a/tests/api/routers/test_filing_api.py b/tests/api/routers/test_filing_api.py index 6c71fb4e..13f38e8c 100644 --- a/tests/api/routers/test_filing_api.py +++ b/tests/api/routers/test_filing_api.py @@ -993,6 +993,7 @@ async def test_good_sign_filing( submission_time=datetime.datetime.now(), filename="file1.csv", ) + get_filing_mock.return_value.is_voluntary = False add_sig_mock = mocker.patch("sbl_filing_api.entities.repos.submission_repo.add_user_action") add_sig_mock.return_value = UserActionDAO( @@ -1079,16 +1080,14 @@ async def test_errors_sign_filing( filename="file1.csv", ) - """ - get_filing_mock.return_value.institution_snapshot_id = None res = client.put("/v1/filing/institutions/1234567890ABCDEFGH00/filings/2024/sign") assert res.status_code == 403 assert ( - res.json() - == "Cannot sign filing. Filing for 1234567890ABCDEFGH00 for period 2024 does not have institution snapshot id defined." + res.json()["error_detail"] + == "Cannot sign filing. Filing for 1234567890ABCDEFGH00 for period 2024 does not have a selection of is_voluntary defined." ) - """ + get_filing_mock.return_value.is_voluntary = True get_filing_mock.return_value.contact_info = None res = client.put("/v1/filing/institutions/1234567890ABCDEFGH00/filings/2024/sign") assert res.status_code == 403 From 2a8c8c8b6c708358375b65b5f72b046f4f510242 Mon Sep 17 00:00:00 2001 From: Adam <41971533+jcadam14@users.noreply.github.com> Date: Tue, 29 Oct 2024 11:27:00 -0600 Subject: [PATCH 3/4] Removed blanket delete of existing is_voluntary default --- .../63138f5cf036_change_is_voluntary_to_nullable.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py b/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py index 173dffc1..01d6e78a 100644 --- a/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py +++ b/db_revisions/versions/63138f5cf036_change_is_voluntary_to_nullable.py @@ -9,7 +9,6 @@ from typing import Sequence, Union from alembic import op -import sqlalchemy as sa # revision identifiers, used by Alembic. @@ -23,15 +22,6 @@ def upgrade() -> None: with op.batch_alter_table("filing", schema=None) as batch_op: batch_op.alter_column("is_voluntary", nullable=True) - conn = op.get_bind() - conn.execute( - sa.text( - """ - UPDATE filing SET is_voluntary = null - """ - ) - ) - def downgrade() -> None: with op.batch_alter_table("filing", schema=None) as batch_op: From a2138dd0dce1e6352c3ebe749db3ae34df6b441c Mon Sep 17 00:00:00 2001 From: Adam <41971533+jcadam14@users.noreply.github.com> Date: Tue, 29 Oct 2024 12:16:36 -0600 Subject: [PATCH 4/4] Updated endpoints to use submission counter and DTO --- src/sbl_filing_api/entities/models/dto.py | 7 +++++++ .../entities/repos/submission_repo.py | 4 ++++ src/sbl_filing_api/routers/filing.py | 20 +++++++++---------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/sbl_filing_api/entities/models/dto.py b/src/sbl_filing_api/entities/models/dto.py index 5f535dc3..798268b0 100644 --- a/src/sbl_filing_api/entities/models/dto.py +++ b/src/sbl_filing_api/entities/models/dto.py @@ -3,6 +3,7 @@ from typing import Dict, Any, List from pydantic import BaseModel, ConfigDict, Field, model_validator from sbl_filing_api.entities.models.model_enums import FilingType, FilingTaskState, SubmissionState, UserActionType +from sbl_filing_api.entities.models.dao import SubmissionDAO class UserActionDTO(BaseModel): @@ -28,6 +29,12 @@ class SubmissionDTO(BaseModel): submitter: UserActionDTO accepter: UserActionDTO | None = None + def from_orm(cls, obj: SubmissionDAO): + obj_dict = obj.__dict__.copy() + obj_dict['id'] = obj_dict['counter'] + return cls.parse_obj(obj_dict) + + class FilingTaskDTO(BaseModel): model_config = ConfigDict(from_attributes=True) diff --git a/src/sbl_filing_api/entities/repos/submission_repo.py b/src/sbl_filing_api/entities/repos/submission_repo.py index fb888b0f..c62bc2ba 100644 --- a/src/sbl_filing_api/entities/repos/submission_repo.py +++ b/src/sbl_filing_api/entities/repos/submission_repo.py @@ -55,6 +55,10 @@ async def get_submission(session: AsyncSession, submission_id: int) -> Submissio result = await query_helper(session, SubmissionDAO, id=submission_id) return result[0] if result else None +async def get_submission_by_counter(session: AsyncSession, lei: str, filing_period: str, counter: int) -> SubmissionDAO: + filing = await get_filing(session, lei=lei, filing_period=filing_period) + result = await query_helper(session, SubmissionDAO, filing=filing.id, counter=counter) + return result[0] if result else None async def get_filing(session: AsyncSession, lei: str, filing_period: str) -> FilingDAO: result = await query_helper(session, FilingDAO, lei=lei, filing_period=filing_period) diff --git a/src/sbl_filing_api/routers/filing.py b/src/sbl_filing_api/routers/filing.py index 9fdc237f..48e4c212 100644 --- a/src/sbl_filing_api/routers/filing.py +++ b/src/sbl_filing_api/routers/filing.py @@ -256,24 +256,24 @@ async def get_submission_latest(request: Request, lei: str, period_code: str): return Response(status_code=status.HTTP_204_NO_CONTENT) -@router.get("/institutions/{lei}/filings/{period_code}/submissions/{id}", response_model=SubmissionDTO | None) +@router.get("/institutions/{lei}/filings/{period_code}/submissions/{counter}", response_model=SubmissionDTO | None) @requires("authenticated") async def get_submission(request: Request, response: Response, id: int): - result = await repo.get_submission(request.state.db_session, id) + result = await repo.get_submission_by_counter(request.state.db_session, lei, period_code, counter) if result: return result response.status_code = status.HTTP_404_NOT_FOUND -@router.put("/institutions/{lei}/filings/{period_code}/submissions/{id}/accept", response_model=SubmissionDTO) +@router.put("/institutions/{lei}/filings/{period_code}/submissions/{counter}/accept", response_model=SubmissionDTO) @requires("authenticated") async def accept_submission(request: Request, id: int, lei: str, period_code: str): - submission = await repo.get_submission(request.state.db_session, id) + submission = await repo.get_submission_by_counter(request.state.db_session, lei, period_code, counter) if not submission: raise RegTechHttpException( status_code=status.HTTP_404_NOT_FOUND, name="Submission Not Found", - detail=f"Submission ID {id} does not exist, cannot accept a non-existing submission.", + detail=f"Submission {counter} for LEI {lei} in filing period {period_code} does not exist, cannot accept a non-existing submission.", ) if ( submission.state != SubmissionState.VALIDATION_SUCCESSFUL @@ -282,7 +282,7 @@ async def accept_submission(request: Request, id: int, lei: str, period_code: st raise RegTechHttpException( status_code=status.HTTP_403_FORBIDDEN, name="Submission Action Forbidden", - detail=f"Submission {id} for LEI {lei} in filing period {period_code} is not in an acceptable state. Submissions must be validated successfully or with only warnings to be accepted.", + detail=f"Submission {counter} for LEI {lei} in filing period {period_code} is not in an acceptable state. Submissions must be validated successfully or with only warnings to be accepted.", ) accepter = await repo.add_user_action( @@ -383,12 +383,12 @@ async def get_latest_submission_report(request: Request, lei: str, period_code: @router.get( - "/institutions/{lei}/filings/{period_code}/submissions/{id}/report", + "/institutions/{lei}/filings/{period_code}/submissions/{counter}/report", responses={200: {"content": {"text/plain; charset=utf-8": {}}}}, ) @requires("authenticated") -async def get_submission_report(request: Request, response: Response, lei: str, period_code: str, id: int): - sub = await repo.get_submission(request.state.db_session, id) +async def get_submission_report(request: Request, response: Response, lei: str, period_code: str, counter: int): + sub = await repo.get_submission_by_counter(request.state.db_session, lei, period_code, counter) if sub and sub.state in [ SubmissionState.VALIDATION_SUCCESSFUL, SubmissionState.VALIDATION_WITH_ERRORS, @@ -410,7 +410,7 @@ async def get_submission_report(request: Request, response: Response, lei: str, raise RegTechHttpException( status_code=status.HTTP_404_NOT_FOUND, name="Report Not Found", - detail=f"Report for ({id}) does not exist.", + detail=f"Report for ({lei}-{period_code}-{counter}) does not exist.", )