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 and test user purge for dashboard.email_preference #22681
Fix and test user purge for dashboard.email_preference #22681
Conversation
…ts_users From [spec](https://docs.google.com/spreadsheets/d/1RybuZunt5F6zSXp33oC4KdaBl20VMHVi0yprI8P30y0/edit#gid=670705846) (requires login). Expected behavior: Associations between purged users and districts (as members or as contacts) are destroyed.
From [spec](https://docs.google.com/spreadsheets/d/1RybuZunt5F6zSXp33oC4KdaBl20VMHVi0yprI8P30y0/edit#gid=670705846) (requires login). Expected behavior: All EmailPreference rows associated with a purged user's email address are also deleted.
# Removes EmailPreference records associated with this email address. | ||
# @param [String] email An email address | ||
def remove_email_preferences(email) | ||
EmailPreference.where(email: email).each(&:destroy) |
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.
EmailPreference has a unique constraint on email
so maybe use find_by(email: email)
?
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.
I assume the email being passed as an argument by the caller came from the User model and has already been stripped and lower-cased?
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.
Hmm, good question. I'll have to investigate. I know we strip and downcase before we hash emails, but we might store the original formatting for display back to the user.
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.
Looks like User model strips and downcases before save:
code-dot-org/dashboard/app/models/user.rb
Lines 544 to 562 in 12f8a84
before_save :make_teachers_21, | |
:normalize_email, | |
:hash_email, | |
:sanitize_race_data_set_urm, | |
:fix_by_user_type | |
before_save :log_admin_save, if: -> {admin_changed? && User.should_log?} | |
before_validation :update_share_setting, unless: :under_13? | |
def make_teachers_21 | |
return unless teacher? | |
self.age = 21 | |
end | |
def normalize_email | |
return unless email.present? | |
self.email = email.strip.downcase | |
end |
Fix and test user purge for dashboard.email_preference
From spec (requires login).
Expected behavior: All EmailPreference rows associated with a purged user's email address are also deleted.