Skip to content

Commit

Permalink
address bilka comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Cesium-Ice committed Sep 22, 2024
1 parent 85681cb commit 13bd956
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 27 deletions.
8 changes: 5 additions & 3 deletions app/controllers/challenge_assignments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ###
Expand Down
15 changes: 11 additions & 4 deletions app/models/challenge_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
45 changes: 34 additions & 11 deletions app/models/collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Check failure

Code scanning / Brakeman

Insufficient validation for header_image_url using /\.(png|gif|jpg)\z/. Use \A and \z as anchors. Error

Insufficient validation for header\_image\_url using /\.(png|gif|jpg)\z/. Use \A and \z as anchors.

validates :tags_after_saving,
length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
Expand Down Expand Up @@ -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
Expand Down
13 changes: 10 additions & 3 deletions app/models/potential_match.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion config/locales/mailers/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}:'
Expand Down
6 changes: 4 additions & 2 deletions features/gift_exchanges/notification_emails.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 13bd956

Please sign in to comment.