From 731ee4bf69a1b5838240ac3a4bf6a526eb09b122 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sat, 22 Jan 2022 22:48:55 +0100 Subject: [PATCH] Address review comments from @nilsding --- app/controllers/ajax/relationship_controller.rb | 12 ++++++------ config/routes.rb | 2 ++ db/schema.rb | 1 + lib/use_case/relationship/create.rb | 8 ++++---- lib/use_case/relationship/destroy.rb | 8 ++++++-- spec/lib/use_case/relationship/create_spec.rb | 12 ++++++------ spec/lib/use_case/relationship/destroy_spec.rb | 10 +++++----- 7 files changed, 30 insertions(+), 23 deletions(-) diff --git a/app/controllers/ajax/relationship_controller.rb b/app/controllers/ajax/relationship_controller.rb index bdf0c6c5..efc684e1 100644 --- a/app/controllers/ajax/relationship_controller.rb +++ b/app/controllers/ajax/relationship_controller.rb @@ -11,9 +11,9 @@ class Ajax::RelationshipController < AjaxController params.require :screen_name UseCase::Relationship::Create.call( - current_user: current_user.screen_name, - target_user: params[:screen_name], - type: params[:type] + source_user: current_user.screen_name, + target_user: params[:screen_name], + type: params[:type] ) @response[:success] = true @response[:message] = t("messages.friend.create.okay") @@ -25,9 +25,9 @@ class Ajax::RelationshipController < AjaxController def destroy UseCase::Relationship::Destroy.call( - current_user: current_user.screen_name, - target_user: params[:screen_name], - type: params[:type] + source_user: current_user.screen_name, + target_user: params[:screen_name], + type: params[:type] ) @response[:success] = true @response[:message] = t("messages.friend.destroy.okay") diff --git a/config/routes.rb b/config/routes.rb index 007cdc73..6b6d443e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -136,11 +136,13 @@ Rails.application.routes.draw do 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/followings(/p/:page)', to: 'user#followings', via: 'get', as: :show_user_followings_alt, defaults: {page: 1} + match '/@:username/friends(/p/:page)', to: redirect('/@%{username}/followings/p/%{page}'), via: 'get', 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/followings(/p/:page)', to: 'user#followings', via: 'get', as: :show_user_followings, defaults: {page: 1} + match '/:username/friends(/p/:page)', to: redirect('/%{username}/followings/p/%{page}'), via: 'get', 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/schema.rb b/db/schema.rb index 764937c1..d8b953db 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -323,5 +323,6 @@ ActiveRecord::Schema.define(version: 2022_01_05_171216) do t.index ["user_id"], name: "index_users_roles_on_user_id" end + add_foreign_key "mute_rules", "users" add_foreign_key "profiles", "users" end diff --git a/lib/use_case/relationship/create.rb b/lib/use_case/relationship/create.rb index adb0db0a..b90a5daf 100644 --- a/lib/use_case/relationship/create.rb +++ b/lib/use_case/relationship/create.rb @@ -6,8 +6,8 @@ require "errors" module UseCase module Relationship class Create < UseCase::Base - option :current_user, type: Types::Coercible::String - option :target_user, type: Types::Coercible::String + option :source_user, type: Types::Coercible::String | Types.Instance(User) + option :target_user, type: Types::Coercible::String | Types.Instance(User) option :type, type: Types::RelationshipTypes def call @@ -28,9 +28,9 @@ module UseCase private def find_source_user - return current_user if current_user.is_a?(::User) + return source_user if source_user.is_a?(::User) - ::User.find_by!(screen_name: current_user) + ::User.find_by!(screen_name: source_user) rescue ActiveRecord::RecordNotFound raise Errors::UserNotFound end diff --git a/lib/use_case/relationship/destroy.rb b/lib/use_case/relationship/destroy.rb index b73fe6d0..ca62cd88 100644 --- a/lib/use_case/relationship/destroy.rb +++ b/lib/use_case/relationship/destroy.rb @@ -6,7 +6,7 @@ require "errors" module UseCase module Relationship class Destroy < UseCase::Base - option :current_user, type: Types::Coercible::String | Types.Instance(User) + option :source_user, type: Types::Coercible::String | Types.Instance(User) option :target_user, type: Types::Coercible::String | Types.Instance(User) option :type, type: Types::RelationshipTypes @@ -28,12 +28,16 @@ module UseCase private def find_source_user - ::User.find_by!(screen_name: current_user) + return source_user if source_user.is_a?(::User) + + ::User.find_by!(screen_name: source_user) rescue ActiveRecord::RecordNotFound raise Errors::UserNotFound end def find_target_user + return target_user if target_user.is_a?(::User) + ::User.find_by!(screen_name: target_user) rescue ActiveRecord::RecordNotFound raise Errors::UserNotFound diff --git a/spec/lib/use_case/relationship/create_spec.rb b/spec/lib/use_case/relationship/create_spec.rb index 5fa4be12..095a60ab 100644 --- a/spec/lib/use_case/relationship/create_spec.rb +++ b/spec/lib/use_case/relationship/create_spec.rb @@ -7,8 +7,8 @@ require "errors" describe UseCase::Relationship::Create do shared_examples_for "valid relationship type" do - context "current_user does not exist" do - let(:current_user) { "Schweinsbraten" } + context "source_user does not exist" do + let(:source_user) { "Schweinsbraten" } include_examples "raises an error", Errors::UserNotFound end @@ -19,13 +19,13 @@ describe UseCase::Relationship::Create do include_examples "raises an error", Errors::UserNotFound end - context "target_user is current_user" do + context "target_user is source_user" do let(:target_user) { user1.screen_name } include_examples "raises an error", Errors::SelfAction end - context "target_user is different from current_user" do + context "target_user is different from source_user" do its([:status]) { is_expected.to eq(201) } its([:extra]) { is_expected.to eq(target_user: user2) } @@ -40,13 +40,13 @@ describe UseCase::Relationship::Create do let(:base_params) do { - current_user: current_user, + source_user: source_user, target_user: target_user, type: type } end let(:params) { base_params } - let(:current_user) { user1.screen_name } + let(:source_user) { user1.screen_name } let(:target_user) { user2.screen_name } let(:type) { nil } diff --git a/spec/lib/use_case/relationship/destroy_spec.rb b/spec/lib/use_case/relationship/destroy_spec.rb index c7c32146..2cecbc05 100644 --- a/spec/lib/use_case/relationship/destroy_spec.rb +++ b/spec/lib/use_case/relationship/destroy_spec.rb @@ -26,15 +26,15 @@ describe UseCase::Relationship::Destroy do end end - context "current_user does not exist" do - let(:current_user) { "Schweinsbraten" } + context "source_user does not exist" do + let(:source_user) { "Schweinsbraten" } let(:target_user) { user2.screen_name } include_examples "raises an error", Errors::UserNotFound end context "target_user does not exist" do - let(:current_user) { user1.screen_name } + let(:source_user) { user1.screen_name } let(:target_user) { "peterwitzig" } include_examples "raises an error", Errors::UserNotFound @@ -43,13 +43,13 @@ describe UseCase::Relationship::Destroy do let(:base_params) do { - current_user: current_user, + source_user: source_user, target_user: target_user, type: type } end let(:params) { base_params } - let(:current_user) { user1.screen_name } + let(:source_user) { user1.screen_name } let(:target_user) { user2.screen_name } let(:type) { nil }