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 exception when there are no more tuitions to pay #829

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

LuisDuarte1
Copy link
Member

Closes #817

Fixes bug when there are no more tuitions to pay and causes the worker to be rescheduled and be ran more often.

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@LuisDuarte1
Copy link
Member Author

LuisDuarte1 commented Jul 10, 2023

Also, I don't like the logic of returning Sem data when there are no tuitions available, this makes the app more difficult to translate, should I handle it here?

@LuisDuarte1 LuisDuarte1 requested review from bdmendes and Sirze01 July 10, 2023 16:37
@bdmendes
Copy link
Collaborator

@LuisDuarte1 absolutely: it also makes the value very prone to bugs like these. If possible, refactor the parser to return a DateTime.

@LuisDuarte1
Copy link
Member Author

@LuisDuarte1 absolutely: it also makes the value very prone to bugs like these. If possible, refactor the parser to return a DateTime.

Done

@bdmendes bdmendes added this to the Mid-Summer 2023 Release milestone Jul 12, 2023
Copy link
Collaborator

@Sirze01 Sirze01 left a comment

Choose a reason for hiding this comment

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

Well done. Good catch on converting the due date to a DateTime in both places.

@bdmendes bdmendes force-pushed the fix/no-tuition-notification branch from 7c44320 to 6e0f6ba Compare July 12, 2023 22:27
@bdmendes bdmendes merged commit f8e0861 into develop Jul 12, 2023
@bdmendes bdmendes deleted the fix/no-tuition-notification branch July 12, 2023 22:35
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.

Exception parsing date in TuitionNotification.shouldDisplay
3 participants