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

AO3-6872 Only notify creators when a work is added to a collection by an archivist #5024

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

AO3-6872 Only notify creators when a work is added to a collection by an archivist #5024

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e57e2e4
AO3-6872 only notify creators when a work is added to a collection
marcus8448 Jan 11, 2025
62bb586
AO3-6872 add test for archivist collection notification
marcus8448 Jan 11, 2025
a292d86
AO3-6872 add test for archivist bookmark creation
marcus8448 Jan 11, 2025
a3968e7
AO3-6872 simplify test name
marcus8448 Jan 11, 2025
b62656c
Revert "AO3-6872 add test for archivist collection notification"
marcus8448 Jan 11, 2025
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
2 changes: 1 addition & 1 deletion app/models/collection_item.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def notify_of_association
# Sends emails to item creator(s) in the case that an archivist
# has added them to the collection.
def notify_archivist_added
return unless User.current_user&.archivist && collection.user_is_maintainer?(User.current_user)
return unless item.is_a?(Work) && User.current_user&.archivist && collection.user_is_maintainer?(User.current_user)

item.users.each do |email_recipient|
next if email_recipient.preference.collection_emails_off
Expand Down
10 changes: 10 additions & 0 deletions features/bookmarks/bookmark_create.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,3 +448,13 @@ Scenario: A bookmark with duplicate tags other than capitalization has only firs
And I fill in "Relationships" with "Relationship 1, Relationship 2"
And I press "Create"
Then I should see "Fandom, relationship, and character tags must not add up to more than 5. You have entered 6 of these tags, so you must remove 1 of them."

Scenario: Bookmark can be added to collection by Archivist without error (Issue 6872)
marcus8448 marked this conversation as resolved.
Show resolved Hide resolved
Given I have an archivist "archivist"
And I am logged in as "archivist"
And I create the collection "My Collection" with name "MyCollection"
When I open a bookmarkable work
And I follow "Bookmark"
And I fill in "bookmark_collection_names" with "MyCollection"
And I press "Create"
Then I should see "Bookmark was successfully created"
12 changes: 12 additions & 0 deletions features/collections/collection_notification.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ Feature: Collectible items email
And I should see "Antarctic Penguins"
And 1 email should be delivered to [email protected]

Scenario: Work added to collection by Archivist sends notification email
Given I have a work "cool work"
And I have an archivist "archivist"
And all emails have been delivered
When I am logged in as "archivist"
And I create the collection "Cold Collection" with name "ColdCollection"
And I view the work "cool work"
And I follow "Add to Collections"
And I fill in "collection_names" with "ColdCollection"
And I press "Add"
Then 1 email should be delivered
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a check for the email subject or text to make sure it's the right email?

Copy link
Member

Choose a reason for hiding this comment

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

FYI looks like #5023 is adding the same test for this mailer. Let's get that one merged first.

Copy link
Author

Choose a reason for hiding this comment

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

I've removed the test (as it is slightly tangential to the bug being fixed). If necessary, I can append a check for the email recipient and body to the test in #5023 after it is merged.


Scenario: Bookmark added to collection sends notification email
Given all email have been delivered
When I have the collection "Dont Bookmark Me Bro" with name "dont_bookmark_me_bro"
Expand Down
Loading