AO3-3620 Fix subscriptions to other creator(s) for works co-authored with orphan_account

app/models/subscription.rb

@@ -37,13 +37,25 @@ def name
     end
   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
+    # 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
+    true
+  end
+
   # 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 +64,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)
 

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,72 @@ 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
 
     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"
+      | 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"
+      # 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
       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
 

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}"

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
@@ -111,12 +114,13 @@
   describe "#valid_notification_entry?" do
     let(:subscription) { build(:subscription) }
     let(:series) { build(:series) }
-    let(:work) { build(:work) }
+    let(:author_pseud) { create(:user).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) { create(:chapter, authors: [author_pseud]) }
+    let(:anon_work) { create(: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) { 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
@@ -132,11 +136,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 +150,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 +199,7 @@
     end
 
     context "when subscribable is a user" do
-      let(:subscription) { build(:subscription, subscribable: create(:user)) }
+      let(:subscription) { build(:subscription, subscribable: author_pseud.user) }
 
       it "returns true for a non-anonymous work" do
         expect(subscription.valid_notification_entry?(work)).to be_truthy
@@ -222,6 +216,31 @@
       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, 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
 
     context "when subscribable is a work" do
@@ -232,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