From 65ef18a7157f8cfcc494ad7a72ce083e79e38d26 Mon Sep 17 00:00:00 2001 From: dtluna Date: Wed, 12 Apr 2017 17:34:36 +0300 Subject: [PATCH] Add error messages for repeated follows and unfollows --- lib/pleroma/user.ex | 29 +++++++++++------- lib/pleroma/web/twitter_api/twitter_api.ex | 6 +++- .../web/twitter_api/twitter_api_controller.ex | 30 ++++++++++++------- test/web/twitter_api/twitter_api_test.exs | 17 +++++++---- 4 files changed, 55 insertions(+), 27 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index c77704db0..5f5bc1c38 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -31,22 +31,31 @@ def follow_changeset(struct, params \\ %{}) do def follow(%User{} = follower, %User{} = followed) do ap_followers = User.ap_followers(followed) - following = [ap_followers | follower.following] - |> Enum.uniq + if following?(follower, followed) do + { :error, + "Could not follow user: #{followed.nickname} is already on your list." } + else + following = [ap_followers | follower.following] + |> Enum.uniq - follower - |> follow_changeset(%{following: following}) - |> Repo.update + follower + |> follow_changeset(%{following: following}) + |> Repo.update + end end def unfollow(%User{} = follower, %User{} = followed) do ap_followers = User.ap_followers(followed) - following = follower.following - |> List.delete(ap_followers) + if following?(follower, followed) do + following = follower.following + |> List.delete(ap_followers) - follower - |> follow_changeset(%{following: following}) - |> Repo.update + follower + |> follow_changeset(%{following: following}) + |> Repo.update + else + { :error, "Not subscribed!" } + end end def following?(%User{} = follower, %User{} = followed) do diff --git a/lib/pleroma/web/twitter_api/twitter_api.ex b/lib/pleroma/web/twitter_api/twitter_api.ex index 0a942e880..66f78f340 100644 --- a/lib/pleroma/web/twitter_api/twitter_api.ex +++ b/lib/pleroma/web/twitter_api/twitter_api.ex @@ -103,7 +103,7 @@ def fetch_status(user, id) do def follow(%User{} = follower, followed_id) do with %User{} = followed <- Repo.get(User, followed_id), - { :ok, follower } <- User.follow(follower, followed), + { :ok, follower } <- User.follow(follower, followed), { :ok, activity } <- ActivityPub.insert(%{ "type" => "Follow", "actor" => follower.ap_id, @@ -112,6 +112,8 @@ def follow(%User{} = follower, followed_id) do }) do { :ok, follower, followed, activity } + else + err -> err end end @@ -120,6 +122,8 @@ def unfollow(%User{} = follower, followed_id) do { :ok, follower } <- User.unfollow(follower, followed) do { :ok, follower, followed } + else + err -> err end end diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index f2c893e96..f6574b8de 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -44,21 +44,24 @@ def friends_timeline(%{assigns: %{user: user}} = conn, params) do end def follow(%{assigns: %{user: user}} = conn, %{ "user_id" => followed_id }) do - { :ok, _user, follower, _activity } = TwitterAPI.follow(user, followed_id) + case TwitterAPI.follow(user, followed_id) do + { :ok, _user, followed, _activity } -> + response = followed |> UserRepresenter.to_json(%{for: user}) + conn |> json_reply(200, response) + { :error, msg } -> forbidden_json_reply(conn, msg) + end - response = follower |> UserRepresenter.to_json(%{for: user}) - - conn - |> json_reply(200, response) end def unfollow(%{assigns: %{user: user}} = conn, %{ "user_id" => followed_id }) do - { :ok, user, follower } = TwitterAPI.unfollow(user, followed_id) + case TwitterAPI.unfollow(user, followed_id) do + { :ok, user, followed } -> + response = followed |> UserRepresenter.to_json(%{for: user}) - response = follower |> UserRepresenter.to_json(%{for: user}) - - conn - |> json_reply(200, response) + conn + |> json_reply(200, response) + { :error, msg } -> forbidden_json_reply(conn, msg) + end end def fetch_status(%{assigns: %{user: user}} = conn, %{ "id" => id }) do @@ -88,4 +91,11 @@ defp json_reply(conn, status, json) do |> put_resp_content_type("application/json") |> send_resp(status, json) end + + defp forbidden_json_reply(conn, error_message) do + json = %{"error" => error_message, "request" => conn.request_path} + |> Poison.encode! + + json_reply(conn, 403, json) + end end diff --git a/test/web/twitter_api/twitter_api_test.exs b/test/web/twitter_api/twitter_api_test.exs index ad932131a..c1f5881b8 100644 --- a/test/web/twitter_api/twitter_api_test.exs +++ b/test/web/twitter_api/twitter_api_test.exs @@ -105,26 +105,31 @@ test "fetch a single status" do test "Follow another user" do { :ok, user } = UserBuilder.insert - { :ok, following } = UserBuilder.insert(%{nickname: "guy"}) + { :ok, followed } = UserBuilder.insert(%{nickname: "guy"}) - {:ok, user, following, activity } = TwitterAPI.follow(user, following.id) + { :ok, user, followed, activity } = TwitterAPI.follow(user, followed.id) user = Repo.get(User, user.id) follow = Repo.get(Activity, activity.id) - assert user.following == [User.ap_followers(following)] + assert user.following == [User.ap_followers(followed)] assert follow == activity + + { :error, msg } = TwitterAPI.follow(user, followed.id) + assert msg == "Could not follow user: #{followed.nickname} is already on your list." end test "Unfollow another user" do - { :ok, following } = UserBuilder.insert(%{nickname: "guy"}) - { :ok, user } = UserBuilder.insert(%{following: [User.ap_followers(following)]}) + { :ok, followed } = UserBuilder.insert(%{nickname: "guy"}) + { :ok, user } = UserBuilder.insert(%{following: [User.ap_followers(followed)]}) - {:ok, user, _following } = TwitterAPI.unfollow(user, following.id) + { :ok, user, _followed } = TwitterAPI.unfollow(user, followed.id) user = Repo.get(User, user.id) assert user.following == [] + { :error, msg } = TwitterAPI.unfollow(user, followed.id) + assert msg == "Not subscribed!" end test "fetch statuses in a context using the conversation id" do