-
Notifications
You must be signed in to change notification settings - Fork 2
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: don't link to locked or sanitised sessions in resume emails #2713
Conversation
Removed vultr server and associated DNS entries |
…ss/update-resume-template-checks
…ss/update-resume-template-checks
{ deleted_at: { _lt: $retentionPeriod } } | ||
{ submitted_at: { _lt: $retentionPeriod } } | ||
{ locked_at: { _lt: $retentionPeriod } } | ||
] |
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.
Previously, I think this is would have failed to find un-submitted sessions older than the retention period? Do these updated conditions seem correct to others as well?
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.
Yep - this makes sense - the previous condition is relying on submitted_at
being set
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 getting to the bottom of this rather opaque one! Additional tests always a welcome site also thank you 👌
{ deleted_at: { _lt: $retentionPeriod } } | ||
{ submitted_at: { _lt: $retentionPeriod } } | ||
{ locked_at: { _lt: $retentionPeriod } } | ||
] |
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.
Yep - this makes sense - the previous condition is relying on submitted_at
being set
There's currently
lowcal_sessions
that are neither submitted, locked, deleted, nor sanitised and created before our expiry timeframe (28 days) on production - which explains the buggy resume email I received (screenshot in attached Trello ticket).Eg try this query:
Changes:
locked_at
condition to resume template which should legitimately be there for invite to pay users, addssanitised_at
as a cautionary fallback