From 25e6ed19bf7f4edc22c8cad5f4b034e06a610057 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Mon, 16 Sep 2024 00:07:41 -0700
Subject: [PATCH 01/20] let each challenge owner receive their own email rather
 than all challenge owners get cc-ed on the same email

---
 app/mailers/user_mailer.rb         | 14 ++++++--------
 app/models/challenge_assignment.rb |  5 ++++-
 app/models/collection.rb           | 13 +++++++------
 app/models/potential_match.rb      |  5 ++++-
 spec/mailers/user_mailer_spec.rb   |  6 +++---
 5 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index a7e4e3b23e3..1050cb38a8b 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -178,32 +178,30 @@ def invite_request_declined(user_id, total, reason)
     end
   end
 
-  # TODO: This may be sent to multiple users simultaneously. We need to ensure
-  # each user gets the email for their preferred locale.
-  def collection_notification(collection_id, subject, message)
+  def collection_notification(collection_id, subject, message, email)
     @message = message
     @collection = Collection.find(collection_id)
     mail(
-      to: @collection.get_maintainers_email,
+      to: email,
       subject: "[#{ArchiveConfig.APP_SHORT_NAME}][#{@collection.title}] #{subject}"
     )
   end
 
-  def invalid_signup_notification(collection_id, invalid_signup_ids)
+  def invalid_signup_notification(collection_id, invalid_signup_ids, email)
     @collection = Collection.find(collection_id)
     @invalid_signups = invalid_signup_ids
     mail(
-      to: @collection.get_maintainers_email,
+      to: email,
       subject: "[#{ArchiveConfig.APP_SHORT_NAME}][#{@collection.title}] Invalid sign-ups found"
     )
   end
 
   # This is sent at the end of matching, i.e., after assignments are generated.
   # It is also sent when assignments are regenerated.
-  def potential_match_generation_notification(collection_id)
+  def potential_match_generation_notification(collection_id, email)
     @collection = Collection.find(collection_id)
     mail(
-      to: @collection.get_maintainers_email,
+      to: email,
       subject: "[#{ArchiveConfig.APP_SHORT_NAME}][#{@collection.title}] Potential assignment generation complete"
     )
   end
diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index 93986f1eb15..2f4699da4ef 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -374,7 +374,10 @@ def self.delayed_generate(collection_id)
       end
     end
     REDIS_GENERAL.del(progress_key(collection))
-    UserMailer.potential_match_generation_notification(collection.id).deliver_later
+    @maintainers = collection.get_maintainers_list
+    @maintainers.each do |i|
+      UserMailer.potential_match_generation_notification(collection.id, i.email).deliver_later
+    end 
   end
 
   # go through the request's potential matches in order from best to worst and try and assign
diff --git a/app/models/collection.rb b/app/models/collection.rb
index d1d71b2466f..2999996036d 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -334,15 +334,16 @@ def prompt_meme?
     return self.challenge_type == "PromptMeme"
   end
 
-  def get_maintainers_email
-    return self.email if !self.email.blank?
-    return parent.email if parent && !parent.email.blank?
-    "#{self.maintainers.collect(&:user).flatten.uniq.collect(&:email).join(',')}"
+  def get_maintainers_list
+    return self.maintainers.collect(&:user).flatten.uniq
   end
 
   def notify_maintainers(subject, message)
-    # send maintainers a notice via email
-    UserMailer.collection_notification(self.id, subject, message).deliver_later
+    @maintainers = self.get_maintainers_list
+    # loop through maintainers and send each a notice via email
+    @maintainers.each do |i|
+      UserMailer.collection_notification(self.id, subject, message, i.email).deliver_later
+    end
   end
 
   include AsyncWithResque
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index 251e2da1503..b35bdca5eda 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -92,7 +92,10 @@ def self.generate_in_background(collection_id)
     invalid_signup_ids = collection.signups.select {|s| !s.valid?}.collect(&:id)
     unless invalid_signup_ids.empty?
       invalid_signup_ids.each {|sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid}
-      UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids).deliver_later
+      @maintainers = collection.get_maintainers_list
+      @maintainers.each do |i|
+        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, i.email).deliver_later
+      end
       PotentialMatch.cancel_generation(collection)
     else
 
diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb
index 09aca017cee..d2afdba11b0 100644
--- a/spec/mailers/user_mailer_spec.rb
+++ b/spec/mailers/user_mailer_spec.rb
@@ -933,7 +933,7 @@
   end
 
   describe "potential_match_generation_notification" do
-    subject(:email) { UserMailer.potential_match_generation_notification(collection.id) }
+    subject(:email) { UserMailer.potential_match_generation_notification(collection.id, "test@example.com") }
 
     let(:collection) { create(:collection) }
 
@@ -966,7 +966,7 @@
   end
 
   describe "invalid_signup_notification" do
-    subject(:email) { UserMailer.invalid_signup_notification(collection.id, [signup.id]) }
+    subject(:email) { UserMailer.invalid_signup_notification(collection.id, [signup.id], "test@example.com") }
 
     let(:collection) { create(:collection) }
     let(:signup) { create(:challenge_signup) }
@@ -998,7 +998,7 @@
   end
 
   describe "collection_notification" do
-    subject(:email) { UserMailer.collection_notification(collection.id, subject_text, message_text) }
+    subject(:email) { UserMailer.collection_notification(collection.id, subject_text, message_text, "test@example.com") }
 
     let(:collection) { create(:collection) }
     let(:subject_text) { Faker::Hipster.sentence }

From fdfca331f56b52c27c13b57631eda28b9f10131c Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Mon, 16 Sep 2024 00:21:58 -0700
Subject: [PATCH 02/20] address rubocop

---
 app/models/potential_match.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index b35bdca5eda..94b795523c2 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -90,7 +90,7 @@ def self.generate_in_background(collection_id)
     # check for invalid signups
     PotentialMatch.clear_invalid_signups(collection)
     invalid_signup_ids = collection.signups.select {|s| !s.valid?}.collect(&:id)
-    unless invalid_signup_ids.empty?
+    if invalid_signup_ids.present?
       invalid_signup_ids.each {|sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid}
       @maintainers = collection.get_maintainers_list
       @maintainers.each do |i|

From c8d8fe2421cd2e3fd457d5944320dd21d547ed5c Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Mon, 16 Sep 2024 00:33:56 -0700
Subject: [PATCH 03/20] remove redundant return

---
 app/models/collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 2999996036d..91991b5beab 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -335,7 +335,7 @@ def prompt_meme?
   end
 
   def get_maintainers_list
-    return self.maintainers.collect(&:user).flatten.uniq
+    @maintainers_list = self.maintainers.collect(&:user).flatten.uniq
   end
 
   def notify_maintainers(subject, message)

From 7fb0d0bf25f0606f623e87e63c567aa75d831dc9 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Mon, 16 Sep 2024 00:43:22 -0700
Subject: [PATCH 04/20] another reviewdog fix

---
 app/models/challenge_assignment.rb | 2 +-
 app/models/collection.rb           | 4 ++--
 app/models/potential_match.rb      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index 2f4699da4ef..5de705e2f93 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -374,7 +374,7 @@ def self.delayed_generate(collection_id)
       end
     end
     REDIS_GENERAL.del(progress_key(collection))
-    @maintainers = collection.get_maintainers_list
+    @maintainers = collection.maintainers_list
     @maintainers.each do |i|
       UserMailer.potential_match_generation_notification(collection.id, i.email).deliver_later
     end 
diff --git a/app/models/collection.rb b/app/models/collection.rb
index 91991b5beab..f35e4049df2 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -334,12 +334,12 @@ def prompt_meme?
     return self.challenge_type == "PromptMeme"
   end
 
-  def get_maintainers_list
+  def maintainers_list
     @maintainers_list = self.maintainers.collect(&:user).flatten.uniq
   end
 
   def notify_maintainers(subject, message)
-    @maintainers = self.get_maintainers_list
+    @maintainers = self.maintainers_list
     # loop through maintainers and send each a notice via email
     @maintainers.each do |i|
       UserMailer.collection_notification(self.id, subject, message, i.email).deliver_later
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index 94b795523c2..f24312ed1d9 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -92,7 +92,7 @@ def self.generate_in_background(collection_id)
     invalid_signup_ids = collection.signups.select {|s| !s.valid?}.collect(&:id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each {|sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid}
-      @maintainers = collection.get_maintainers_list
+      @maintainers = collection.maintainers_list
       @maintainers.each do |i|
         UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, i.email).deliver_later
       end

From 7b51ffdf7678e82bf3a49e72a5fb0f11f8758088 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Tue, 17 Sep 2024 23:11:53 -0700
Subject: [PATCH 05/20] rubocop autocorrections

---
 app/mailers/user_mailer.rb         |  45 +++---
 app/models/challenge_assignment.rb | 116 +++++++++-------
 app/models/collection.rb           | 215 +++++++++++++++--------------
 app/models/potential_match.rb      |  41 +++---
 spec/mailers/user_mailer_spec.rb   |  11 +-
 5 files changed, 220 insertions(+), 208 deletions(-)

diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 1050cb38a8b..6d19299fd70 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -30,9 +30,9 @@ def invited_to_collection_notification(user_id, work_id, collection_id)
     @work = Work.find(work_id)
     @collection = Collection.find(collection_id)
     mail(
-         to: @user.email,
-         subject: "[#{ArchiveConfig.APP_SHORT_NAME}]#{'[' + @collection.title + ']'} Request to include work in a collection"
-    )
+      to: @user.email,
+      subject: "[#{ArchiveConfig.APP_SHORT_NAME}]#{'[' + @collection.title + ']'} Request to include work in a collection"
+       )
   end
 
   # We use an options hash here, instead of keyword arguments, to avoid
@@ -72,7 +72,7 @@ def anonymous_or_unrevealed_notification(user_id, work_id, collection_id, option
   # TODO refactor to make it asynchronous
   def invitation(invitation_id)
     @invitation = Invitation.find(invitation_id)
-    @user_name = (@invitation.creator.is_a?(User) ? @invitation.creator.login : '')
+    @user_name = (@invitation.creator.is_a?(User) ? @invitation.creator.login : "")
     mail(
       to: @invitation.invitee_email,
       subject: t("user_mailer.invitation.subject", app_name: ArchiveConfig.APP_SHORT_NAME)
@@ -92,7 +92,7 @@ def invitation_to_claim(invitation_id, archivist_login)
   end
 
   # Notifies a writer that their imported works have been claimed
-  def claim_notification(creator_id, claimed_work_ids, is_user=false)
+  def claim_notification(creator_id, claimed_work_ids, is_user = false)
     if is_user
       creator = User.find(creator_id)
       locale = creator.preference.locale.iso
@@ -116,6 +116,7 @@ def batch_subscription_notification(subscription_id, entries)
     # then the resque job does not error and we just silently fail.
     @subscription = Subscription.find_by(id: subscription_id)
     return if @subscription.nil?
+
     creation_entries = JSON.parse(entries)
     @creations = []
     # look up all the creations that have generated updates for this subscription
@@ -123,13 +124,14 @@ def batch_subscription_notification(subscription_id, entries)
       creation_type, creation_id = creation_info.split("_")
       creation = creation_type.constantize.where(id: creation_id).first
       next unless creation && creation.try(:posted)
-      next if (creation.is_a?(Chapter) && !creation.work.try(:posted))
-      next if creation.pseuds.any? {|p| p.user == User.orphan_account} # no notifications for orphan works
+      next if creation.is_a?(Chapter) && !creation.work.try(:posted)
+      next if creation.pseuds.any? { |p| p.user == User.orphan_account } # no notifications for orphan works
+
       # TODO: allow subscriptions to orphan_account to receive notifications
 
       # If the subscription notification is for a user subscription, we don't
       # want to send updates about works that have recently become anonymous.
-      if @subscription.subscribable_type == 'User'
+      if @subscription.subscribable_type == "User"
         next if Subscription.anonymous_creation?(creation)
       end
 
@@ -142,9 +144,7 @@ def batch_subscription_notification(subscription_id, entries)
     @creations.uniq!
 
     subject = @subscription.subject_text(@creations.first)
-    if @creations.count > 1
-      subject += " and #{@creations.count - 1} more"
-    end
+    subject += " and #{@creations.count - 1} more" if @creations.count > 1
     I18n.with_locale(@subscription.user.preference.locale.iso) do
       mail(
         to: @subscription.user.email,
@@ -160,7 +160,7 @@ def invite_increase_notification(user_id, total)
     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}"
+        subject: t("user_mailer.invite_increase_notification.subject", app_name: ArchiveConfig.APP_SHORT_NAME.to_s)
       )
     end
   end
@@ -173,7 +173,7 @@ def invite_request_declined(user_id, total, reason)
     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)
+        subject: t("user_mailer.invite_request_declined.subject", app_name: ArchiveConfig.APP_SHORT_NAME)
       )
     end
   end
@@ -225,7 +225,7 @@ def signup_notification(user_id)
     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)
+        subject: t("user_mailer.signup_notification.subject", app_name: ArchiveConfig.APP_SHORT_NAME)
       )
     end
   end
@@ -238,7 +238,7 @@ def change_email(user_id, old_email, new_email)
     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)
+        subject: t("user_mailer.change_email.subject", app_name: ArchiveConfig.APP_SHORT_NAME)
       )
     end
   end
@@ -320,7 +320,7 @@ def recipient_notification(user_id, work_id, collection_id = nil)
   end
 
   # Emails a prompter to say that a response has been posted to their prompt
-  def prompter_notification(work_id, collection_id=nil)
+  def prompter_notification(work_id, collection_id = nil)
     @work = Work.find(work_id)
     @collection = Collection.find(collection_id) if collection_id
     @work.challenge_claims.each do |claim|
@@ -349,7 +349,7 @@ def delete_work_notification(user, work)
     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)
+        subject: t("user_mailer.delete_work_notification.subject", app_name: ArchiveConfig.APP_SHORT_NAME)
       )
     end
   end
@@ -369,7 +369,7 @@ def admin_deleted_work_notification(user, work)
     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)
+        subject: t("user_mailer.admin_deleted_work_notification.subject", app_name: ArchiveConfig.APP_SHORT_NAME)
       )
     end
   end
@@ -392,9 +392,9 @@ def admin_spam_work_notification(creation_id, user_id)
     @work = Work.find_by(id: creation_id)
 
     mail(
-        to: @user.email,
-        subject: "[#{ArchiveConfig.APP_SHORT_NAME}] Your work was hidden as spam"
-    )
+      to: @user.email,
+      subject: "[#{ArchiveConfig.APP_SHORT_NAME}] Your work was hidden as spam"
+      )
   end
 
   ### OTHER NOTIFICATIONS ###
@@ -403,6 +403,7 @@ def admin_spam_work_notification(creation_id, user_id)
   def feedback(feedback_id)
     feedback = Feedback.find(feedback_id)
     return unless feedback.email
+
     @summary = feedback.summary
     @comment = feedback.comment
     @username = feedback.username if feedback.username.present?
@@ -426,6 +427,4 @@ def abuse_report(abuse_report_id)
     )
   end
 
-  protected
-
 end
diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index 5de705e2f93..7dc4c05052f 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -14,29 +14,29 @@ class ChallengeAssignment < ApplicationRecord
   validate :signups_match, on: :update
   def signups_match
     if self.sent_at.nil? &&
-      self.request_signup.present? &&
-      self.offer_signup.present? &&
-      !self.request_signup.request_potential_matches.pluck(:offer_signup_id).include?(self.offer_signup_id)
+       self.request_signup.present? &&
+       self.offer_signup.present? &&
+       !self.request_signup.request_potential_matches.pluck(:offer_signup_id).include?(self.offer_signup_id)
       errors.add(:base, ts("does not match. Did you mean to write-in a giver?"))
     end
   end
 
-  scope :for_request_signup, lambda {|signup| where('request_signup_id = ?', signup.id)}
+  scope :for_request_signup, ->(signup) { where("request_signup_id = ?", signup.id) }
 
-  scope :for_offer_signup, lambda {|signup| where('offer_signup_id = ?', signup.id)}
+  scope :for_offer_signup, ->(signup) { where("offer_signup_id = ?", signup.id) }
 
-  scope :in_collection, lambda {|collection| where('challenge_assignments.collection_id = ?', collection.id) }
+  scope :in_collection, ->(collection) { where("challenge_assignments.collection_id = ?", collection.id) }
 
-  scope :defaulted, -> { where("defaulted_at IS NOT NULL") }
+  scope :defaulted, -> { where.not(defaulted_at: nil) }
   scope :undefaulted, -> { where("defaulted_at IS NULL") }
   scope :uncovered, -> { where("covered_at IS NULL") }
-  scope :covered, -> { where("covered_at IS NOT NULL") }
-  scope :sent, -> { where("sent_at IS NOT NULL") }
+  scope :covered, -> { where.not(covered_at: nil) }
+  scope :sent, -> { where.not(sent_at: nil) }
 
-  scope :with_pinch_hitter, -> { where("pinch_hitter_id IS NOT NULL") }
+  scope :with_pinch_hitter, -> { where.not(pinch_hitter_id: nil) }
 
   scope :with_offer, -> { where("offer_signup_id IS NOT NULL OR pinch_hitter_id IS NOT NULL") }
-  scope :with_request, -> { where("request_signup_id IS NOT NULL") }
+  scope :with_request, -> { where.not(request_signup_id: nil) }
   scope :with_no_request, -> { where("request_signup_id IS NULL") }
   scope :with_no_offer, -> { where("offer_signup_id IS NULL AND pinch_hitter_id IS NULL") }
 
@@ -53,13 +53,12 @@ def signups_match
 
   scope :order_by_offering_pseud, -> { joins(OFFERING_PSEUD_JOIN).order("pseuds.name") }
 
-
   # Get all of a user's assignments
-  scope :by_offering_user, lambda {|user|
-    select("DISTINCT challenge_assignments.*").
-    joins(OFFERING_PSEUD_JOIN).
-    joins("INNER JOIN users ON pseuds.user_id = users.id").
-    where('users.id = ?', user.id)
+  scope :by_offering_user, lambda { |user|
+    select("DISTINCT challenge_assignments.*")
+      .joins(OFFERING_PSEUD_JOIN)
+      .joins("INNER JOIN users ON pseuds.user_id = users.id")
+      .where("users.id = ?", user.id)
   }
 
   # sorting by fulfilled/posted status
@@ -67,26 +66,25 @@ def signups_match
                                                            collection_items.item_id = challenge_assignments.creation_id AND
                                                            collection_items.item_type = challenge_assignments.creation_type)"
 
-  COLLECTION_ITEMS_LEFT_JOIN =  "LEFT JOIN collection_items ON (collection_items.collection_id = challenge_assignments.collection_id AND
+  COLLECTION_ITEMS_LEFT_JOIN = "LEFT JOIN collection_items ON (collection_items.collection_id = challenge_assignments.collection_id AND
                                                                 collection_items.item_id = challenge_assignments.creation_id AND
                                                                 collection_items.item_type = challenge_assignments.creation_type)"
 
   WORKS_JOIN = "INNER JOIN works ON works.id = challenge_assignments.creation_id AND challenge_assignments.creation_type = 'Work'"
   WORKS_LEFT_JOIN = "LEFT JOIN works ON works.id = challenge_assignments.creation_id AND challenge_assignments.creation_type = 'Work'"
 
-  scope :fulfilled, -> {
+  scope :fulfilled, lambda {
     joins(COLLECTION_ITEMS_JOIN).joins(WORKS_JOIN)
       .where("challenge_assignments.creation_id IS NOT NULL AND collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ? AND works.posted = 1",
              CollectionItem.user_approval_statuses[:approved], CollectionItem.collection_approval_statuses[:approved])
   }
 
-
   scope :posted, -> { joins(WORKS_JOIN).where("challenge_assignments.creation_id IS NOT NULL AND works.posted = 1") }
 
   # should be faster than unfulfilled scope because no giant left joins
   def self.unfulfilled_in_collection(collection)
     fulfilled_ids = ChallengeAssignment.in_collection(collection).fulfilled.pluck(:id)
-    fulfilled_ids.empty? ? in_collection(collection) : in_collection(collection).where("challenge_assignments.id NOT IN (?)", fulfilled_ids)
+    fulfilled_ids.empty? ? in_collection(collection) : in_collection(collection).where.not(challenge_assignments: { id: fulfilled_ids })
   end
 
   # faster than unposted scope because no left join!
@@ -106,7 +104,7 @@ def self.duplicate_recipients(collection)
   end
 
   # has to be a left join to get assignments that don't have a collection item
-  scope :unfulfilled, -> {
+  scope :unfulfilled, lambda {
     joins(COLLECTION_ITEMS_LEFT_JOIN).joins(WORKS_LEFT_JOIN)
       .where("challenge_assignments.creation_id IS NULL OR collection_items.user_approval_status != ? OR collection_items.collection_approval_status != ? OR works.posted = 0",
              CollectionItem.user_approval_statuses[:approved], CollectionItem.collection_approval_statuses[:approved])
@@ -117,7 +115,6 @@ def self.duplicate_recipients(collection)
 
   scope :unstarted, -> { where("challenge_assignments.creation_id IS NULL") }
 
-
   before_destroy :clear_assignment
   def clear_assignment
     if offer_signup
@@ -137,6 +134,7 @@ def request
 
   def get_collection_item
     return nil unless self.creation
+
     CollectionItem.where("collection_id = ? AND item_id = ? AND item_type = ?", self.collection_id, self.creation_id, self.creation_type).first
   end
 
@@ -153,17 +151,13 @@ def posted?
   end
 
   def defaulted=(value)
-    if value == "1"
-      self.defaulted_at = Time.now
-    else
-      self.defaulted_at = nil
-    end
+    self.defaulted_at = (Time.now if value == "1")
   end
 
   def defaulted
     !self.defaulted_at.nil?
   end
-  alias_method :defaulted?, :defaulted
+  alias defaulted? defaulted
 
   def offer_signup_pseud=(pseud_byline)
     if pseud_byline.blank?
@@ -209,16 +203,27 @@ def offering_pseud
   end
 
   def requesting_pseud
-    request_signup ? request_signup.pseud : (pinch_request_signup ? pinch_request_signup.pseud : nil)
+    if request_signup
+      request_signup.pseud
+    else
+      (pinch_request_signup ? pinch_request_signup.pseud : nil)
+    end
   end
 
-
   def offer_byline
-    offer_signup && offer_signup.pseud ? offer_signup.pseud.byline : (pinch_hitter ? (pinch_hitter.byline + "* (pinch hitter)") : "- none -")
+    if offer_signup && offer_signup.pseud
+      offer_signup.pseud.byline
+    else
+      (pinch_hitter ? (pinch_hitter.byline + "* (pinch hitter)") : "- none -")
+    end
   end
 
   def request_byline
-    request_signup && request_signup.pseud ? request_signup.pseud.byline : (pinch_request_signup ? (pinch_request_byline + "* (pinch recipient)") : "- None -")
+    if request_signup && request_signup.pseud
+      request_signup.pseud.byline
+    else
+      (pinch_request_signup ? (pinch_request_byline + "* (pinch recipient)") : "- None -")
+    end
   end
 
   def pinch_hitter_byline
@@ -260,7 +265,11 @@ def send_out
     unless self.sent_at
       self.sent_at = Time.now
       save
-      assigned_to = self.offer_signup ? self.offer_signup.pseud.user : (self.pinch_hitter ? self.pinch_hitter.user : nil)
+      assigned_to = if self.offer_signup
+                      self.offer_signup.pseud.user
+                    else
+                      (self.pinch_hitter ? self.pinch_hitter.user : nil)
+                    end
       request = self.request_signup || self.pinch_request_signup
       UserMailer.challenge_assignment_notification(collection.id, assigned_to.id, self.id).deliver_later if assigned_to && request
     end
@@ -332,13 +341,14 @@ def self.delayed_generate(collection_id)
     @offer_match_buckets = {}
     @max_match_count = 0
     if settings.nil? || settings.no_match_required?
-       # stuff everyone into the same bucket
-       @max_match_count = 1
-       @request_match_buckets[1] = collection.signups
-       @offer_match_buckets[1] = collection.signups
+      # stuff everyone into the same bucket
+      @max_match_count = 1
+      @request_match_buckets[1] = collection.signups
+      @offer_match_buckets[1] = collection.signups
     else
       collection.signups.find_each do |signup|
         next if signup.nil?
+
         request_match_count = signup.request_potential_matches.count
         @request_match_buckets[request_match_count] ||= []
         @request_match_buckets[request_match_count] << signup
@@ -357,20 +367,24 @@ def self.delayed_generate(collection_id)
     # matches.)
     0.upto(@max_match_count) do |count|
       if @request_match_buckets[count]
-        @request_match_buckets[count].sort_by {rand}.each do |request_signup|
+        @request_match_buckets[count].sort_by { rand }
+          .each do |request_signup|
           # go through the potential matches in order from best to worst and try and assign
           request_signup.reload
           next if request_signup.assigned_as_request
+
           ChallengeAssignment.assign_request!(collection, request_signup)
         end
       end
 
-      if @offer_match_buckets[count]
-        @offer_match_buckets[count].sort_by {rand}.each do |offer_signup|
-          offer_signup.reload
-          next if offer_signup.assigned_as_offer
-          ChallengeAssignment.assign_offer!(collection, offer_signup)
-        end
+      next unless @offer_match_buckets[count]
+
+      @offer_match_buckets[count].sort_by { rand }
+        .each do |offer_signup|
+        offer_signup.reload
+        next if offer_signup.assigned_as_offer
+
+        ChallengeAssignment.assign_offer!(collection, offer_signup)
       end
     end
     REDIS_GENERAL.del(progress_key(collection))
@@ -391,7 +405,7 @@ def self.assign_request!(collection, request_signup)
 
       # if there's a circular match let's save it as our last choice
       if potential_match.offer_signup.assigned_as_request && !last_choice &&
-          collection.assignments.for_request_signup(potential_match.offer_signup).first.offer_signup == request_signup
+         collection.assignments.for_request_signup(potential_match.offer_signup).first.offer_signup == request_signup
         last_choice = potential_match
         next
       end
@@ -401,9 +415,7 @@ def self.assign_request!(collection, request_signup)
       break
     end
 
-    if !assigned && last_choice
-      ChallengeAssignment.do_assign_request!(assignment, last_choice)
-    end
+    ChallengeAssignment.do_assign_request!(assignment, last_choice) if !assigned && last_choice
 
     request_signup.assigned_as_request = true
     request_signup.save!
@@ -423,7 +435,7 @@ def self.assign_offer!(collection, offer_signup)
 
       # if there's a circular match let's save it as our last choice
       if potential_match.request_signup.assigned_as_offer && !last_choice &&
-          collection.assignments.for_offer_signup(potential_match.request_signup).first.request_signup == offer_signup
+         collection.assignments.for_offer_signup(potential_match.request_signup).first.request_signup == offer_signup
         last_choice = potential_match
         next
       end
@@ -433,9 +445,7 @@ def self.assign_offer!(collection, offer_signup)
       break
     end
 
-    if !assigned && last_choice
-      ChallengeAssignment.do_assign_offer!(assignment, last_choice)
-    end
+    ChallengeAssignment.do_assign_offer!(assignment, last_choice) if !assigned && last_choice
 
     offer_signup.assigned_as_offer = true
     offer_signup.save!
diff --git a/app/models/collection.rb b/app/models/collection.rb
index f35e4049df2..f8df6c930e9 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -3,14 +3,14 @@ class Collection < ApplicationRecord
   include WorksOwner
 
   has_attached_file :icon,
-  styles: { standard: "100x100>" },
-  url: "/system/:class/:attachment/:id/:style/:basename.:extension",
-  path: %w(staging production).include?(Rails.env) ? ":class/:attachment/:id/:style.:extension" : ":rails_root/public:url",
-  storage: %w(staging production).include?(Rails.env) ? :s3 : :filesystem,
-  s3_protocol: "https",
-  default_url: "/images/skins/iconsets/default/icon_collection.png"
-
-  validates_attachment_content_type :icon, content_type: /image\/\S+/, allow_nil: true
+                    styles: { standard: "100x100>" },
+                    url: "/system/:class/:attachment/:id/:style/:basename.:extension",
+                    path: %w[staging production].include?(Rails.env) ? ":class/:attachment/:id/:style.:extension" : ":rails_root/public:url",
+                    storage: %w[staging production].include?(Rails.env) ? :s3 : :filesystem,
+                    s3_protocol: "https",
+                    default_url: "/images/skins/iconsets/default/icon_collection.png"
+
+  validates_attachment_content_type :icon, content_type: %r{image/\S+}, allow_nil: true
   validates_attachment_size :icon, less_than: 500.kilobytes, allow_nil: true
 
   belongs_to :parent, class_name: "Collection", inverse_of: :children
@@ -22,13 +22,14 @@ class Collection < ApplicationRecord
   has_one :collection_preference, dependent: :destroy
   accepts_nested_attributes_for :collection_preference
 
+  before_validation :clear_icon
+  before_validation :cleanup_url
   before_create :ensure_associated
   def ensure_associated
-    self.collection_preference = CollectionPreference.new unless  self.collection_preference
-    self.collection_profile = CollectionProfile.new unless  self.collection_profile
+    self.collection_preference = CollectionPreference.new unless self.collection_preference
+    self.collection_profile = CollectionProfile.new unless self.collection_profile
   end
 
-
   belongs_to :challenge, dependent: :destroy, polymorphic: true
   has_many :prompts, dependent: :destroy
 
@@ -52,10 +53,10 @@ def clean_up_challenge
   accepts_nested_attributes_for :collection_items, allow_destroy: true
   has_many :approved_collection_items, -> { approved_by_both }, class_name: "CollectionItem"
 
-  has_many :works, through: :collection_items, source: :item, source_type: 'Work'
+  has_many :works, through: :collection_items, source: :item, source_type: "Work"
   has_many :approved_works, -> { posted }, through: :approved_collection_items, source: :item, source_type: "Work"
 
-  has_many :bookmarks, through: :collection_items, source: :item, source_type: 'Bookmark'
+  has_many :bookmarks, through: :collection_items, source: :item, source_type: "Bookmark"
   has_many :approved_bookmarks, through: :approved_collection_items, source: :item, source_type: "Bookmark"
 
   has_many :collection_participants, dependent: :destroy
@@ -63,41 +64,34 @@ def clean_up_challenge
 
   has_many :participants, through: :collection_participants, source: :pseud
   has_many :users, through: :participants, source: :user
-  has_many :invited, -> { where('collection_participants.participant_role = ?', CollectionParticipant::INVITED) }, through: :collection_participants, source: :pseud
-  has_many :owners, -> { where('collection_participants.participant_role = ?', CollectionParticipant::OWNER) }, through: :collection_participants, source: :pseud
-  has_many :moderators, -> { where('collection_participants.participant_role = ?', CollectionParticipant::MODERATOR) }, through: :collection_participants, source: :pseud
-  has_many :members, -> { where('collection_participants.participant_role = ?', CollectionParticipant::MEMBER) }, through: :collection_participants, source: :pseud
-  has_many :posting_participants, -> { where('collection_participants.participant_role in (?)', [CollectionParticipant::MEMBER,CollectionParticipant::MODERATOR, CollectionParticipant::OWNER ] ) }, through: :collection_participants, source: :pseud
-
-
+  has_many :invited, -> { where("collection_participants.participant_role = ?", CollectionParticipant::INVITED) }, through: :collection_participants, source: :pseud
+  has_many :owners, -> { where("collection_participants.participant_role = ?", CollectionParticipant::OWNER) }, through: :collection_participants, source: :pseud
+  has_many :moderators, -> { where("collection_participants.participant_role = ?", CollectionParticipant::MODERATOR) }, through: :collection_participants, source: :pseud
+  has_many :members, -> { where("collection_participants.participant_role = ?", CollectionParticipant::MEMBER) }, through: :collection_participants, source: :pseud
+  has_many :posting_participants, -> { where("collection_participants.participant_role in (?)", [CollectionParticipant::MEMBER, CollectionParticipant::MODERATOR, CollectionParticipant::OWNER]) }, through: :collection_participants, source: :pseud
 
   CHALLENGE_TYPE_OPTIONS = [
-                             ["", ""],
-                             [ts("Gift Exchange"), "GiftExchange"],
-                             [ts("Prompt Meme"), "PromptMeme"],
-                           ]
-
-  before_validation :clear_icon
+    ["", ""],
+    [ts("Gift Exchange"), "GiftExchange"],
+    [ts("Prompt Meme"), "PromptMeme"]
+  ]
 
   validate :must_have_owners
   def must_have_owners
     # we have to use collection participants because the association may not exist until after
     # the collection is saved
-    errors.add(:base, ts("Collection has no valid owners.")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select {|p| p.is_owner?}.empty?
+    errors.add(:base, ts("Collection has no valid owners.")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select { |p| p.is_owner? }
+      .empty?
   end
 
   validate :collection_depth
   def collection_depth
-    if (self.parent && self.parent.parent) || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
-      errors.add(:base, ts("Sorry, but %{name} is a subcollection, so it can't also be a parent collection.", name: parent.name))
-    end
+    errors.add(:base, ts("Sorry, but %{name} is a subcollection, so it can't also be a parent collection.", name: parent.name)) if (self.parent && self.parent.parent) || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
   end
 
   validate :parent_exists
   def parent_exists
-    unless parent_name.blank? || Collection.find_by(name: parent_name)
-      errors.add(:base, ts("We couldn't find a collection with name %{name}.", name: parent_name))
-    end
+    errors.add(:base, ts("We couldn't find a collection with name %{name}.", name: parent_name)) unless parent_name.blank? || Collection.find_by(name: parent_name)
   end
 
   validate :parent_is_allowed
@@ -111,49 +105,49 @@ def parent_is_allowed
     end
   end
 
-  validates_presence_of :name, message: ts("Please enter a name for your collection.")
+  validates :name, presence: { message: ts("Please enter a name for your collection.") }
   validates :name, uniqueness: { message: ts("Sorry, that name is already taken. Try again, please!") }
-  validates_length_of :name,
-    minimum: ArchiveConfig.TITLE_MIN,
-    too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN)
-  validates_length_of :name,
-    maximum: ArchiveConfig.TITLE_MAX,
-    too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX)
-  validates_format_of :name,
-    message: ts('must begin and end with a letter or number; it may also contain underscores. It may not contain any other characters, including spaces.'),
-    with: /\A[A-Za-z0-9]\w*[A-Za-z0-9]\Z/
-  validates_length_of :icon_alt_text, allow_blank: true, maximum: ArchiveConfig.ICON_ALT_MAX,
-    too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_ALT_MAX)
-  validates_length_of :icon_comment_text, allow_blank: true, maximum: ArchiveConfig.ICON_COMMENT_MAX,
-    too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_COMMENT_MAX)
+  validates :name,
+            length: { minimum: ArchiveConfig.TITLE_MIN,
+                      too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN) }
+  validates :name,
+            length: { maximum: ArchiveConfig.TITLE_MAX,
+                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX) }
+  validates :name,
+            format: { message: ts("must begin and end with a letter or number; it may also contain underscores. It may not contain any other characters, including spaces."),
+                      with: /\A[A-Za-z0-9]\w*[A-Za-z0-9]\Z/ }
+  validates :icon_alt_text, length: { allow_blank: true, maximum: ArchiveConfig.ICON_ALT_MAX,
+                                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_ALT_MAX) }
+  validates :icon_comment_text, length: { allow_blank: true, maximum: ArchiveConfig.ICON_COMMENT_MAX,
+                                          too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_COMMENT_MAX) }
 
   validates :email, email_format: { allow_blank: true }
 
-  validates_presence_of :title, message: ts("Please enter a title to be displayed for your collection.")
-  validates_length_of :title,
-    minimum: ArchiveConfig.TITLE_MIN,
-    too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN)
-  validates_length_of :title,
-    maximum: ArchiveConfig.TITLE_MAX,
-    too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX)
+  validates :title, presence: { message: ts("Please enter a title to be displayed for your collection.") }
+  validates :title,
+            length: { minimum: ArchiveConfig.TITLE_MIN,
+                      too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN) }
+  validates :title,
+            length: { maximum: ArchiveConfig.TITLE_MAX,
+                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX) }
   validate :no_reserved_strings
   def no_reserved_strings
     errors.add(:title, ts("^Sorry, the ',' character cannot be in a collection Display Title.")) if
-      title.match(/\,/)
+      title.match(/,/)
   end
 
   # return title.html_safe to overcome escaping done by sanitiser
   def title
-    read_attribute(:title).try(:html_safe)
+    self[:title].try(:html_safe)
   end
 
-  validates_length_of :description,
-    allow_blank: true,
-    maximum: ArchiveConfig.SUMMARY_MAX,
-    too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX)
+  validates :description,
+            length: { allow_blank: true,
+                      maximum: ArchiveConfig.SUMMARY_MAX,
+                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
-  validates_format_of :header_image_url, allow_blank: true, with: URI::regexp(%w(http https)), message: ts("is not a valid URL.")
-  validates_format_of :header_image_url, allow_blank: true, with: /\.(png|gif|jpg)$/, message: ts("can only point to a gif, jpg, or png file."), multiline: true
+  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
+  validates :header_image_url, format: { allow_blank: true, with: /\.(png|gif|jpg)$/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
@@ -167,12 +161,11 @@ def title
   scope :unrevealed, -> { joins(:collection_preference).where("collection_preferences.unrevealed = ?", true) }
   scope :anonymous, -> { joins(:collection_preference).where("collection_preferences.anonymous = ?", true) }
   scope :no_challenge, -> { where(challenge_type: nil) }
-  scope :gift_exchange, -> { where(challenge_type: 'GiftExchange') }
-  scope :prompt_meme, -> { where(challenge_type: 'PromptMeme') }
+  scope :gift_exchange, -> { where(challenge_type: "GiftExchange") }
+  scope :prompt_meme, -> { where(challenge_type: "PromptMeme") }
   scope :name_only, -> { select("collections.name") }
   scope :by_title, -> { order(:title) }
 
-  before_validation :cleanup_url
   def cleanup_url
     self.header_image_url = Addressable::URI.heuristic_parse(self.header_image_url) if self.header_image_url
   end
@@ -180,42 +173,42 @@ def cleanup_url
   # Get only collections with running challenges
   def self.signup_open(challenge_type)
     if challenge_type == "PromptMeme"
-      not_closed.where(challenge_type: challenge_type).
-        joins("INNER JOIN prompt_memes on prompt_memes.id = challenge_id").where("prompt_memes.signup_open = 1").
-        where("prompt_memes.signups_close_at > ?", Time.now).order("prompt_memes.signups_close_at DESC")
+      not_closed.where(challenge_type: challenge_type)
+        .joins("INNER JOIN prompt_memes on prompt_memes.id = challenge_id").where("prompt_memes.signup_open = 1")
+        .where("prompt_memes.signups_close_at > ?", Time.now).order("prompt_memes.signups_close_at DESC")
     elsif challenge_type == "GiftExchange"
-      not_closed.where(challenge_type: challenge_type).
-        joins("INNER JOIN gift_exchanges on gift_exchanges.id = challenge_id").where("gift_exchanges.signup_open = 1").
-        where("gift_exchanges.signups_close_at > ?", Time.now).order("gift_exchanges.signups_close_at DESC")
+      not_closed.where(challenge_type: challenge_type)
+        .joins("INNER JOIN gift_exchanges on gift_exchanges.id = challenge_id").where("gift_exchanges.signup_open = 1")
+        .where("gift_exchanges.signups_close_at > ?", Time.now).order("gift_exchanges.signups_close_at DESC")
     end
   end
 
-  scope :with_name_like, lambda {|name|
-    where("collections.name LIKE ?", '%' + name + '%').
-    limit(10)
+  scope :with_name_like, lambda { |name|
+    where("collections.name LIKE ?", "%" + name + "%")
+      .limit(10)
   }
 
-  scope :with_title_like, lambda {|title|
-    where("collections.title LIKE ?", '%' + title + '%')
+  scope :with_title_like, lambda { |title|
+    where("collections.title LIKE ?", "%" + title + "%")
   }
 
-  scope :with_item_count, -> {
-    select("collections.*, count(distinct collection_items.id) as item_count").
-    joins("left join collections child_collections on child_collections.parent_id = collections.id
+  scope :with_item_count, lambda {
+    select("collections.*, count(distinct collection_items.id) as item_count")
+      .joins("left join collections child_collections on child_collections.parent_id = collections.id
            left join collection_items on ( (collection_items.collection_id = child_collections.id OR collection_items.collection_id = collections.id)
                                      AND collection_items.user_approval_status = 1
-                                     AND collection_items.collection_approval_status = 1)").
-    group("collections.id")
+                                     AND collection_items.collection_approval_status = 1)")
+      .group("collections.id")
   }
 
   def to_param
-   name_was
+    name_was
   end
 
   # Change membership of collection(s) from a particular pseud to the orphan account
-  def self.orphan(pseuds, collections, default=true)
-    for pseud in pseuds
-      for collection in collections
+  def self.orphan(pseuds, collections, default = true)
+    pseuds.each do |pseud|
+      collections.each do |collection|
         if pseud && collection && collection.owners.include?(pseud)
           orphan_pseud = default ? User.orphan_account.default_pseud : User.orphan_account.pseuds.find_or_create_by(name: pseud.name)
           pseud.change_membership(collection, orphan_pseud)
@@ -237,8 +230,8 @@ def autocomplete_search_string_before_last_save
   end
 
   def autocomplete_prefixes
-    [ "autocomplete_collection_all",
-      "autocomplete_collection_#{closed? ? 'closed' : 'open'}" ]
+    ["autocomplete_collection_all",
+     "autocomplete_collection_#{closed? ? 'closed' : 'open'}"]
   end
 
   def autocomplete_score
@@ -246,7 +239,6 @@ def autocomplete_score
   end
   ## END AUTOCOMPLETE
 
-
   def parent_name=(name)
     @parent_name = name
     self.parent = Collection.find_by(name: name)
@@ -310,6 +302,7 @@ def get_participating_pseuds_for_user(user)
 
   def get_participants_for_user(user)
     return [] unless user
+
     CollectionParticipant.in_collection(self).for_user(user)
   end
 
@@ -321,17 +314,22 @@ def gift_notification
     self.collection_profile.gift_notification || (parent ? parent.collection_profile.gift_notification : "")
   end
 
-  def moderated? ; self.collection_preference.moderated ; end
-  def closed? ; self.collection_preference.closed ; end
-  def unrevealed? ; self.collection_preference.unrevealed ; end
-  def anonymous? ; self.collection_preference.anonymous ; end
-  def challenge? ; !self.challenge.nil? ; end
+  def moderated?() = self.collection_preference.moderated
+
+  def closed?() = self.collection_preference.closed
+
+  def unrevealed?() = self.collection_preference.unrevealed
+
+  def anonymous?() = self.collection_preference.anonymous
+
+  def challenge?() = !self.challenge.nil?
 
   def gift_exchange?
-    return self.challenge_type == "GiftExchange"
+    self.challenge_type == "GiftExchange"
   end
+
   def prompt_meme?
-    return self.challenge_type == "PromptMeme"
+    self.challenge_type == "PromptMeme"
   end
 
   def maintainers_list
@@ -367,25 +365,28 @@ def reveal_collection_item_authors
   end
 
   def send_reveal_notifications
-    approved_collection_items.each {|collection_item| collection_item.notify_of_reveal}
+    approved_collection_items.each { |collection_item| collection_item.notify_of_reveal }
   end
 
   def self.sorted_and_filtered(sort, filters, page)
-    pagination_args = {page: page}
+    pagination_args = { page: page }
 
     # build up the query with scopes based on the options the user specifies
     query = Collection.top_level
 
-    if !filters[:title].blank?
+    if filters[:title].present?
       # we get the matching collections out of autocomplete and use their ids
       ids = Collection.autocomplete_lookup(search_param: filters[:title],
-                autocomplete_prefix: (filters[:closed].blank? ? "autocomplete_collection_all" : (filters[:closed] ? "autocomplete_collection_closed" : "autocomplete_collection_open"))
-             ).map {|result| Collection.id_from_autocomplete(result)}
+                                           autocomplete_prefix: (if filters[:closed].blank?
+                                                                   "autocomplete_collection_all"
+                                                                 else
+                                                                   (filters[:closed] ? "autocomplete_collection_closed" : "autocomplete_collection_open")
+                                                                 end)).map { |result| Collection.id_from_autocomplete(result) }
       query = query.where("collections.id in (?)", ids)
-    else
-      query = (filters[:closed] == "true" ? query.closed : query.not_closed) if !filters[:closed].blank?
+    elsif filters[:closed].present?
+      query = (filters[:closed] == "true" ? query.closed : query.not_closed)
     end
-    query = (filters[:moderated] == "true" ? query.moderated : query.unmoderated) if !filters[:moderated].blank?
+    query = (filters[:moderated] == "true" ? query.moderated : query.unmoderated) if filters[:moderated].present?
     if filters[:challenge_type].present?
       if filters[:challenge_type] == "gift_exchange"
         query = query.gift_exchange
@@ -397,15 +398,15 @@ def self.sorted_and_filtered(sort, filters, page)
     end
     query = query.order(sort)
 
-    if !filters[:fandom].blank?
+    if filters[:fandom].blank?
+      query.paginate(pagination_args)
+    else
       fandom = Fandom.find_by_name(filters[:fandom])
       if fandom
         (fandom.approved_collections & query).paginate(pagination_args)
       else
         []
       end
-    else
-      query.paginate(pagination_args)
     end
   end
 
@@ -417,7 +418,7 @@ def delete_icon=(value)
   def delete_icon
     !!@delete_icon
   end
-  alias_method :delete_icon?, :delete_icon
+  alias delete_icon? delete_icon
 
   def clear_icon
     self.icon = nil if delete_icon? && !icon.dirty?
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index f24312ed1d9..152b5bd1acf 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -1,5 +1,4 @@
 class PotentialMatch < ApplicationRecord
-
   # We use "-1" to represent all the requested items matching
   ALL = -1
 
@@ -12,26 +11,23 @@ class PotentialMatch < ApplicationRecord
   belongs_to :offer_signup, class_name: "ChallengeSignup"
   belongs_to :request_signup, class_name: "ChallengeSignup"
 
-protected
-
+  
   def self.progress_key(collection)
-    CACHE_PROGRESS_KEY + "#{collection.id}"
+    CACHE_PROGRESS_KEY + collection.id.to_s
   end
 
   def self.signup_key(collection)
-    CACHE_SIGNUP_KEY + "#{collection.id}"
+    CACHE_SIGNUP_KEY + collection.id.to_s
   end
 
   def self.interrupt_key(collection)
-    CACHE_INTERRUPT_KEY + "#{collection.id}"
+    CACHE_INTERRUPT_KEY + collection.id.to_s
   end
 
   def self.invalid_signup_key(collection)
-    CACHE_INVALID_SIGNUP_KEY + "#{collection.id}"
+    CACHE_INVALID_SIGNUP_KEY + collection.id.to_s
   end
 
-public
-
   def self.clear!(collection)
     # rapidly delete all potential prompt matches and potential matches
     # WITHOUT CALLBACKS
@@ -89,9 +85,10 @@ def self.generate_in_background(collection_id)
 
     # check for invalid signups
     PotentialMatch.clear_invalid_signups(collection)
-    invalid_signup_ids = collection.signups.select {|s| !s.valid?}.collect(&:id)
+    invalid_signup_ids = collection.signups.select { |s| !s.valid? }
+      .collect(&:id)
     if invalid_signup_ids.present?
-      invalid_signup_ids.each {|sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid}
+      invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
       @maintainers = collection.maintainers_list
       @maintainers.each do |i|
         UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, i.email).deliver_later
@@ -165,9 +162,9 @@ def self.matching_signup_ids(collection, signup, collection_tag_sets, required_t
       # and now we look up any signups that have one of those tagsets in the opposite position -- ie,
       # if this signup is a request, we are looking for offers with the same tag; if it's an offer, we're
       # looking for requests with the same tag.
-      matching_signup_ids = (prompt_type == "request" ? Offer : Request).
-                            where("tag_set_id IN (?) OR optional_tag_set_id IN (?)", match_tagsets, match_tagsets).
-                            pluck(:challenge_signup_id).compact
+      matching_signup_ids = (prompt_type == "request" ? Offer : Request)
+        .where("tag_set_id IN (?) OR optional_tag_set_id IN (?)", match_tagsets, match_tagsets)
+        .pluck(:challenge_signup_id).compact
 
       # now add on "any" matches for the required types
       condition = case required_types.first.underscore
@@ -207,14 +204,14 @@ def self.regenerate_for_signup_in_background(signup_id)
     # Get all the data
     settings = collection.challenge.potential_match_settings
     collection_tag_sets = Prompt.where(collection_id: collection.id).pluck(:tag_set_id, :optional_tag_set_id).flatten.compact
-    required_types = settings.required_types.map {|t| t.classify}
+    required_types = settings.required_types.map { |t| t.classify }
 
     # clear the existing potential matches for this signup in each direction
     signup.offer_potential_matches.destroy_all
     signup.request_potential_matches.destroy_all
 
     # We check the signup in both directions -- as a request signup and as an offer signup
-    %w(request offer).each do |prompt_type|
+    %w[request offer].each do |prompt_type|
       PotentialMatch.generate_for_signup(collection, signup, settings, collection_tag_sets, required_types, prompt_type)
     end
   end
@@ -266,12 +263,16 @@ def <=>(other)
     return cmp unless cmp == 0
 
     # if we're a match down to here just match on id
-    return self.id <=> other.id
+    self.id <=> other.id
   end
 
-protected
+  protected
+
   def compare_all(self_value, other_value)
-    self_value == ALL ? (other_value == ALL ? 0 : 1) : (other_value == ALL ? -1 : self_value <=> other_value)
+    if self_value == ALL
+      other_value == ALL ? 0 : 1
+    else
+      (other_value == ALL ? -1 : self_value <=> other_value)
+    end
   end
-
 end
diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb
index d2afdba11b0..f2fc7c3e5ef 100644
--- a/spec/mailers/user_mailer_spec.rb
+++ b/spec/mailers/user_mailer_spec.rb
@@ -678,11 +678,12 @@
       end
 
       it "formats the date rightfully in French" do
-        I18n.locale = "fr"
-        travel_to "2022-03-14 13:27:09 +0000" do
-          expect(email).to have_html_part_content("Envoyé le 14 mars 2022 13h 27min 09s.")
-          expect(email).to have_text_part_content("Envoyé le 14 mars 2022 13h 27min 09s.")
-        end
+        I18n.with_locale("fr") do
+          travel_to "2022-03-14 13:27:09 +0000" do
+            expect(email).to have_html_part_content("Envoyé le 14 mars 2022 13h 27min 09s.")
+            expect(email).to have_text_part_content("Envoyé le 14 mars 2022 13h 27min 09s.")
+          end
+        end 
       end
     end
   end

From d4da2dadedd61ee87a59c4bcabf527fa2415d0b0 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Wed, 18 Sep 2024 12:40:08 -0700
Subject: [PATCH 06/20] address Bilka comments

---
 .../challenge_assignments_controller.rb       |  7 +++--
 app/models/challenge_assignment.rb            | 11 +++----
 app/models/collection.rb                      | 10 ++++---
 app/models/potential_match.rb                 |  7 +++--
 config/locales/mailers/en.yml                 |  3 ++
 .../notification_emails.feature               | 29 +++++++++++++++++++
 6 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/app/controllers/challenge_assignments_controller.rb b/app/controllers/challenge_assignments_controller.rb
index 55555e4705e..57197215c21 100644
--- a/app/controllers/challenge_assignments_controller.rb
+++ b/app/controllers/challenge_assignments_controller.rb
@@ -219,9 +219,10 @@ def default_all
   def default
     @challenge_assignment.defaulted_at = Time.now
     @challenge_assignment.save
-    @challenge_assignment.collection.notify_maintainers("Challenge default by #{@challenge_assignment.offer_byline}",
-        "Signed-up participant #{@challenge_assignment.offer_byline} has defaulted on their assignment for #{@challenge_assignment.request_byline}. " +
-        "You may want to assign a pinch hitter on the collection assignments page: #{collection_assignments_url(@challenge_assignment.collection)}")
+    subject = t("user_mailer.collection_notification.challenge_default.subject", offer_byline: @challenge_assignment.offer_byline )
+    message = t("user_mailer.collection_notification.challenge_default.complete", offer_byline: @challenge_assignment.offer_byline, request_byline: @challenge_assignment.request_byline, assignments_page_link: collection_assignments_url(@challenge_assignment.collection))
+    @challenge_assignment.collection.notify_maintainers(subject, message)
+
     flash[:notice] = "We have notified the collection maintainers that you had to default on your assignment."
     redirect_to user_assignments_path(current_user)
   end
diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index 7dc4c05052f..fa8220cbda2 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -298,8 +298,8 @@ def self.delayed_send_out(collection_id)
     collection.assignments.each do |assignment|
       assignment.send_out
     end
-    subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
-    message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
+    subject = "user_mailer.collection_notification.assignments_sent.subject"
+    message = "user_mailer.collection_notification.assignments_sent.complete"
     collection.notify_maintainers(subject, message)
 
     # purge the potential matches! we don't want bazillions of them in our db
@@ -388,9 +388,10 @@ def self.delayed_generate(collection_id)
       end
     end
     REDIS_GENERAL.del(progress_key(collection))
-    @maintainers = collection.maintainers_list
-    @maintainers.each do |i|
-      UserMailer.potential_match_generation_notification(collection.id, i.email).deliver_later
+    collection.maintainers_list.each do |user|
+      I18n.with_locale(user.preference.locale.iso) do
+        UserMailer.potential_match_generation_notification(collection.id, user.email).deliver_later
+      end 
     end 
   end
 
diff --git a/app/models/collection.rb b/app/models/collection.rb
index f8df6c930e9..832e3981836 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -333,14 +333,16 @@ def prompt_meme?
   end
 
   def maintainers_list
-    @maintainers_list = self.maintainers.collect(&:user).flatten.uniq
+    self.maintainers.collect(&:user).flatten.uniq
   end
 
   def notify_maintainers(subject, message)
-    @maintainers = self.maintainers_list
+    
     # loop through maintainers and send each a notice via email
-    @maintainers.each do |i|
-      UserMailer.collection_notification(self.id, subject, message, i.email).deliver_later
+    self.maintainers_list.each do |user|
+      I18n.with_locale(user.preference.locale.iso) do
+        UserMailer.collection_notification(self.id, I18n.t(subject, default: subject), I18n.t(message, default: message), user.email).deliver_later
+      end
     end
   end
 
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index 152b5bd1acf..bbca85c4838 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -89,9 +89,10 @@ def self.generate_in_background(collection_id)
       .collect(&:id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
-      @maintainers = collection.maintainers_list
-      @maintainers.each do |i|
-        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, i.email).deliver_later
+      collection.maintainers_list.each do |user|
+        I18n.with_locale(user.preference.locale.iso) do
+          UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, user.email).deliver_later
+        end
       end
       PotentialMatch.cancel_generation(collection)
     else
diff --git a/config/locales/mailers/en.yml b/config/locales/mailers/en.yml
index 073826e1bc6..18b73a46227 100644
--- a/config/locales/mailers/en.yml
+++ b/config/locales/mailers/en.yml
@@ -251,6 +251,9 @@ en:
       assignments_sent:
         complete: All assignments have now been sent out.
         subject: Assignments sent
+      challenge_default:
+        complete: 'Signed-up participant %{offer_byline} has defaulted on their assignment for %{request_byline}. You may want to assign a pinch hitter on the collection assignments page: %{assignments_page_link}'
+        subject: Challenge default by %{offer_byline}
       html:
         received_message: 'You have received a message about your collection %{collection_link}:'
       text:
diff --git a/features/gift_exchanges/notification_emails.feature b/features/gift_exchanges/notification_emails.feature
index df426b0db16..3aa1f1e6629 100644
--- a/features/gift_exchanges/notification_emails.feature
+++ b/features/gift_exchanges/notification_emails.feature
@@ -1,6 +1,35 @@
 Feature: Gift Exchange Notification Emails
   Make sure that gift exchange notification emails are formatted properly
 
+  Scenario: Assignment notification emails should be sent to two owners in their respective locales
+    Given I have created the tagless gift exchange "Holiday Swap"
+      And I open signups for "Holiday Swap"
+    
+    When I am logged in as "participant1"
+      And I start signing up for "Holiday Swap"
+      And I press "Submit"
+    Then I should see "Sign-up was successfully created."
+
+    When I am logged in as "participant2"
+      And I start signing up for "Holiday Swap"
+      And I press "Submit"
+    Then I should see "Sign-up was successfully created."
+
+    Given I have added a co-moderator "mod2" to collection "Holiday Swap"
+      And a locale with translated emails
+      And the user "mod1" enables translated emails
+    When I close signups for "Holiday Swap"
+      And I have generated matches for "Holiday Swap"
+      And I have sent assignments for "Holiday Swap"
+
+    Then 4 emails should be delivered
+      And "mod1" should receive 1 email
+      And the email to "mod1" should be translated
+      And "mod2" should receive 1 email
+      And the email to "mod2" should be non-translated
+      And "participant1" should receive 1 email
+      And "participant2" should receive 1 email
+
   Scenario: Assignment notifications with linebreaks.
     Given I have created the tagless gift exchange "Holiday Swap"
       And I open signups for "Holiday Swap"

From 1f0242e271a15e40c738fb27a4dba13a53bafc56 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Wed, 18 Sep 2024 13:53:11 -0700
Subject: [PATCH 07/20] rubocop linter changes

---
 app/models/potential_match.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index bbca85c4838..3ba07f8c9f5 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -85,7 +85,7 @@ def self.generate_in_background(collection_id)
 
     # check for invalid signups
     PotentialMatch.clear_invalid_signups(collection)
-    invalid_signup_ids = collection.signups.select { |s| !s.valid? }
+    invalid_signup_ids = collection.signups.reject(&:valid?)
       .collect(&:id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
@@ -134,7 +134,7 @@ def self.generate_for_signup(collection, signup, settings, collection_tag_sets,
         potential_match = other_signup.match(signup, settings)
       end
 
-      potential_match.save if potential_match && potential_match.valid?
+      potential_match.save if potential_match&.valid?
     end
   end
 
@@ -151,7 +151,7 @@ def self.matching_signup_ids(collection, signup, collection_tag_sets, required_t
     signup_tagsets = signup.send(prompt_type.pluralize).pluck(:tag_set_id, :optional_tag_set_id).flatten.compact
 
     # get the ids of all the tags of the required types in the signup's tagsets
-    signup_tags = SetTagging.where(tag_set_id: signup_tagsets).joins(:tag).where("tags.type IN (?)", required_types).pluck(:tag_id)
+    signup_tags = SetTagging.where(tag_set_id: signup_tagsets).joins(:tag).where(tags: { type: required_types }).pluck(:tag_id)
 
     if signup_tags.empty?
       # a match is required by the settings but the user hasn't put any of the required tags in, meaning they are open to anything
@@ -205,7 +205,7 @@ def self.regenerate_for_signup_in_background(signup_id)
     # Get all the data
     settings = collection.challenge.potential_match_settings
     collection_tag_sets = Prompt.where(collection_id: collection.id).pluck(:tag_set_id, :optional_tag_set_id).flatten.compact
-    required_types = settings.required_types.map { |t| t.classify }
+    required_types = settings.required_types.map(&:classify)
 
     # clear the existing potential matches for this signup in each direction
     signup.offer_potential_matches.destroy_all
@@ -255,13 +255,13 @@ def <=>(other)
 
     # start with seeing how many offers/requests match
     cmp = compare_all(self.num_prompts_matched, other.num_prompts_matched)
-    return cmp unless cmp == 0
+    return cmp unless cmp.zero?
 
     # compare the "quality" of the best prompt match
     # (i.e. the number of matching tags between the most closely-matching
     # request prompt/offer prompt pair)
     cmp = compare_all(max_tags_matched, other.max_tags_matched)
-    return cmp unless cmp == 0
+    return cmp unless cmp.zero?
 
     # if we're a match down to here just match on id
     self.id <=> other.id

From f9e91ce27a48d8382bdd22f6d526f361bb509ab4 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Wed, 18 Sep 2024 13:55:54 -0700
Subject: [PATCH 08/20] fix unused translation

---
 app/models/challenge_assignment.rb |  4 +---
 app/models/collection.rb           | 12 ++++++++++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index fa8220cbda2..5c34636d5a6 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -298,9 +298,7 @@ def self.delayed_send_out(collection_id)
     collection.assignments.each do |assignment|
       assignment.send_out
     end
-    subject = "user_mailer.collection_notification.assignments_sent.subject"
-    message = "user_mailer.collection_notification.assignments_sent.complete"
-    collection.notify_maintainers(subject, message)
+    collection.notify_maintainers_challenge_sent
 
     # purge the potential matches! we don't want bazillions of them in our db
     PotentialMatch.clear!(collection)
diff --git a/app/models/collection.rb b/app/models/collection.rb
index 832e3981836..5b597a9781b 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -336,12 +336,20 @@ def maintainers_list
     self.maintainers.collect(&:user).flatten.uniq
   end
 
+  def notify_maintainers_challenge_sent
+    # loop through maintainers and send each a notice via email
+    self.maintainers_list.each do |user|
+      I18n.with_locale(user.preference.locale.iso) do
+        UserMailer.collection_notification(self.id, t("user_mailer.collection_notification.assignments_sent.subject"), t("user_mailer.collection_notification.assignments_sent.complete"), user.email).deliver_later
+      end
+    end
+  end
+
   def notify_maintainers(subject, message)
-    
     # loop through maintainers and send each a notice via email
     self.maintainers_list.each do |user|
       I18n.with_locale(user.preference.locale.iso) do
-        UserMailer.collection_notification(self.id, I18n.t(subject, default: subject), I18n.t(message, default: message), user.email).deliver_later
+        UserMailer.collection_notification(self.id, subject, message, user.email).deliver_later
       end
     end
   end

From 5e92d685016a6837372d7e886fea3fd589ecf561 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Wed, 18 Sep 2024 14:19:30 -0700
Subject: [PATCH 09/20] yet more rubocop autocorrection changes

---
 app/models/collection.rb | 64 +++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 5b597a9781b..79f5d211863 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -64,29 +64,29 @@ def clean_up_challenge
 
   has_many :participants, through: :collection_participants, source: :pseud
   has_many :users, through: :participants, source: :user
-  has_many :invited, -> { where("collection_participants.participant_role = ?", CollectionParticipant::INVITED) }, through: :collection_participants, source: :pseud
-  has_many :owners, -> { where("collection_participants.participant_role = ?", CollectionParticipant::OWNER) }, through: :collection_participants, source: :pseud
-  has_many :moderators, -> { where("collection_participants.participant_role = ?", CollectionParticipant::MODERATOR) }, through: :collection_participants, source: :pseud
-  has_many :members, -> { where("collection_participants.participant_role = ?", CollectionParticipant::MEMBER) }, through: :collection_participants, source: :pseud
-  has_many :posting_participants, -> { where("collection_participants.participant_role in (?)", [CollectionParticipant::MEMBER, CollectionParticipant::MODERATOR, CollectionParticipant::OWNER]) }, through: :collection_participants, source: :pseud
+  has_many :invited, -> { where(collection_participants: { participant_role: CollectionParticipant::INVITED }) }, through: :collection_participants, source: :pseud
+  has_many :owners, -> { where(collection_participants: { participant_role: CollectionParticipant::OWNER }) }, through: :collection_participants, source: :pseud
+  has_many :moderators, -> { where(collection_participants: { participant_role: CollectionParticipant::MODERATOR }) }, through: :collection_participants, source: :pseud
+  has_many :members, -> { where(collection_participants: { participant_role: CollectionParticipant::MEMBER }) }, through: :collection_participants, source: :pseud
+  has_many :posting_participants, -> { where(collection_participants: { participant_role: [CollectionParticipant::MEMBER, CollectionParticipant::MODERATOR, CollectionParticipant::OWNER] }) }, through: :collection_participants, source: :pseud
 
   CHALLENGE_TYPE_OPTIONS = [
     ["", ""],
     [ts("Gift Exchange"), "GiftExchange"],
     [ts("Prompt Meme"), "PromptMeme"]
-  ]
+  ].freeze
 
   validate :must_have_owners
   def must_have_owners
     # we have to use collection participants because the association may not exist until after
     # the collection is saved
-    errors.add(:base, ts("Collection has no valid owners.")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select { |p| p.is_owner? }
+    errors.add(:base, ts("Collection has no valid owners.")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select(&:is_owner?)
       .empty?
   end
 
   validate :collection_depth
   def collection_depth
-    errors.add(:base, ts("Sorry, but %{name} is a subcollection, so it can't also be a parent collection.", name: parent.name)) if (self.parent && self.parent.parent) || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
+    errors.add(:base, ts("Sorry, but %{name} is a subcollection, so it can't also be a parent collection.", name: parent.name)) if self.parent&.parent || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
   end
 
   validate :parent_exists
@@ -154,12 +154,12 @@ def title
                       message: "^Sorry, a collection can only have %{count} tags." }
 
   scope :top_level, -> { where(parent_id: nil) }
-  scope :closed, -> { joins(:collection_preference).where("collection_preferences.closed = ?", true) }
-  scope :not_closed, -> { joins(:collection_preference).where("collection_preferences.closed = ?", false) }
-  scope :moderated, -> { joins(:collection_preference).where("collection_preferences.moderated = ?", true) }
-  scope :unmoderated, -> { joins(:collection_preference).where("collection_preferences.moderated = ?", false) }
-  scope :unrevealed, -> { joins(:collection_preference).where("collection_preferences.unrevealed = ?", true) }
-  scope :anonymous, -> { joins(:collection_preference).where("collection_preferences.anonymous = ?", true) }
+  scope :closed, -> { joins(:collection_preference).where(collection_preferences: { closed: true }) }
+  scope :not_closed, -> { joins(:collection_preference).where(collection_preferences: { closed: false }) }
+  scope :moderated, -> { joins(:collection_preference).where(collection_preferences: { moderated: true }) }
+  scope :unmoderated, -> { joins(:collection_preference).where(collection_preferences: { moderated: false }) }
+  scope :unrevealed, -> { joins(:collection_preference).where(collection_preferences: { unrevealed: true }) }
+  scope :anonymous, -> { joins(:collection_preference).where(collection_preferences: { anonymous: true }) }
   scope :no_challenge, -> { where(challenge_type: nil) }
   scope :gift_exchange, -> { where(challenge_type: "GiftExchange") }
   scope :prompt_meme, -> { where(challenge_type: "PromptMeme") }
@@ -172,24 +172,25 @@ def cleanup_url
 
   # Get only collections with running challenges
   def self.signup_open(challenge_type)
-    if challenge_type == "PromptMeme"
+    case challenge_type
+    when "PromptMeme"
       not_closed.where(challenge_type: challenge_type)
         .joins("INNER JOIN prompt_memes on prompt_memes.id = challenge_id").where("prompt_memes.signup_open = 1")
-        .where("prompt_memes.signups_close_at > ?", Time.now).order("prompt_memes.signups_close_at DESC")
-    elsif challenge_type == "GiftExchange"
+        .where("prompt_memes.signups_close_at > ?", Time.zone.now).order("prompt_memes.signups_close_at DESC")
+    when "GiftExchange"
       not_closed.where(challenge_type: challenge_type)
         .joins("INNER JOIN gift_exchanges on gift_exchanges.id = challenge_id").where("gift_exchanges.signup_open = 1")
-        .where("gift_exchanges.signups_close_at > ?", Time.now).order("gift_exchanges.signups_close_at DESC")
+        .where("gift_exchanges.signups_close_at > ?", Time.zone.now).order("gift_exchanges.signups_close_at DESC")
     end
   end
 
   scope :with_name_like, lambda { |name|
-    where("collections.name LIKE ?", "%" + name + "%")
+    where("collections.name LIKE ?", "%#{name}%")
       .limit(10)
   }
 
   scope :with_title_like, lambda { |title|
-    where("collections.title LIKE ?", "%" + title + "%")
+    where("collections.title LIKE ?", "%#{title}%")
   }
 
   scope :with_item_count, lambda {
@@ -277,27 +278,27 @@ def maintainers
   end
 
   def user_is_owner?(user)
-    user && user != :false && !(user.pseuds & self.all_owners).empty?
+    user && user != false && !(user.pseuds & self.all_owners).empty?
   end
 
   def user_is_moderator?(user)
-    user && user != :false && !(user.pseuds & self.all_moderators).empty?
+    user && user != false && !(user.pseuds & self.all_moderators).empty?
   end
 
   def user_is_maintainer?(user)
-    user && user != :false && !(user.pseuds & (self.all_moderators + self.all_owners)).empty?
+    user && user != false && !(user.pseuds & (self.all_moderators + self.all_owners)).empty?
   end
 
   def user_is_participant?(user)
-    user && user != :false && !get_participating_pseuds_for_user(user).empty?
+    user && user != false && !get_participating_pseuds_for_user(user).empty?
   end
 
   def user_is_posting_participant?(user)
-    user && user != :false && !(user.pseuds & self.all_posting_participants).empty?
+    user && user != false && !(user.pseuds & self.all_posting_participants).empty?
   end
 
   def get_participating_pseuds_for_user(user)
-    (user && user != :false) ? user.pseuds & self.all_participants : []
+    (user && user != false) ? user.pseuds & self.all_participants : []
   end
 
   def get_participants_for_user(user)
@@ -375,7 +376,7 @@ def reveal_collection_item_authors
   end
 
   def send_reveal_notifications
-    approved_collection_items.each { |collection_item| collection_item.notify_of_reveal }
+    approved_collection_items.each(&:notify_of_reveal)
   end
 
   def self.sorted_and_filtered(sort, filters, page)
@@ -392,17 +393,18 @@ def self.sorted_and_filtered(sort, filters, page)
                                                                  else
                                                                    (filters[:closed] ? "autocomplete_collection_closed" : "autocomplete_collection_open")
                                                                  end)).map { |result| Collection.id_from_autocomplete(result) }
-      query = query.where("collections.id in (?)", ids)
+      query = query.where(collections: { id: ids })
     elsif filters[:closed].present?
       query = (filters[:closed] == "true" ? query.closed : query.not_closed)
     end
     query = (filters[:moderated] == "true" ? query.moderated : query.unmoderated) if filters[:moderated].present?
     if filters[:challenge_type].present?
-      if filters[:challenge_type] == "gift_exchange"
+      case filters[:challenge_type]
+      when "gift_exchange"
         query = query.gift_exchange
-      elsif filters[:challenge_type] == "prompt_meme"
+      when "prompt_meme"
         query = query.prompt_meme
-      elsif filters[:challenge_type] == "no_challenge"
+      when "no_challenge"
         query = query.no_challenge
       end
     end

From a4f6c633dceaaf1d8ed07d5662218391f21afb15 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Wed, 18 Sep 2024 14:21:10 -0700
Subject: [PATCH 10/20] fix broken UTs by fixing defining t in
 notify_maintainers_challenge_sent function

---
 app/models/collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 79f5d211863..39c17cfa38c 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -341,7 +341,7 @@ def notify_maintainers_challenge_sent
     # loop through maintainers and send each a notice via email
     self.maintainers_list.each do |user|
       I18n.with_locale(user.preference.locale.iso) do
-        UserMailer.collection_notification(self.id, t("user_mailer.collection_notification.assignments_sent.subject"), t("user_mailer.collection_notification.assignments_sent.complete"), user.email).deliver_later
+        UserMailer.collection_notification(self.id, I18n.t("user_mailer.collection_notification.assignments_sent.subject"), I18n.t("user_mailer.collection_notification.assignments_sent.complete"), user.email).deliver_later
       end
     end
   end

From 85681cb1183b8c37115c0ecf962e4a4b5cad612c Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Wed, 18 Sep 2024 14:26:56 -0700
Subject: [PATCH 11/20] rubocop change keyword argument from default = true to
 default: true

---
 app/models/collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 39c17cfa38c..0281b03ef85 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -207,7 +207,7 @@ def to_param
   end
 
   # Change membership of collection(s) from a particular pseud to the orphan account
-  def self.orphan(pseuds, collections, default = true)
+  def self.orphan(pseuds, collections, default: true)
     pseuds.each do |pseud|
       collections.each do |collection|
         if pseud && collection && collection.owners.include?(pseud)

From 13bd95635b911cb35f2939b77b45997db1f2b352 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 00:16:28 -0700
Subject: [PATCH 12/20] address bilka comments

---
 .../challenge_assignments_controller.rb       |  8 ++--
 app/mailers/user_mailer.rb                    |  6 +--
 app/models/challenge_assignment.rb            | 15 +++++--
 app/models/collection.rb                      | 45 ++++++++++++++-----
 app/models/potential_match.rb                 | 13 ++++--
 config/locales/mailers/en.yml                 |  2 +-
 .../notification_emails.feature               |  6 ++-
 7 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/app/controllers/challenge_assignments_controller.rb b/app/controllers/challenge_assignments_controller.rb
index 57197215c21..3f959ec8302 100644
--- a/app/controllers/challenge_assignments_controller.rb
+++ b/app/controllers/challenge_assignments_controller.rb
@@ -219,9 +219,11 @@ def default_all
   def default
     @challenge_assignment.defaulted_at = Time.now
     @challenge_assignment.save
-    subject = t("user_mailer.collection_notification.challenge_default.subject", offer_byline: @challenge_assignment.offer_byline )
-    message = t("user_mailer.collection_notification.challenge_default.complete", offer_byline: @challenge_assignment.offer_byline, request_byline: @challenge_assignment.request_byline, assignments_page_link: collection_assignments_url(@challenge_assignment.collection))
-    @challenge_assignment.collection.notify_maintainers(subject, message)
+    offer_byline = @challenge_assignment.offer_byline
+    request_byline = @challenge_assignment.request_byline
+    assignments_page_url = collection_assignments_url(@challenge_assignment.collection)
+    
+    @challenge_assignment.collection.notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
 
     flash[:notice] = "We have notified the collection maintainers that you had to default on your assignment."
     redirect_to user_assignments_path(current_user)
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 6d19299fd70..0e487fb387d 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -92,7 +92,7 @@ def invitation_to_claim(invitation_id, archivist_login)
   end
 
   # Notifies a writer that their imported works have been claimed
-  def claim_notification(creator_id, claimed_work_ids, is_user = false)
+  def claim_notification(creator_id, claimed_work_ids, is_user=false)
     if is_user
       creator = User.find(creator_id)
       locale = creator.preference.locale.iso
@@ -160,7 +160,7 @@ def invite_increase_notification(user_id, total)
     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.to_s)
+        subject: default_i18n_subject(app_name: ArchiveConfig.APP_SHORT_NAME)
       )
     end
   end
@@ -394,7 +394,7 @@ def admin_spam_work_notification(creation_id, user_id)
     mail(
       to: @user.email,
       subject: "[#{ArchiveConfig.APP_SHORT_NAME}] Your work was hidden as spam"
-      )
+    )
   end
 
   ### OTHER NOTIFICATIONS ###
diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index 5c34636d5a6..c2b5df7b950 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -298,7 +298,7 @@ def self.delayed_send_out(collection_id)
     collection.assignments.each do |assignment|
       assignment.send_out
     end
-    collection.notify_maintainers_challenge_sent
+    collection.notify_maintainers_assignments_sent
 
     # purge the potential matches! we don't want bazillions of them in our db
     PotentialMatch.clear!(collection)
@@ -386,9 +386,16 @@ def self.delayed_generate(collection_id)
       end
     end
     REDIS_GENERAL.del(progress_key(collection))
-    collection.maintainers_list.each do |user|
-      I18n.with_locale(user.preference.locale.iso) do
-        UserMailer.potential_match_generation_notification(collection.id, user.email).deliver_later
+
+    if !collection.email.blank?
+      UserMailer.potential_match_generation_notification(collection.id, collection.email).deliver_later
+    elsif collection.parent && !collection.parent.email.blank?
+      UserMailer.potential_match_generation_notification(collection.id, collection.parent.email).deliver_later
+    else
+      collection.maintainers_list.each do |user|
+        I18n.with_locale(user.preference.locale.iso) do
+          UserMailer.potential_match_generation_notification(collection.id, user.email).deliver_later
+        end
       end 
     end 
   end
diff --git a/app/models/collection.rb b/app/models/collection.rb
index 0281b03ef85..9b64ba163f1 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -147,7 +147,7 @@ def title
                       too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
   validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
-  validates :header_image_url, format: { allow_blank: true, with: /\.(png|gif|jpg)$/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: /\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
@@ -337,20 +337,43 @@ def maintainers_list
     self.maintainers.collect(&:user).flatten.uniq
   end
 
-  def notify_maintainers_challenge_sent
-    # loop through maintainers and send each a notice via email
-    self.maintainers_list.each do |user|
-      I18n.with_locale(user.preference.locale.iso) do
-        UserMailer.collection_notification(self.id, I18n.t("user_mailer.collection_notification.assignments_sent.subject"), I18n.t("user_mailer.collection_notification.assignments_sent.complete"), user.email).deliver_later
+  def notify_maintainers_assignments_sent
+    subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
+    message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
+    if !self.email.blank?
+      UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
+    elsif
+      self.parent && !self.parent.email.blank?
+      UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
+    else
+    # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
+      self.maintainers_list.each do |user|
+        I18n.with_locale(user.preference.locale.iso) do
+          translated_subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
+          translated_message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
+          UserMailer.collection_notification(self.id, translated_subject, translated_message, user.email).deliver_later
+        end
       end
     end
   end
 
-  def notify_maintainers(subject, message)
-    # loop through maintainers and send each a notice via email
-    self.maintainers_list.each do |user|
-      I18n.with_locale(user.preference.locale.iso) do
-        UserMailer.collection_notification(self.id, subject, message, user.email).deliver_later
+  def notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
+    subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline )
+    message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
+
+    if !self.email.blank?
+      UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
+    elsif
+      self.parent && !self.parent.email.blank?
+      UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
+    else
+    # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
+      self.maintainers_list.each do |user|
+        I18n.with_locale(user.preference.locale.iso) do
+          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline )
+          translated_message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
+          UserMailer.collection_notification(self.id, translated_subject, translated_message, user.email).deliver_later
+        end
       end
     end
   end
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index 3ba07f8c9f5..05cd5c169b8 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -89,9 +89,16 @@ def self.generate_in_background(collection_id)
       .collect(&:id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
-      collection.maintainers_list.each do |user|
-        I18n.with_locale(user.preference.locale.iso) do
-          UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, user.email).deliver_later
+      
+      if !collection.email.blank?
+        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.email).deliver_later
+      elsif collection.parent && !collection.parent.email.blank?
+        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.parent.email).deliver_later
+      else
+        collection.maintainers_list.each do |user|
+          I18n.with_locale(user.preference.locale.iso) do
+            UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, user.email).deliver_later
+          end
         end
       end
       PotentialMatch.cancel_generation(collection)
diff --git a/config/locales/mailers/en.yml b/config/locales/mailers/en.yml
index 18b73a46227..16c85b34011 100644
--- a/config/locales/mailers/en.yml
+++ b/config/locales/mailers/en.yml
@@ -252,7 +252,7 @@ en:
         complete: All assignments have now been sent out.
         subject: Assignments sent
       challenge_default:
-        complete: 'Signed-up participant %{offer_byline} has defaulted on their assignment for %{request_byline}. You may want to assign a pinch hitter on the collection assignments page: %{assignments_page_link}'
+        complete: 'Signed-up participant %{offer_byline} has defaulted on their assignment for %{request_byline}. You may want to assign a pinch hitter on the collection assignments page: %{assignments_page_url}'
         subject: Challenge default by %{offer_byline}
       html:
         received_message: 'You have received a message about your collection %{collection_link}:'
diff --git a/features/gift_exchanges/notification_emails.feature b/features/gift_exchanges/notification_emails.feature
index 3aa1f1e6629..edbcc197fbc 100644
--- a/features/gift_exchanges/notification_emails.feature
+++ b/features/gift_exchanges/notification_emails.feature
@@ -15,18 +15,20 @@ Feature: Gift Exchange Notification Emails
       And I press "Submit"
     Then I should see "Sign-up was successfully created."
 
-    Given I have added a co-moderator "mod2" to collection "Holiday Swap"
+    When I have added a co-moderator "mod2" to collection "Holiday Swap"
       And a locale with translated emails
       And the user "mod1" enables translated emails
-    When I close signups for "Holiday Swap"
+      And I close signups for "Holiday Swap"
       And I have generated matches for "Holiday Swap"
       And I have sent assignments for "Holiday Swap"
 
     Then 4 emails should be delivered
       And "mod1" should receive 1 email
       And the email to "mod1" should be translated
+      And the email should contain "You have received a message about your collection"
       And "mod2" should receive 1 email
       And the email to "mod2" should be non-translated
+      And the email should contain "You have received a message about your collection"
       And "participant1" should receive 1 email
       And "participant2" should receive 1 email
 

From cde3a7470533e8e17cb2c842e8a43e4eccb939fe Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 00:48:56 -0700
Subject: [PATCH 13/20] address rubocop and fix broken cucumber test

---
 app/models/challenge_assignment.rb |  4 ++--
 app/models/collection.rb           | 12 ++++++------
 app/models/potential_match.rb      |  4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index c2b5df7b950..ab4dee0ff42 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -387,9 +387,9 @@ def self.delayed_generate(collection_id)
     end
     REDIS_GENERAL.del(progress_key(collection))
 
-    if !collection.email.blank?
+    if collection.email.present?
       UserMailer.potential_match_generation_notification(collection.id, collection.email).deliver_later
-    elsif collection.parent && !collection.parent.email.blank?
+    elsif collection.parent && collection.parent.email.present?
       UserMailer.potential_match_generation_notification(collection.id, collection.parent.email).deliver_later
     else
       collection.maintainers_list.each do |user|
diff --git a/app/models/collection.rb b/app/models/collection.rb
index 9b64ba163f1..3e9b4f2817f 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -147,7 +147,7 @@ def title
                       too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
   validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
-  validates :header_image_url, format: { allow_blank: true, with: /\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
@@ -207,7 +207,7 @@ def to_param
   end
 
   # Change membership of collection(s) from a particular pseud to the orphan account
-  def self.orphan(pseuds, collections, default: true)
+  def self.orphan(pseuds, collections, default=true)
     pseuds.each do |pseud|
       collections.each do |collection|
         if pseud && collection && collection.owners.include?(pseud)
@@ -340,10 +340,10 @@ def maintainers_list
   def notify_maintainers_assignments_sent
     subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
     message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
-    if !self.email.blank?
+    if self.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
     elsif
-      self.parent && !self.parent.email.blank?
+      self.parent && self.parent.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
     else
     # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
@@ -361,10 +361,10 @@ def notify_maintainers_challenge_default(offer_byline, request_byline, assignmen
     subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline )
     message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
 
-    if !self.email.blank?
+    if self.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
     elsif
-      self.parent && !self.parent.email.blank?
+      self.parent && self.parent.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
     else
     # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index 05cd5c169b8..4ee894dc5a9 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -90,9 +90,9 @@ def self.generate_in_background(collection_id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
       
-      if !collection.email.blank?
+      if collection.email.present?
         UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.email).deliver_later
-      elsif collection.parent && !collection.parent.email.blank?
+      elsif collection.parent && collection.parent.email.present?
         UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.parent.email).deliver_later
       else
         collection.maintainers_list.each do |user|

From 047a8751b18a09156a27bfeb152f28dac3ce7e05 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 00:57:46 -0700
Subject: [PATCH 14/20] will rubocop finally be satisfied?

---
 app/models/collection.rb | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 3e9b4f2817f..c8eeb38b395 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -342,11 +342,10 @@ def notify_maintainers_assignments_sent
     message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
     if self.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
-    elsif
-      self.parent && self.parent.email.present?
+    elsif self.parent && self.parent.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
     else
-    # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
+      # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
       self.maintainers_list.each do |user|
         I18n.with_locale(user.preference.locale.iso) do
           translated_subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
@@ -358,19 +357,18 @@ def notify_maintainers_assignments_sent
   end
 
   def notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
-    subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline )
+    subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline)
     message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
 
     if self.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
-    elsif
-      self.parent && self.parent.email.present?
+    elsif self.parent && self.parent.email.present?
       UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
     else
-    # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
+      # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
       self.maintainers_list.each do |user|
         I18n.with_locale(user.preference.locale.iso) do
-          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline )
+          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline)
           translated_message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
           UserMailer.collection_notification(self.id, translated_subject, translated_message, user.email).deliver_later
         end

From 450dbeb5b9d068841d91fc5f143b9234a22390bf Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 01:38:47 -0700
Subject: [PATCH 15/20] see if rubocop will let me get away with not fixing all
 instances of ts()

---
 app/models/collection.rb     | 6 +++---
 config/locales/models/en.yml | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index c8eeb38b395..2d4210abc91 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -146,8 +146,8 @@ def title
                       maximum: ArchiveConfig.SUMMARY_MAX,
                       too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
-  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
-  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: I18n.t("collection.header_image.invalid_url") }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: I18n.t("collection.header_image.invalid_file_type"), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
@@ -207,7 +207,7 @@ def to_param
   end
 
   # Change membership of collection(s) from a particular pseud to the orphan account
-  def self.orphan(pseuds, collections, default=true)
+  def self.orphan(pseuds, collections, default = true)
     pseuds.each do |pseud|
       collections.each do |collection|
         if pseud && collection && collection.owners.include?(pseud)
diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
index 83d9a8a6b4d..cba0f3d7fe1 100644
--- a/config/locales/models/en.yml
+++ b/config/locales/models/en.yml
@@ -235,6 +235,10 @@ en:
         other: Works
   attributes:
     ticket_number: Ticket ID
+  collection:
+    header_image:
+      invalid_file_type: can only point to a gif, jpg, or png file.
+      invalid_url: is not a valid URL.
   errors:
     attributes:
       ticket_number:

From c28acc5fc48abf84039ad65f70c0b66434b9392b Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 01:55:30 -0700
Subject: [PATCH 16/20] rubocop change not about ts

---
 app/controllers/users_controller.rb | 2 +-
 app/models/collection.rb            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index b4429d20adf..99e2e14f949 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -315,7 +315,7 @@ def destroy_author
       use_default = params[:use_default] == 'true' || params[:sole_author] == 'orphan_pseud'
 
       Creatorship.orphan(pseuds, works, use_default)
-      Collection.orphan(pseuds, @sole_owned_collections, use_default)
+      Collection.orphan(pseuds, @sole_owned_collections, default: use_default)
     elsif params[:sole_author] == 'delete'
       # Deletes works where user is sole author
       @sole_authored_works.each(&:destroy)
diff --git a/app/models/collection.rb b/app/models/collection.rb
index 2d4210abc91..0c7b8b5278b 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -207,7 +207,7 @@ def to_param
   end
 
   # Change membership of collection(s) from a particular pseud to the orphan account
-  def self.orphan(pseuds, collections, default = true)
+  def self.orphan(pseuds, collections, default: true)
     pseuds.each do |pseud|
       collections.each do |collection|
         if pseud && collection && collection.owners.include?(pseud)

From ec338b0cf2b7c00da62ceec451504a15736aa105 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 02:56:12 -0700
Subject: [PATCH 17/20] internationalize collection.rb

---
 app/models/collection.rb     | 52 ++++++++++++++++++------------------
 config/locales/models/en.yml | 24 ++++++++++++++---
 2 files changed, 47 insertions(+), 29 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 0c7b8b5278b..8088a640f30 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -72,67 +72,67 @@ def clean_up_challenge
 
   CHALLENGE_TYPE_OPTIONS = [
     ["", ""],
-    [ts("Gift Exchange"), "GiftExchange"],
-    [ts("Prompt Meme"), "PromptMeme"]
+    [I18n.t("collection.gift_exchange"), "GiftExchange"],
+    [I18n.t("collection.prompt_meme"), "PromptMeme"]
   ].freeze
 
   validate :must_have_owners
   def must_have_owners
     # we have to use collection participants because the association may not exist until after
     # the collection is saved
-    errors.add(:base, ts("Collection has no valid owners.")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select(&:is_owner?)
+    errors.add(:base, I18n.t("collection.validation.no_valid_owners")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select(&:is_owner?)
       .empty?
   end
 
   validate :collection_depth
   def collection_depth
-    errors.add(:base, ts("Sorry, but %{name} is a subcollection, so it can't also be a parent collection.", name: parent.name)) if self.parent&.parent || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
+    errors.add(:base, I18n.t("collection.validation.collection_depth", name: parent.name)) if self.parent&.parent || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
   end
 
   validate :parent_exists
   def parent_exists
-    errors.add(:base, ts("We couldn't find a collection with name %{name}.", name: parent_name)) unless parent_name.blank? || Collection.find_by(name: parent_name)
+    errors.add(:base, I18n.t("collection.validation.collection_not_found", name: parent_name)) unless parent_name.blank? || Collection.find_by(name: parent_name)
   end
 
   validate :parent_is_allowed
   def parent_is_allowed
-    if parent
-      if parent == self
-        errors.add(:base, ts("You can't make a collection its own parent."))
-      elsif parent_id_changed? && !parent.user_is_maintainer?(User.current_user)
-        errors.add(:base, ts("You have to be a maintainer of %{name} to make a subcollection.", name: parent.name))
-      end
+    return unless parent
+    
+    if parent == self
+      errors.add(:base, I18n.t("collection.validation.invalid_parent"))
+    elsif parent_id_changed? && !parent.user_is_maintainer?(User.current_user)
+      errors.add(:base, I18n.t("collection.validation.subcollection", name: parent.name))
     end
   end
 
-  validates :name, presence: { message: ts("Please enter a name for your collection.") }
-  validates :name, uniqueness: { message: ts("Sorry, that name is already taken. Try again, please!") }
+  validates :name, presence: { message: I18n.t("collection.validation.name.presence") }
+  validates :name, uniqueness: { message: I18n.t("collection.validation.name.uniqueness") }
   validates :name,
             length: { minimum: ArchiveConfig.TITLE_MIN,
-                      too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN) }
+                      too_short: I18n.t("collection.validation.length.min", min: ArchiveConfig.TITLE_MIN) }
   validates :name,
             length: { maximum: ArchiveConfig.TITLE_MAX,
-                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX) }
+                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.TITLE_MAX) }
   validates :name,
-            format: { message: ts("must begin and end with a letter or number; it may also contain underscores. It may not contain any other characters, including spaces."),
+            format: { message: I18n.t("collection.validation.name.format"),
                       with: /\A[A-Za-z0-9]\w*[A-Za-z0-9]\Z/ }
   validates :icon_alt_text, length: { allow_blank: true, maximum: ArchiveConfig.ICON_ALT_MAX,
-                                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_ALT_MAX) }
+                                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.ICON_ALT_MAX) }
   validates :icon_comment_text, length: { allow_blank: true, maximum: ArchiveConfig.ICON_COMMENT_MAX,
-                                          too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_COMMENT_MAX) }
+                                          too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.ICON_COMMENT_MAX) }
 
   validates :email, email_format: { allow_blank: true }
 
-  validates :title, presence: { message: ts("Please enter a title to be displayed for your collection.") }
+  validates :title, presence: { message: I18n.t("collection.validation.title") }
   validates :title,
             length: { minimum: ArchiveConfig.TITLE_MIN,
-                      too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN) }
+                      too_short: I18n.t("collection.validation.length.min", min: ArchiveConfig.TITLE_MIN) }
   validates :title,
             length: { maximum: ArchiveConfig.TITLE_MAX,
-                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX) }
+                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.TITLE_MAX) }
   validate :no_reserved_strings
   def no_reserved_strings
-    errors.add(:title, ts("^Sorry, the ',' character cannot be in a collection Display Title.")) if
+    errors.add(:title, I18n.t("collection.validation.no_reserved_strings")) if
       title.match(/,/)
   end
 
@@ -144,14 +144,14 @@ def title
   validates :description,
             length: { allow_blank: true,
                       maximum: ArchiveConfig.SUMMARY_MAX,
-                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
+                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.SUMMARY_MAX) }
 
-  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: I18n.t("collection.header_image.invalid_url") }
-  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: I18n.t("collection.header_image.invalid_file_type"), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: I18n.t("collection.validation.header_image.invalid_url") }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: I18n.t("collection.validation.header_image.invalid_file_type"), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
-                      message: "^Sorry, a collection can only have %{count} tags." }
+                      message: I18n.t("collection.validation.num_tags") }
 
   scope :top_level, -> { where(parent_id: nil) }
   scope :closed, -> { joins(:collection_preference).where(collection_preferences: { closed: true }) }
diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
index cba0f3d7fe1..9409bac2471 100644
--- a/config/locales/models/en.yml
+++ b/config/locales/models/en.yml
@@ -236,9 +236,27 @@ en:
   attributes:
     ticket_number: Ticket ID
   collection:
-    header_image:
-      invalid_file_type: can only point to a gif, jpg, or png file.
-      invalid_url: is not a valid URL.
+    gift_exchange: Gift Exchange
+    prompt_meme: Prompt Meme
+    validation:
+      collection_depth: Sorry, but %{name} is a subcollection, so it can't also be a parent collection.
+      collection_not_found: We couldn't find a collection with name %{name}.
+      header_image:
+        invalid_file_type: can only point to a gif, jpg, or png file.
+        invalid_url: is not a valid URL.
+      invalid_parent: You can't make a collection its own parent.
+      length:
+        max: must be less than %{max} characters long.
+        min: must be at least %{min} characters long.
+      name:
+        format: must begin and end with a letter or number; it may also contain underscores. It may not contain any other characters, including spaces.
+        presence: Please enter a name for your collection.
+        uniqueness: Sorry, that name is already taken. Try again, please!
+      no_reserved_strings: "^Sorry, the ',' character cannot be in a collection Display Title."
+      no_valid_owners: Collection has no valid owners.
+      num_tags: "^Sorry, a collection can only have %{count} tags."
+      subcollection: You have to be a maintainer of %{name} to make a subcollection.
+      title: Please enter a title to be displayed for your collection.
   errors:
     attributes:
       ticket_number:

From e05c37b0f540a2fe1d31c1a7820d8716de56cd15 Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 22:10:41 -0700
Subject: [PATCH 18/20] Revert "internationalize collection.rb"

This reverts commit ec338b0cf2b7c00da62ceec451504a15736aa105.
---
 app/models/collection.rb     | 52 ++++++++++++++++++------------------
 config/locales/models/en.yml | 24 +++--------------
 2 files changed, 29 insertions(+), 47 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 8088a640f30..0c7b8b5278b 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -72,67 +72,67 @@ def clean_up_challenge
 
   CHALLENGE_TYPE_OPTIONS = [
     ["", ""],
-    [I18n.t("collection.gift_exchange"), "GiftExchange"],
-    [I18n.t("collection.prompt_meme"), "PromptMeme"]
+    [ts("Gift Exchange"), "GiftExchange"],
+    [ts("Prompt Meme"), "PromptMeme"]
   ].freeze
 
   validate :must_have_owners
   def must_have_owners
     # we have to use collection participants because the association may not exist until after
     # the collection is saved
-    errors.add(:base, I18n.t("collection.validation.no_valid_owners")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select(&:is_owner?)
+    errors.add(:base, ts("Collection has no valid owners.")) if (self.collection_participants + (self.parent ? self.parent.collection_participants : [])).select(&:is_owner?)
       .empty?
   end
 
   validate :collection_depth
   def collection_depth
-    errors.add(:base, I18n.t("collection.validation.collection_depth", name: parent.name)) if self.parent&.parent || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
+    errors.add(:base, ts("Sorry, but %{name} is a subcollection, so it can't also be a parent collection.", name: parent.name)) if self.parent&.parent || (self.parent && !self.children.empty?) || (!self.children.empty? && !self.children.collect(&:children).flatten.empty?)
   end
 
   validate :parent_exists
   def parent_exists
-    errors.add(:base, I18n.t("collection.validation.collection_not_found", name: parent_name)) unless parent_name.blank? || Collection.find_by(name: parent_name)
+    errors.add(:base, ts("We couldn't find a collection with name %{name}.", name: parent_name)) unless parent_name.blank? || Collection.find_by(name: parent_name)
   end
 
   validate :parent_is_allowed
   def parent_is_allowed
-    return unless parent
-    
-    if parent == self
-      errors.add(:base, I18n.t("collection.validation.invalid_parent"))
-    elsif parent_id_changed? && !parent.user_is_maintainer?(User.current_user)
-      errors.add(:base, I18n.t("collection.validation.subcollection", name: parent.name))
+    if parent
+      if parent == self
+        errors.add(:base, ts("You can't make a collection its own parent."))
+      elsif parent_id_changed? && !parent.user_is_maintainer?(User.current_user)
+        errors.add(:base, ts("You have to be a maintainer of %{name} to make a subcollection.", name: parent.name))
+      end
     end
   end
 
-  validates :name, presence: { message: I18n.t("collection.validation.name.presence") }
-  validates :name, uniqueness: { message: I18n.t("collection.validation.name.uniqueness") }
+  validates :name, presence: { message: ts("Please enter a name for your collection.") }
+  validates :name, uniqueness: { message: ts("Sorry, that name is already taken. Try again, please!") }
   validates :name,
             length: { minimum: ArchiveConfig.TITLE_MIN,
-                      too_short: I18n.t("collection.validation.length.min", min: ArchiveConfig.TITLE_MIN) }
+                      too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN) }
   validates :name,
             length: { maximum: ArchiveConfig.TITLE_MAX,
-                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.TITLE_MAX) }
+                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX) }
   validates :name,
-            format: { message: I18n.t("collection.validation.name.format"),
+            format: { message: ts("must begin and end with a letter or number; it may also contain underscores. It may not contain any other characters, including spaces."),
                       with: /\A[A-Za-z0-9]\w*[A-Za-z0-9]\Z/ }
   validates :icon_alt_text, length: { allow_blank: true, maximum: ArchiveConfig.ICON_ALT_MAX,
-                                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.ICON_ALT_MAX) }
+                                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_ALT_MAX) }
   validates :icon_comment_text, length: { allow_blank: true, maximum: ArchiveConfig.ICON_COMMENT_MAX,
-                                          too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.ICON_COMMENT_MAX) }
+                                          too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_COMMENT_MAX) }
 
   validates :email, email_format: { allow_blank: true }
 
-  validates :title, presence: { message: I18n.t("collection.validation.title") }
+  validates :title, presence: { message: ts("Please enter a title to be displayed for your collection.") }
   validates :title,
             length: { minimum: ArchiveConfig.TITLE_MIN,
-                      too_short: I18n.t("collection.validation.length.min", min: ArchiveConfig.TITLE_MIN) }
+                      too_short: ts("must be at least %{min} characters long.", min: ArchiveConfig.TITLE_MIN) }
   validates :title,
             length: { maximum: ArchiveConfig.TITLE_MAX,
-                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.TITLE_MAX) }
+                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.TITLE_MAX) }
   validate :no_reserved_strings
   def no_reserved_strings
-    errors.add(:title, I18n.t("collection.validation.no_reserved_strings")) if
+    errors.add(:title, ts("^Sorry, the ',' character cannot be in a collection Display Title.")) if
       title.match(/,/)
   end
 
@@ -144,14 +144,14 @@ def title
   validates :description,
             length: { allow_blank: true,
                       maximum: ArchiveConfig.SUMMARY_MAX,
-                      too_long: I18n.t("collection.validation.length.max", max: ArchiveConfig.SUMMARY_MAX) }
+                      too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
-  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: I18n.t("collection.validation.header_image.invalid_url") }
-  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: I18n.t("collection.validation.header_image.invalid_file_type"), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: I18n.t("collection.header_image.invalid_url") }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: I18n.t("collection.header_image.invalid_file_type"), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
-                      message: I18n.t("collection.validation.num_tags") }
+                      message: "^Sorry, a collection can only have %{count} tags." }
 
   scope :top_level, -> { where(parent_id: nil) }
   scope :closed, -> { joins(:collection_preference).where(collection_preferences: { closed: true }) }
diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
index 9409bac2471..cba0f3d7fe1 100644
--- a/config/locales/models/en.yml
+++ b/config/locales/models/en.yml
@@ -236,27 +236,9 @@ en:
   attributes:
     ticket_number: Ticket ID
   collection:
-    gift_exchange: Gift Exchange
-    prompt_meme: Prompt Meme
-    validation:
-      collection_depth: Sorry, but %{name} is a subcollection, so it can't also be a parent collection.
-      collection_not_found: We couldn't find a collection with name %{name}.
-      header_image:
-        invalid_file_type: can only point to a gif, jpg, or png file.
-        invalid_url: is not a valid URL.
-      invalid_parent: You can't make a collection its own parent.
-      length:
-        max: must be less than %{max} characters long.
-        min: must be at least %{min} characters long.
-      name:
-        format: must begin and end with a letter or number; it may also contain underscores. It may not contain any other characters, including spaces.
-        presence: Please enter a name for your collection.
-        uniqueness: Sorry, that name is already taken. Try again, please!
-      no_reserved_strings: "^Sorry, the ',' character cannot be in a collection Display Title."
-      no_valid_owners: Collection has no valid owners.
-      num_tags: "^Sorry, a collection can only have %{count} tags."
-      subcollection: You have to be a maintainer of %{name} to make a subcollection.
-      title: Please enter a title to be displayed for your collection.
+    header_image:
+      invalid_file_type: can only point to a gif, jpg, or png file.
+      invalid_url: is not a valid URL.
   errors:
     attributes:
       ticket_number:

From ff9a9c098ef1fcc5f0c4124ec807d495013e9dca Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Sun, 22 Sep 2024 22:11:55 -0700
Subject: [PATCH 19/20] Revert "see if rubocop will let me get away with not
 fixing all instances of ts()"

This reverts commit 450dbeb5b9d068841d91fc5f143b9234a22390bf.
---
 app/models/collection.rb     | 4 ++--
 config/locales/models/en.yml | 4 ----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 0c7b8b5278b..c36f80ac8e4 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -146,8 +146,8 @@ def title
                       maximum: ArchiveConfig.SUMMARY_MAX,
                       too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
-  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: I18n.t("collection.header_image.invalid_url") }
-  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: I18n.t("collection.header_image.invalid_file_type"), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
index cba0f3d7fe1..83d9a8a6b4d 100644
--- a/config/locales/models/en.yml
+++ b/config/locales/models/en.yml
@@ -235,10 +235,6 @@ en:
         other: Works
   attributes:
     ticket_number: Ticket ID
-  collection:
-    header_image:
-      invalid_file_type: can only point to a gif, jpg, or png file.
-      invalid_url: is not a valid URL.
   errors:
     attributes:
       ticket_number:

From e86c8a0e40f7195de00bba870d19d08f97aa0ada Mon Sep 17 00:00:00 2001
From: Cesium <cindydxz@gmail.com>
Date: Mon, 23 Sep 2024 17:37:05 -0700
Subject: [PATCH 20/20] internationalize default email, minor refactoring, add
 more UTs

---
 .../challenge_assignments_controller.rb       |  5 +--
 app/models/challenge_assignment.rb            | 10 ++---
 app/models/collection.rb                      | 30 ++++++-------
 app/models/potential_match.rb                 |  6 +--
 config/locales/models/en.yml                  |  7 ++++
 .../notification_emails.feature               | 42 ++++++++++++++++++-
 .../step_definitions/email_custom_steps.rb    | 28 +++++++++++++
 7 files changed, 99 insertions(+), 29 deletions(-)

diff --git a/app/controllers/challenge_assignments_controller.rb b/app/controllers/challenge_assignments_controller.rb
index 3f959ec8302..1e90132adde 100644
--- a/app/controllers/challenge_assignments_controller.rb
+++ b/app/controllers/challenge_assignments_controller.rb
@@ -219,11 +219,10 @@ def default_all
   def default
     @challenge_assignment.defaulted_at = Time.now
     @challenge_assignment.save
-    offer_byline = @challenge_assignment.offer_byline
-    request_byline = @challenge_assignment.request_byline
+    
     assignments_page_url = collection_assignments_url(@challenge_assignment.collection)
     
-    @challenge_assignment.collection.notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
+    @challenge_assignment.collection.notify_maintainers_challenge_default(@challenge_assignment, assignments_page_url)
 
     flash[:notice] = "We have notified the collection maintainers that you had to default on your assignment."
     redirect_to user_assignments_path(current_user)
diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
index ab4dee0ff42..80b37738451 100755
--- a/app/models/challenge_assignment.rb
+++ b/app/models/challenge_assignment.rb
@@ -214,7 +214,7 @@ def offer_byline
     if offer_signup && offer_signup.pseud
       offer_signup.pseud.byline
     else
-      (pinch_hitter ? (pinch_hitter.byline + "* (pinch hitter)") : "- none -")
+      (pinch_hitter ? I18n.t("challenge_assignment.offer_byline.pinch_hitter", pinch_hitter_byline: pinch_hitter.byline) : I18n.t("challenge_assignment.offer_byline.none"))
     end
   end
 
@@ -222,7 +222,7 @@ def request_byline
     if request_signup && request_signup.pseud
       request_signup.pseud.byline
     else
-      (pinch_request_signup ? (pinch_request_byline + "* (pinch recipient)") : "- None -")
+      (pinch_request_signup ? I18n.t("challenge_assignment.request_byline.pinch_recipient", pinch_request_byline: pinch_request_byline) : I18n.t("challenge_assignment.request_byline.none"))
     end
   end
 
@@ -387,10 +387,8 @@ def self.delayed_generate(collection_id)
     end
     REDIS_GENERAL.del(progress_key(collection))
 
-    if collection.email.present?
-      UserMailer.potential_match_generation_notification(collection.id, collection.email).deliver_later
-    elsif collection.parent && collection.parent.email.present?
-      UserMailer.potential_match_generation_notification(collection.id, collection.parent.email).deliver_later
+    if collection.collection_email.present?
+      UserMailer.potential_match_generation_notification(collection.id, collection.collection_email).deliver_later
     else
       collection.maintainers_list.each do |user|
         I18n.with_locale(user.preference.locale.iso) do
diff --git a/app/models/collection.rb b/app/models/collection.rb
index c36f80ac8e4..46732943b46 100755
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -147,7 +147,7 @@ def title
                       too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
   validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
-  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file.") }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
@@ -337,13 +337,16 @@ def maintainers_list
     self.maintainers.collect(&:user).flatten.uniq
   end
 
+  def collection_email
+    return self.email if self.email.present?
+    return parent.email if parent && parent.email.present? 
+  end
+
   def notify_maintainers_assignments_sent
     subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
     message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
-    if self.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
-    elsif self.parent && self.parent.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
+    if self.collection_email.present?
+      UserMailer.collection_notification(self.id, subject, message, self.collection_email).deliver_later
     else
       # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
       self.maintainers_list.each do |user|
@@ -356,20 +359,17 @@ def notify_maintainers_assignments_sent
     end
   end
 
-  def notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
-    subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline)
-    message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
-
-    if self.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
-    elsif self.parent && self.parent.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
+  def notify_maintainers_challenge_default(challenge_assignment, assignments_page_url)
+    if self.collection_email.present?
+      subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: challenge_assignment.offer_byline)
+      message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: challenge_assignment.offer_byline, request_byline: challenge_assignment.request_byline, assignments_page_url: assignments_page_url)
+      UserMailer.collection_notification(self.id, subject, message, self.collection_email).deliver_later
     else
       # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
       self.maintainers_list.each do |user|
         I18n.with_locale(user.preference.locale.iso) do
-          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline)
-          translated_message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
+          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: challenge_assignment.offer_byline)
+          translated_message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: challenge_assignment.offer_byline, request_byline: challenge_assignment.request_byline, assignments_page_url: assignments_page_url)
           UserMailer.collection_notification(self.id, translated_subject, translated_message, user.email).deliver_later
         end
       end
diff --git a/app/models/potential_match.rb b/app/models/potential_match.rb
index 4ee894dc5a9..0927aeb660c 100644
--- a/app/models/potential_match.rb
+++ b/app/models/potential_match.rb
@@ -90,10 +90,8 @@ def self.generate_in_background(collection_id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
       
-      if collection.email.present?
-        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.email).deliver_later
-      elsif collection.parent && collection.parent.email.present?
-        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.parent.email).deliver_later
+      if collection.collection_email.present?
+        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.collection_email).deliver_later
       else
         collection.maintainers_list.each do |user|
           I18n.with_locale(user.preference.locale.iso) do
diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
index 83d9a8a6b4d..0424c928c2f 100644
--- a/config/locales/models/en.yml
+++ b/config/locales/models/en.yml
@@ -235,6 +235,13 @@ en:
         other: Works
   attributes:
     ticket_number: Ticket ID
+  challenge_assignment:
+    offer_byline:
+      none: "- none -"
+      pinch_hitter: "%{pinch_hitter_byline}* (pinch hitter)"
+    request_byline:
+      none: "- None -"
+      pinch_recipient: "%{pinch_request_byline}* (pinch recipient)"
   errors:
     attributes:
       ticket_number:
diff --git a/features/gift_exchanges/notification_emails.feature b/features/gift_exchanges/notification_emails.feature
index edbcc197fbc..c36449ba17f 100644
--- a/features/gift_exchanges/notification_emails.feature
+++ b/features/gift_exchanges/notification_emails.feature
@@ -1,7 +1,7 @@
 Feature: Gift Exchange Notification Emails
   Make sure that gift exchange notification emails are formatted properly
 
-  Scenario: Assignment notification emails should be sent to two owners in their respective locales
+  Scenario: Assignment notification emails should be sent to two owners in their respective locales when assignments are generated
     Given I have created the tagless gift exchange "Holiday Swap"
       And I open signups for "Holiday Swap"
     
@@ -32,6 +32,46 @@ Feature: Gift Exchange Notification Emails
       And "participant1" should receive 1 email
       And "participant2" should receive 1 email
 
+  Scenario: If collection email is set, use the collection email instead of moderator emails
+    Given I have created the tagless gift exchange "Holiday Swap"
+      And I open signups for "Holiday Swap"
+      And I am logged in as "participant1"
+      And I start signing up for "Holiday Swap"
+      And I press "Submit"
+      And I am logged in as "participant2"
+      And I start signing up for "Holiday Swap"
+      And I press "Submit"
+      And I have added a co-moderator "mod2" to collection "Holiday Swap"
+      And I go to "Holiday Swap" collection's page
+      And I follow "Collection Settings"
+      And I fill in "Collection email" with "test@archiveofourown.org"
+      And I press "Update"
+      And I close signups for "Holiday Swap"
+      And I have generated matches for "Holiday Swap"
+      And I have sent assignments for "Holiday Swap"
+    Then 3 emails should be delivered
+      And 1 email should be delivered to test@archiveofourown.org
+      And the email should contain "You have received a message about your collection"
+
+  Scenario: Default notification emails should be sent to two owners in their respective locales when a user defaults on an assignment
+    
+    Given everyone has their assignments for "Holiday Swap"
+      And I have added a co-moderator "mod2" to collection "Holiday Swap"
+      And a locale with translated emails
+      And the user "mod1" enables translated emails
+
+    When I am logged in as "myname1"
+      And I go to my assignments page
+      And I follow "Default"
+    Then I should see "We have notified the collection maintainers that you had to default on your assignment."
+      And 7 emails should be delivered
+      And "mod1" should receive 2 emails
+      And the last email to "mod1" should be translated
+      And the last email should contain "defaulted on their assignment"
+      And "mod2" should receive 1 email
+      And the email to "mod2" should be non-translated
+      And the email should contain "defaulted on their assignment"
+
   Scenario: Assignment notifications with linebreaks.
     Given I have created the tagless gift exchange "Holiday Swap"
       And I open signups for "Holiday Swap"
diff --git a/features/step_definitions/email_custom_steps.rb b/features/step_definitions/email_custom_steps.rb
index b9c3b2932a0..e03db12e2da 100644
--- a/features/step_definitions/email_custom_steps.rb
+++ b/features/step_definitions/email_custom_steps.rb
@@ -27,6 +27,12 @@
   step(%{the email to "#{user}" should not contain "translation missing"}) # missing translations in the target language fall back to English
 end
 
+Then "the last email to {string} should be translated" do |user|
+  step(%{the last email to "#{user}" should contain "Translated footer"})
+  step(%{the last email to "#{user}" should not contain "fan-run and fan-supported archive"}) # untranslated English text
+  step(%{the last email to "#{user}" should not contain "translation missing"}) # missing translations in the target language fall back to English
+end
+
 Then "the email to {string} should be non-translated" do |user|
   step(%{the email to "#{user}" should not contain "Translated footer"})
   step(%{the email to "#{user}" should contain "fan-run and fan-supported archive"})
@@ -54,6 +60,17 @@
   end
 end
 
+Then "the last email to {string} should contain {string}" do |user, text|
+  @user = User.find_by(login: user)
+  email = emails("to: \"#{email_for(@user.email)}\"").last
+  if email.multipart?
+    expect(email.text_part.body).to match(text)
+    expect(email.html_part.body).to match(text)
+  else
+    expect(email.body).to match(text)
+  end
+end
+
 Then "the email to {string} should not contain {string}" do |user, text|
   @user = User.find_by(login: user)
   email = emails("to: \"#{email_for(@user.email)}\"").first
@@ -65,6 +82,17 @@
   end
 end
 
+Then "the last email to {string} should not contain {string}" do |user, text|
+  @user = User.find_by(login: user)
+  email = emails("to: \"#{email_for(@user.email)}\"").last
+  if email.multipart?
+    expect(email.text_part.body).not_to match(text)
+    expect(email.html_part.body).not_to match(text)
+  else
+    expect(email.body).not_to match(text)
+  end
+end
+
 Then "{string} should receive {int} email(s)" do |user, count|
   @user = User.find_by(login: user)
   expect(emails("to: \"#{email_for(@user.email)}\"").size).to eq(count.to_i)