From ce951453a263c862f814658a31619cbf7f43a3c4 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Fri, 31 Dec 2021 22:19:21 +0100 Subject: [PATCH] Make relationships polymorphic --- app/controllers/user_controller.rb | 8 ++-- app/models/relationship.rb | 20 ---------- app/models/relationships/follow.rb | 19 +++++++++ app/models/user.rb | 15 +------ app/models/user/relationship.rb | 23 +++++++++++ app/models/user/relationship/follow.rb | 39 +++++++++++++++++++ app/models/user/relationship_methods.rb | 6 +-- app/models/user/timeline_methods.rb | 2 +- app/views/tabs/_profile.haml | 4 +- config/routes.rb | 4 +- ...0211231155702_add_type_to_relationships.rb | 16 ++++++++ db/schema.rb | 2 + .../ajax/friend_controller_spec.rb | 18 ++++----- 13 files changed, 122 insertions(+), 54 deletions(-) create mode 100644 app/models/relationships/follow.rb create mode 100644 app/models/user/relationship.rb create mode 100644 app/models/user/relationship/follow.rb create mode 100644 db/migrate/20211231155702_add_type_to_relationships.rb diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index a0537064..c11f0757 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -10,7 +10,7 @@ class UserController < ApplicationController @more_data_available = !@user.cursored_answers(last_id: @answers_last_id, size: 1).count.zero? if user_signed_in? - notif = Notification.where(target_type: "Relationship", target_id: @user.active_relationships.where(target_id: current_user.id).pluck(:id), recipient_id: current_user.id, new: true).first + notif = Notification.where(target_type: "Relationship", target_id: @user.active_follow_relationships.where(target_id: current_user.id).pluck(:id), recipient_id: current_user.id, new: true).first unless notif.nil? notif.new = false notif.save @@ -96,12 +96,12 @@ class UserController < ApplicationController end end - def friends + def followings @title = 'Following' @user = User.where('LOWER(screen_name) = ?', params[:username].downcase).includes(:profile).first! - @users = @user.cursored_friends(last_id: params[:last_id]) + @users = @user.cursored_followings(last_id: params[:last_id]) @users_last_id = @users.map(&:id).min - @more_data_available = !@user.cursored_friends(last_id: @users_last_id, size: 1).count.zero? + @more_data_available = !@user.cursored_followings(last_id: @users_last_id, size: 1).count.zero? @type = :friend respond_to do |format| diff --git a/app/models/relationship.rb b/app/models/relationship.rb index b956df38..6a96d5cb 100644 --- a/app/models/relationship.rb +++ b/app/models/relationship.rb @@ -5,24 +5,4 @@ class Relationship < ApplicationRecord validates :target_id, presence: true default_scope { order(created_at: :asc) } - - after_create do - Notification.notify target, self - - # increment counts - source.increment! :friend_count - target.increment! :follower_count - end - - before_destroy do - Notification.denotify target, self - - # decrement counts - source.decrement! :friend_count - target.decrement! :follower_count - end - - def notification_type(*_args) - Notifications::StartedFollowing - end end diff --git a/app/models/relationships/follow.rb b/app/models/relationships/follow.rb new file mode 100644 index 00000000..f8d3c657 --- /dev/null +++ b/app/models/relationships/follow.rb @@ -0,0 +1,19 @@ +class Relationships::Follow < Relationship + after_create do + Notification.notify target, self + # increment counts + source.increment! :friend_count + target.increment! :follower_count + end + + before_destroy do + Notification.denotify target, self + # decrement counts + source.decrement! :friend_count + target.decrement! :follower_count + end + + def notification_type(*_args) + Notifications::StartedFollowing + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 1f175539..c11991e7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,4 +1,6 @@ class User < ApplicationRecord + include User::Relationship + include User::Relationship::Follow include User::AnswerMethods include User::InboxMethods include User::QuestionMethods @@ -24,14 +26,6 @@ class User < ApplicationRecord has_many :answers, dependent: :destroy has_many :comments, dependent: :destroy has_many :inboxes, dependent: :destroy - has_many :active_relationships, class_name: 'Relationship', - foreign_key: 'source_id', - dependent: :destroy - has_many :passive_relationships, class_name: 'Relationship', - foreign_key: 'target_id', - dependent: :destroy - has_many :friends, through: :active_relationships, source: :target - has_many :followers, through: :passive_relationships, source: :source has_many :smiles, dependent: :destroy has_many :comment_smiles, dependent: :destroy has_many :services, dependent: :destroy @@ -111,11 +105,6 @@ class User < ApplicationRecord active_relationships.find_by(target: target_user).destroy end - # @return [Boolean] true if +self+ is following +target_user+ - def following?(target_user) - friends.include? target_user - end - # @param list [List] # @return [Boolean] true if +self+ is a member of +list+ def member_of?(list) diff --git a/app/models/user/relationship.rb b/app/models/user/relationship.rb new file mode 100644 index 00000000..851fea31 --- /dev/null +++ b/app/models/user/relationship.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class User + module Relationship + extend ActiveSupport::Concern + + private + + # Create a relationship for `type` with `target_user` as target. + def create_relationship(type, target_user) + type.create(target: target_user) + end + + # Destroy a relationship for `type` with `target_user` as target. + def destroy_relationship(type, target_user) + type.find_by(target: target_user)&.destroy + end + + def relationship_active?(type, target_user) + type.include?(target_user) + end + end +end diff --git a/app/models/user/relationship/follow.rb b/app/models/user/relationship/follow.rb new file mode 100644 index 00000000..a2a9869d --- /dev/null +++ b/app/models/user/relationship/follow.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require "errors" + +class User + module Relationship + module Follow + extend ActiveSupport::Concern + + included do + has_many :active_follow_relationships, class_name: "Relationships::Follow", + foreign_key: "source_id", + dependent: :destroy + has_many :passive_follow_relationships, class_name: "Relationships::Follow", + foreign_key: "target_id", + dependent: :destroy + has_many :followings, through: :active_follow_relationships, source: :target + has_many :followers, through: :passive_follow_relationships, source: :source + end + + # Follow an user + def follow(target_user) + raise Justask::Errors::FollowingOtherBlockedSelf if target_user.blocking?(self) + raise Justask::Errors::FollowingSelfBlockedOther if blocking?(target_user) + raise Justask::Errors::FollowingSelf if target_user == self + create_relationship(active_follow_relationships, target_user) + end + + # Unfollow an user + def unfollow(target_user) + destroy_relationship(active_follow_relationships, target_user) + end + + def following?(target_user) + relationship_active?(followings, target_user) + end + end + end +end diff --git a/app/models/user/relationship_methods.rb b/app/models/user/relationship_methods.rb index b527fe25..1392a331 100644 --- a/app/models/user/relationship_methods.rb +++ b/app/models/user/relationship_methods.rb @@ -3,11 +3,11 @@ module User::RelationshipMethods include CursorPaginatable - define_cursor_paginator :cursored_friends, :ordered_friends + define_cursor_paginator :cursored_followings, :ordered_followings define_cursor_paginator :cursored_followers, :ordered_followers - def ordered_friends - friends.reverse_order.includes(:profile) + def ordered_followings + followings.reverse_order.includes(:profile) end def ordered_followers diff --git a/app/models/user/timeline_methods.rb b/app/models/user/timeline_methods.rb index 34127835..57c79115 100644 --- a/app/models/user/timeline_methods.rb +++ b/app/models/user/timeline_methods.rb @@ -7,6 +7,6 @@ module User::TimelineMethods # @return [Array] the users' timeline def timeline - Answer.where('user_id in (?) OR user_id = ?', friend_ids, id).order(:created_at).reverse_order.includes(comments: [:user, :smiles], question: [:user], user: [:profile], smiles: [:user]) + Answer.where('user_id in (?) OR user_id = ?', following_ids, id).order(:created_at).reverse_order.includes(comments: [:user, :smiles], question: [:user], user: [:profile], smiles: [:user]) end end diff --git a/app/views/tabs/_profile.haml b/app/views/tabs/_profile.haml index 82e3d9f2..5df6f699 100644 --- a/app/views/tabs/_profile.haml +++ b/app/views/tabs/_profile.haml @@ -2,5 +2,5 @@ .list-group.list-group-horizontal-sm.text-center = list_group_item 'Answers', show_user_profile_path(user.screen_name), badge: user.answered_count = list_group_item 'Questions', show_user_questions_path(user.screen_name), badge: user.asked_count - = list_group_item 'Followers', show_user_followers_path(user.screen_name), badge: user.follower_count - = list_group_item 'Following', show_user_friends_path(user.screen_name), badge: user.friend_count + = list_group_item 'Followers', show_user_followers_path(user.screen_name), badge: user.followers.count + = list_group_item 'Following', show_user_followings_path(user.screen_name), badge: user.followings.count diff --git a/config/routes.rb b/config/routes.rb index d274489c..31c8976a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -135,12 +135,12 @@ Rails.application.routes.draw do match '/@:username/a/:id', to: 'answer#show', via: 'get', as: :show_user_answer_alt match '/@:username/q/:id', to: 'question#show', via: 'get', as: :show_user_question_alt match '/@:username/followers(/p/:page)', to: 'user#followers', via: 'get', as: :show_user_followers_alt, defaults: {page: 1} - match '/@:username/friends(/p/:page)', to: 'user#friends', via: 'get', as: :show_user_friends_alt, defaults: {page: 1} + match '/@:username/followings(/p/:page)', to: 'user#followings', via: 'get', as: :show_user_followings_alt, defaults: {page: 1} match '/:username(/p/:page)', to: 'user#show', via: 'get', as: :show_user_profile, defaults: {page: 1} match '/:username/a/:id', to: 'answer#show', via: 'get', as: :show_user_answer match '/:username/q/:id', to: 'question#show', via: 'get', as: :show_user_question match '/:username/followers(/p/:page)', to: 'user#followers', via: 'get', as: :show_user_followers, defaults: {page: 1} - match '/:username/friends(/p/:page)', to: 'user#friends', via: 'get', as: :show_user_friends, defaults: {page: 1} + match '/:username/followings(/p/:page)', to: 'user#followings', via: 'get', as: :show_user_followings, defaults: {page: 1} match '/:username/lists(/p/:page)', to: 'user#lists', via: 'get', as: :show_user_lists, defaults: {page: 1} match '/:username/questions(/p/:page)', to: 'user#questions', via: 'get', as: :show_user_questions, defaults: {page: 1} diff --git a/db/migrate/20211231155702_add_type_to_relationships.rb b/db/migrate/20211231155702_add_type_to_relationships.rb new file mode 100644 index 00000000..dd393b71 --- /dev/null +++ b/db/migrate/20211231155702_add_type_to_relationships.rb @@ -0,0 +1,16 @@ +class AddTypeToRelationships < ActiveRecord::Migration[5.2] + def up + add_column :relationships, :type, :string + + execute %(update relationships set type = 'Relationships::Follow') + + change_column_null :relationships, :type, false + add_index :relationships, :type + end + + def down + execute %(delete from relationships where type <> 'Relationships::Follow') + + remove_column :relationships, :type + end +end diff --git a/db/schema.rb b/db/schema.rb index 5087ead2..3db5ff31 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -157,9 +157,11 @@ ActiveRecord::Schema.define(version: 2022_01_05_171216) do t.bigint "target_id" t.datetime "created_at" t.datetime "updated_at" + t.string "type", null: false t.index ["source_id", "target_id"], name: "index_relationships_on_source_id_and_target_id", unique: true t.index ["source_id"], name: "index_relationships_on_source_id" t.index ["target_id"], name: "index_relationships_on_target_id" + t.index ["type"], name: "index_relationships_on_type" end create_table "reports", id: :serial, force: :cascade do |t| diff --git a/spec/controllers/ajax/friend_controller_spec.rb b/spec/controllers/ajax/friend_controller_spec.rb index 89b846cf..51daddf6 100644 --- a/spec/controllers/ajax/friend_controller_spec.rb +++ b/spec/controllers/ajax/friend_controller_spec.rb @@ -28,9 +28,9 @@ describe Ajax::FriendController, :ajax_controller, type: :controller do end it "creates a follow relationship" do - expect(user.friends.ids).not_to include(target_user.id) - expect { subject }.to(change { user.friends.count }.by(1)) - expect(user.friends.ids).to include(target_user.id) + expect(user.followings.ids).not_to include(target_user.id) + expect { subject }.to(change { user.followings.count }.by(1)) + expect(user.followings.ids).to include(target_user.id) end include_examples "returns the expected response" @@ -48,7 +48,7 @@ describe Ajax::FriendController, :ajax_controller, type: :controller do end it "does not create a follow relationship" do - expect { subject }.not_to(change { user.friends.count }) + expect { subject }.not_to(change { user.followings.count }) end include_examples "returns the expected response" @@ -99,9 +99,9 @@ describe Ajax::FriendController, :ajax_controller, type: :controller do before(:each) { user.follow target_user } it "destroys a follow relationship" do - expect(user.friends.ids).to include(target_user.id) - expect { subject }.to(change { user.friends.count }.by(-1)) - expect(user.friends.ids).not_to include(target_user.id) + expect(user.followings.ids).to include(target_user.id) + expect { subject }.to(change { user.followings.count }.by(-1)) + expect(user.followings.ids).not_to include(target_user.id) end include_examples "returns the expected response" @@ -117,7 +117,7 @@ describe Ajax::FriendController, :ajax_controller, type: :controller do end it "does not destroy a follow relationship" do - expect { subject }.not_to(change { user.friends.count }) + expect { subject }.not_to(change { user.followings.count }) end include_examples "returns the expected response" @@ -136,7 +136,7 @@ describe Ajax::FriendController, :ajax_controller, type: :controller do end it "does not destroy a follow relationship" do - expect { subject }.not_to(change { user.friends.count }) + expect { subject }.not_to(change { user.followings.count }) end include_examples "returns the expected response"