AO3-4250 Made gift recipients update on blurbs on pseud / username changes - #5063
AO3-4250 Made gift recipients update on blurbs on pseud / username changes#5063melinath wants to merge 5 commits into
Conversation
| pseud.works.each do |work| | ||
| work.touch | ||
| work.expire_caches | ||
| end | ||
| pseud.gift_works.each do |work| | ||
| work.touch | ||
| work.expire_caches | ||
| end |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
For now the discussion looks to be concluded, we'll go with touching things synchronously for the time being
|
|
||
| after_update :check_default_pseud | ||
| after_update :expire_caches | ||
| after_destroy :expire_caches |
There was a problem hiding this comment.
Since we already have a check for saved_change_to_name?, could this be moved to after_commit to avoid duplicating with both after_update and after_destroy?
| Given I have no users | ||
| And the following activated users exist | ||
| | login | password | email | id | | ||
| | gifter | something | gifter@example.com | 1 | | ||
| | giftee1 | something | giftee1@example.com | 2 | | ||
| And a pseud exists with name: "Me2", user_id: 2 |
There was a problem hiding this comment.
For this and the following tests, could you use
to avoid hardcoding user IDs? Then, we should be able to drop theGiven I have no users and remove the ID column from the next step
|
Adopted at #5967 based on the policy for stalled pull requests |
3 participants
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-4250
Purpose
Made it so that changing a pseud or username, deleting a pseud, or deleting a user account will expire the cache for all gift works (so that the byline will no longer display the old pseud or username.)
For deleting a pseud, this is done by expiring the cache on all gifts for the default pseud (since gifts are reassigned to the default pseud.) This will clear more caches than necessary - technically we only need to expire the cache for the gifts that were previously assigned to pseud being deleted. I did it this way for now because it's easier & keeps all the code in one place, which I think is easier to understand / less likely to break; however, I can definitely go back and change this if desired.
I also tweaked some of the tests related to series blurb updates (to ensure that they behave properly if only the username is updated and no pseuds) and fixed some minor whitespace issues on adjacent tests.
Credit
Stephen Lewis, he/him