-
Notifications
You must be signed in to change notification settings - Fork 81
31949 - Staff Filing Access limited #3997
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
31949 - Staff Filing Access limited #3997
Conversation
loneil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think it will do the trick but couple minor question/comment
| staff_filing_types = [ | ||
| CoreFiling.FilingTypes.AMALGAMATIONOUT.value, | ||
| CoreFiling.FilingTypes.CONTINUATIONOUT.value, | ||
| CoreFiling.FilingTypes.COURTORDER.value, | ||
| CoreFiling.FilingTypes.PUTBACKON.value, | ||
| CoreFiling.FilingTypes.PUTBACKOFF.value, | ||
| CoreFiling.FilingTypes.ADMIN_FREEZE.value, | ||
| CoreFiling.FilingTypes.REGISTRARSNOTATION.value, | ||
| CoreFiling.FilingTypes.REGISTRARSORDER.value | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything more global to the application already (maybe not?) than this local list for "what is a staff filing" that could be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| CoreFiling.FilingTypes.REGISTRARSORDER.value | ||
| ] | ||
|
|
||
| if filing_type in staff_filing_types: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be good to add this additional logic to unit testing in test_permissions.py
| CoreFiling.FilingTypes.ADMIN_FREEZE.value, | ||
| CoreFiling.FilingTypes.REGISTRARSNOTATION.value, | ||
| CoreFiling.FilingTypes.REGISTRARSORDER.value | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing changeOfLiquidators, changeOfReceivers, conversion, correction, dissolution (administrative)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
severinbeauvais
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this PR solve the problem described in the ticket?
| CoreFiling.FilingTypes.CHANGEOFRECEIVERS.value, | ||
| CoreFiling.FilingTypes.CONVERSION.value, | ||
| CoreFiling.FilingTypes.CORRECTION.value | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this being done separately from the other filings (eg, lines 153-196 below)?
The existing permissions mappings/tables should be able to handle staff filings types and staff permissions.
What the code above does is hard-code some of these mappings, which I think is a bad idea.

Issue #: /bcgov/entity#31949
bcgov/entity#31918
bcgov/entity#31919
Description of changes:
Limit The Access to staff filings :
AMALGAMATIONOUT
CONTINUATIONOUT
COURTORDER
PUTBACKON
PUTBACKOFF
ADMIN_FREEZE
REGISTRARSNOTATION
REGISTRARSORDER
CHANGEOFLIQUIDATORS
CHANGEOFRECEIVERS
CONVERSION
CORRECTION
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).