Skip to content

Commit

Permalink
AO3-6042 Fix that emails ignored the locale preference feature flag (#…
Browse files Browse the repository at this point in the history
…4707)

* AO3-6042 Add test

* AO3-6042 Fix that emails ignored the locale preference feature flag

* AO3-6042 Move test to user mailer

* AO3-6042 Override preferred_locale instead

* AO3-6042 Turn preferred_locale into Rails relation
  • Loading branch information
Bilka2 authored Feb 17, 2024
1 parent 8873b8f commit e7b2708
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 25 deletions.
5 changes: 2 additions & 3 deletions app/controllers/archive_faqs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,8 @@ def default_url_options
def set_locale
session[:language_id] = params[:language_id] if Locale.exists?(iso: params[:language_id])

if current_user.present? && $rollout.active?(:set_locale_preference,
current_user)
@i18n_locale = session[:language_id].presence || Locale.find(current_user.preference.preferred_locale).iso
if current_user.present?
@i18n_locale = session[:language_id].presence || current_user.preference.locale.iso
else
@i18n_locale = session[:language_id].presence || I18n.default_locale
end
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/archive_devise_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def reset_password_instructions(record, token, options = {})
devise_mail(record, :reset_password_instructions,
options.merge(subject: subject))
else
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
subject = t("users.mailer.reset_password_instructions.subject",
app_name: ArchiveConfig.APP_SHORT_NAME)
devise_mail(record, :reset_password_instructions,
Expand Down
4 changes: 2 additions & 2 deletions app/mailers/comment_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class CommentMailer < ApplicationMailer
def comment_notification(user, comment)
@comment = comment
@owner = user
I18n.with_locale(Locale.find(user.preference.preferred_locale).iso) do
I18n.with_locale(user.preference.locale.iso) do
mail(
to: user.email,
subject: "[#{ArchiveConfig.APP_SHORT_NAME}] Comment on " + (@comment.ultimate_parent.is_a?(Tag) ? "the tag " : "") + @comment.ultimate_parent.commentable_name.gsub("&gt;", ">").gsub("&lt;", "<")
Expand All @@ -14,7 +14,7 @@ def comment_notification(user, comment)
# Sends email to an owner of the top-level commentable when a comment is edited
def edited_comment_notification(user, comment)
@comment = comment
I18n.with_locale(Locale.find(user.preference.preferred_locale).iso) do
I18n.with_locale(user.preference.locale.iso) do
mail(
to: user.email,
subject: "[#{ArchiveConfig.APP_SHORT_NAME}] Edited comment on " + (@comment.ultimate_parent.is_a?(Tag) ? "the tag " : "") + @comment.ultimate_parent.commentable_name.gsub("&gt;", ">").gsub("&lt;", "<")
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/kudo_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def batch_kudo_notification(user_id, user_kudos)
user = User.find(user_id)
kudos_hash = JSON.parse(user_kudos)

I18n.with_locale(Locale.find(user.preference.preferred_locale).iso) do
I18n.with_locale(user.preference.locale.iso) do
kudos_hash.each_pair do |commentable_info, kudo_givers_hash|
# Parse the key to extract the type and id of the commentable so we can
# weed out any commentables that no longer exist.
Expand Down
28 changes: 14 additions & 14 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def anonymous_or_unrevealed_notification(user_id, work_id, collection_id, option
:unrevealed
end

I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @user.email,
subject: t(".subject.#{@status}",
Expand Down Expand Up @@ -95,7 +95,7 @@ def invitation_to_claim(invitation_id, archivist_login)
def claim_notification(creator_id, claimed_work_ids, is_user=false)
if is_user
creator = User.find(creator_id)
locale = Locale.find(creator.preference.preferred_locale).iso
locale = creator.preference.locale.iso
else
creator = ExternalAuthor.find(creator_id)
locale = I18n.default_locale
Expand Down Expand Up @@ -145,7 +145,7 @@ def batch_subscription_notification(subscription_id, entries)
if @creations.count > 1
subject += " and #{@creations.count - 1} more"
end
I18n.with_locale(Locale.find(@subscription.user.preference.preferred_locale).iso) do
I18n.with_locale(@subscription.user.preference.locale.iso) do
mail(
to: @subscription.user.email,
subject: "[#{ArchiveConfig.APP_SHORT_NAME}] #{subject}"
Expand All @@ -157,7 +157,7 @@ def batch_subscription_notification(subscription_id, entries)
def invite_increase_notification(user_id, total)
@user = User.find(user_id)
@total = total
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @user.email,
subject: "#{t 'user_mailer.invite_increase_notification.subject', app_name: ArchiveConfig.APP_SHORT_NAME}"
Expand All @@ -170,7 +170,7 @@ def invite_request_declined(user_id, total, reason)
@user = User.find(user_id)
@total = total
@reason = reason
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @user.email,
subject: t('user_mailer.invite_request_declined.subject', app_name: ArchiveConfig.APP_SHORT_NAME)
Expand Down Expand Up @@ -213,7 +213,7 @@ def challenge_assignment_notification(collection_id, assigned_user_id, assignmen
@assigned_user = User.find(assigned_user_id)
assignment = ChallengeAssignment.find(assignment_id)
@request = (assignment.request_signup || assignment.pinch_request_signup)
I18n.with_locale(Locale.find(@assigned_user.preference.preferred_locale).iso) do
I18n.with_locale(@assigned_user.preference.locale.iso) do
mail(
to: @assigned_user.email,
# i18n-tasks-use t('user_mailer.challenge_assignment_notification.subject')
Expand All @@ -225,7 +225,7 @@ def challenge_assignment_notification(collection_id, assigned_user_id, assignmen
# Asks a user to validate and activate their new account
def signup_notification(user_id)
@user = User.find(user_id)
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @user.email,
subject: t('user_mailer.signup_notification.subject', app_name: ArchiveConfig.APP_SHORT_NAME)
Expand All @@ -238,7 +238,7 @@ def change_email(user_id, old_email, new_email)
@user = User.find(user_id)
@old_email = old_email
@new_email = new_email
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @old_email,
subject: t('user_mailer.change_email.subject', app_name: ArchiveConfig.APP_SHORT_NAME)
Expand Down Expand Up @@ -293,7 +293,7 @@ def related_work_notification(user_id, related_work_id)
@related_work = RelatedWork.find(related_work_id)
@related_parent_link = url_for(controller: :works, action: :show, id: @related_work.parent)
@related_child_link = url_for(controller: :works, action: :show, id: @related_work.work)
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @user.email,
subject: "[#{ArchiveConfig.APP_SHORT_NAME}] Related work notification"
Expand All @@ -306,7 +306,7 @@ def recipient_notification(user_id, work_id, collection_id = nil)
@user = User.find(user_id)
@work = Work.find(work_id)
@collection = Collection.find(collection_id) if collection_id
I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
subject = if @collection
t("user_mailer.recipient_notification.subject.collection",
app_name: ArchiveConfig.APP_SHORT_NAME,
Expand All @@ -328,7 +328,7 @@ def prompter_notification(work_id, collection_id=nil)
@collection = Collection.find(collection_id) if collection_id
@work.challenge_claims.each do |claim|
user = User.find(claim.request_signup.pseud.user.id)
I18n.with_locale(Locale.find(user.preference.preferred_locale).iso) do
I18n.with_locale(user.preference.locale.iso) do
mail(
to: user.email,
subject: "[#{ArchiveConfig.APP_SHORT_NAME}] A response to your prompt"
Expand All @@ -349,7 +349,7 @@ def delete_work_notification(user, work)
attachments["#{download.file_name}.html"] = { content: html, encoding: "base64" }
attachments["#{download.file_name}.txt"] = { content: html, encoding: "base64" }

I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: user.email,
subject: t('user_mailer.delete_work_notification.subject', app_name: ArchiveConfig.APP_SHORT_NAME)
Expand All @@ -369,7 +369,7 @@ def admin_deleted_work_notification(user, work)
attachments["#{download.file_name}.html"] = { content: html, encoding: "base64" }
attachments["#{download.file_name}.txt"] = { content: html, encoding: "base64" }

I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: user.email,
subject: t('user_mailer.admin_deleted_work_notification.subject', app_name: ArchiveConfig.APP_SHORT_NAME)
Expand All @@ -382,7 +382,7 @@ def admin_hidden_work_notification(creation_id, user_id)
@user = User.find_by(id: user_id)
@work = Work.find_by(id: creation_id)

I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do
I18n.with_locale(@user.preference.locale.iso) do
mail(
to: @user.email,
# i18n-tasks-use t('user_mailer.admin_hidden_work_notification.subject')
Expand Down
2 changes: 1 addition & 1 deletion app/models/creatorship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def notify_creator
pseud.user != User.current_user &&
pseud.user != User.orphan_account

I18n.with_locale(Locale.find(pseud.user.preference.preferred_locale).iso) do
I18n.with_locale(pseud.user.preference.locale.iso) do
if approved?
if User.current_user.try(:is_archivist?)
UserMailer.creatorship_notification_archivist(id, User.current_user.id).deliver_later
Expand Down
5 changes: 5 additions & 0 deletions app/models/preference.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class Preference < ApplicationRecord
belongs_to :user
belongs_to :skin
belongs_to :locale, foreign_key: "preferred_locale"

validates :work_title_format,
format: {
Expand Down Expand Up @@ -29,4 +30,8 @@ def can_use_skin

errors.add(:base, "You don't have permission to use that skin!")
end

def locale
$rollout.active?(:set_locale_preference, user) ? super : Locale.default
end
end
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ def activate
def create_default_associateds
self.pseuds << Pseud.new(name: self.login, is_default: true)
self.profile = Profile.new
self.preference = Preference.new(preferred_locale: Locale.default.id)
self.preference = Preference.new(locale: Locale.default)
end

def prevent_password_resets?
Expand Down
2 changes: 1 addition & 1 deletion app/views/preferences/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
</dd>
<dt><%= f.label :time_zone, ts('Your time zone') %></dt>
<dd><%= f.time_zone_select :time_zone, nil, :default => Time.zone.name %></dd>
<% if Rails.env.test? || Rails.env.development? || $rollout.active?(:set_locale_preference, @user) %>
<% if $rollout.active?(:set_locale_preference, @user) %>
<dt><%= f.label :preferred_locale, ts('Your locale') %> <%= link_to_help 'locale-preferences' %></dt>
<dd><%= f.select :preferred_locale, locale_options_for_select(@available_locales, "id"),
default: @preference.preferred_locale %></dd>
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/archive_faqs_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
let(:user_locale) { create(:locale) }
let(:user) do
user = create(:user)
user.preference.update!(preferred_locale: user_locale.id)
user.preference.update!(locale: user_locale)
user
end

Expand Down
50 changes: 50 additions & 0 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1200,4 +1200,54 @@
it_behaves_like "an email with a deleted work with draft chapters attached"
end
end

describe "obeys the set locale preference feature flag" do
let(:user) { create(:user) }
let(:work) { create(:work, authors: [user.default_pseud]) }
let(:locale) { create(:locale) }

context "when the set locale preference feature flag is on" do
before { $rollout.activate_user(:set_locale_preference, user) }

context "and the user has non-default locale set" do
before { user.preference.update!(locale: locale) }

it "sends a localised email" do
expect(I18n).to receive(:with_locale).with(locale.iso)
expect(UserMailer.admin_hidden_work_notification(work.id, user.id)).to be_truthy
end
end

context "and the user has the default locale set" do
before { user.preference.update!(locale: Locale.default) }

it "sends an English email" do
expect(I18n).to receive(:with_locale).with("en")
expect(UserMailer.admin_hidden_work_notification(work.id, user.id)).to be_truthy
end
end
end

context "when the set locale preference feature flag is off" do
before { $rollout.deactivate_user(:set_locale_preference, user) }

context "and the user has non-default locale set" do
before { user.preference.update!(locale: locale) }

it "sends an English email" do
expect(I18n).to receive(:with_locale).with("en")
expect(UserMailer.admin_hidden_work_notification(work.id, user.id)).to be_truthy
end
end

context "and the user has the default locale set" do
before { user.preference.update!(locale: Locale.default) }

it "sends an English email" do
expect(I18n).to receive(:with_locale).with("en")
expect(UserMailer.admin_hidden_work_notification(work.id, user.id)).to be_truthy
end
end
end
end
end

0 comments on commit e7b2708

Please sign in to comment.