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

Use optional chaining to avoid undefined monthly_monitor_report #5511

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

rhelmer
Copy link
Collaborator

@rhelmer rhelmer commented Jan 17, 2025

References:

Jira: MNTOR-3848

Description

Use optional chaining to avoid monthly_monitor_report being undefined.

I believe that the problem here is that monthly_monitor_report can be null in the database if the monthly report has not run successfully for this account. I also believe we started getting more of these reports because the cron job that sends this email broke temporarily (it's now fixed), but there's still a possibility for this to happen for e.g. new users.

I did not add a unit test, but I did change the typescript types to reflect that this may be undefined.

How to test

The easiest way to to test locally is to ensure that monthly_monitor_report is null in the subscribers table.

Checklist (Definition of Done)

  • Commits in this PR are minimal and have descriptive commit messages.
  • Jira ticket has been updated (if needed) to match changes made during the development process.
  • Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

@rhelmer rhelmer requested review from Vinnl, codemist and flozia January 17, 2025 20:46
Copy link

@Vinnl Vinnl mentioned this pull request Jan 20, 2025
10 tasks
@Vinnl
Copy link
Collaborator

Vinnl commented Jan 20, 2025

The question is: if the value can also be undefined, then why didn't TS warn us when we passed that into these components? So I traced that down to a couple of overeager type assertions, which I removed in #5521.

I'm also not 100% sure what the values of the checkboxes initialised with this value should be if it's undefined, but I would guess that they should actually be checked by default, rather than unchecked as they are now. My PR does that too.

(This PR is the target of that PR.)

@rhelmer rhelmer removed request for codemist and flozia January 22, 2025 18:05
@rhelmer rhelmer merged commit 846bc04 into main Jan 22, 2025
16 checks passed
@rhelmer rhelmer deleted the MNTOR-3848/optional-chaining-for-monthly_monitor_report branch January 22, 2025 18:10
Copy link

Cleanup completed - database 'blurts-server-pr-5511' destroyed, cloud run service 'blurts-server-pr-5511' destroyed

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