96 add filing sign endpoint

[jcadam14]

Closes #96

This originally had #100 merged in since I needed the SUBMISSION_ACCEPTED state and that PR hadn't been accepted yet, so the commits look like a lot, but it's not.

• Adds /institutions/{lei}/filings/{period_name}/sign endpoint
  • Checks that the Filing exists, a latest submission for the filing exists and is in the SUBMISSION_ACCEPTED state, and that the contact_info is set before allowing creation of the signature
• Adds a signature table to store signing info, with relationship to Filing DAO/table
• Adds add_signature function that creates and stores the SignatureDAO
• pytests for endpoint, submission_repo and migration
• Updated pytests LEIs for the filing mock so the user leis match the get_filing_mock.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
src/sbl_filing_api/entities/models
dao.py					119
dto.py
src/sbl_filing_api/entities/repos
submission_repo.py
src/sbl_filing_api/routers
filing.py
Project Total

This report was generated by python-coverage-comment-action

[hkeeler]

Overall I think this is a good first pass. Most of my comments are longer-term questions, and not something that should hang up this PR. I can spin them off as separate issues.

Adding the email address would be nice, and I think pretty easy though. But if you wanna spin that off too, OK by me. I know it's end of day and we're prepping to demo this tomorrow if possible.

[hkeeler]

Probably worth adding email address while we're at it.

[jcadam14]

Updated to add the signer email with some small rewrites to support that

[hkeeler]

Similar to this Contact Info check, do we need something similar for institution data?

[hkeeler]

Looking at where the filing is originally created, it looks like we're still mocking that Institution Snapshot data.

sbl-filing-api/src/sbl_filing_api/entities/repos/submission_repo.py

Lines 130 to 138 in a7a9a08

	async def create_new_filing(session: AsyncSession, lei: str, filing_period: str) -> FilingDAO:
	new_filing = FilingDAO(
	filing_period=filing_period,
	lei=lei,
	institution_snapshot_id="v1", # need story to retrieve this from user-fi I believe
	)
	new_filing = await upsert_helper(session, new_filing, FilingDAO)
	new_filing = await populate_missing_tasks(session, [new_filing])
	return new_filing[0]

So, it seems like there'll always be a snapshot value set, but it's not currently valid?

Also, I see we have a PUT institution-snapshot-id endpoint...

sbl-filing-api/src/sbl_filing_api/routers/filing.py

Lines 161 to 171 in a7a9a08

	@router.put("/institutions/{lei}/filings/{period_code}/institution-snapshot-id", response_model=FilingDTO)
	@requires("authenticated")
	async def put_institution_snapshot(request: Request, lei: str, period_code: str, update_value: SnapshotUpdateDTO):
	result = await repo.get_filing(request.state.db_session, lei, period_code)
	if result:
	result.institution_snapshot_id = update_value.institution_snapshot_id
	return await repo.upsert_filing(request.state.db_session, result)
	return JSONResponse(
	status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
	content=f"A Filing for the LEI ({lei}) and period ({period_code}) that was attempted to be updated does not exist.",
	)

...but I'm not sure where in the current flow we'd call that. Around when we set the SBL institution type? And how would we know to call it if there's newer/different institution data? Lookup the latest one the Sign and submit page, updating it if there is one?

...and do we need to solve any of this for MVP? Is it good enough to just show the latest version of institution data on sign and submit?

Sorry, I know this is now off-topic for this PR. 😆 I'll create a separate issue to work through what (if anything) we want to do for MVP.

[jcadam14]

Similar to this Contact Info check, do we need something similar for institution data?

I thought about this. Right now we set the institution snapshot ID to a default of "v1". But I think we want to have that start out, like contact info, as None and when they check or update their institution data, the FE sets the snapshot ID and that's how we know they've completed that.

[jcadam14]

Created story #143

[hkeeler]

How do we know when a filing needs to be re-signed once changes have been made to an already-signed filing?

We could...

1. Associate signatures to submissions rather than filings. Then on the signing screen if the submission associated to the filing isn't signed, you now you need to sign it? That wouldn't fix needing to re-sign after changing contact info or institution data, though.

2. Disassociate a signature from a filing if anything changes on a signed filing. Not sure how that'd work in the current one-to-many sigs per filing. Add a invalid/expired type flag?

   Alternatively, don't have it as a one-to-many, and just hang the signature off the filing record similar to other fields. If anything changes on the filing, the signature gets dropped. We lose history of signature, but makes it simple. If we want history, we could implement it the same way we do for institution data at a later time.

Anyway, I don't think we need to solve this for this PR, but I think we will before we go live.

[jcadam14]

Created #146

[lchen-2101]

merge conflict, also, is this one still relevant since we talked about standardizing the submitter, accepter, and signer models?

[jcadam14]

merge conflict, also, is this one still relevant since we talked about standardizing the submitter, accepter, and signer models?

Still relevant but we can either wait for that update to be made and then incorporate into here or complete this one and, as part of the db update, update this piece. We've actually demo'd this at a sprint review but the PR has been out here for awhile. I would prefer the latter simply because this one is going to keep getting behind main, it's been reviewed and updates made and new stories written to address concerns for future work, but then it stalled. Y'alls call though.

[lchen-2101]

merge conflict, also, is this one still relevant since we talked about standardizing the submitter, accepter, and signer models?

Still relevant but we can either wait for that update to be made and then incorporate into here or complete this one and, as part of the db update, update this piece. We've actually demo'd this at a sprint review but the PR has been out here for awhile. I would prefer the latter simply because this one is going to keep getting behind main, it's been reviewed and updates made and new stories written to address concerns for future work, but then it stalled. Y'alls call though.

gotcha, let's get the conflict resolved, and have this merged in, then we'll do the refactor that consolidates all the tables, daos, and dtos.

[lchen-2101]

LGTM, this will get refactored to be standardized between submitter, accepter, and signer.