From 3962671135419e0868e413fc3babf8cc35709df4 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 3 Jan 2022 00:31:55 +0100 Subject: [PATCH] Implement relationship logic as use case --- app/controllers/ajax/friend_controller.rb | 39 ----- .../ajax/relationship_controller.rb | 37 ++++ .../retrospring/features/user/action.ts | 5 +- app/models/user.rb | 10 -- app/models/user/relationship/follow.rb | 4 +- config/locales/en.yml | 21 +++ config/routes.rb | 4 +- db/schema.rb | 7 - lib/errors.rb | 22 +++ lib/use_case/base.rb | 8 + lib/use_case/relationship/create.rb | 54 ++++++ lib/use_case/relationship/destroy.rb | 54 ++++++ .../ajax/friend_controller_spec.rb | 159 ------------------ .../ajax/relationship_controller_spec.rb | 122 ++++++++++++++ spec/shared_examples/error_raising.rb | 9 + spec/shared_examples/requires_login.rb | 14 ++ 16 files changed, 347 insertions(+), 222 deletions(-) delete mode 100644 app/controllers/ajax/friend_controller.rb create mode 100644 app/controllers/ajax/relationship_controller.rb create mode 100644 lib/use_case/relationship/create.rb create mode 100644 lib/use_case/relationship/destroy.rb delete mode 100644 spec/controllers/ajax/friend_controller_spec.rb create mode 100644 spec/controllers/ajax/relationship_controller_spec.rb create mode 100644 spec/shared_examples/error_raising.rb create mode 100644 spec/shared_examples/requires_login.rb diff --git a/app/controllers/ajax/friend_controller.rb b/app/controllers/ajax/friend_controller.rb deleted file mode 100644 index 4dc849ea..00000000 --- a/app/controllers/ajax/friend_controller.rb +++ /dev/null @@ -1,39 +0,0 @@ -class Ajax::FriendController < AjaxController - def create - params.require :screen_name - - target_user = User.find_by_screen_name(params[:screen_name]) - - begin - current_user.follow target_user - rescue => e - Sentry.capture_exception(e) - @response[:status] = :fail - @response[:message] = I18n.t('messages.friend.create.fail') - return - end - - @response[:status] = :okay - @response[:message] = I18n.t('messages.friend.create.okay') - @response[:success] = true - end - - def destroy - params.require :screen_name - - target_user = User.find_by_screen_name(params[:screen_name]) - - begin - current_user.unfollow target_user - rescue => e - Sentry.capture_exception(e) - @response[:status] = :fail - @response[:message] = I18n.t('messages.friend.destroy.fail') - return - end - - @response[:status] = :okay - @response[:message] = I18n.t('messages.friend.destroy.okay') - @response[:success] = true - end -end diff --git a/app/controllers/ajax/relationship_controller.rb b/app/controllers/ajax/relationship_controller.rb new file mode 100644 index 00000000..5adbd958 --- /dev/null +++ b/app/controllers/ajax/relationship_controller.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "use_case/relationship/create" +require "use_case/relationship/destroy" +require "errors" + +class Ajax::RelationshipController < AjaxController + before_action :authenticate_user! + + def create + UseCase::Relationship::Create.call( + current_user_id: current_user.id, + target_user: params[:target_user], + type: params[:type] + ) + @response[:success] = true + @response[:message] = t(".success") + rescue Errors::Base => e + @response[:message] = t(e.locale_tag) + ensure + return_response + end + + def destroy + UseCase::Relationship::Destroy.call( + current_user_id: current_user.id, + target_user: params[:target_user], + type: params[:type] + ) + @response[:success] = true + @response[:message] = t(".success") + rescue Errors::Base => e + @response[:message] = t(e.locale_tag) + ensure + return_response + end +end diff --git a/app/javascript/retrospring/features/user/action.ts b/app/javascript/retrospring/features/user/action.ts index 7e64c747..bbac9b08 100644 --- a/app/javascript/retrospring/features/user/action.ts +++ b/app/javascript/retrospring/features/user/action.ts @@ -7,14 +7,15 @@ export function userActionHandler(event: Event): void { const target = button.dataset.target; const action = button.dataset.action; - const targetURL = action === 'follow' ? '/ajax/create_friend' : '/ajax/destroy_friend'; + const targetURL = action === 'follow' ? '/ajax/create_relationship' : '/ajax/destroy_relationship'; let success = false; Rails.ajax({ url: targetURL, type: 'POST', data: new URLSearchParams({ - screen_name: target + screen_name: target, + type: "follow" }).toString(), success: (data) => { success = data.success; diff --git a/app/models/user.rb b/app/models/user.rb index c11991e7..1b74c939 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -95,16 +95,6 @@ class User < ApplicationRecord end end - # follows an user. - def follow(target_user) - active_relationships.create(target: target_user) - end - - # unfollows an user - def unfollow(target_user) - active_relationships.find_by(target: target_user).destroy - 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/follow.rb b/app/models/user/relationship/follow.rb index a2a9869d..a85b969d 100644 --- a/app/models/user/relationship/follow.rb +++ b/app/models/user/relationship/follow.rb @@ -20,9 +20,7 @@ class User # 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 + raise Errors::FollowingSelf if target_user == self create_relationship(active_follow_relationships, target_user) end diff --git a/config/locales/en.yml b/config/locales/en.yml index 715581f5..6341783d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -457,3 +457,24 @@ en: invalid_code: "The code you entered was invalid." setup: success: "Two factor authentication has been enabled for your account." + errors: + base: "An error occurred" + + bad_request: "Bad Request" + param_is_missing: "param is missing" + + forbidden: "This is illegal, you know" + blocked: "You have been blocked from performing this request" + other_blocked_self: "You have been blocked by this user" + asking_other_blocked_self: "You have been blocked from asking this user questions" + following_other_blocked_self: "You have been blocked from following this user" + self_blocked_other: "You cannot do this while blocking this user" + asking_self_blocked_other: "You cannot ask an user who you are currently blocking" + following_self_blocked_other: "You cannot follow an user who you are currently blocking" + self_action: "You cannot do this to yourself" + following_self: "You cannot follow yourself" + + not_found: "That does not exist" + user_not_found: "User not found" + + conflict: "This already exists" diff --git a/config/routes.rb b/config/routes.rb index 31c8976a..007cdc73 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -102,8 +102,8 @@ Rails.application.routes.draw do match '/delete_all_inbox/:author', to: 'inbox#remove_all_author', via: :post, as: :delete_all_author match '/answer', to: 'answer#create', via: :post, as: :answer match '/destroy_answer', to: 'answer#destroy', via: :post, as: :destroy_answer - match '/create_friend', to: 'friend#create', via: :post, as: :create_friend - match '/destroy_friend', to: 'friend#destroy', via: :post, as: :destroy_friend + match '/create_relationship', to: 'relationship#create', via: :post, as: :create_relationship + match '/destroy_relationship', to: 'relationship#destroy', via: :post, as: :destroy_relationship match '/create_smile', to: 'smile#create', via: :post, as: :create_smile match '/destroy_smile', to: 'smile#destroy', via: :post, as: :destroy_smile match '/create_comment_smile', to: 'smile#create_comment', via: :post, as: :create_comment_smile diff --git a/db/schema.rb b/db/schema.rb index 55de0a22..764937c1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -122,7 +122,6 @@ ActiveRecord::Schema.define(version: 2022_01_05_171216) do t.boolean "new" t.datetime "created_at" t.datetime "updated_at" - t.string "state" t.index ["new"], name: "index_notifications_on_new" t.index ["recipient_id"], name: "index_notifications_on_recipient_id" end @@ -278,12 +277,7 @@ ActiveRecord::Schema.define(version: 2022_01_05_171216) do t.integer "asked_count", default: 0, null: false t.integer "answered_count", default: 0, null: false t.integer "commented_count", default: 0, null: false - t.string "display_name" t.integer "smiled_count", default: 0, null: false - t.string "motivation_header", default: "", null: false - t.string "website", default: "", null: false - t.string "location", default: "", null: false - t.text "bio", default: "", null: false t.string "profile_picture_file_name" t.boolean "profile_picture_processing" t.integer "profile_picture_x" @@ -329,6 +323,5 @@ 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/errors.rb b/lib/errors.rb index 1ddb9198..20220f55 100644 --- a/lib/errors.rb +++ b/lib/errors.rb @@ -7,6 +7,10 @@ module Errors def code @code ||= self.class.name.sub('Errors::', '').underscore end + + def locale_tag + @locale_tag ||= "errors.#{code}" + end end class BadRequest < Base @@ -15,6 +19,9 @@ module Errors end end + class ParamIsMissing < BadRequest + end + class InvalidBanDuration < BadRequest end @@ -23,4 +30,19 @@ module Errors 403 end end + + class SelfAction < Forbidden + end + + class FollowingSelf < SelfAction + end + + class NotFound < Base + def status + 404 + end + end + + class UserNotFound < NotFound + end end diff --git a/lib/use_case/base.rb b/lib/use_case/base.rb index 5468e0ff..2d9578a2 100644 --- a/lib/use_case/base.rb +++ b/lib/use_case/base.rb @@ -15,5 +15,13 @@ module UseCase def call raise NotImplementedError end + + private + + def not_blank!(*args) + args.each do |arg| + raise Errors::ParamIsMissing if instance_variable_get("@#{arg}").blank? + end + end end end \ No newline at end of file diff --git a/lib/use_case/relationship/create.rb b/lib/use_case/relationship/create.rb new file mode 100644 index 00000000..61d7cbcf --- /dev/null +++ b/lib/use_case/relationship/create.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require "use_case/base" +require "errors" + +module UseCase + module Relationship + class Create < UseCase::Base + option :current_user_id, type: Types::Coercible::Integer + option :target_user, type: Types::Coercible::String + option :type, type: Types::Coercible::String + + def call + not_blank! :current_user_id, :target_user, :type + + type = @type.downcase + ensure_type(type) + source_user = find_source_user + target_user = find_target_user + + source_user.public_send(type, target_user) + + { + status: 201, + resource: true, + extra: { + target_user: target_user + } + } + end + + private + + def ensure_type(type) + raise Errors::BadRequest unless type == 'follow' + end + + def find_source_user + user_id = @current_user_id + User.find(user_id) + rescue ActiveRecord::RecordNotFound + raise Errors::UserNotFound + end + + def find_target_user + target_user = @target_user + return target_user if target_user.is_a?(User) + User.find_by!(screen_name: target_user) + rescue ActiveRecord::RecordNotFound + raise Errors::UserNotFound + end + end + end +end diff --git a/lib/use_case/relationship/destroy.rb b/lib/use_case/relationship/destroy.rb new file mode 100644 index 00000000..0b0fcf54 --- /dev/null +++ b/lib/use_case/relationship/destroy.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require "use_case/base" +require "errors" + +module UseCase + module Relationship + class Destroy < UseCase::Base + option :current_user_id, type: Types::Coercible::Integer + option :target_user, type: Types::Coercible::String + option :type, type: Types::Coercible::String + + def call + not_blank! :current_user_id, :target_user, :type + + type = @type.downcase + ensure_type(type) + source_user = find_source_user + target_user = find_target_user + + source_user.public_send("un#{type}", target_user) + + { + status: 204, + resource: nil, + extra: { + target_user: target_user + } + } + end + + private + + def ensure_type(type) + raise Errors::BadRequest unless type == 'follow' + end + + def find_source_user + user_id = @current_user_id + User.find(user_id) + rescue ActiveRecord::RecordNotFound + raise Errors::UserNotFound + end + + def find_target_user + target_user = @target_user + return target_user if target_user.is_a?(User) + User.find_by!(screen_name: target_user) + rescue ActiveRecord::RecordNotFound + raise Errors::UserNotFound + end + end + end +end diff --git a/spec/controllers/ajax/friend_controller_spec.rb b/spec/controllers/ajax/friend_controller_spec.rb deleted file mode 100644 index 51daddf6..00000000 --- a/spec/controllers/ajax/friend_controller_spec.rb +++ /dev/null @@ -1,159 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -describe Ajax::FriendController, :ajax_controller, type: :controller do - describe "#create" do - let(:params) do - { - screen_name: screen_name - } - end - - subject { post(:create, params: params) } - - context "when user is signed in" do - before(:each) { sign_in(user) } - - context "when target user exists" do - let(:target_user) { FactoryBot.create(:user) } - let(:screen_name) { target_user.screen_name } - - let(:expected_response) do - { - "success" => true, - "status" => "okay", - "message" => anything - } - end - - it "creates a follow relationship" do - 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" - end - - context "when target user does not exist" do - let(:screen_name) { "tripmeister_eder" } - - let(:expected_response) do - { - "success" => true, - "status" => "okay", - "message" => anything - } - end - - it "does not create a follow relationship" do - expect { subject }.not_to(change { user.followings.count }) - end - - include_examples "returns the expected response" - end - end - - context "when user is not signed in" do - let(:screen_name) { "tutenchamun" } - let(:expected_response) do - { - "success" => false, - "status" => "fail", - "message" => anything - } - end - - include_examples "returns the expected response" - end - end - - describe "#destroy" do - let(:params) do - { - screen_name: screen_name - } - end - - subject { delete(:destroy, params: params) } - - context "when user is signed in" do - before(:each) { sign_in(user) } - - context "when target user exists" do - let(:target_user) { FactoryBot.create(:user) } - let(:screen_name) { target_user.screen_name } - - before(:each) { target_user } - - context "when user follows target" do - let(:expected_response) do - { - "success" => true, - "status" => "okay", - "message" => anything - } - end - - before(:each) { user.follow target_user } - - it "destroys a follow relationship" do - 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" - end - - context "when user does not already follow target" do - let(:expected_response) do - { - "success" => false, - "status" => "fail", - "message" => anything - } - end - - it "does not destroy a follow relationship" do - expect { subject }.not_to(change { user.followings.count }) - end - - include_examples "returns the expected response" - end - end - - context "when target user does not exist" do - let(:screen_name) { "tripmeister_eder" } - - let(:expected_response) do - { - "success" => false, - "status" => "fail", - "message" => anything - } - end - - it "does not destroy a follow relationship" do - expect { subject }.not_to(change { user.followings.count }) - end - - include_examples "returns the expected response" - end - end - - context "when user is not signed in" do - let(:screen_name) { "tutenchamun" } - let(:expected_response) do - { - "success" => false, - "status" => "fail", - "message" => anything - } - end - - include_examples "returns the expected response" - end - end -end diff --git a/spec/controllers/ajax/relationship_controller_spec.rb b/spec/controllers/ajax/relationship_controller_spec.rb new file mode 100644 index 00000000..047b57cd --- /dev/null +++ b/spec/controllers/ajax/relationship_controller_spec.rb @@ -0,0 +1,122 @@ +# coding: utf-8 +# frozen_string_literal: true + +require "rails_helper" + +describe Ajax::RelationshipController, type: :controller do + shared_examples_for "params is empty" do + let(:params) { { type: type } } # type is still required as it's part of the route + + include_examples "ajax does not succeed", "param is missing" + end + + let!(:user) { FactoryBot.create(:user) } + let!(:user2) { FactoryBot.create(:user) } + + describe "#create" do + shared_examples_for "valid relationship type" do + it_behaves_like "params is empty" + + context "target_user does not exist" do + let(:target_user) { "peter-witzig" } + + include_examples "ajax does not succeed", "not found" + end + + context "target_user is current user" do + let(:target_user) { user.screen_name } + + include_examples "ajax does not succeed", "yourself" + end + + context "target_user is different from current_user" do + let(:target_user) { user2.screen_name } + + it "creates the relationship" do + expect { subject }.to change { Relationship.count }.by(1) + expect(Relationship.last.target.screen_name).to eq(target_user) + end + end + end + + let(:type) { "Sauerkraut" } + let(:target_user) { user2.screen_name } + let(:params) { { type: type, target_user: target_user } } + + subject { post(:create, params: params) } + + it_behaves_like "requires login" + + context "user signed in" do + before(:each) { sign_in(user) } + + context "type = 'follow'" do + let(:type) { "follow" } + + include_examples "valid relationship type" + end + + context "type = 'dick'" do + let(:type) { "dick" } + + it_behaves_like "params is empty" + include_examples "ajax does not succeed", "Bad Request" + end + end + end + + describe "#destroy" do + shared_examples_for "valid relationship type" do + let(:target_user) { user2.screen_name } + + context "relationship exists" do + before do + user.public_send(type, user2) + end + + it "destroys a relationship" do + expect { subject }.to change { Relationship.count }.by(-1) + end + end + + context "relationship does not exist" do + it "does not change anything at all" do + expect { subject }.to change { Relationship.count }.by(0) + end + end + + it_behaves_like "params is empty" + + context "target_user does not exist" do + let(:target_user) { "peter-witzig" } + + include_examples "ajax does not succeed", "not found" + end + end + + let(:type) { "Sauerkraut" } + let(:target_user) { user2.screen_name } + let(:params) { { type: type, target_user: target_user } } + + subject { delete(:destroy, params: params) } + + it_behaves_like "requires login" + + context "user signed in" do + before(:each) { sign_in(user) } + + context "type = 'follow'" do + let(:type) { "follow" } + + include_examples "valid relationship type" + end + + context "type = 'dick'" do + let(:type) { "dick" } + + it_behaves_like "params is empty" + include_examples "ajax does not succeed", "Bad Request" + end + end + end +end diff --git a/spec/shared_examples/error_raising.rb b/spec/shared_examples/error_raising.rb new file mode 100644 index 00000000..5eb073ed --- /dev/null +++ b/spec/shared_examples/error_raising.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +RSpec.shared_examples_for "ajax does not succeed" do |part_of_error_message| + it "ajax does not succeed" do + subject + expect(assigns(:response)[:success]).to be false + expect(assigns(:response)[:message]).to include(part_of_error_message) + end +end diff --git a/spec/shared_examples/requires_login.rb b/spec/shared_examples/requires_login.rb new file mode 100644 index 00000000..7f96d0c0 --- /dev/null +++ b/spec/shared_examples/requires_login.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +RSpec.shared_examples_for "requires login" do + it "redirects to the login page for guests" do + subject + expect(response).to redirect_to(new_user_session_path) + end + + it "does not redirect to the login page for users" do + sign_in user + subject + expect(response).to_not redirect_to(new_user_session_path) + end +end