From bac6628aea6376b1dfd8e18e6ca3b169b1c274c6 Mon Sep 17 00:00:00 2001
From: Thomas Citharel <tcit@tcit.fr>
Date: Mon, 25 Oct 2021 13:20:33 +0200
Subject: [PATCH] Improve changing current actor

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
---
 lib/graphql/resolvers/user.ex | 23 ++++++++++++-----------
 lib/mobilizon/users/users.ex  | 24 ++++++++----------------
 lib/web/auth/guardian.ex      |  2 +-
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/lib/graphql/resolvers/user.ex b/lib/graphql/resolvers/user.ex
index 6300028ca..f9eee0bac 100644
--- a/lib/graphql/resolvers/user.ex
+++ b/lib/graphql/resolvers/user.ex
@@ -311,19 +311,20 @@ defmodule Mobilizon.GraphQL.Resolvers.User do
   def change_default_actor(
         _parent,
         %{preferred_username: username},
-        %{context: %{current_user: %User{id: user_id} = user}}
+        %{context: %{current_user: %User{} = user}}
       ) do
-    with %Actor{id: actor_id} = actor <- Actors.get_local_actor_by_name(username),
-         {:user_actor, true} <-
-           {:user_actor, actor_id in Enum.map(Users.get_actors_for_user(user), & &1.id)},
-         %User{} = user <- Users.update_user_default_actor(user_id, actor) do
-      {:ok, user}
-    else
-      {:user_actor, _} ->
-        {:error, :actor_not_from_user}
+    case Actors.get_local_actor_by_name(username) do
+      %Actor{id: actor_id} = actor ->
+        if actor_id in Enum.map(Users.get_actors_for_user(user), & &1.id) do
+          %User{} = user = Users.update_user_default_actor(user, actor)
+          {:ok, user}
+        else
+          {:error, dgettext("errors", "This profile does not belong to you")}
+        end
 
-      _error ->
-        {:error, :unable_to_change_default_actor}
+      nil ->
+        {:error,
+         dgettext("errors", "Profile with username %{username} not found", %{username: username})}
     end
   end
 
diff --git a/lib/mobilizon/users/users.ex b/lib/mobilizon/users/users.ex
index 380d6ac3e..1337d04dc 100644
--- a/lib/mobilizon/users/users.ex
+++ b/lib/mobilizon/users/users.ex
@@ -250,19 +250,15 @@ defmodule Mobilizon.Users do
   Updates user's default actor.
   Raises `Ecto.NoResultsError` if the user does not exist.
   """
-  @spec update_user_default_actor(integer | String.t(), Actor.t() | nil) :: User.t()
-  def update_user_default_actor(user_id, actor) do
+  @spec update_user_default_actor(User.t(), Actor.t() | nil) :: User.t()
+  def update_user_default_actor(%User{id: user_id} = user, actor) do
     actor_id = if is_nil(actor), do: nil, else: actor.id
 
     user_id
-    |> update_user_default_actor_query(actor_id)
-    |> Repo.update_all([])
+    |> update_user_default_actor_query()
+    |> Repo.update_all(set: [default_actor_id: actor_id])
 
-    Cachex.put(:default_actors, to_string(user_id), actor)
-
-    user_id
-    |> get_user!()
-    |> Repo.preload([:default_actor])
+    %User{user | default_actor: actor}
   end
 
   @doc """
@@ -524,13 +520,9 @@ defmodule Mobilizon.Users do
     from(a in Actor, where: a.user_id == ^user_id)
   end
 
-  @spec update_user_default_actor_query(integer | String.t(), integer | String.t() | nil) ::
+  @spec update_user_default_actor_query(integer | String.t()) ::
           Ecto.Query.t()
-  defp update_user_default_actor_query(user_id, actor_id) do
-    from(
-      u in User,
-      where: u.id == ^user_id,
-      update: [set: [default_actor_id: ^actor_id]]
-    )
+  defp update_user_default_actor_query(user_id) do
+    where(User, [u], u.id == ^user_id)
   end
 end
diff --git a/lib/web/auth/guardian.ex b/lib/web/auth/guardian.ex
index 446fb5dc9..9709aff9a 100644
--- a/lib/web/auth/guardian.ex
+++ b/lib/web/auth/guardian.ex
@@ -32,7 +32,7 @@ defmodule Mobilizon.Web.Auth.Guardian do
     try do
       case Integer.parse(uid_str) do
         {uid, ""} ->
-          {:ok, Users.get_user_with_actors!(uid)}
+          {:ok, Users.get_user!(uid)}
 
         _ ->
           {:error, :invalid_id}