533 filing status prototype #540
Conversation
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @@ -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 | |||
There was a problem hiding this comment.
Wondering if we should add a check to ensure the Filing is in an OPEN state on signing. @lchen-2101 ?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This validation has been added.
src/sbl_filing_api/routers/filing.py
Outdated
| ) | ||
| @requires("authenticated") | ||
| async def reopen_filing(request: Request, lei: str, period_code: str): | ||
| await repo.add_user_action( |
There was a problem hiding this comment.
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.
|
Not entirely related to this PR, but I'm wondering if we should look into a different approach to user actions than what we are currently doing. At the moment it seems that a new relational mapping table is added everytime a new type is introduced. What I thought of is user actions table being basically a changelog; it would only be related to the filing, so the filing would have a list of actions showing how it got to the state it is in at this moment. |
There was a problem hiding this comment.
I think this looks good now. Just need to demo it for the frontend and discuss. But code looks good so once we get buy in we can merge.
I think we'll want a helm values update for this to set the sign and submit validators to NOT have valid_filing_open just yet, so that when/if this is deployed we don't keep people from resigning in beta.
|
@lchen-2101 Maybe not an endpoint per se, but have something that returns all user actions for a given filing? |
Depends on the type really. Since a filing can be opened/reopened multiple times we have to keep a list of reopeners so we have to have that mapping table. If it was just a single maintained action it's a straight relationship. Unfortunately since user actions don't have any indicator of the filing or submission they're associated with, the mapping table is needed. Now we could add some indicator to the user action and make it easier to determine what filing or submission an action was associated with. |
Further discussion needed on my proposal, shouldn't hold up what you are currently doing; but it would be a list of actions on the filing object. |
Yeap, what I was thinking is the actions table would have a fk pointing to the filing id; and a nullable index pointing to the submission id; since those are the only 2 objects actionable by a user. |
Closes #533