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.

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-4250 Made gift recipients update on blurbs on pseud / username changes #5063

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 14 additions & 3 deletions app/models/pseud.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class Pseud < ApplicationRecord

after_update :check_default_pseud
after_update :expire_caches
after_destroy :expire_caches
after_commit :reindex_creations, :touch_comments

scope :alphabetical, -> { order(:name) }
Expand Down Expand Up @@ -390,9 +391,19 @@ def check_default_pseud
end

def expire_caches
if saved_change_to_name?
works.touch_all
series.each(&:expire_byline_cache)
return unless saved_change_to_name? || (destroyed? && !user.nil?)

pseud = destroyed? ? user.default_pseud : self
return if pseud.nil?

pseud.series.each(&:expire_byline_cache)
pseud.works.each do |work|
work.touch
work.expire_caches
end
pseud.gift_works.each do |work|
work.touch
work.expire_caches
end
Comment on lines +400 to 407
Copy link
Contributor

Choose a reason for hiding this comment

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

The discussion whether touching the works is the right cache busting strategy hasn't quite concluded, but it looks like we were leaning towards it. So, this can be optimized further.

For the gift works, the information is in the blurb. The blurb cache key depends on the work cache key, which depends on the work's updated_at. So it's enough to touch the work to bust the cache, no need to manually do so via expire_caches. That means that this can use the more optimised touch_all instead of looping through the gift works.

Your PR doesn't say why you updated pseud.works to use expire_caches as well, but I think it should be fine to go back to touch_all in this case too.

end

Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ def expire_caches
work.touch
work.expire_caches
end
self.gift_works.each do |work|
work.touch
work.expire_caches
end
end

def remove_user_from_kudos
Expand Down
31 changes: 31 additions & 0 deletions features/other_a/pseud_delete.feature
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,34 @@ Feature: Delete pseud.
Then I should see "My Collection Thing"
And I should not see "other_pseud (myself)" within "#main"
And I should see "myself" within "#main"

Scenario: Deleting a pseud updates series blurbs

Given I am logged in as "Myself"
And I add the work "Great Work" to series "Best Series" as "Me2"
When I go to the dashboard page for user "Myself" with pseud "Me2"
And I follow "Series"
Then I should see "Best Series by Me2 (Myself)"

When I delete the pseud "Me2"
And I follow "Series"
Then I should see "Best Series by Myself"

Scenario: Deleting a pseud updates gift blurbs
Given I have no users
And the following activated users exist
| login | password | email | id |
| gifter | something | [email protected] | 1 |
| giftee1 | something | [email protected] | 2 |
And a pseud exists with name: "Me2", user_id: 2
And the user "giftee1" allows gifts
And I am logged in as "gifter" with password "something"
And I set up the draft "GiftStory1"
And I give the work to "Me2 (giftee1)"
And I press "Post"
And I am logged in as "giftee1" with password "something"
When I go to my gifts page
Then I should see "GiftStory1 by gifter for Me2 (giftee1)"
When I delete the pseud "Me2"
And I go to my gifts page
Then I should see "GiftStory1 by gifter for giftee1"
24 changes: 24 additions & 0 deletions features/other_a/pseuds.feature
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,30 @@ Scenario: Edit pseud updates series blurbs
When I follow "Series"
Then I should see "Best Series by Me3 (Myself)"

Scenario: Edit pseud updates gift blurbs
Given I have no users
And the following activated users exist
| login | password | email | id |
| gifter | something | [email protected] | 1 |
| giftee1 | something | [email protected] | 2 |
And a pseud exists with name: "Me2", user_id: 2
And the user "giftee1" allows gifts
And I am logged in as "gifter" with password "something"
And I set up the draft "GiftStory1"
And I give the work to "Me2 (giftee1)"
And I press "Post"
And I am logged in as "giftee1" with password "something"
When I go to my gifts page
Then I should see "GiftStory1 by gifter for Me2 (giftee1)"
When I go to my profile page
And I follow "Manage My Pseuds"
And I follow "Edit Me2"
And I fill in "Name" with "Me3"
And I press "Update"
Then I should see "Pseud was successfully updated."
When I go to my gifts page
Then I should see "GiftStory1 by gifter for Me3 (giftee1)"

Scenario: Change details as an admin

Given "someone" has the pseud "alt"
Expand Down
2 changes: 1 addition & 1 deletion features/step_definitions/pseud_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

When(/^I delete the pseud "([^\"]*)"$/) do |pseud|
visit user_pseuds_path(User.current_user)
click_link("delete_#{pseud}")
click_link("delete_#{pseud.downcase}")
end

When /^"([^\"]*)" creates the default pseud "([^"]*)"$/ do |username, newpseud|
Expand Down
23 changes: 23 additions & 0 deletions features/users/user_delete.feature
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,26 @@ Scenario: Delete a user who has coauthored a work
And a user account should not exist for "testuser"
When I go to orphan_account's series page
Then I should see "Epic"

Scenario: Deleting a user updates gift blurbs
Given I have no users
And I have no works or comments
And the following activated users exist
| login | password | email | id |
| gifter | something | [email protected] | 1 |
| giftee1 | something | [email protected] | 2 |
And the user "giftee1" allows gifts
And I am logged in as "gifter" with password "something"
And I set up the draft "GiftStory1"
And I give the work to "giftee1"
And I press "Post"
And I am logged in as "giftee1" with password "something"
When I am on gifter's works page
Then I should see "GiftStory1 by gifter for giftee1"
When I try to delete my account as giftee1
Then I should see "You have successfully deleted your account."
And a user account should not exist for "giftee1"
And I should be logged out
When I am on gifter's works page
Then I should see "GiftStory1 by gifter"
And I should not see "GiftStory1 by gifter for giftee1"
101 changes: 85 additions & 16 deletions features/users/user_rename.feature
Original file line number Diff line number Diff line change
Expand Up @@ -182,35 +182,104 @@ Feature:
And I should see "after" within "#main"
And I should not see "before" within "#main"

Scenario: Changing username updates series blurbs
Scenario: Changing only username updates series blurbs
Given I have no users
And I am logged in as "oldusername" with password "password"
And the following activated user exists
| login | password | id |
| oldusername | secret | 1 |
And a pseud exists with name: "newusername", user_id: 1
And I am logged in as "oldusername" with password "secret"
And I add the work "Great Work" to series "Best Series"
When I go to the dashboard page for user "oldusername" with pseud "oldusername"
And I follow "Series"
Then I should see "Best Series by oldusername"
When I visit the change username page for oldusername
And I fill in "New user name" with "newusername"
And I fill in "Password" with "password"
And I fill in "Password" with "secret"
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, newusername"
When I follow "Series"
Then I should see "Best Series by oldusername (newusername)"

Scenario: Changing username and pseud updates series blurbs
Given I have no users
And I am logged in as "oldusername" with password "secret"
And I add the work "Great Work" to series "Best Series"
When I go to the dashboard page for user "oldusername" with pseud "oldusername"
And I follow "Series"
Then I should see "Best Series by oldusername"
When I visit the change username page for oldusername
And I fill in "New user name" with "newusername"
And I fill in "Password" with "secret"
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, newusername"
When I follow "Series"
Then I should see "Best Series by newusername"
And I should not see "Best Series by oldusername"

Scenario: Changing only username updates gift blurbs
Given I have no users
And the following activated users exist
| login | password | email | id |
| gifter | something | [email protected] | 1 |
| giftee1 | something | [email protected] | 2 |
And a pseud exists with name: "newusername", user_id: 2
And the user "giftee1" allows gifts
And I am logged in as "gifter" with password "something"
And I set up the draft "GiftStory1"
And I give the work to "giftee1"
And I press "Post"
And I am logged in as "giftee1" with password "something"
When I go to my gifts page
Then I should see "GiftStory1 by gifter for giftee1"
When I visit the change username page for giftee1
And I fill in "New user name" with "newusername"
And I fill in "Password" with "something"
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, newusername"
When I go to my gifts page
Then I should see "GiftStory1 by gifter for giftee1 (newusername)"

Scenario: Changing the username from a forbidden name to non-forbidden
Given I have no users
And the following activated user exists
| login | password |
| forbidden | secret |
And the user name "forbidden" is on the forbidden list
When I am logged in as "forbidden" with password "secret"
And I visit the change username page for forbidden
And I fill in "New user name" with "notforbidden"
And I fill in "Password" with "secret"
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, notforbidden"
Scenario: Changing username and pseud updates gift blurbs
Given I have no users
And the following activated users exist
| login | password | email | id |
| gifter | something | [email protected] | 1 |
| giftee1 | something | [email protected] | 2 |
And the user "giftee1" allows gifts
And I am logged in as "gifter" with password "something"
And I set up the draft "GiftStory1"
And I give the work to "giftee1"
And I press "Post"
And I am logged in as "giftee1" with password "something"
When I go to my gifts page
Then I should see "GiftStory1 by gifter for giftee1"
When I visit the change username page for giftee1
And I fill in "New user name" with "newusername"
And I fill in "Password" with "something"
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, newusername"
When I go to my gifts page
Then I should see "GiftStory1 by gifter for newusername"
And I should not see "GiftStory1 by gifter for giftee1"

Scenario: Changing the username from a forbidden name to non-forbidden
Given I have no users
And the following activated user exists
| login | password |
| forbidden | secret |
And the user name "forbidden" is on the forbidden list
When I am logged in as "forbidden" with password "secret"
And I visit the change username page for forbidden
And I fill in "New user name" with "notforbidden"
And I fill in "Password" with "secret"
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, notforbidden"

Scenario: Tag wrangling supervisors are emailed about tag wrangler username changes
Given the user "before" exists and is activated
Expand Down
Loading