Merge branch 'feature/delete-identity-last-group-admin' into 'master'

Don't delete the last admin of a group

Closes #135

See merge request framasoft/mobilizon!170

lib/mobilizon/actors/member.ex

@@ -68,6 +68,33 @@ defmodule Mobilizon.Actors.Member do
     )
   end
 
+  @doc """
+  Get all group ids where the actor_id is the last administrator
+  """
+  def list_group_id_where_last_administrator(actor_id) do
+    in_query =
+      from(
+        m in Member,
+        where: m.actor_id == ^actor_id and (m.role == ^:creator or m.role == ^:administrator),
+        select: m.parent_id
+      )
+
+    Repo.all(
+      from(
+        m in Member,
+        where: m.role == ^:creator or m.role == ^:administrator,
+        join: m2 in subquery(in_query),
+        on: m.parent_id == m2.parent_id,
+        group_by: m.parent_id,
+        select: m.parent_id,
+        having: count(m.actor_id) == 1
+      )
+    )
+  end
+
+  @doc """
+  Returns true if the member is an administrator (admin or creator) of the group
+  """
   def is_administrator(%Member{role: :administrator}), do: {:is_admin, true}
   def is_administrator(%Member{role: :creator}), do: {:is_admin, true}
   def is_administrator(%Member{}), do: {:is_admin, false}

lib/mobilizon_web/resolvers/group.ex

@@ -178,7 +178,7 @@ defmodule MobilizonWeb.Resolvers.Group do
     with {:is_owned, true, actor} <- User.owns_actor(user, actor_id),
          {:ok, %Member{} = member} <- Member.get_member(actor.id, group_id),
          {:only_administrator, false} <-
-           {:only_administrator, check_that_member_is_not_only_administrator(group_id, actor_id)},
+           {:only_administrator, check_that_member_is_not_last_administrator(group_id, actor_id)},
          {:ok, _} <-
            Mobilizon.Actors.delete_member(member) do
       {
@@ -211,8 +211,8 @@ defmodule MobilizonWeb.Resolvers.Group do
   # We check that the actor asking to leave the group is not it's only administrator
   # We start by fetching the list of administrator or creators and if there's only one of them
   # and that it's the actor requesting leaving the group we return true
-  @spec check_that_member_is_not_only_administrator(integer(), integer()) :: boolean()
-  defp check_that_member_is_not_only_administrator(group_id, actor_id) do
+  @spec check_that_member_is_not_last_administrator(integer(), integer()) :: boolean()
+  defp check_that_member_is_not_last_administrator(group_id, actor_id) do
     case Member.list_administrator_members_for_group(group_id) do
       [
         %Member{

lib/mobilizon_web/resolvers/person.ex

@@ -3,7 +3,7 @@ defmodule MobilizonWeb.Resolvers.Person do
   Handles the person-related GraphQL calls
   """
   alias Mobilizon.Actors
-  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Actors.{Actor, Member}
   alias Mobilizon.Users.User
   alias Mobilizon.Users
   alias Mobilizon.Events
@@ -118,6 +118,7 @@ defmodule MobilizonWeb.Resolvers.Person do
            {:find_actor, Actors.get_actor_by_name(preferred_username)},
          {:is_owned, true, _} <- User.owns_actor(user, actor.id),
          {:last_identity, false} <- {:last_identity, last_identity?(user)},
+         {:last_admin, false} <- {:last_admin, last_admin_of_a_group?(actor.id)},
          {:ok, actor} <- Actors.delete_actor(actor) do
       {:ok, actor}
     else
@@ -127,6 +128,9 @@ defmodule MobilizonWeb.Resolvers.Person do
       {:last_identity, true} ->
         {:error, "Cannot remove the last identity of a user"}
 
+      {:last_admin, true} ->
+        {:error, "Cannot remove the last administrator of a group"}
+
       {:is_owned, false} ->
         {:error, "Actor is not owned by authenticated user"}
     end
@@ -213,6 +217,12 @@ defmodule MobilizonWeb.Resolvers.Person do
     |> proxify_banner
   end
 
+  # We check that the actor is not the last administrator/creator of a group
+  @spec last_admin_of_a_group?(integer()) :: boolean()
+  defp last_admin_of_a_group?(actor_id) do
+    length(Member.list_group_id_where_last_administrator(actor_id)) > 0
+  end
+
   @spec proxify_avatar(Actor.t()) :: Actor.t()
   defp proxify_avatar(%Actor{avatar: %{url: avatar_url} = avatar} = actor) do
     actor |> Map.put(:avatar, avatar |> Map.put(:url, MobilizonWeb.MediaProxy.url(avatar_url)))

test/mobilizon_web/resolvers/person_resolver_test.exs

@@ -354,7 +354,7 @@ defmodule MobilizonWeb.Resolvers.PersonResolverTest do
         |> auth_conn(user1)
         |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
 
-      assert json_response(res, 200)["data"]["updatePerson"] == nil
+      assert json_response(res, 200)["data"]["deletePerson"] == nil
 
       assert hd(json_response(res, 200)["errors"])["message"] ==
                "Actor is not owned by authenticated user"
@@ -377,7 +377,7 @@ defmodule MobilizonWeb.Resolvers.PersonResolverTest do
         |> auth_conn(user)
         |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
 
-      assert json_response(res, 200)["data"]["updatePerson"] == nil
+      assert json_response(res, 200)["data"]["deletePerson"] == nil
 
       assert hd(json_response(res, 200)["errors"])["message"] ==
                "Actor not found"
@@ -400,12 +400,44 @@ defmodule MobilizonWeb.Resolvers.PersonResolverTest do
         |> auth_conn(user)
         |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
 
-      assert json_response(res, 200)["data"]["updatePerson"] == nil
+      assert json_response(res, 200)["data"]["deletePerson"] == nil
 
       assert hd(json_response(res, 200)["errors"])["message"] ==
                "Cannot remove the last identity of a user"
     end
 
+    test "delete_person/3 should fail to delete an identity that is the last admin of a group",
+         context do
+      group = insert(:group)
+      classic_user = insert(:user)
+      classic_actor = insert(:actor, user: classic_user, preferred_username: "classic_user")
+
+      admin_user = insert(:user)
+      admin_actor = insert(:actor, user: admin_user, preferred_username: "last_admin")
+      insert(:actor, user: admin_user)
+
+      insert(:member, %{actor: admin_actor, role: :creator, parent: group})
+      insert(:member, %{actor: classic_actor, role: :member, parent: group})
+
+      mutation = """
+          mutation {
+            deletePerson(preferredUsername: "last_admin") {
+              id,
+            }
+          }
+      """
+
+      res =
+        context.conn
+        |> auth_conn(admin_user)
+        |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+
+      assert json_response(res, 200)["data"]["deletePerson"] == nil
+
+      assert hd(json_response(res, 200)["errors"])["message"] ==
+               "Cannot remove the last administrator of a group"
+    end
+
     test "delete_person/3 should delete a user identity", context do
       user = insert(:user)
       insert(:actor, user: user, preferred_username: "riri")