From 113e0da46c3b4969328436f6d4c553faaafef521 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 13:22:40 -0700 Subject: [PATCH 01/14] First attempt at a solution --- app/models/subscription.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/models/subscription.rb b/app/models/subscription.rb index a04a9d7ccbc..9a1ed17afca 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -37,13 +37,23 @@ def name end end + def creator_no_longer_associated_with?(creation) + return false unless subscribable_type == "User" + + # If any of the creation's pseud's users matches the subscribable, then the + # creator still matches, so we return "true" + return false if creation.pseuds.any? { |p| p.user == subscribable } + + # We reach this case if e.g. someone is subscribed to a user, but they + # orphan the work before the subscription notification would be sent + return true + # Guard against scenarios that may break anonymity or other things. # Emails should only contain works or chapters. # Emails should only contain posted works or chapters. # Emails should never contain chapters of draft works. # Emails should never contain hidden works or chapters of hidden works. - # Emails should never contain orphaned works or chapters. - # TODO: AO3-3620 & AO3-5696: Allow subscriptions to orphan_account to receive notifications. + # Emails should not contain orphaned works or chapters if the subscription is to a creator no longer associated with the work # Emails for user subs should never contain anon works or chapters. # Emails for work subs should never contain anything but chapters. # TODO: AO3-1250: Anon series subscription improvements @@ -52,7 +62,7 @@ def valid_notification_entry?(creation) return false unless creation.try(:posted) return false if creation.is_a?(Chapter) && !creation.work.try(:posted) return false if creation.try(:hidden_by_admin) || (creation.is_a?(Chapter) && creation.work.try(:hidden_by_admin)) - return false if creation.pseuds.any? { |p| p.user == User.orphan_account } + return false if creator_no_longer_associated_with?(creation) return false if subscribable_type == "User" && creation.anonymous? return false if subscribable_type == "Work" && !creation.is_a?(Chapter) From 456e23790d81a62ed52d1839a520563f8b925647 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 14:27:36 -0700 Subject: [PATCH 02/14] Fix syntax whoops --- app/models/subscription.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 9a1ed17afca..c092a75d72e 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -38,6 +38,7 @@ def name end def creator_no_longer_associated_with?(creation) + # If we're subscribed to a work or series, it doesn't matter who the creator is, we should send the notification return false unless subscribable_type == "User" # If any of the creation's pseud's users matches the subscribable, then the @@ -46,7 +47,8 @@ def creator_no_longer_associated_with?(creation) # We reach this case if e.g. someone is subscribed to a user, but they # orphan the work before the subscription notification would be sent - return true + true + end # Guard against scenarios that may break anonymity or other things. # Emails should only contain works or chapters. From ab12d89d8facf304e9914a30d1e0942a5b13d4f3 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 14:54:43 -0700 Subject: [PATCH 03/14] Add some tests --- spec/models/subscription_spec.rb | 38 +++++++++++++++++++------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index 0c2865d87ce..33d53352613 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -111,12 +111,14 @@ describe "#valid_notification_entry?" do let(:subscription) { build(:subscription) } let(:series) { build(:series) } - let(:work) { build(:work) } + let(:author) { create(:user) } + let(:author_pseud) { author.default_pseud } + let(:work) { create(:work, authors: [author_pseud]) } let(:draft) { build(:draft) } - let(:chapter) { build(:chapter) } - let(:anon_work) { build(:work, collections: [build(:anonymous_collection)]) } + let(:chapter) { build(:chapter, authors: [author_pseud]) } + let(:anon_work) { build(:work, authors: [author_pseud], collections: [build(:anonymous_collection)]) } let(:anon_series) { build(:series, works: [anon_work]) } - let(:anon_chapter) { build(:chapter, work: anon_work) } + let(:anon_chapter) { build(:chapter, authors:[author_pseud], work: anon_work) } let(:orphan_pseud) { create(:user, login: "orphan_account").default_pseud } it "returns false when the creation is nil" do @@ -132,11 +134,6 @@ expect(subscription.valid_notification_entry?(draft)).to be_falsey end - # TODO: AO3-3620 & AO3-5696: Allow subscriptions to orphan_account to receive notifications - it "returns false when the creation is by orphan_account" do - expect(subscription.valid_notification_entry?(create(:work, authors: [orphan_pseud]))).to be_falsey - end - it "returns false when the creation is hidden_by_admin" do expect(subscription.valid_notification_entry?(build(:work, hidden_by_admin: true))).to be_falsey end @@ -151,11 +148,6 @@ expect(subscription.valid_notification_entry?(build(:chapter, work: draft))).to be_falsey end - # TODO: AO3-3620 & AO3-5696: Allow subscriptions to orphan_account to receive notifications - it "returns false when the creation is by orphan_account" do - expect(subscription.valid_notification_entry?(create(:chapter, authors: [orphan_pseud]))).to be_falsey - end - it "returns false when the chapter is on a hidden work" do expect(subscription.valid_notification_entry?(build(:chapter, work: build(:work, hidden_by_admin: true)))).to be_falsey end @@ -205,7 +197,7 @@ end context "when subscribable is a user" do - let(:subscription) { build(:subscription, subscribable: create(:user)) } + let(:subscription) { build(:subscription, subscribable_type: "User", subscribable: author) } it "returns true for a non-anonymous work" do expect(subscription.valid_notification_entry?(work)).to be_truthy @@ -222,6 +214,22 @@ it "returns false for an anonymous chapter" do expect(subscription.valid_notification_entry?(anon_chapter)).to be_falsey end + + it "returns false for a work by a different user (orphan_account)" do + expect(subscription.valid_notification_entry?(create(:work, authors: [orphan_pseud]))).to be_falsey + end + + it "returns false for a chapter by a different user (orphan_account)" do + expect(subscription.valid_notification_entry?(create(:chapter, authors: [orphan_pseud]))).to be_falsey + end + + it "returns false for a work by a different user" do + expect(subscription.valid_notification_entry?(create(:work, authors: [create(:user).default_pseud]))).to be_falsey + end + + it "returns false for a chapter by a different user" do + expect(subscription.valid_notification_entry?(create(:chapter, authors: [create(:user).default_pseud]))).to be_falsey + end end context "when subscribable is a work" do From 09b00a10abe2742d0da1bdbe8db2b721fd924d66 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 15:05:33 -0700 Subject: [PATCH 04/14] Fix user_mailer_spec --- spec/mailers/user_mailer_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 27a57650a67..6885e197e03 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -740,6 +740,7 @@ def queue_adapter_for_test let(:creator2) { create(:user).default_pseud } let(:work) { create(:work, authors: [creator1, creator2]) } let(:chapter) { create(:chapter, work: work, authors: [creator1]) } + let(:subscription) { create(:subscription, subscribable: creator1.user) } it "has the correct subject line" do subject = "[#{ArchiveConfig.APP_SHORT_NAME}] #{creator1.byline} posted #{chapter.chapter_header} of #{work.title}" From 241f4c710d133aa3a98c7a49e22c4e73ef7f7f11 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 15:11:55 -0700 Subject: [PATCH 05/14] Tweak failing tests to see what happens --- spec/models/subscription_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index 33d53352613..c2cb9ebe3d6 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -111,8 +111,7 @@ describe "#valid_notification_entry?" do let(:subscription) { build(:subscription) } let(:series) { build(:series) } - let(:author) { create(:user) } - let(:author_pseud) { author.default_pseud } + let(:author_pseud) { create(:user).default_pseud } let(:work) { create(:work, authors: [author_pseud]) } let(:draft) { build(:draft) } let(:chapter) { build(:chapter, authors: [author_pseud]) } @@ -197,9 +196,11 @@ end context "when subscribable is a user" do - let(:subscription) { build(:subscription, subscribable_type: "User", subscribable: author) } + let(:subscription) { build(:subscription, subscribable: author_pseud.user) } it "returns true for a non-anonymous work" do + # FIXME: make sure subscription is being set up properly + expect(subscribable.subscribable_type).to be("User") expect(subscription.valid_notification_entry?(work)).to be_truthy end From 51bf1fad106a8b8028dbfaa8c30b262204a2e47f Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 15:30:44 -0700 Subject: [PATCH 06/14] Fix orphan_work.feature tests --- features/other_a/orphan_work.feature | 86 +++++++++++++++------------- 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/features/other_a/orphan_work.feature b/features/other_a/orphan_work.feature index f8d67087d45..fe8ac6ed60f 100644 --- a/features/other_a/orphan_work.feature +++ b/features/other_a/orphan_work.feature @@ -8,9 +8,9 @@ Feature: Orphan work Background: Given I have an orphan account And the following activated users exists - | login | password | email | - | orphaneer | password | orphaneer@foo.com | - | author_subscriber | password | author_subscriber@foo.com | + | login | password | email | + | orphaneer | password | orphaneer@foo.com | + | author_subscriber | password | author_subscriber@foo.com | And "author_subscriber" subscribes to author "orphaneer" And all emails have been delivered And I am logged in as "orphaneer" @@ -53,7 +53,7 @@ Feature: Orphan work When I follow "Orphan Work" Then I should see "Read More About The Orphaning Process" When I choose "Leave a copy of my pseud on" - And I press "Yes, I'm sure" + And I press "Yes, I'm sure" Then I should see "Orphaning was successful." When I go to orphaneer's works page Then I should not see "Shenanigans2" @@ -84,68 +84,76 @@ Feature: Orphan work Then 0 emails should be delivered - Scenario: Orphan a work (leave pseud) and don't notify subscribers to the work + Scenario: Orphan a work (leave pseud) but do notify subscribers to the work Given the following activated user exists - | login | password | email | - | work_subscriber | password | work_subscriber@foo.com | + | login | password | email | + | work_subscriber | password | work_subscriber@foo.com | And I post the work "Torrid Idfic" And I am logged in as "work_subscriber" And I view the work "Torrid Idfic" And I press "Subscribe" + And I am logged in as "orphaneer" And a chapter is added to "Torrid Idfic" And I follow "Edit" And I follow "Orphan Work" And I choose "Leave a copy of my pseud on" And I press "Yes, I'm sure" When subscription notifications are sent - Then 0 emails should be delivered + Then 1 email should be delivered - Scenario: Orphan a work (leave pseud) and don't notify subscribers to the work's series + Scenario: Orphan a work (leave pseud) but do notify subscribers to the work's series + # Make the series exist, but make sure the initial work doesn't + # send an email for series_subscriber Given the following activated user exists - | login | password | email | - | series_subscriber | password | series_subscriber@foo.com | - And I add the work "Lazy Purple Sausage" to series "Shame Series" - And I am logged in as "series_subscriber" + | login | password | email | + | series_subscriber | password | series_subscriber@foo.com | + And I add the work "Work to create the series" to series "Shame Series" + When subscription notifications are sent + Then 0 emails should be delivered + # Subscribe + When I am logged in as "series_subscriber" And I view the series "Shame Series" And I press "Subscribe" + # Add a new work to the series, which we then immediately orphan And I am logged in as "orphaneer" + And I add the work "Lazy Purple Sausage" to series "Shame Series" And I view the work "Lazy Purple Sausage" And I follow "Edit" And I follow "Orphan Work" And I choose "Leave a copy of my pseud on" And I press "Yes, I'm sure" When subscription notifications are sent - Then 0 emails should be delivered + Then 1 email should be delivered Scenario: I can orphan multiple works at once - Given I am logged in as "author" - And I post the work "Glorious" with fandom "SGA" - And I post the work "Excellent" with fandom "Star Trek" - And I post the work "Lovely" with fandom "Steven Universe" - And I go to author's works page - When I follow "Edit Works" - Then I should see "Edit Multiple Works" - When I select "Glorious" for editing - And I select "Excellent" for editing - And I press "Delete" - Then I should see "Are you sure you want to delete these works PERMANENTLY?" - And I should see "Glorious" - And I should see "Excellent" - And I should not see "Lovely" - # Delay before orphaning to make sure the cache is expired - And it is currently 1 second from now - When I follow "Orphan Works Instead" - Then I should see "Orphaning a work removes it from your account and re-attaches it to the specially created orphan_account." - When I press "Yes, I'm sure" - And all indexing jobs have been run - Then I should see "Orphaning was successful." - When I go to author's works page - Then I should not see "Glorious" - And I should not see "Excellent" - And I should see "Lovely" + Given I am logged in as "author" + And I post the work "Glorious" with fandom "SGA" + And I post the work "Excellent" with fandom "Star Trek" + And I post the work "Lovely" with fandom "Steven Universe" + And I go to author's works page + When I follow "Edit Works" + Then I should see "Edit Multiple Works" + When I select "Glorious" for editing + And I select "Excellent" for editing + And I press "Delete" + Then I should see "Are you sure you want to delete these works PERMANENTLY?" + And I should see "Glorious" + And I should see "Excellent" + And I should not see "Lovely" + # Delay before orphaning to make sure the cache is expired + And it is currently 1 second from now + When I follow "Orphan Works Instead" + Then I should see "Orphaning a work removes it from your account and re-attaches it to the specially created orphan_account." + When I press "Yes, I'm sure" + And all indexing jobs have been run + Then I should see "Orphaning was successful." + When I go to author's works page + Then I should not see "Glorious" + And I should not see "Excellent" + And I should see "Lovely" Scenario: Orphaning a shared work should not affect chapters created solely by the other creator From 64457f769724533d046c58bf5ee7012d26af86b1 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 15:32:22 -0700 Subject: [PATCH 07/14] Fix typo --- spec/models/subscription_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index c2cb9ebe3d6..122a1823f77 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -200,7 +200,7 @@ it "returns true for a non-anonymous work" do # FIXME: make sure subscription is being set up properly - expect(subscribable.subscribable_type).to be("User") + expect(subscription.subscribable_type).to be("User") expect(subscription.valid_notification_entry?(work)).to be_truthy end From f64052e29d6110a36361eff437c1b6203ca4e435 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 15:57:32 -0700 Subject: [PATCH 08/14] Make some tweaks to try to undertand email tests --- features/other_a/orphan_work.feature | 20 +++++++++++++++++--- spec/models/subscription_spec.rb | 2 -- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/features/other_a/orphan_work.feature b/features/other_a/orphan_work.feature index fe8ac6ed60f..a6a590b37d2 100644 --- a/features/other_a/orphan_work.feature +++ b/features/other_a/orphan_work.feature @@ -90,7 +90,10 @@ Feature: Orphan work | login | password | email | | work_subscriber | password | work_subscriber@foo.com | And I post the work "Torrid Idfic" - And I am logged in as "work_subscriber" + # FIXME: remove, I'm just trying to understand email logic + When subscription notifications are sent + Then 0 emails should be delivered to "series_subscriber@foo.com" + When I am logged in as "work_subscriber" And I view the work "Torrid Idfic" And I press "Subscribe" And I am logged in as "orphaneer" @@ -102,6 +105,17 @@ Feature: Orphan work When subscription notifications are sent Then 1 email should be delivered + # FIXME: remove duplicate + Scenario: Test initial email is 0 + + # Make the series exist, but make sure the initial work doesn't + # send an email for series_subscriber + Given the following activated user exists + | login | password | email | + | series_subscriber | password | series_subscriber@foo.com | + And I add the work "Work to create the series" to series "Shame Seriesss" + When subscription notifications are sent + Then 0 emails should be delivered to "series_subscriber@foo.com" Scenario: Orphan a work (leave pseud) but do notify subscribers to the work's series @@ -112,7 +126,7 @@ Feature: Orphan work | series_subscriber | password | series_subscriber@foo.com | And I add the work "Work to create the series" to series "Shame Series" When subscription notifications are sent - Then 0 emails should be delivered + Then 0 emails should be delivered to "series_subscriber@foo.com" # Subscribe When I am logged in as "series_subscriber" And I view the series "Shame Series" @@ -126,7 +140,7 @@ Feature: Orphan work And I choose "Leave a copy of my pseud on" And I press "Yes, I'm sure" When subscription notifications are sent - Then 1 email should be delivered + Then 1 email should be delivered to "series_subscriber@foo.com" Scenario: I can orphan multiple works at once Given I am logged in as "author" diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index 122a1823f77..c14620f1902 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -199,8 +199,6 @@ let(:subscription) { build(:subscription, subscribable: author_pseud.user) } it "returns true for a non-anonymous work" do - # FIXME: make sure subscription is being set up properly - expect(subscription.subscribable_type).to be("User") expect(subscription.valid_notification_entry?(work)).to be_truthy end From 682cd80f393b6f87986e04013557b474d9c6721a Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 16:00:09 -0700 Subject: [PATCH 09/14] Add another check to try to understand --- spec/models/subscription_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index c14620f1902..24b21b13e3d 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -203,6 +203,7 @@ end it "returns true for a non-anonymous chapter" do + expect(chapter.pseuds).to contain(author_pseud) expect(subscription.valid_notification_entry?(chapter)).to be_truthy end From 3a3538ad209997f1547a25d9c904e711dc8571cc Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Thu, 25 Dec 2025 16:10:01 -0700 Subject: [PATCH 10/14] Swap some 'build's to 'create's to see if that helps --- spec/models/subscription_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index 24b21b13e3d..770adef0d86 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -114,10 +114,10 @@ let(:author_pseud) { create(:user).default_pseud } let(:work) { create(:work, authors: [author_pseud]) } let(:draft) { build(:draft) } - let(:chapter) { build(:chapter, authors: [author_pseud]) } - let(:anon_work) { build(:work, authors: [author_pseud], collections: [build(:anonymous_collection)]) } - let(:anon_series) { build(:series, works: [anon_work]) } - let(:anon_chapter) { build(:chapter, authors:[author_pseud], work: anon_work) } + let(:chapter) { create(:chapter, authors: [author_pseud]) } + let(:anon_work) { create(:work, authors: [author_pseud], collections: [build(:anonymous_collection)]) } + let(:anon_series) { create(:series, works: [anon_work]) } + let(:anon_chapter) { create(:chapter, authors:[author_pseud], work: anon_work) } let(:orphan_pseud) { create(:user, login: "orphan_account").default_pseud } it "returns false when the creation is nil" do From 367d19b4aae60839cad1711beb1076b5121638ff Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Fri, 26 Dec 2025 01:04:46 +0000 Subject: [PATCH 11/14] Test passes locally?? --- spec/models/subscription_spec.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index 770adef0d86..e00839412eb 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe Subscription do let(:subscription) { build(:subscription) } @@ -22,7 +22,8 @@ end it "should be destroyed" do - expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound + expect { subscription.reload } + .to raise_error ActiveRecord::RecordNotFound end end end @@ -43,7 +44,8 @@ end it "should be destroyed" do - expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound + expect { subscription.reload } + .to raise_error ActiveRecord::RecordNotFound end end end @@ -64,7 +66,8 @@ end it "should be destroyed" do - expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound + expect { subscription.reload } + .to raise_error ActiveRecord::RecordNotFound end end end @@ -117,7 +120,7 @@ let(:chapter) { create(:chapter, authors: [author_pseud]) } let(:anon_work) { create(:work, authors: [author_pseud], collections: [build(:anonymous_collection)]) } let(:anon_series) { create(:series, works: [anon_work]) } - let(:anon_chapter) { create(:chapter, authors:[author_pseud], work: anon_work) } + let(:anon_chapter) { create(:chapter, authors: [author_pseud], work: anon_work) } let(:orphan_pseud) { create(:user, login: "orphan_account").default_pseud } it "returns false when the creation is nil" do @@ -134,7 +137,7 @@ end it "returns false when the creation is hidden_by_admin" do - expect(subscription.valid_notification_entry?(build(:work, hidden_by_admin: true))).to be_falsey + expect(subscription.valid_notification_entry?(build(:work, hidden_by_admin: true))).to be_truthy end end @@ -203,7 +206,6 @@ end it "returns true for a non-anonymous chapter" do - expect(chapter.pseuds).to contain(author_pseud) expect(subscription.valid_notification_entry?(chapter)).to be_truthy end From ef4eb54fde079084cbcde74ff4c4ba942db798ab Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Fri, 26 Dec 2025 01:19:06 +0000 Subject: [PATCH 12/14] Improve orphan_work.features test --- features/other_a/orphan_work.feature | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/features/other_a/orphan_work.feature b/features/other_a/orphan_work.feature index a6a590b37d2..a696b74ed19 100644 --- a/features/other_a/orphan_work.feature +++ b/features/other_a/orphan_work.feature @@ -90,10 +90,7 @@ Feature: Orphan work | login | password | email | | work_subscriber | password | work_subscriber@foo.com | And I post the work "Torrid Idfic" - # FIXME: remove, I'm just trying to understand email logic - When subscription notifications are sent - Then 0 emails should be delivered to "series_subscriber@foo.com" - When I am logged in as "work_subscriber" + And I am logged in as "work_subscriber" And I view the work "Torrid Idfic" And I press "Subscribe" And I am logged in as "orphaneer" @@ -105,30 +102,15 @@ Feature: Orphan work When subscription notifications are sent Then 1 email should be delivered - # FIXME: remove duplicate - Scenario: Test initial email is 0 - - # Make the series exist, but make sure the initial work doesn't - # send an email for series_subscriber - Given the following activated user exists - | login | password | email | - | series_subscriber | password | series_subscriber@foo.com | - And I add the work "Work to create the series" to series "Shame Seriesss" - When subscription notifications are sent - Then 0 emails should be delivered to "series_subscriber@foo.com" - Scenario: Orphan a work (leave pseud) but do notify subscribers to the work's series - # Make the series exist, but make sure the initial work doesn't - # send an email for series_subscriber Given the following activated user exists | login | password | email | | series_subscriber | password | series_subscriber@foo.com | + # Create the series, so we can subscribe to it before we add a work-to-be-orphaned And I add the work "Work to create the series" to series "Shame Series" - When subscription notifications are sent - Then 0 emails should be delivered to "series_subscriber@foo.com" - # Subscribe - When I am logged in as "series_subscriber" + # Subscribe + And I am logged in as "series_subscriber" And I view the series "Shame Series" And I press "Subscribe" # Add a new work to the series, which we then immediately orphan From 687333cfbe39cd68b51cc1d54ad9ccfaff6a07ad Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Fri, 26 Dec 2025 01:20:52 +0000 Subject: [PATCH 13/14] Revert accidental failing test --- spec/models/subscription_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index e00839412eb..ee4463067f9 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -137,7 +137,7 @@ end it "returns false when the creation is hidden_by_admin" do - expect(subscription.valid_notification_entry?(build(:work, hidden_by_admin: true))).to be_truthy + expect(subscription.valid_notification_entry?(build(:work, hidden_by_admin: true))).to be_falsey end end From db351a6d62d8f92dd347b8eca8b9ea82f4d1d401 Mon Sep 17 00:00:00 2001 From: irrationalpie Date: Fri, 26 Dec 2025 01:43:46 +0000 Subject: [PATCH 14/14] Tweak tests --- features/other_a/orphan_work.feature | 2 +- spec/models/subscription_spec.rb | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/features/other_a/orphan_work.feature b/features/other_a/orphan_work.feature index a696b74ed19..6081995e466 100644 --- a/features/other_a/orphan_work.feature +++ b/features/other_a/orphan_work.feature @@ -122,7 +122,7 @@ Feature: Orphan work And I choose "Leave a copy of my pseud on" And I press "Yes, I'm sure" When subscription notifications are sent - Then 1 email should be delivered to "series_subscriber@foo.com" + Then 1 email should be delivered Scenario: I can orphan multiple works at once Given I am logged in as "author" diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index ee4463067f9..ccc6344d474 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -119,7 +119,7 @@ let(:draft) { build(:draft) } let(:chapter) { create(:chapter, authors: [author_pseud]) } let(:anon_work) { create(:work, authors: [author_pseud], collections: [build(:anonymous_collection)]) } - let(:anon_series) { create(:series, works: [anon_work]) } + let(:anon_series) { build(:series, works: [anon_work]) } let(:anon_chapter) { create(:chapter, authors: [author_pseud], work: anon_work) } let(:orphan_pseud) { create(:user, login: "orphan_account").default_pseud } @@ -229,7 +229,16 @@ expect(subscription.valid_notification_entry?(create(:work, authors: [create(:user).default_pseud]))).to be_falsey end - it "returns false for a chapter by a different user" do + it "returns false for a chapter by a different user, even if the subscribed-to user is one of the work creators" do + # subscription is for author_pseud + author2 = create(:user).default_pseud + work = create(:work, authors: [author_pseud, author2]) + # chapter is by author2 only + chapter = create(:chapter, authors: [author2], work: work, position: 2) + expect(subscription.valid_notification_entry?(chapter)).to be_falsey + end + + it "returns false for a chapter by a different user, even if the user is one of the work creators" do expect(subscription.valid_notification_entry?(create(:chapter, authors: [create(:user).default_pseud]))).to be_falsey end end @@ -242,6 +251,11 @@ it "returns true for a non-anonymous chapter" do expect(subscription.valid_notification_entry?(chapter)).to be_truthy end + + # if the subscribable type is work, creation must be a chapter + it "returns false when the creation is a work" do + expect(subscription.valid_notification_entry?(work)).to be_falsey + end end context "when work is anonymous" do