-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(repeatingAppointments): NASS-1572: Clean up appointments using sync hook #7187
base: main
Are you sure you want to change the base?
Conversation
… was behaving more like intialForm values
…ttps://github.com/beyondessential/tamanu into feat/nass-1422-modify-repeating-enable-on-frontend
…clean-up-appointments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! This was a tricky corner case to reason about haha
Just one issue I think I noticed tho possibly I might not be following exactly how it's working
isFullyGenerated: true, | ||
untilDate: previousAppointment ? previousAppointment.startTime : appointment.startTime, | ||
cancelledAtDate: previousAppointment ? previousAppointment.startTime : appointment.startTime, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to be setting untilDate
to this value as well? Otherwise the next time that the GenerateRepeatingAppointments
scheduled task runs it might re-generate these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for grabbing this PR. Ohh let me look into this I think you are right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to keep untilDate as record of intended generation end time. But adding the cancelled_at_date clause and deleted_at as well while I was at it in case of db delete. 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yep that makes sense! Nice stuff :)
Android builds 📱
|
🍹
|
Changes
this and all future appointments
Reproduction path
This will mean that repeating appointment will hit limit at second generated appointment - meaning that the task will kick in to generate another two
Generate repeating appointment with occurrence count 4 and weekly frequency (it should create 2 appointments hitting max)
Make sure it has synced once to central and then before the next sync cancel repeating appointment from first appointment into future
After sync ensure there are no appointments that have appeared at week 3 and 4 ( I will check database too )
Deploys
Remember to...