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

fix: corrected get last ar filed date method #3001

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

EPortman
Copy link
Contributor

Issue #: https://app.zenhub.com/workspaces/names-team-board-new-655554cbddd49510027dad2e/issues/gh/bcgov/business-ar/318

Description of changes:
When a Business AR filing is received from the new system, the corporation is looked up using find_by_identifier, which creates a founding_date based on the recognition_date (converted from Pacific Time to UTC).
https://github.com/bcgov/lear/blob/main/colin-api/src/colin_api/models/business.py#L327

When _get_last_ar_filed_date is called, it uses this founding_date by extracting the month and day. The issue was that the founding_date was not being converted back to Pacific Time. As a result, filings for businesses with a recognition_date after 5 PM local time were always showing a last_ar_filed_date one day ahead.

This PR fixes this by converting the founding_date back to pacific time before creating the last_ar_filed_date

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).

@EPortman EPortman marked this pull request as ready for review September 25, 2024 17:42
@EPortman EPortman force-pushed the 271_fix_last_ar_filed_date branch from 491b882 to 416efbc Compare September 25, 2024 18:01
@EPortman EPortman marked this pull request as draft September 25, 2024 18:29
@vysakh-menon-aot
Copy link
Collaborator

@EPortman If its ready you can change this from Draft to Ready for review and assign yourself as Assignees

@EPortman
Copy link
Contributor Author

@vysakh-menon-aot I was getting an error when calling convert_to_pacific_time() since the founding_date_str had -00:00 in it when it should be +00.00.
image

So just had to control for that.

@EPortman EPortman marked this pull request as ready for review September 25, 2024 19:03
@EPortman
Copy link
Contributor Author

I also do not have the option to assign myself as the assignee. I think a permission issue?

@EPortman EPortman marked this pull request as draft September 25, 2024 19:09
@EPortman EPortman force-pushed the 271_fix_last_ar_filed_date branch from 0eacd53 to 4547d95 Compare September 25, 2024 19:18
Copy link

@EPortman
Copy link
Contributor Author

This is now ready. I tested it with a filing (Corp number 0705869) and the result in Colin was like so:

image

The day and month are now correctly in sync with the recogniton_date when the time is after 5pm and the year matches the year of the AR filing.

@EPortman EPortman marked this pull request as ready for review September 25, 2024 19:21
Comment on lines +1302 to +1304
# If the founding date has `-00:00`, replace it with `+00:00` before passing to the function
if '-00:00' in founding_date_str:
founding_date_str = founding_date_str.replace('-00:00', '+00:00')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not really sure about this. Approving it since this function is used only for Business AR sync

@vysakh-menon-aot vysakh-menon-aot merged commit 82aec4b into bcgov:main Sep 25, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants