-
Notifications
You must be signed in to change notification settings - Fork 650
AO3-3620 Fix subscriptions to other creator(s) for works co-authored with orphan_account #5523
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.
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
base: master
Are you sure you want to change the base?
AO3-3620 Fix subscriptions to other creator(s) for works co-authored with orphan_account #5523
Conversation
app/models/subscription.rb
Outdated
| # 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible we may also reach this check if:
- A and B have a work together
- User subscribes to A
- B posts a new chapter to the work, without listing A as co-creator for the chapter
The Jira issue for https://otwarchive.atlassian.net/browse/AO3-5696 suggests this behavior is not desired, so I need to do more testing/checking here to see what the behavior is in practice
| 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 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My formatting settings appear to have changed a fair amount of whitespace in this file. If there's a specific formatter I could be using and some recommended settings, I'm happy to swap back! Otherwise it doesn't seem worth individually reverting the whitespace-only lines
| let(:series) { build(:series) } | ||
| let(:work) { build(:work) } | ||
| let(:author_pseud) { create(:user).default_pseud } | ||
| let(:work) { create(:work, authors: [author_pseud]) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we now compare the subscribable to the creation.pseuds, it made more sense to me to create :work and similar variables with a known set of authors
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-3620
Purpose
Instead of checking whether orphan account is one of the creators to block notifications for newly-orphaned works, checks whether the creator associated with the subscription is still associated with the creation
Note
A lot of the issue instructions talk about removing the ability to subscribe to orphan account, which
appears to already be the case(edit: I fully goofed here, I was logged out when I was looking at the orphan account page 😂🙃), so I figured this issue was still open because of it breaking subscriptions to other creators, and named this PR after what it actually fixesNote
With this PR, if someone is subscribed to author A but not author B, where A and B are co-creators of a work, the user will NOT get a notification if author B adds a new chapter without author A
Testing Instructions
How can the Archive's QA team verify that this is working as you intended?
References
Related issue: https://otwarchive.atlassian.net/browse/AO3-5696
Credit
irrationalpie (they/them)