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.

Sign up for GitHub

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

Jump to bottom

AO3-5053 Update bookmarker byline when pseud or username changes #4773

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

AO3-5053 Update bookmarker byline when pseud or username changes #4773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2df0e2e
AO3-5053 Update bookmarker name when pseud/login name changes
irrationalpie7 Feb 16, 2023
4048cb6
AO3-5053 revert test file formatting
irrationalpie7 Mar 29, 2024
ad766d5
AO3-5053 Remove byline from cache instead of touching all bookmarks
irrationalpie7 Mar 30, 2024
0c84baf
AO3-5053 Fix typos
irrationalpie7 Mar 31, 2024
1b79932
AO3-5053 Fix typo again
irrationalpie7 Mar 31, 2024
673ae22
AO3-5053 removed unnecessary spec tests
irrationalpie7 Apr 2, 2024
68c3cb6
AO3-5053 fix linter warning
irrationalpie7 Apr 2, 2024
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
2 changes: 1 addition & 1 deletion app/models/bookmark.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def check_new_external_work

scope :latest, -> { is_public.limit(ArchiveConfig.ITEMS_PER_PAGE).join_work }

scope :for_blurb, -> { includes(:bookmarkable, :pseud, :tags, :collections) }
scope :for_blurb, -> { includes(:bookmarkable, :tags, :collections, pseud: [:user]) }

# a complicated dynamic scope here:
# if the user is an Admin, we use the "visible_to_admin" scope
Expand Down
11 changes: 6 additions & 5 deletions app/views/bookmarks/_bookmark_user_module.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
<% # expects "bookmark" %>
<div class="<% if is_author_of?(bookmark) %>own <% end %>user module group">
<% blurb_cache_key = (is_author_of?(bookmark) ? "bookmark-owner-blurb-#{bookmark.cache_key}-v2" : "bookmark-blurb-#{bookmark.cache_key}-v2") %>
<!--bookmarker-->
<h5 class="byline heading">
<%= t("Bookmarked by") %> <%= link_to(bookmark.pseud.byline, user_pseud_bookmarks_path(bookmark.pseud.user, bookmark.pseud)) %>
Copy link
Contributor

@Bilka2 Bilka2 Apr 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The feature test error is happening because you switched from ts to t. The switch is great, t is preferred. But then the link to the user should be integrated into the translation call so it looks like t(".bookmarked_by_html", pseud_link: link_to(....)) and the translation needs to be in config/locales/views/en.yml, in this case in a new section for bookmarks:

bookmarks:
  bookmark_user_module:
    bookmarked_by_html: Bookmarked by %{pseud_link}

Copy link
Contributor

Choose a reason for hiding this comment

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

For context: Normally the tests would let you know that the translation for "Bookmarked by" is missing (in the yml file). The missing translation caused it to get converted to "Bookmarked By" in the feature tests. And the test for missing translations fails, as it should. However, the reporting for it on GitHub is broken, see https://otwarchive.atlassian.net/browse/AO3-6703.

I found what the issue was because I tried to look at the blurb in my local dev install and got the "translation missing" error there.

</h5>
<% blurb_cache_key = (is_author_of?(bookmark) ? "bookmark-owner-blurb-#{bookmark.cache_key}-v3" : "bookmark-blurb-#{bookmark.cache_key}-v3") %>
<% cache(blurb_cache_key, skip_digest: true) do %>
<!--bookmarker, time-->
<h5 class="byline heading">
<%= ts('Bookmarked by') %> <%= link_to(bookmark.pseud.byline, user_pseud_bookmarks_path(bookmark.pseud.user, bookmark.pseud)) %>
</h5>
<!--time-->
<p class="datetime"><%= set_format_for_date(bookmark.created_at) %></p>

<% # information added by the bookmark owner %>
Expand Down
16 changes: 16 additions & 0 deletions features/other_a/pseuds.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,19 @@ Scenario: Change details as an admin
Then I should see "Pseud was successfully updated."
When I go to the admin-activities page
Then I should see 1 admin activity log entry


Scenario: Bookmarks reflect pseud changes immediately

Given the work "Interesting"
And I am logged in as "myself"
And I add the pseud "before"
And I bookmark the work "Interesting" as "before"
And I go to myself's bookmarks page
Then I should see "Bookmarked by before (myself)"

When it is currently 1 second from now
And I change the pseud "before" to "after"
And I go to myself's bookmarks page
Then I should see "Bookmarked by after (myself)"
And I should not see "Bookmarked by before (myself)"
17 changes: 17 additions & 0 deletions features/users/user_rename.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,20 @@ Feature:
And I press "Change User Name"
Then I should get confirmation that I changed my username
And I should see "Hi, notforbidden"

Scenario: Bookmarker's bookmark blurbs reflect username changes immediately
Given the work "Interesting"
And I am logged in as "before" with password "password"
And I add the pseud "mine"
And I bookmark the work "Interesting" as "mine"
And I go to before's bookmarks page
Then I should see "Bookmarked by mine (before)"

When it is currently 1 second from now
And I visit the change username page for before
And I fill in "New user name" with "after"
And I fill in "Password" with "password"
And I press "Change User Name"
And I go to after's bookmarks page
Then I should see "Bookmarked by mine (after)"
And I should not see "Bookmarked by mine (before)"
Loading