23543 COLIN BE Services - Sync involuntary dissolution stages back to COLIN #3027
Conversation
(cherry picked from commit 831100eafe40a2e528ae1321409d353d6007c79c)
leodube-aot
requested review from
argush3,
kzdev420,
chenhongjing,
vysakh-menon-aot,
AimeeGao and
eason-pan-bc
| @@ -154,6 +154,7 @@ def get(legal_type, identifier, name_type): | |||
| @cors_preflight('GET') | |||
| @API.route('/internal/<string:info_type>', methods=['GET']) | |||
| @API.route('/internal/<string:legal_type>/<string:identifier>/<string:info_type>', methods=['GET']) | |||
| @API.route('/internal/<string:legal_type>/<string:identifier>', methods=['PATCH']) | |||
There was a problem hiding this comment.
I wasn't entirely sure where the best place was to add this endpoint. I figured since we are updating the corp state and not actually creating a filing, then using an internal PATCH endpoint on the business made the most sense.
|
| colin_ids = send_filing(app, token, filing, filing_id) | ||
| update = None | ||
| if colin_ids and any(colin_ids): | ||
| update = update_colin_id(app, token, filing_id, colin_ids) |
There was a problem hiding this comment.
I found that colin_ids can return as [None], which actually evaluates to True. We need to make sure to not attempt updating colin ids if they are None, as it will still insert into the colin_event_ids table using a default sequence value. Using if colin_ids and any(colin_ids) does the trick
There was a problem hiding this comment.
Its not expected to receive [None]. When did you receive it? any specific filing?
There was a problem hiding this comment.
For a successful sync we must receive an event id and if failed send_filing will return None
There was a problem hiding this comment.
Yup so I tracked down the source of the issue.
It's caused when an involuntary dissolution filing is passed to colin api without any meta_data, or with incorrectly formed meta_data. You can see that in Filing.add_involuntary_dissolution_event can return None if this is the case. This gets returned as event_id = None and saved into the colinIds for the filing (with HTTP status CREATED, even though nothing was created).
It's probably pretty low risk, as I only encountered it because I made a mistake manually creating local test data.
Thinking more about this, it should probably be fixed in colin api rather than the updater job like I did here. This PR may not get merged (see Argus' comment), but this bug might still be worth fixing.
| BatchProcessing.BatchProcessingStep.WARNING_LEVEL_1, | ||
| BatchProcessing.BatchProcessingStep.WARNING_LEVEL_2, | ||
| ])).\ | ||
| filter(BatchProcessing.status == BatchProcessing.BatchProcessingStatus.PROCESSING).\ |
There was a problem hiding this comment.
Thinking if it's proper to only select batch processing in PROCESSING status...?
There was a problem hiding this comment.
This ticket is on hold for now, but to still answer your question: yes I think it should only select PROCESSING status. My analysis on the statuses is that:
ERROR,HOLD, andQUEUEDstatuses we don't want to to sync since there is some issue or wait associated with the batch processing.COMPLETEDstatus will always have a dissolution filing. This dissolution filing is already synced correctly (creates and EVENT and sets the CORP_STATE).WITHDRAWNstatus should always have an annual report filing. This AR filing sync needs to be verified in [VERIFY] Update Colin Filings Job - Check that WITHDRAWN dissolutions sync correctly entity#23545
If we were to sync COMPLETED and WITHDRAWN status batch processings, we would get duplicates in COLIN since we would be syncing both the batch processing and their associated filings.
|
As per our conversation, this work is on hold until we take another look at what really needs to be synced back to COLIN if anything. |
|
Closing as no longer a requirement |
Issue #: /bcgov/entity#23543
Description of changes:
BatchProcessingmodel:colin_event_identry for synced batch processings.Local Testing
In depth local testing can be found in the test notes of the ticket.
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).