Fixed the send to get data for notification_count_for_the_year #2029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🧪 Review environmenthttps://kro5se4mi6h2mqh2jnzicmzdg40jopyo.lambda-url.ca-central-1.on.aws/ |
jzbahrai
force-pushed
the
task/fix-data-send
branch
from
1ccafbb to
5e3977e
Compare
smcmurtry
approved these changes
Jan 3, 2025
There was a problem hiding this comment.
I was able to reproduce the bug locally by doing the following on the main branches of admin and api:
- create a new template
- upload a csv via the admin interface and do a bulk send
- visit the template page that was used for the send
- observe the 500 error page:
Sorry, we’re experiencing technical difficulties Try again later.and on the api:
data[month][row.notification_type][row.notification_status] += row.count
KeyError: '2025-01'
Then I check out Jumana's branch and refresh the page, and I no longer see an error.
This change has the side effect of making the annual limits apply to the financial year and not the calendar year, which is what we want.
Looks good!
andrewleith
approved these changes
Jan 6, 2025
| @@ -83,8 +82,10 @@ def get_limit_stats(self, service: Service): | |||
| } | |||
| """ | |||
|
|
|||
| current_financial_year = get_current_financial_year() | |||
There was a problem hiding this comment.
Yep my bad, I was passing the current year here where it should have been fiscal. Thanks and sorry for the alarms 😞
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary | Résumé
When we get the monthly notification count, we need the count for this current fiscal year.
The code was previously sending "2025" as the year to get the monthly data information, instead
in the time period april 2024 - march 2025, we need to call the monthly_stats function with "2024".
Testing:
I tested this locally, and I am sending the correct data.
Release plan
Release this to production, and then manually test the change to see if templates can be fetched.