From: nertc Date: Tue, 14 Jan 2025 07:33:56 +0000 (+0400) Subject: Refactor friendships controller and model X-Git-Tag: live~280^2 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/be11f2075e8bbde0a1e0640fd254c64ee0c94f25 Refactor friendships controller and model --- diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 9516a3012..4c5288307 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -42,7 +42,7 @@ class Ability can [:create, :subscribe, :unsubscribe], DiaryEntry can :update, DiaryEntry, :user => user can [:create], DiaryComment - can [:make_friend, :remove_friend], Friendship + can [:show, :create, :destroy], Follow can [:read, :create, :mark, :unmute, :destroy], Message can [:close, :reopen], Note can [:read, :update], :preference diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb index 928f1c1ec..aa9d81343 100644 --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@ -55,7 +55,7 @@ class ChangesetsController < ApplicationController elsif @params[:bbox] changesets = conditions_bbox(changesets, BoundingBox.from_bbox_params(params)) elsif @params[:friends] && current_user - changesets = changesets.where(:user => current_user.friends.identifiable) + changesets = changesets.where(:user => current_user.followings.identifiable) elsif @params[:nearby] && current_user changesets = changesets.where(:user => current_user.nearby) end diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index 5f39a0886..94876e72a 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -29,7 +29,7 @@ class DiaryEntriesController < ApplicationController elsif params[:friends] if current_user @title = t ".title_followed" - entries = DiaryEntry.where(:user => current_user.friends) + entries = DiaryEntry.where(:user => current_user.followings) else require_user return diff --git a/app/controllers/follows_controller.rb b/app/controllers/follows_controller.rb new file mode 100644 index 000000000..d267777c1 --- /dev/null +++ b/app/controllers/follows_controller.rb @@ -0,0 +1,61 @@ +class FollowsController < ApplicationController + include UserMethods + + layout "site" + + before_action :authorize_web + before_action :set_locale + before_action :check_database_readable + + authorize_resource + + before_action :check_database_writable + before_action :lookup_friend + + def show + @already_follows = current_user.friends_with?(@friend) + end + + def create + follow = Follow.new + follow.follower = current_user + follow.following = @friend + if current_user.friends_with?(@friend) + flash[:warning] = t ".already_followed", :name => @friend.display_name + elsif current_user.follows.where(:created_at => Time.now.utc - 1.hour..).count >= current_user.max_friends_per_hour + flash[:error] = t ".limit_exceeded" + elsif follow.save + flash[:notice] = t ".success", :name => @friend.display_name + UserMailer.friendship_notification(follow).deliver_later + else + follow.add_error(t(".failed", :name => @friend.display_name)) + end + + referer = safe_referer(params[:referer]) if params[:referer] + + redirect_to referer || user_path + end + + def destroy + if current_user.friends_with?(@friend) + Follow.where(:follower => current_user, :following => @friend).delete_all + flash[:notice] = t ".success", :name => @friend.display_name + else + flash[:error] = t ".not_followed", :name => @friend.display_name + end + + referer = safe_referer(params[:referer]) if params[:referer] + + redirect_to referer || user_path + end + + private + + ## + # ensure that there is a "friend" instance variable + def lookup_friend + @friend = User.active.find_by!(:display_name => params[:display_name]) + rescue ActiveRecord::RecordNotFound + render_unknown_user params[:display_name] + end +end diff --git a/app/controllers/friendships_controller.rb b/app/controllers/friendships_controller.rb deleted file mode 100644 index 8de438fd2..000000000 --- a/app/controllers/friendships_controller.rb +++ /dev/null @@ -1,61 +0,0 @@ -class FriendshipsController < ApplicationController - include UserMethods - - layout "site" - - before_action :authorize_web - before_action :set_locale - before_action :check_database_readable - - authorize_resource - - before_action :check_database_writable, :only => [:make_friend, :remove_friend] - before_action :lookup_friend, :only => [:make_friend, :remove_friend] - - def make_friend - if request.post? - friendship = Friendship.new - friendship.befriender = current_user - friendship.befriendee = @friend - if current_user.friends_with?(@friend) - flash[:warning] = t ".already_followed", :name => @friend.display_name - elsif current_user.friendships.where(:created_at => Time.now.utc - 1.hour..).count >= current_user.max_friends_per_hour - flash[:error] = t ".limit_exceeded" - elsif friendship.save - flash[:notice] = t ".success", :name => @friend.display_name - UserMailer.friendship_notification(friendship).deliver_later - else - friendship.add_error(t(".failed", :name => @friend.display_name)) - end - - referer = safe_referer(params[:referer]) if params[:referer] - - redirect_to referer || user_path - end - end - - def remove_friend - if request.post? - if current_user.friends_with?(@friend) - Friendship.where(:befriender => current_user, :befriendee => @friend).delete_all - flash[:notice] = t ".success", :name => @friend.display_name - else - flash[:error] = t ".not_followed", :name => @friend.display_name - end - - referer = safe_referer(params[:referer]) if params[:referer] - - redirect_to referer || user_path - end - end - - private - - ## - # ensure that there is a "friend" instance variable - def lookup_friend - @friend = User.active.find_by!(:display_name => params[:display_name]) - rescue ActiveRecord::RecordNotFound - render_unknown_user params[:display_name] - end -end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index fc1b10551..fea73c710 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -119,16 +119,16 @@ class UserMailer < ApplicationMailer end end - def friendship_notification(friendship) - with_recipient_locale friendship.befriendee do - @friendship = friendship - @viewurl = user_url(@friendship.befriender) - @followurl = follow_url(@friendship.befriender) - @author = @friendship.befriender.display_name - - attach_user_avatar(@friendship.befriender) - mail :to => friendship.befriendee.email, - :subject => t(".subject", :user => friendship.befriender.display_name) + def friendship_notification(follow) + with_recipient_locale follow.following do + @follow = follow + @viewurl = user_url(@follow.follower) + @followurl = follow_url(@follow.follower) + @author = @follow.follower.display_name + + attach_user_avatar(@follow.follower) + mail :to => follow.following.email, + :subject => t(".subject", :user => follow.follower.display_name) end end diff --git a/app/models/friendship.rb b/app/models/follow.rb similarity index 69% rename from app/models/friendship.rb rename to app/models/follow.rb index 2b1c7ce00..ebf3ad735 100644 --- a/app/models/friendship.rb +++ b/app/models/follow.rb @@ -18,9 +18,9 @@ # friends_user_id_fkey (user_id => users.id) # -class Friendship < ApplicationRecord +class Follow < ApplicationRecord self.table_name = "friends" - belongs_to :befriender, :class_name => "User", :foreign_key => :user_id - belongs_to :befriendee, :class_name => "User", :foreign_key => :friend_user_id + belongs_to :follower, :class_name => "User", :foreign_key => :user_id, :inverse_of => :follows + belongs_to :following, :class_name => "User", :foreign_key => :friend_user_id, :inverse_of => :follows end diff --git a/app/models/user.rb b/app/models/user.rb index 917faca21..ec3883bc6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -58,8 +58,8 @@ class User < ApplicationRecord has_many :new_messages, -> { where(:to_user_visible => true, :muted => false, :message_read => false).order(:sent_on => :desc) }, :class_name => "Message", :foreign_key => :to_user_id has_many :sent_messages, -> { where(:from_user_visible => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :class_name => "Message", :foreign_key => :from_user_id has_many :muted_messages, -> { where(:to_user_visible => true, :muted => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :class_name => "Message", :foreign_key => :to_user_id - has_many :friendships, -> { joins(:befriendee).where(:users => { :status => %w[active confirmed] }) } - has_many :friends, :through => :friendships, :source => :befriendee + has_many :follows, -> { joins(:following).where(:users => { :status => %w[active confirmed] }) } + has_many :followings, :through => :follows, :source => :following has_many :preferences, :class_name => "UserPreference" has_many :changesets, -> { order(:created_at => :desc) }, :inverse_of => :user has_many :changeset_comments, :foreign_key => :author_id, :inverse_of => :author @@ -283,7 +283,7 @@ class User < ApplicationRecord end def friends_with?(new_friend) - friendships.exists?(:befriendee => new_friend) + follows.exists?(:following => new_friend) end ## @@ -414,7 +414,7 @@ class User < ApplicationRecord def max_friends_per_hour account_age_in_seconds = Time.now.utc - created_at account_age_in_hours = account_age_in_seconds / 3600 - recent_friends = Friendship.where(:befriendee => self).where(:created_at => Time.now.utc - 3600..).count + recent_friends = Follow.where(:following => self).where(:created_at => Time.now.utc - 3600..).count max_friends = account_age_in_hours.ceil + recent_friends - (active_reports * 10) max_friends.clamp(0, Settings.max_friends_per_hour) end diff --git a/app/views/dashboards/_contact.html.erb b/app/views/dashboards/_contact.html.erb index 41d0a24f2..0b3b42fb5 100644 --- a/app/views/dashboards/_contact.html.erb +++ b/app/views/dashboards/_contact.html.erb @@ -36,9 +36,9 @@
  • <%= link_to t("users.show.send message"), new_message_path(contact) %>
  • <% if current_user.friends_with?(contact) %> - <%= link_to t("users.show.unfollow"), remove_friend_path(:display_name => contact.display_name, :referer => request.fullpath), :method => :post %> + <%= link_to t("users.show.unfollow"), follow_path(:display_name => contact.display_name, :referer => request.fullpath), :method => :delete %> <% else %> - <%= link_to t("users.follow"), make_friend_path(:display_name => contact.display_name, :referer => request.fullpath), :method => :post %> + <%= link_to t("users.follow"), follow_path(:display_name => contact.display_name, :referer => request.fullpath), :method => :post %> <% end %>
    • diff --git a/app/views/dashboards/show.html.erb b/app/views/dashboards/show.html.erb index b1ca90e85..be4461a00 100644 --- a/app/views/dashboards/show.html.erb +++ b/app/views/dashboards/show.html.erb @@ -22,7 +22,7 @@ <%= tag.div "", :id => "map", :class => "content_map border border-secondary-subtle rounded z-0", :data => { :user => user_data } %> <% end %> - <% friends = @user.friends %> + <% friends = @user.followings %> <% nearby = @user.nearby - friends %> diff --git a/app/views/follows/show.html.erb b/app/views/follows/show.html.erb new file mode 100644 index 000000000..3b9ef8dc3 --- /dev/null +++ b/app/views/follows/show.html.erb @@ -0,0 +1,8 @@ +<% content_for :heading do %> +

    <%= t(@already_follows ? ".unfollow.heading" : ".follow.heading", :user => @friend.display_name) %>

    +<% end %> + +<%= link_to t(@already_follows ? ".unfollow.button" : ".follow.button"), + follow_path(:display_name => @friend.display_name, :referer => params[:referer]), + :method => (@already_follows ? :delete : :post), + :class => "btn btn-sm btn-primary" %> diff --git a/app/views/friendships/make_friend.html.erb b/app/views/friendships/make_friend.html.erb deleted file mode 100644 index f5c2b9c0c..000000000 --- a/app/views/friendships/make_friend.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<% content_for :heading do %> -

    <%= t ".heading", :user => @friend.display_name %>

    -<% end %> - -<%= bootstrap_form_tag do |f| %> - <% if params[:referer] -%> - <%= f.hidden_field :referer, :value => params[:referer] %> - <% end -%> - <%= f.primary t(".button") %> -<% end %> diff --git a/app/views/friendships/remove_friend.html.erb b/app/views/friendships/remove_friend.html.erb deleted file mode 100644 index f5c2b9c0c..000000000 --- a/app/views/friendships/remove_friend.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<% content_for :heading do %> -

    <%= t ".heading", :user => @friend.display_name %>

    -<% end %> - -<%= bootstrap_form_tag do |f| %> - <% if params[:referer] -%> - <%= f.hidden_field :referer, :value => params[:referer] %> - <% end -%> - <%= f.primary t(".button") %> -<% end %> diff --git a/app/views/user_mailer/friendship_notification.html.erb b/app/views/user_mailer/friendship_notification.html.erb index fffad9fa2..16ddcad49 100644 --- a/app/views/user_mailer/friendship_notification.html.erb +++ b/app/views/user_mailer/friendship_notification.html.erb @@ -1,11 +1,11 @@ -

    <%= t ".hi", :to_user => @friendship.befriendee.display_name %>

    +

    <%= t ".hi", :to_user => @follow.following.display_name %>

    -

    <%= t ".followed_you", :user => @friendship.befriender.display_name %>

    +

    <%= t ".followed_you", :user => @follow.follower.display_name %>

    <%= message_body do %>

    <%= t ".see_their_profile_html", :userurl => link_to(@viewurl, @viewurl) %>

    - <% unless @friendship.befriendee.friends_with?(@friendship.befriender) -%> + <% unless @follow.following.friends_with?(@follow.follower) -%>

    <%= t ".follow_them_html", :followurl => link_to(@followurl, @followurl) %>

    <% end -%> <% end %> diff --git a/app/views/user_mailer/friendship_notification.text.erb b/app/views/user_mailer/friendship_notification.text.erb index 593bc765a..624ba92b7 100644 --- a/app/views/user_mailer/friendship_notification.text.erb +++ b/app/views/user_mailer/friendship_notification.text.erb @@ -1,9 +1,9 @@ -<%= t ".hi", :to_user => @friendship.befriendee.display_name %> +<%= t ".hi", :to_user => @follow.following.display_name %> -<%= t '.followed_you', :user => @friendship.befriender.display_name %> +<%= t '.followed_you', :user => @follow.follower.display_name %> <%= t '.see_their_profile', :userurl => @viewurl %> -<% unless @friendship.befriendee.friends_with?(@friendship.befriender) -%> +<% unless @follow.following.friends_with?(@follow.follower) -%> <%= t '.follow_them', :followurl => @followurl %> <% end -%> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index efeac90bc..5a8c116c1 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -84,9 +84,9 @@ <% if current_user %>
  • <% if current_user.friends_with?(@user) %> - <%= link_to t(".unfollow"), remove_friend_path(:display_name => @user.display_name), :method => :post %> + <%= link_to t(".unfollow"), follow_path(:display_name => @user.display_name), :method => :delete %> <% else %> - <%= link_to t(".follow"), make_friend_path(:display_name => @user.display_name), :method => :post %> + <%= link_to t(".follow"), follow_path(:display_name => @user.display_name), :method => :post %> <% end %>
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index f9fe33041..ff43a0787 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -686,17 +686,20 @@ en: not_found: title: File not found description: Couldn't find a file/directory/API operation by that name on the OpenStreetMap server (HTTP 404) - friendships: - make_friend: - heading: "Do you want to follow %{user}?" - button: "Follow User" + follows: + show: + follow: + heading: "Do you want to follow %{user}?" + button: "Follow User" + unfollow: + heading: "Do you want to unfollow %{user}?" + button: "Unfollow User" + create: success: "You are now following %{name}!" failed: "Sorry, your request to follow %{name} has failed." already_followed: "You already follow %{name}." limit_exceeded: "You have followed a lot of users recently. Please wait a while before trying to follow any more." - remove_friend: - heading: "Do you want to unfollow %{user}?" - button: "Unfollow" + destroy: success: "You successfully unfollowed %{name}." not_followed: "You are not following %{name}." geocoder: diff --git a/config/routes.rb b/config/routes.rb index 479d35346..0b18c47fd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -288,8 +288,12 @@ OpenStreetMap::Application.routes.draw do resource :profile, :only => [:edit, :update] # friendships - match "/user/:display_name/make_friend" => "friendships#make_friend", :via => [:get, :post], :as => "make_friend" - match "/user/:display_name/remove_friend" => "friendships#remove_friend", :via => [:get, :post], :as => "remove_friend" + scope "/user/:display_name" do + resource :follow, :only => [:create, :destroy, :show], :path => "follow" + + get "make_friend", :to => redirect("/user/%{display_name}/follow") + get "remove_friend", :to => redirect("/user/%{display_name}/follow") + end # user lists namespace :users do diff --git a/test/controllers/changesets_controller_test.rb b/test/controllers/changesets_controller_test.rb index a486e4b5e..9bbca2ab1 100644 --- a/test/controllers/changesets_controller_test.rb +++ b/test/controllers/changesets_controller_test.rb @@ -190,8 +190,8 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest # Checks the display of the friends changesets listing def test_index_friends private_user = create(:user, :data_public => true) - friendship = create(:friendship, :befriender => private_user) - changeset = create(:changeset, :user => friendship.befriendee, :num_changes => 1) + follow = create(:follow, :follower => private_user) + changeset = create(:changeset, :user => follow.following, :num_changes => 1) _changeset2 = create(:changeset, :user => create(:user), :num_changes => 1) get friend_changesets_path diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index a1c22fff8..c96c433bf 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@ -384,8 +384,8 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest def test_index_friends user = create(:user) other_user = create(:user) - friendship = create(:friendship, :befriender => user) - diary_entry = create(:diary_entry, :user => friendship.befriendee) + follow = create(:follow, :follower => user) + diary_entry = create(:diary_entry, :user => follow.following) _other_entry = create(:diary_entry, :user => other_user) # Try a list of diary entries for your friends when not logged in diff --git a/test/controllers/follows_controller_test.rb b/test/controllers/follows_controller_test.rb new file mode 100644 index 000000000..93bb3bc3e --- /dev/null +++ b/test/controllers/follows_controller_test.rb @@ -0,0 +1,182 @@ +require "test_helper" + +class FollowsControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/user/username/follow", :method => :get }, + { :controller => "follows", :action => "show", :display_name => "username" } + ) + assert_routing( + { :path => "/user/username/follow", :method => :post }, + { :controller => "follows", :action => "create", :display_name => "username" } + ) + assert_routing( + { :path => "/user/username/follow", :method => :delete }, + { :controller => "follows", :action => "destroy", :display_name => "username" } + ) + end + + def test_follow + # Get users to work with + user = create(:user) + follow = create(:user) + + # Check that the users aren't already friends + assert_nil Follow.find_by(:follower => user, :following => follow) + + # When not logged in a GET should ask us to login + get follow_path(follow) + assert_redirected_to login_path(:referer => follow_path(follow)) + + # When not logged in a POST should error + post follow_path(follow) + assert_response :forbidden + assert_nil Follow.find_by(:follower => user, :following => follow) + + session_for(user) + + # When logged in a GET should get a confirmation page + get follow_path(follow) + assert_response :success + assert_template :show + assert_select "a[href*='test']", 0 + assert_nil Follow.find_by(:follower => user, :following => follow) + + # When logged in a POST should add the follow + assert_difference "ActionMailer::Base.deliveries.size", 1 do + perform_enqueued_jobs do + post follow_path(follow) + end + end + assert_redirected_to user_path(follow) + assert_match(/You are now following/, flash[:notice]) + assert Follow.find_by(:follower => user, :following => follow) + email = ActionMailer::Base.deliveries.first + assert_equal 1, email.to.count + assert_equal follow.email, email.to.first + ActionMailer::Base.deliveries.clear + + # A second POST should report that the follow already exists + assert_no_difference "ActionMailer::Base.deliveries.size" do + perform_enqueued_jobs do + post follow_path(follow) + end + end + assert_redirected_to user_path(follow) + assert_match(/You already follow/, flash[:warning]) + assert Follow.find_by(:follower => user, :following => follow) + end + + def test_follow_with_referer + # Get users to work with + user = create(:user) + follow = create(:user) + session_for(user) + + # Check that the users aren't already friends + assert_nil Follow.find_by(:follower => user, :following => follow) + + # The GET should preserve any referer + get follow_path(follow), :params => { :referer => "/test" } + assert_response :success + assert_template :show + assert_select "a[href*='test']" + assert_nil Follow.find_by(:follower => user, :following => follow) + + # When logged in a POST should add the follow and refer us + assert_difference "ActionMailer::Base.deliveries.size", 1 do + perform_enqueued_jobs do + post follow_path(follow), :params => { :referer => "/test" } + end + end + assert_redirected_to "/test" + assert_match(/You are now following/, flash[:notice]) + assert Follow.find_by(:follower => user, :following => follow) + email = ActionMailer::Base.deliveries.first + assert_equal 1, email.to.count + assert_equal follow.email, email.to.first + ActionMailer::Base.deliveries.clear + end + + def test_follow_unknown_user + # Should error when a bogus user is specified + session_for(create(:user)) + get follow_path("No Such User") + assert_response :not_found + assert_template :no_such_user + end + + def test_unfollow + # Get users to work with + user = create(:user) + follow = create(:user) + create(:follow, :follower => user, :following => follow) + + # Check that the users are friends + assert Follow.find_by(:follower => user, :following => follow) + + # When not logged in a GET should ask us to login + get follow_path(follow) + assert_redirected_to login_path(:referer => follow_path(follow)) + + # When not logged in a POST should error + delete follow_path, :params => { :display_name => follow.display_name } + assert_response :forbidden + assert Follow.find_by(:follower => user, :following => follow) + + session_for(user) + + # When logged in a GET should get a confirmation page + get follow_path(follow) + assert_response :success + assert_template :show + assert_select "a[href*='test']", 0 + assert Follow.find_by(:follower => user, :following => follow) + + # When logged in a DELETE should remove the follow + delete follow_path(follow) + assert_redirected_to user_path(follow) + assert_match(/You successfully unfollowed/, flash[:notice]) + assert_nil Follow.find_by(:follower => user, :following => follow) + + # A second DELETE should report that the follow does not exist + delete follow_path(follow) + assert_redirected_to user_path(follow) + assert_match(/You are not following/, flash[:error]) + assert_nil Follow.find_by(:follower => user, :following => follow) + end + + def test_unfollow_with_referer + # Get users to work with + user = create(:user) + follow = create(:user) + create(:follow, :follower => user, :following => follow) + session_for(user) + + # Check that the users are friends + assert Follow.find_by(:follower => user, :following => follow) + + # The GET should preserve any referer + get follow_path(follow), :params => { :referer => "/test" } + assert_response :success + assert_template :show + assert_select "a[href*='test']" + assert Follow.find_by(:follower => user, :following => follow) + + # When logged in a POST should remove the follow and refer + delete follow_path(follow), :params => { :referer => "/test" } + assert_redirected_to "/test" + assert_match(/You successfully unfollowed/, flash[:notice]) + assert_nil Follow.find_by(:follower => user, :following => follow) + end + + def test_unfollow_unknown_user + # Should error when a bogus user is specified + session_for(create(:user)) + get follow_path("No Such User") + assert_response :not_found + assert_template :no_such_user + end +end diff --git a/test/controllers/friendships_controller_test.rb b/test/controllers/friendships_controller_test.rb deleted file mode 100644 index 5e5cf1344..000000000 --- a/test/controllers/friendships_controller_test.rb +++ /dev/null @@ -1,198 +0,0 @@ -require "test_helper" - -class FriendshipsControllerTest < ActionDispatch::IntegrationTest - ## - # test all routes which lead to this controller - def test_routes - assert_routing( - { :path => "/user/username/follow", :method => :get }, - { :controller => "friendships", :action => "follow", :display_name => "username" } - ) - assert_routing( - { :path => "/user/username/follow", :method => :post }, - { :controller => "friendships", :action => "follow", :display_name => "username" } - ) - assert_routing( - { :path => "/user/username/unfollow", :method => :get }, - { :controller => "friendships", :action => "unfollow", :display_name => "username" } - ) - assert_routing( - { :path => "/user/username/unfollow", :method => :post }, - { :controller => "friendships", :action => "unfollow", :display_name => "username" } - ) - end - - def test_follow - # Get users to work with - user = create(:user) - friend = create(:user) - - # Check that the users aren't already friends - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - - # When not logged in a GET should ask us to login - get make_friend_path(friend) - assert_redirected_to login_path(:referer => make_friend_path(friend)) - - # When not logged in a POST should error - post make_friend_path(friend) - assert_response :forbidden - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - - session_for(user) - - # When logged in a GET should get a confirmation page - get make_friend_path(friend) - assert_response :success - assert_template :follow - assert_select "form" do - assert_select "input[type='hidden'][name='referer']", 0 - assert_select "input[type='submit']", 1 - end - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - - # When logged in a POST should add the friendship - assert_difference "ActionMailer::Base.deliveries.size", 1 do - perform_enqueued_jobs do - post make_friend_path(friend) - end - end - assert_redirected_to user_path(friend) - assert_match(/You are now following/, flash[:notice]) - assert Friendship.find_by(:befriender => user, :befriendee => friend) - email = ActionMailer::Base.deliveries.first - assert_equal 1, email.to.count - assert_equal friend.email, email.to.first - ActionMailer::Base.deliveries.clear - - # A second POST should report that the friendship already exists - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post make_friend_path(friend) - end - end - assert_redirected_to user_path(friend) - assert_match(/You already follow/, flash[:warning]) - assert Friendship.find_by(:befriender => user, :befriendee => friend) - end - - def test_follow_with_referer - # Get users to work with - user = create(:user) - friend = create(:user) - session_for(user) - - # Check that the users aren't already friends - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - - # The GET should preserve any referer - get make_friend_path(friend), :params => { :referer => "/test" } - assert_response :success - assert_template :follow - assert_select "form" do - assert_select "input[type='hidden'][name='referer'][value='/test']", 1 - assert_select "input[type='submit']", 1 - end - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - - # When logged in a POST should add the friendship and refer us - assert_difference "ActionMailer::Base.deliveries.size", 1 do - perform_enqueued_jobs do - post make_friend_path(friend), :params => { :referer => "/test" } - end - end - assert_redirected_to "/test" - assert_match(/You are now following/, flash[:notice]) - assert Friendship.find_by(:befriender => user, :befriendee => friend) - email = ActionMailer::Base.deliveries.first - assert_equal 1, email.to.count - assert_equal friend.email, email.to.first - ActionMailer::Base.deliveries.clear - end - - def test_follow_unknown_user - # Should error when a bogus user is specified - session_for(create(:user)) - get make_friend_path("No Such User") - assert_response :not_found - assert_template :no_such_user - end - - def test_unfollow - # Get users to work with - user = create(:user) - friend = create(:user) - create(:friendship, :befriender => user, :befriendee => friend) - - # Check that the users are friends - assert Friendship.find_by(:befriender => user, :befriendee => friend) - - # When not logged in a GET should ask us to login - get remove_friend_path(friend) - assert_redirected_to login_path(:referer => remove_friend_path(friend)) - - # When not logged in a POST should error - post remove_friend_path, :params => { :display_name => friend.display_name } - assert_response :forbidden - assert Friendship.find_by(:befriender => user, :befriendee => friend) - - session_for(user) - - # When logged in a GET should get a confirmation page - get remove_friend_path(friend) - assert_response :success - assert_template :unfollow - assert_select "form" do - assert_select "input[type='hidden'][name='referer']", 0 - assert_select "input[type='submit']", 1 - end - assert Friendship.find_by(:befriender => user, :befriendee => friend) - - # When logged in a POST should remove the friendship - post remove_friend_path(friend) - assert_redirected_to user_path(friend) - assert_match(/You successfully unfollowed/, flash[:notice]) - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - - # A second POST should report that the friendship does not exist - post remove_friend_path(friend) - assert_redirected_to user_path(friend) - assert_match(/You are not following/, flash[:error]) - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - end - - def test_unfollow_with_referer - # Get users to work with - user = create(:user) - friend = create(:user) - create(:friendship, :befriender => user, :befriendee => friend) - session_for(user) - - # Check that the users are friends - assert Friendship.find_by(:befriender => user, :befriendee => friend) - - # The GET should preserve any referer - get remove_friend_path(friend), :params => { :referer => "/test" } - assert_response :success - assert_template :unfollow - assert_select "form" do - assert_select "input[type='hidden'][name='referer'][value='/test']", 1 - assert_select "input[type='submit']", 1 - end - assert Friendship.find_by(:befriender => user, :befriendee => friend) - - # When logged in a POST should remove the friendship and refer - post remove_friend_path(friend), :params => { :referer => "/test" } - assert_redirected_to "/test" - assert_match(/You successfully unfollowed/, flash[:notice]) - assert_nil Friendship.find_by(:befriender => user, :befriendee => friend) - end - - def test_unfollow_unknown_user - # Should error when a bogus user is specified - session_for(create(:user)) - get remove_friend_path("No Such User") - assert_response :not_found - assert_template :no_such_user - end -end diff --git a/test/factories/follows.rb b/test/factories/follows.rb new file mode 100644 index 000000000..01dad52e0 --- /dev/null +++ b/test/factories/follows.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :follow do + follower :factory => :user + following :factory => :user + end +end diff --git a/test/factories/friendships.rb b/test/factories/friendships.rb deleted file mode 100644 index 12df2a945..000000000 --- a/test/factories/friendships.rb +++ /dev/null @@ -1,6 +0,0 @@ -FactoryBot.define do - factory :friendship do - befriender :factory => :user - befriendee :factory => :user - end -end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 6836c4f70..3b600fc87 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -140,7 +140,7 @@ class UserTest < ActiveSupport::TestCase alice = create(:user, :active) bob = create(:user, :active) charlie = create(:user, :active) - create(:friendship, :befriender => alice, :befriendee => bob) + create(:follow, :follower => alice, :following => bob) assert alice.friends_with?(bob) assert_not alice.friends_with?(charlie) @@ -174,13 +174,13 @@ class UserTest < ActiveSupport::TestCase def test_friends norm = create(:user, :active) sec = create(:user, :active) - create(:friendship, :befriender => norm, :befriendee => sec) + create(:follow, :follower => norm, :following => sec) - assert_equal [sec], norm.friends - assert_equal 1, norm.friends.size + assert_equal [sec], norm.followings + assert_equal 1, norm.followings.size - assert_empty sec.friends - assert_equal 0, sec.friends.size + assert_empty sec.followings + assert_equal 0, sec.followings.size end def test_user_preferred_editor diff --git a/test/system/dashboard_test.rb b/test/system/dashboard_test.rb index ef1b952dc..2ab4db5fa 100644 --- a/test/system/dashboard_test.rb +++ b/test/system/dashboard_test.rb @@ -12,7 +12,7 @@ class DashboardSystemTest < ApplicationSystemTestCase test "show users if have friends" do user = create(:user, :home_lon => 1.1, :home_lat => 1.1) friend_user = create(:user, :home_lon => 1.2, :home_lat => 1.2) - create(:friendship, :befriender => user, :befriendee => friend_user) + create(:follow, :follower => user, :following => friend_user) create(:changeset, :user => friend_user) sign_in_as(user) diff --git a/test/system/friendships_test.rb b/test/system/follows_test.rb similarity index 74% rename from test/system/friendships_test.rb rename to test/system/follows_test.rb index 25b627f53..f9f00ff7f 100644 --- a/test/system/friendships_test.rb +++ b/test/system/follows_test.rb @@ -1,13 +1,13 @@ require "application_system_test_case" -class FriendshipsTest < ApplicationSystemTestCase +class FollowsTest < ApplicationSystemTestCase test "show message when max frienships limit is exceeded" do - befriendee = create(:user) + following = create(:user) sign_in_as create(:user) with_settings(:max_friends_per_hour => 0) do - visit user_path(befriendee) + visit user_path(following) assert_link "Follow" click_on "Follow"