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
Set interstitial last seen to future date when clearing school_info_id in users #18304
Conversation
exit | ||
end | ||
end.parse! | ||
puts "Called with options: #{options}" |
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.
Nice
if options[:actually_update] | ||
user.last_seen_school_info_interstitial = DateTime.now + options[:interstitial_days] | ||
user.school_info_id = nil | ||
user.save!(validate: false) |
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.
Why skip validation? I'm guessing that's for the same reason as this, in which case please add a small comment.
As an aside, we really should try to fix our users table so they all pass validation at some point :(
if ACTUALLY_UPDATE | ||
user.update!(school_info_id: nil) | ||
if options[:actually_update] | ||
user.last_seen_school_info_interstitial = DateTime.now + options[:interstitial_days] |
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.
Why are we setting it to today (or the future)? Is this just to give them at least a week before asking again?
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.
Marina asked that we delay the popup until after the school lookup is added. However, they need to fix the data now in order to run the other script that sets the school_info_id based on past PD enrollments.
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.
Gotcha. We will (obviously) need to make sure we add the school lookup before 7 days from whenever we set these to.
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.
We are going to future date it, 35 days from now on November 15th. With the future date the number of days is negative and the check will return false, which means don't show.
Setting the interstitial dialog to show in the future.
Related to: #18254