Skip to content

Commit

Permalink
474 update is voluntary field to be nullable (#475)
Browse files Browse the repository at this point in the history
Closes #474 

Need to update either this or #473 due to each PR have an alembic
script, so will need to make whichever PR is reviewed/approved last
alembic based on the first PR merged in.
  • Loading branch information
jcadam14 authored Oct 29, 2024
1 parent 54fc55d commit 71d24a3
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
"""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


# 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)


def downgrade() -> None:
with op.batch_alter_table("filing", schema=None) as batch_op:
batch_op.alter_column("is_voluntary", nullable=False)
2 changes: 1 addition & 1 deletion src/sbl_filing_api/entities/models/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
2 changes: 1 addition & 1 deletion src/sbl_filing_api/entities/models/dto.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
6 changes: 6 additions & 0 deletions src/sbl_filing_api/routers/filing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 0 additions & 5 deletions tests/api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ def get_filing_mock(mocker: MockerFixture) -> Mock:
action_type=UserActionType.SUBMIT,
timestamp=datetime.now(),
),
is_voluntary=False,
)
return mock

Expand Down Expand Up @@ -130,7 +129,6 @@ def get_filings_mock(mocker: MockerFixture) -> Mock:
action_type=UserActionType.SUBMIT,
timestamp=datetime.now(),
),
is_voluntary=False,
),
FilingDAO(
id=1,
Expand Down Expand Up @@ -159,7 +157,6 @@ def get_filings_mock(mocker: MockerFixture) -> Mock:
action_type=UserActionType.SUBMIT,
timestamp=datetime.now(),
),
is_voluntary=False,
),
FilingDAO(
id=1,
Expand Down Expand Up @@ -188,7 +185,6 @@ def get_filings_mock(mocker: MockerFixture) -> Mock:
action_type=UserActionType.SUBMIT,
timestamp=datetime.now(),
),
is_voluntary=False,
),
]
return mock
Expand Down Expand Up @@ -224,7 +220,6 @@ def post_filing_mock(mocker: MockerFixture) -> Mock:
action_type=UserActionType.CREATE,
timestamp=datetime.now(),
),
is_voluntary=False,
)
return mock

Expand Down
11 changes: 4 additions & 7 deletions tests/api/routers/test_filing_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,6 @@ def test_put_contact_info(
action_type=UserActionType.CREATE,
timestamp=datetime.datetime.now(),
),
is_voluntary=False,
)

client = TestClient(app_fixture)
Expand Down Expand Up @@ -807,7 +806,6 @@ def test_no_extension(
action_type=UserActionType.CREATE,
timestamp=datetime.datetime.now(),
),
is_voluntary=True,
)

client = TestClient(app_fixture)
Expand Down Expand Up @@ -995,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(
Expand Down Expand Up @@ -1081,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
Expand Down
8 changes: 8 additions & 0 deletions tests/migrations/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

0 comments on commit 71d24a3

Please sign in to comment.