Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AO3-6268 Fix All challenge owners/moderators get CCed in "Assignments sent" notification #4912

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

AO3-6268 Fix All challenge owners/moderators get CCed in "Assignments sent" notification #4912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
25e6ed1
let each challenge owner receive their own email rather than all chal…
Cesium-Ice Sep 16, 2024
fdfca33
address rubocop
Cesium-Ice Sep 16, 2024
c8d8fe2
remove redundant return
Cesium-Ice Sep 16, 2024
7fb0d0b
another reviewdog fix
Cesium-Ice Sep 16, 2024
7b51ffd
rubocop autocorrections
Cesium-Ice Sep 18, 2024
d4da2da
address Bilka comments
Cesium-Ice Sep 18, 2024
1f0242e
rubocop linter changes
Cesium-Ice Sep 18, 2024
f9e91ce
fix unused translation
Cesium-Ice Sep 18, 2024
5e92d68
yet more rubocop autocorrection changes
Cesium-Ice Sep 18, 2024
a4f6c63
fix broken UTs by fixing defining t in notify_maintainers_challenge_s…
Cesium-Ice Sep 18, 2024
85681cb
rubocop change keyword argument from default = true to default: true
Cesium-Ice Sep 18, 2024
13bd956
address bilka comments
Cesium-Ice Sep 22, 2024
cde3a74
address rubocop and fix broken cucumber test
Cesium-Ice Sep 22, 2024
047a875
will rubocop finally be satisfied?
Cesium-Ice Sep 22, 2024
450dbeb
see if rubocop will let me get away with not fixing all instances of …
Cesium-Ice Sep 22, 2024
c28acc5
rubocop change not about ts
Cesium-Ice Sep 22, 2024
ec338b0
internationalize collection.rb
Cesium-Ice Sep 22, 2024
e05c37b
Revert "internationalize collection.rb"
Cesium-Ice Sep 23, 2024
ff9a9c0
Revert "see if rubocop will let me get away with not fixing all insta…
Cesium-Ice Sep 23, 2024
e86c8a0
internationalize default email, minor refactoring, add more UTs
Cesium-Ice Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions app/controllers/challenge_assignments_controller.rb
View file
Open in desktop
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
@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)}")

assignments_page_url = collection_assignments_url(@challenge_assignment.collection)

@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)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@
end
end

if params[:sole_author] == 'keep_pseud' || params[:sole_author] == 'orphan_pseud'

Check warning on line 308 in app/controllers/users_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Convert `if-elsif` to `case-when`.

Raw Output:
app/controllers/users_controller.rb:308:5: C: Style/CaseLikeIf: Convert `if-elsif` to `case-when`.
# Orphans works where user is the sole author.

pseuds = @user.pseuds
Expand All @@ -315,7 +315,7 @@
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)
Expand Down
55 changes: 26 additions & 29 deletions app/mailers/user_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
@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"

Check warning on line 34 in app/mailers/user_mailer.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer string interpolation to string concatenation.

Raw Output:
app/mailers/user_mailer.rb:34:52: C: Style/StringConcatenation: Prefer string interpolation to string concatenation.
)

Check warning on line 35 in app/mailers/user_mailer.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Indent `)` to column 4 (not 7)

Raw Output:
app/mailers/user_mailer.rb:35:8: C: Layout/ClosingParenthesisIndentation: Indent `)` to column 4 (not 7)
Comment on lines -33 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to avoid this specific rabbithole of rubocop warnings from reviewdog by not changing this unrelated bit of code at all.

end

# We use an options hash here, instead of keyword arguments, to avoid
Expand Down Expand Up @@ -72,7 +72,7 @@
# 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)
Expand Down Expand Up @@ -116,20 +116,22 @@
# 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
creation_entries.each do |creation_info|
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

Expand All @@ -142,9 +144,7 @@
@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,
Expand All @@ -160,7 +160,7 @@
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: default_i18n_subject(app_name: ArchiveConfig.APP_SHORT_NAME)
)
end
end
Expand All @@ -173,37 +173,35 @@
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

# 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}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Regarding your comment on Jira, this is the related issue: The subject locale should include the whole string with the placeholders for the short_name and the collection title, not just the last part.

But not relevant for this PR.)

)
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
Expand All @@ -227,7 +225,7 @@
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
Expand All @@ -240,7 +238,7 @@
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
Expand Down Expand Up @@ -322,7 +320,7 @@
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|
Expand Down Expand Up @@ -351,7 +349,7 @@
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
Expand All @@ -371,7 +369,7 @@
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
Expand All @@ -394,8 +392,8 @@
@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

Expand All @@ -405,6 +403,7 @@
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?
Expand All @@ -428,6 +427,4 @@
)
end

protected

end
Loading
Loading