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: don't link to locked or sanitised sessions in resume emails #2713

Merged
merged 6 commits into from
Jan 31, 2024

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Jan 29, 2024

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:

query MyQuery {
  lowcal_sessions(where: {sanitised_at: {_is_null: true}, deleted_at: {_is_null: true}, submitted_at: {_is_null: true}, email: {_eq: "[email protected]"}, locked_at: {_is_null: true}}, order_by: {created_at: asc}) {
    id
    created_at
    flow {
      slug
      team {
        slug
      }
    }
  }
}

Changes:

  • Adds locked_at condition to resume template which should legitimately be there for invite to pay users, adds sanitised_at as a cautionary fallback
  • Adds an extra check when prepping template content to skip expired sessions, updates tests accordingly
  • Updates the Hasura webhook query to find sessions ready for sanitation that should retroactively handle any outstanding sessions on next/future runs now

@jessicamcinchak
Copy link
Member Author

Copy link

github-actions bot commented Jan 29, 2024

Removed vultr server and associated DNS entries

{ deleted_at: { _lt: $retentionPeriod } }
{ submitted_at: { _lt: $retentionPeriod } }
{ locked_at: { _lt: $retentionPeriod } }
]
Copy link
Member Author

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?

Copy link
Contributor

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

@jessicamcinchak jessicamcinchak requested a review from a team January 31, 2024 10:21
@jessicamcinchak jessicamcinchak marked this pull request as ready for review January 31, 2024 10:23
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a 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 } }
]
Copy link
Contributor

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

@jessicamcinchak jessicamcinchak merged commit 9d933e0 into main Jan 31, 2024
12 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/update-resume-template-checks branch January 31, 2024 11:13
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