Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

533 filing status prototype #540

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
"""Add filing state

Revision ID: 7fe49d38726b
Revises: 6ec12afa5b37
Create Date: 2024-12-30 13:05:01.303998

"""

from typing import Sequence, Union

from alembic import op, context
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision: str = "7fe49d38726b"
down_revision: Union[str, None] = "6ec12afa5b37"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None

filing_state_enum = postgresql.ENUM(
"OPEN",
"CLOSED",
name="filingstate",
create_type=False,
)

old_user_action = postgresql.ENUM(
"SUBMIT",
"ACCEPT",
"SIGNED",
"CREATE",
name="useractiontype",
create_type=False,
)

new_user_action = postgresql.ENUM(
"SUBMIT",
"ACCEPT",
"SIGNED",
"CREATE",
"REOPEN",
name="useractiontype",
create_type=False,
)


def upgrade() -> None:
filing_state_enum.create(op.get_bind(), checkfirst=True)
op.add_column(
"filing",
sa.Column("state", filing_state_enum),
)

if "sqlite" not in context.get_context().dialect.name:
op.execute("ALTER TYPE useractiontype RENAME TO useractiontype_old")
new_user_action.create(op.get_bind(), checkfirst=True)
op.execute(
"ALTER TABLE user_action ALTER COLUMN action_type TYPE useractiontype USING user_action::text::useractiontype"
)
op.execute("DROP TYPE useractiontype_old")


def downgrade() -> None:
op.drop_column("filing", "state")
if "sqlite" not in context.get_context().dialect.name:
op.execute(sa.DDL("DROP TYPE filingstate"))

op.execute("ALTER TYPE useractiontype RENAME TO useractiontype_old")
old_user_action.create(op.get_bind(), checkfirst=True)
op.execute(
"ALTER TABLE user_action ALTER COLUMN action_type TYPE useractiontype USING user_action::text::useractiontype"
)
op.execute("DROP TYPE useractiontype_old")
5 changes: 3 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ env = [
"FS_DOWNLOAD_CONFIG__PROTOCOL=file",
"ENV=TEST",
"MAIL_API_URL=http://mail-api:8765/internal/confirmation/send",
'REQUEST_VALIDATORS__SIGN_AND_SUBMIT=["valid_lei_status","valid_lei_tin","valid_filing_exists","valid_sub_accepted","valid_voluntary_filer","valid_contact_info"]',
'REQUEST_VALIDATORS__FILING_CREATE=["valid_period_exists", "valid_no_filing_exists"]'
'REQUEST_VALIDATORS__SIGN_AND_SUBMIT=["valid_lei_status","valid_lei_tin","valid_filing_exists_sign","valid_sub_accepted","valid_voluntary_filer","valid_contact_info"]',
'REQUEST_VALIDATORS__FILING_CREATE=["valid_period_exists", "valid_no_filing_exists"]',
'REQUEST_VALIDATORS__FILING_REOPEN=["valid_filing_exists_reopen", "valid_filing_not_open"]'
]
testpaths = ["tests"]

Expand Down
5 changes: 3 additions & 2 deletions src/.env.local
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ FS_UPLOAD_CONFIG__ROOT="../upload"
EXPIRED_SUBMISSION_CHECK_SECS=120
SERVER_CONFIG__RELOAD="true"
MAIL_API_URL=http://mail-api:8765/internal/confirmation/send
REQUEST_VALIDATORS__SIGN_AND_SUBMIT=["valid_lei_status","valid_lei_tin","valid_filing_exists","valid_sub_accepted","valid_voluntary_filer","valid_contact_info"]
REQUEST_VALIDATORS__FILING_CREATE=["valid_period_exists", "valid_no_filing_exists"]
REQUEST_VALIDATORS__SIGN_AND_SUBMIT=["valid_lei_status","valid_lei_tin","valid_filing_exists_sign","valid_sub_accepted","valid_voluntary_filer","valid_contact_info"]
REQUEST_VALIDATORS__FILING_CREATE=["valid_period_exists", "valid_no_filing_exists"]
REQUEST_VALIDATORS__FILING_REOPEN=["valid_filing_exists_reopen", "valid_filing_not_open"]
4 changes: 3 additions & 1 deletion src/sbl_filing_api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,16 @@ class RequestActionValidations(BaseSettings):
sign_and_submit: Set[str] = {
"valid_lei_status",
"valid_lei_tin",
"valid_filing_exists",
"valid_filing_exists_sign",
"valid_sub_accepted",
"valid_voluntary_filer",
"valid_contact_info",
}

filing_create: Set[str] = {"valid_period_exists", "valid_no_filing_exists"}

filing_reopen: Set[str] = {"valid_filing_exists_reopen", "valid_filing_not_open"}

model_config = SettingsConfigDict(env_prefix="request_validators__", env_file=env_files_to_load, extra="allow")


Expand Down
9 changes: 8 additions & 1 deletion src/sbl_filing_api/entities/models/dao.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
from sbl_filing_api.entities.models.model_enums import FilingType, FilingTaskState, SubmissionState, UserActionType
from sbl_filing_api.entities.models.model_enums import (
FilingType,
FilingTaskState,
SubmissionState,
UserActionType,
FilingState,
)
from datetime import datetime
from typing import Any, List
from sqlalchemy import Enum as SAEnum, String, desc
Expand Down Expand Up @@ -125,6 +131,7 @@ class FilingDAO(Base):
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(nullable=True)
state: Mapped[FilingState | None] = 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
9 changes: 8 additions & 1 deletion src/sbl_filing_api/entities/models/dto.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
from datetime import datetime
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.model_enums import (
FilingType,
FilingTaskState,
SubmissionState,
UserActionType,
FilingState,
)


class UserActionDTO(BaseModel):
Expand Down Expand Up @@ -89,6 +95,7 @@ class FilingDTO(BaseModel):
signatures: List[UserActionDTO] = []
creator: UserActionDTO
is_voluntary: bool | None = None
state: FilingState | None = None


class FilingPeriodDTO(BaseModel):
Expand Down
6 changes: 6 additions & 0 deletions src/sbl_filing_api/entities/models/model_enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class UserActionType(str, Enum):
ACCEPT = "ACCEPT"
SIGN = "SIGN"
CREATE = "CREATE"
REOPEN = "REOPEN"


class SubmissionState(str, Enum):
Expand All @@ -30,3 +31,8 @@ class FilingTaskState(str, Enum):

class FilingType(str, Enum):
ANNUAL = "ANNUAL"


class FilingState(str, Enum):
OPEN = "OPEN"
CLOSED = "CLOSED"
4 changes: 2 additions & 2 deletions src/sbl_filing_api/entities/repos/submission_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
UserActionDAO,
)
from sbl_filing_api.entities.models.dto import FilingPeriodDTO, FilingDTO, ContactInfoDTO, UserActionDTO
from sbl_filing_api.entities.models.model_enums import SubmissionState
from sbl_filing_api.entities.models.model_enums import SubmissionState, FilingState

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -139,7 +139,7 @@ async def upsert_filing(session: AsyncSession, filing: FilingDTO) -> FilingDAO:


async def create_new_filing(session: AsyncSession, lei: str, filing_period: str, creator_id: int) -> FilingDAO:
new_filing = FilingDAO(filing_period=filing_period, lei=lei, creator_id=creator_id)
new_filing = FilingDAO(filing_period=filing_period, lei=lei, creator_id=creator_id, state=FilingState.OPEN)
return await upsert_helper(session, new_filing, FilingDAO)


Expand Down
28 changes: 27 additions & 1 deletion src/sbl_filing_api/routers/filing.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from regtech_api_commons.models.auth import AuthenticatedUser

from sbl_filing_api.entities.models.dao import FilingDAO
from sbl_filing_api.entities.models.model_enums import UserActionType
from sbl_filing_api.entities.models.model_enums import UserActionType, FilingState
from sbl_filing_api.services import submission_processor
from sbl_filing_api.services.multithread_handler import handle_submission
from sbl_filing_api.config import request_action_validations
Expand Down Expand Up @@ -139,6 +139,7 @@ async def sign_filing(request: Request, lei: str, period_code: str):
sig_timestamp = int(sig.timestamp.timestamp())
filing.confirmation_id = lei + "-" + period_code + "-" + str(latest_sub.counter) + "-" + str(sig_timestamp)
filing.signatures.append(sig)
filing.state = FilingState.CLOSED
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we should add a check to ensure the Filing is in an OPEN state on signing. @lchen-2101 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we prevent signing altogether if the state is not set to open?
And add a validator check to look at the state before proceeding?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we prevent signing altogether if the state is not set to open? And add a validator check to look at the state before proceeding?

That's what I am thinking, yeah.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation has been added.

send_confirmation_email(
request.user.name, request.user.email, filing.contact_info.email, filing.confirmation_id, sig_timestamp
)
Expand Down Expand Up @@ -411,3 +412,28 @@ async def update_is_voluntary(request: Request, lei: str, period_code: str, upda
name="Filing Not Found",
detail=f"A Filing for the LEI ({lei}) and period ({period_code}) that was attempted to be updated does not exist.",
)


@router.put(
"/institutions/{lei}/filings/{period_code}/reopen",
response_model=FilingDTO,
dependencies=[
Depends(set_context({UserActionContext.FILING})),
Depends(validate_user_action(request_action_validations.filing_reopen, "Filing Reopen Forbidden")),
],
)
@requires("authenticated")
async def reopen_filing(request: Request, lei: str, period_code: str):
await repo.add_user_action(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user action needs to be added to the Filing object as a relationship (like signature, submitter is added to the Submission, etc). Otherwise there's not relationship to tell who reopened a filing. You just have a user action created in the table but nothing that it refers back to.

request.state.db_session,
UserActionDTO(
user_id=request.user.id,
user_name=request.user.name,
user_email=request.user.email,
action_type=UserActionType.REOPEN,
),
)
filing = await repo.get_filing(request.state.db_session, lei, period_code)
filing.state = FilingState.OPEN
res = await repo.upsert_filing(request.state.db_session, filing)
return res
23 changes: 21 additions & 2 deletions src/sbl_filing_api/services/validators/filing_validators.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging

from sbl_filing_api.entities.models.dao import FilingDAO
from sbl_filing_api.entities.models.model_enums import FilingState
from .base_validator import ActionValidator

log = logging.getLogger(__name__)
Expand All @@ -15,15 +16,24 @@ def __call__(self, filing: FilingDAO, period_code: str, lei: str, **kwargs):
return f"Filing already exists for Filing Period {period_code} and LEI {lei}"


class ValidFilingExists(ActionValidator):
class ValidFilingExistsSign(ActionValidator):
def __init__(self):
super().__init__("valid_filing_exists")
super().__init__("valid_filing_exists_sign")

def __call__(self, filing: FilingDAO, lei: str, period_code: str, **kwargs):
if not filing:
return f"There is no Filing for LEI {lei} in period {period_code}, unable to sign a non-existent Filing."


class ValidFilingExistsReopen(ActionValidator):
def __init__(self):
super().__init__("valid_filing_exists_reopen")

def __call__(self, filing: FilingDAO, lei: str, period_code: str, **kwargs):
if not filing:
return f"There is no Filing for LEI {lei} in period {period_code}, unable to reopen a non-existent Filing."


class ValidVoluntaryFiler(ActionValidator):
def __init__(self):
super().__init__("valid_voluntary_filer")
Expand All @@ -40,3 +50,12 @@ def __init__(self):
def __call__(self, filing: FilingDAO, **kwargs):
if filing and not filing.contact_info:
return f"Cannot sign filing. Filing for {filing.lei} for period {filing.filing_period} does not have contact info defined."


class ValidFilingNotOpen(ActionValidator):
def __init__(self):
super().__init__("valid_filing_not_open")

def __call__(self, *args, filing: FilingDAO, **kwargs):
if filing and filing.state is FilingState.OPEN:
return f"Cannot reopen filing. Filing state for {filing.lei} for period {filing.filing_period} is OPEN."
43 changes: 42 additions & 1 deletion tests/api/routers/test_filing_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
UserActionDAO,
)
from sbl_filing_api.entities.models.dto import ContactInfoDTO, UserActionDTO
from sbl_filing_api.entities.models.model_enums import UserActionType
from sbl_filing_api.entities.models.model_enums import UserActionType, FilingState
from sbl_filing_api.services import submission_processor
from sbl_filing_api.services.multithread_handler import handle_submission

Expand Down Expand Up @@ -1001,6 +1001,7 @@ async def test_good_sign_filing(
self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock, get_filing_mock: Mock
):
get_filing_mock.return_value.is_voluntary = True
get_filing_mock.return_value.state = FilingState.CLOSED
get_filing_mock.return_value.submissions = [
SubmissionDAO(
id=1,
Expand Down Expand Up @@ -1302,3 +1303,43 @@ def test_contact_info_invalid_phone_number(
in res.json()["error_detail"]
)
assert res.status_code == 422

async def test_unauthed_reopen_filing(self, mocker: MockerFixture, app_fixture: FastAPI, unauthed_user_mock):
client = TestClient(app_fixture)
res = client.put("/v1/filing/institutions/123456790/filings/2024/reopen")
assert res.status_code == 403

def test_reopen_filing(
self, mocker: MockerFixture, app_fixture: FastAPI, get_filing_mock: Mock, authed_user_mock: Mock
):
add_user_action_mock = mocker.patch("sbl_filing_api.entities.repos.submission_repo.add_user_action")
add_user_action_mock.return_value = None
get_filing_mock.return_value.state = FilingState.OPEN

upsert_mock = mocker.patch("sbl_filing_api.entities.repos.submission_repo.upsert_filing")
updated_filing_obj = deepcopy(get_filing_mock.return_value)
upsert_mock.return_value = updated_filing_obj

client = TestClient(app_fixture)
res = client.put("/v1/filing/institutions/1234567890ABCDEFGH00/filings/2024/reopen")

assert res.status_code == 403
assert res.json()["error_name"] == "Filing Reopen Forbidden"
assert (
res.json()["error_detail"]
== "['Cannot reopen filing. Filing state for 1234567890ABCDEFGH00 for period 2024 is OPEN.']"
)

get_filing_mock.return_value.state = FilingState.CLOSED
res = client.put("/v1/filing/institutions/1234567890ABCDEFGH00/filings/2024/reopen")
assert res.status_code == 200
assert res.json()["state"] == "OPEN"

get_filing_mock.return_value = None
res = client.put("/v1/filing/institutions/1234567890ABCDEFGH00/filings/2024/reopen")
assert res.status_code == 403
assert res.json()["error_name"] == "Filing Reopen Forbidden"
assert (
res.json()["error_detail"]
== "['There is no Filing for LEI 1234567890ABCDEFGH00 in period 2024, unable to reopen a non-existent Filing.']"
)
12 changes: 11 additions & 1 deletion tests/entities/repos/test_submission_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,21 @@ async def setup(
action_type=UserActionType.SIGN,
timestamp=dt.now(),
)
user_action6 = UserActionDAO(
id=6,
user_id="[email protected]",
user_name="signer name",
user_email="[email protected]",
action_type=UserActionType.REOPEN,
timestamp=dt.now(),
)

transaction_session.add(user_action1)
transaction_session.add(user_action2)
transaction_session.add(user_action3)
transaction_session.add(user_action4)
transaction_session.add(user_action5)
transaction_session.add(user_action6)

filing_task_1 = FilingTaskDAO(name="Task-1", task_order=1)
filing_task_2 = FilingTaskDAO(name="Task-2", task_order=2)
Expand Down Expand Up @@ -262,6 +271,7 @@ async def test_add_filing(self, transaction_session: AsyncSession):
assert res.creator.user_name == "test creator"
assert res.creator.user_email == "[email protected]"
assert res.creator.action_type == UserActionType.CREATE
assert res.state == "OPEN"

async def test_modify_filing(self, transaction_session: AsyncSession):
user_action_create = await repo.add_user_action(
Expand Down Expand Up @@ -606,7 +616,7 @@ async def test_get_user_action(self, query_session: AsyncSession):
async def test_get_user_actions(self, query_session: AsyncSession):
res = await repo.get_user_actions(session=query_session)

assert len(res) == 5
assert len(res) == 6
assert res[0].id == 1
assert res[0].user_name == "signer name"

Expand Down
7 changes: 7 additions & 0 deletions tests/migrations/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,3 +462,10 @@ def test_migrations_to_63138f5cf036(alembic_runner: MigrationContext, alembic_en
inspector = sqlalchemy.inspect(alembic_engine)
columns = inspector.get_columns("filing")
assert next(c for c in columns if c["name"] == "is_voluntary")["nullable"]


def test_migrations_to_7fe49d38726b(alembic_runner: MigrationContext, alembic_engine: Engine):
alembic_runner.migrate_up_to("7fe49d38726b")
inspector = sqlalchemy.inspect(alembic_engine)

assert "state" in set([c["name"] for c in inspector.get_columns("filing")])
Loading
Loading