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-5901 Limit number of pseuds on profile and sidebar #4554

Merged
merged 17 commits into from
Jul 6, 2023
Merged
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
31 changes: 27 additions & 4 deletions app/controllers/profile_controller.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
class ProfileController < ApplicationController
before_action :load_user_and_pseuds

def show
@user = User.find_by(login: params[:user_id])
if @user.nil?
flash[:error] = ts("Sorry, there's no user by that name.")
redirect_to '/' and return
elsif @user.profile.nil?
if @user.profile.nil?
Profile.create(user_id: @user.id)
@user.reload
end

@profile = ProfilePresenter.new(@user.profile)
#code the same as the stuff in users_controller
if current_user.respond_to?(:subscriptions)
@subscription = current_user.subscriptions.where(subscribable_id: @user.id,
Expand All @@ -18,4 +18,27 @@ def show
@page_subtitle = ts("%{username} - Profile", username: @user.login)
end

def pseuds
respond_to do |format|
format.html do
redirect_to user_pseuds_path(@user)
end

format.js
end
end

private

def load_user_and_pseuds
@user = User.find_by(login: params[:user_id])

if @user.nil?
flash[:error] = ts("Sorry, there's no user by that name.")
redirect_to root_path
return
end
Comment on lines +34 to +40
Copy link
Contributor

@Bilka2 Bilka2 Jun 26, 2023

Choose a reason for hiding this comment

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

Based on recent other PRs regarding misspelled usernames (#4312 + #4376), I think this should use find_by! so it results in a 404 error instead of redirecting when the user is nonexistent.


@pseuds = @user.pseuds.default_alphabetical.paginate(page: params[:page])
end
end
13 changes: 13 additions & 0 deletions app/decorators/profile_presenter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class ProfilePresenter < SimpleDelegator
def created_at
user.created_at.to_date
end

def date_of_birth
super if user.preference.try(:date_of_birth_visible)
end

def email
user.email if user.preference.try(:email_visible)
end
end
43 changes: 37 additions & 6 deletions app/helpers/pseuds_helper.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,42 @@
module PseudsHelper

# Prints array of pseuds with links to user pages
# used on Profile page
def print_pseud_list(pseuds)
pseuds.includes(:user).collect { |pseud| span_if_current(pseud.name, [pseud.user, pseud]) }.join(", ").html_safe
# Returns a list of pseuds, with links to each pseud.
#
# Used on Profile page, and by ProfileController#pseuds.
#
# The pseuds argument should be a single page of the user's pseuds, generated
# by calling user.pseuds.paginate(page: 1) or similar. This allows us to
# insert a remote link to dynamically insert the next page of pseuds.
def print_pseud_list(user, pseuds, first: true)
links = pseuds.map do |pseud|
link_to(pseud.name, [user, pseud])
end

difference = pseuds.total_entries - pseuds.length - pseuds.offset

if difference.positive?
links << link_to(
t("profile.pseud_list.more_pseuds", count: difference),
pseuds_user_profile_path(user, page: pseuds.next_page),
remote: true, id: "more_pseuds"
)
end

more_pseuds_connector = tag.span(
t("support.array.last_word_connector"),
id: "more_pseuds_connector"
)

if first
to_sentence(links,
last_word_connector: more_pseuds_connector)
else
links.unshift("")
to_sentence(links,
last_word_connector: more_pseuds_connector,
two_words_connector: more_pseuds_connector)
end
end

# used in the sidebar
def print_pseud_selector(pseuds)
pseuds -= [@pseud] if @pseud && @pseud.new_record?
Expand Down
4 changes: 4 additions & 0 deletions app/models/pseud.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ class Pseud < ApplicationRecord
after_update :expire_caches
after_commit :reindex_creations, :touch_comments

scope :alphabetical, -> { order(:name) }
scope :default_alphabetical, -> { order(is_default: :desc).alphabetical }
scope :abbreviated_list, -> { default_alphabetical.limit(ArchiveConfig.ITEMS_PER_PAGE) }

def self.not_orphaned
where("user_id != ?", User.orphan_account)
end
Expand Down
2 changes: 2 additions & 0 deletions app/views/profile/pseuds.js.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
$j("#more_pseuds_connector").remove();
$j("#more_pseuds").replaceWith("<%= j print_pseud_list(@user, @pseuds, first: false) %>")
26 changes: 13 additions & 13 deletions app/views/profile/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,37 @@
<%= render 'users/header' %>

<!--main content-->
<% unless @user.profile.title.blank? %>
<h3 class="heading"><%=h @user.profile.title %></h3>
<% if @profile.title.present? %>
<h3 class="heading"><%=h @profile.title %></h3>
<% end %>

<div class="wrapper">
<dl class="meta">
<dt class="pseuds"><%= link_to ts("My pseuds:"), user_pseuds_path(@user) %></dt>
<dd class="pseuds"><%= print_pseud_list(@user.pseuds) %></dd>
<dt class="pseuds"><%= ts("My pseuds:") %></dt>
<dd class="pseuds"><%= print_pseud_list(@user, @pseuds) %></dd>
<dt><%= ts("I joined on:") %></dt>
<dd><%= ts("%{date}", :date => l(@user.created_at.to_date)) %></dd>
<dd><%= l(@profile.created_at) %></dd>
<dt><%= ts("My user ID is:") %></dt>
<dd><%= @user.id %></dd>
<% if @user.profile.location? %>
<% if @profile.location %>
<dt class="location"><%=h ts("I live in:") %></dt>
<dd><%=h @user.profile.location %></dd>
<dd><%=h @profile.location %></dd>
<% end %>
<% if @user.preference.try(:date_of_birth_visible) %>
<% if @profile.date_of_birth %>
<dt class="birthday"><%=h ts("My birthday:") %></dt>
<dd class="birthday"><%=l(@user.profile.date_of_birth) unless @user.profile.date_of_birth.blank? %></dd>
<dd class="birthday"><%=l(@profile.date_of_birth) %></dd>
<% end %>
<% if @user.preference.try(:email_visible) %>
<% if @profile.email %>
<dt class="email"><%=h ts("My email address:") %></dt>
<dd class="email"><%= mail_to @user.email, nil, :encode => "hex" %></dd>
<dd class="email"><%= mail_to @profile.email, nil, :encode => "hex" %></dd>
<% end %>
</dl>
</div>

<% unless @user.profile.about_me.blank? %>
<% if @profile.about_me.present? %>
<div class="bio module">
<h3 class="heading"><%=h ts("Bio") %></h3>
<blockquote class="userstuff"><%=raw sanitize_field(@user.profile, :about_me) %></blockquote>
<blockquote class="userstuff"><%=raw sanitize_field(@profile, :about_me) %></blockquote>
</div>
<% end %>
<!--/content-->
Expand Down
5 changes: 3 additions & 2 deletions app/views/users/_sidebar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
<li><%= span_if_current ts("Profile"), user_profile_path(@user) %></li>
<% if @user.pseuds.size > 1 %>
<li class="pseud" aria-haspopup="true">
<a href="#" title="<%= ts("Pseud Switcher") %>"><%= ts("Pseuds") %></a>
<% pseud_link_text = current_page?(@user) ? ts("Pseuds") : (@pseud ? @pseud.name : @user.login) %>
<a href="#" title="<%= ts("Pseud Switcher") %>"><%= pseud_link_text %></a>
<ul class="expandable secondary">
<%= print_pseud_selector(@user.pseuds) %>
<%= print_pseud_selector(@user.pseuds.abbreviated_list) %>
<li><%= span_if_current ts("All Pseuds (%{pseud_number})", :pseud_number => @user.pseuds.count), user_pseuds_path(@user) %></li>
</ul>
</li>
Expand Down
5 changes: 5 additions & 0 deletions config/locales/views/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,11 @@ en:
allow_collection_invitation: Allow others to invite my works to collections.
blocked_users: Blocked Users
muted_users: Muted Users
profile:
pseud_list:
more_pseuds:
one: "%{count} more pseud"
other: "%{count} more pseuds"
pseuds:
delete_preview:
cancel: Cancel
Expand Down
6 changes: 5 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,11 @@
end
resources :nominations, controller: "tag_set_nominations", only: [:index]
resources :preferences, only: [:index, :update]
resource :profile, only: [:show], controller: "profile"
resource :profile, only: [:show], controller: "profile" do
collection do
get :pseuds
end
end
resources :pseuds do
resources :works
resources :series
Expand Down
26 changes: 5 additions & 21 deletions features/other_a/preferences_edit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Feature: Edit preferences
And I should see "Turn off the banner showing on every page."


Scenario: View and edit preferences for history, personal details, view entire work
Scenario: View and edit preferences for history, view entire work

Given the following activated user exists
| login | password |
Expand All @@ -50,16 +50,10 @@ Feature: Edit preferences
Then I should not see "My email address"
And I should not see "My birthday"
When I am logged in as "editname" with password "password"
Then I should see "Hi, editname!"
And I should see "Log Out"
When I post the work "This has two chapters"
And I follow "Add Chapter"
And I fill in "content" with "Secondy chapter"
And I press "Preview"
And I press "Post"
Then I should see "Secondy chapter"
And I post the 2 chapter work "This has two chapters"
Then I should be on the 2nd chapter of the work "This has two chapters"
And I follow "Previous Chapter"
Then I should not see "Secondy chapter"
Then I should be on the 1st chapter of the work "This has two chapters"
When I follow "editname"
Then I should see "Dashboard" within "div#dashboard"
And I should see "History" within "div#dashboard"
Expand All @@ -79,22 +73,12 @@ Feature: Edit preferences
Then I should see "Edit My Profile"
When I uncheck "Turn on History"
And I check "Show the whole work by default."
And I check "Show my email address to other people."
And I check "Show my date of birth to other people."
And I press "Update"
Then I should see "Your preferences were successfully updated"
And I should not see "History" within "div#dashboard"
When I go to the works page
And I follow "This has two chapters"
Then I should see "Secondy chapter"
When I log out
And I go to editname's user page
And I follow "Profile"
Then I should see "My email address"
And I should see "My birthday"
When I go to the works page
And I follow "This has two chapters"
Then I should not see "Secondy chapter"
Then I should not see "Next Chapter"

@javascript
Scenario: User can hide warning and freeform tags and reveal them on a case-
Expand Down
6 changes: 6 additions & 0 deletions features/other_a/profile_edit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ Scenario: Changing email address and viewing
And the email should not contain "translation missing"
When I change my preferences to display my email address
Then I should see "My email address: [email protected]"
When I log out
And I go to editname's profile page
Then I should see "My email address: [email protected]"

Scenario: Changing email address after requesting password reset

Expand Down Expand Up @@ -153,6 +156,9 @@ Scenario: Entering date of birth and displaying
When I change my preferences to display my date of birth
Then I should see "My birthday: 1980-11-30"
And 0 emails should be delivered
When I log out
And I go to editname's profile page
Then I should see "My birthday: 1980-11-30"

Scenario: Change password - mistake in typing old password

Expand Down
30 changes: 30 additions & 0 deletions features/other_a/pseuds.feature
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,33 @@ Scenario: Comments reflect pseud changes immediately
And I view the work "Interesting" with comments
Then I should see "after (myself)" within ".comment h4.byline"
And I should not see "before (myself)"

Scenario: Many pseuds

Given there are 3 pseuds per page
And "Zaphod" has the pseud "Slartibartfast"
And "Zaphod" has the pseud "Agrajag"
And "Zaphod" has the pseud "Betelgeuse"
And I am logged in as "Zaphod"

When I view my profile
Then I should see "Zaphod" within "dl.meta"
And I should see "Agrajag" within "dl.meta"
And I should see "Betelgeuse" within "dl.meta"
And I should not see "Slartibartfast" within "dl.meta"
And I should see "1 more pseud" within "dl.meta"

When I go to my user page
Then I should see "Zaphod" within "ul.expandable"
And I should see "Agrajag" within "ul.expandable"
And I should see "Betelgeuse" within "ul.expandable"
And I should not see "Slartibartfast" within "ul.expandable"
And I should see "All Pseuds (4)" within "ul.expandable"

When I go to my "Slartibartfast" pseud page
Then I should see "Slartibartfast" within "li.pseud > a"
And I should not see "Slartibartfast" within "ul.expandable"

When there are 10 pseuds per page
And I view my profile
Then I should see "Zaphod, Agrajag, Betelgeuse, and Slartibartfast" within "dl.meta"
10 changes: 8 additions & 2 deletions features/step_definitions/pseud_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,24 @@
step %{I start a new session}
end

Given "there are {int} pseuds per page" do |amount|
stub_const("ArchiveConfig", OpenStruct.new(ArchiveConfig))
ArchiveConfig.ITEMS_PER_PAGE = amount.to_i
allow(Pseud).to receive(:per_page).and_return(amount)
end

When /^I change the pseud "([^\"]*)" to "([^\"]*)"/ do |old_pseud, new_pseud|
step %{I edit the pseud "#{old_pseud}"}
fill_in("Name", with: new_pseud)
click_button("Update")
end

When /^I edit the pseud "([^\"]*)"/ do |pseud|
When /^I edit the pseud "([^\"]*)"/ do |pseud|
p = Pseud.where(name: pseud, user_id: User.current_user.id).first
visit edit_user_pseud_path(User.current_user, p)
end

When /^I add the pseud "([^\"]*)"/ do |pseud|
When /^I add the pseud "([^\"]*)"/ do |pseud|
visit new_user_pseud_path(User.current_user)
fill_in("Name", with: pseud)
click_button("Create")
Expand Down
28 changes: 20 additions & 8 deletions spec/controllers/profile_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,32 @@
require 'spec_helper'

describe ProfileController do
describe 'show' do
it 'should be an error for a non existent user' do
describe "show" do
let(:user) { create(:user) }

it "redirects and shows an error message for a non existent user" do
get :show, params: { user_id: 999_999_999_999 }

expect(response).to redirect_to(root_path)
expect(flash[:error]).to eq "Sorry, there's no user by that name."
end

it 'should create a new profile if one does not exist' do
@user = FactoryBot.create(:user)
@user.profile.destroy
@user.reload
get :show, params: { user_id: @user }
expect(@user.profile).to be
it "creates a new profile if one does not exist" do
user.profile.destroy
user.reload

get :show, params: { user_id: user }

expect(user.profile).not_to be_nil
end

it "uses the profile presenter for the profile" do
profile_presenter = instance_double(ProfilePresenter)
allow(ProfilePresenter).to receive(:new).and_return(profile_presenter)

get :show, params: { user_id: user }

expect(assigns(:profile)).to eq(profile_presenter)
end
end
end
Loading