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

Trigger slack shift notifications on current shift change #2080

Merged
merged 2 commits into from
Jun 1, 2023

Conversation

matiasb
Copy link
Contributor

@matiasb matiasb commented Jun 1, 2023

Before this change, a diff ical check (which happens with frequency with imported ical), particularly with overrides in an API/terraform schedule would trigger unexpected slack notifications because the prev vs current ical comparison will flag a diff, but when comparing current and previous shifts, current_shifts will have the shift in progress while the prev_shifts calculated from the overrides-only diff will most of the time be empty (unless you set/change an override at current time).

Simplified the checks to always compare previous current shifts (ie. the ones in the schedule from the DB) vs the recalculated ones using the (refreshed) ical data from the schedule.

@matiasb matiasb added the pr:no public docs Added to a PR that does not require public documentation updates label Jun 1, 2023
@matiasb matiasb requested a review from a team June 1, 2023 14:18
Copy link
Member

@vstpme vstpme left a comment

Choose a reason for hiding this comment

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

Nice 👍
Do I understand correctly this should fix repeated messages completely?

@matiasb
Copy link
Contributor Author

matiasb commented Jun 1, 2023

Nice +1 Do I understand correctly this should fix repeated messages completely?

I hope so! 🤞

@matiasb matiasb added this pull request to the merge queue Jun 1, 2023
Merged via the queue into dev with commit 5d383c7 Jun 1, 2023
@matiasb matiasb deleted the matiasb/slack-shift-notifications-simplification branch June 1, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants