5
11
Fork
You've already forked tomato
1

Prevent events from ending before they start #40

Merged
edwardloveall merged 2 commits from el-no-inverted-events into main 2025年10月28日 15:01:09 +01:00

Fixes #17

There was no validation in place to prevent events from having an ends_at datetime before starts_at, which is invalid data. Event starts at 9am, ends at 8am the same day??? This new validation prevents that.

And now that we have that validation we can fix our existing data.

The difference between end and start for the effected events are all over the place. They range from 30 minutes to 22 hours. We're going to:

  • get a backup of the DB right before we deploy
  • deploy which will fix all bad events to 2h events (ends_at 2 hours after starts_at)
  • run some code on the backup to generate a list of changed events
  • email the orgs with links to all the events so they can do a manual audit if that's important to them.
Code to run on backup before deploying
  • get db backup
  • restore db backup locally
Organization.find_each do |org|
 puts "## #{org.name}"
 puts
 org.events.where("ends_at < starts_at").find_each do |event|
 shift = event.shift_event.shift
 zone = org.time_zone
 puts "* [#{event.title}](https://rootable.org/shift_event_occurrences/#{shift.id}-#{event.starts_at.in_time_zone(zone).to_i})"
 end
 puts
end
Fixes #17 There was no validation in place to prevent events from having an `ends_at` datetime _before_ `starts_at`, which is invalid data. Event starts at 9am, ends at 8am the same day??? This new validation prevents that. And now that we have that validation we can fix our existing data. The difference between end and start for the effected events are all over the place. They range from 30 minutes to 22 hours. We're going to: * get a backup of the DB right before we deploy * deploy which will fix all bad events to 2h events (ends_at 2 hours after starts_at) * run some code on the backup to generate a list of changed events * email the orgs with links to all the events so they can do a manual audit if that's important to them. <details> <summary>Code to run on backup before deploying</summary> * get db backup * restore db backup locally ```ruby Organization.find_each do |org| puts "## #{org.name}" puts org.events.where("ends_at < starts_at").find_each do |event| shift = event.shift_event.shift zone = org.time_zone puts "* [#{event.title}](https://rootable.org/shift_event_occurrences/#{shift.id}-#{event.starts_at.in_time_zone(zone).to_i})" end puts end ``` </details>
@ -6,6 +6,7 @@ class Event < ApplicationRecord
validate :validate_recurrence_options, if: proc { |event| event.recurrence_options.present? }, on: :save
validates :title, :starts_at, :ends_at, presence: true
validates :ends_at, comparison: {greater_than_or_equal_to: :starts_at}
First-time contributor
Copy link

Are there any implications to having an event where the starts_at and ends_at are the same? Does an even need a non-zero duration?

Are there any implications to having an event where the `starts_at` and `ends_at` are the same? Does an even need a non-zero duration?
Author
Owner
Copy link

That's a good question. I'm not sure why I picked or_equal_to but thinking about it now, we have no use case for that, and our algo for finding events in a range may struggle with a zero duration event. I'll just change it to greater_than.

That's a good question. I'm not sure why I picked `or_equal_to` but thinking about it now, we have no use case for that, and our algo for finding events in a range _may_ struggle with a zero duration event. I'll just change it to `greater_than`.
edwardloveall force-pushed el-no-inverted-events from 48f977c15b
Some checks failed
Setup Successful
Jest Successful
Static Analysis Successful
RSpec Failed
to 23eec54945
Some checks failed
Setup Successful
Jest Successful
Static Analysis Successful
RSpec Failed
2025年10月28日 14:50:46 +01:00
Compare
Author
Owner
Copy link

RSpec is flaky and passing locally

RSpec is flaky and passing locally
edwardloveall deleted branch el-no-inverted-events 2025年10月28日 15:01:13 +01:00
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rootable/tomato!40
Reference in a new issue
rootable/tomato
No description provided.
Delete branch "el-no-inverted-events"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?