From 5a8ad3ab525e52a5d3d722a6a1ec5df60a28bd0a Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 1 Mar 2019 17:11:28 +0100 Subject: [PATCH] Add join/leave group --- lib/mobilizon/actors/actor.ex | 3 +- lib/mobilizon/actors/actors.ex | 6 + lib/mobilizon/actors/member.ex | 46 ++- lib/mobilizon/events/events.ex | 6 +- lib/mobilizon_web/resolvers/event.ex | 4 +- lib/mobilizon_web/resolvers/group.ex | 138 ++++++++- lib/mobilizon_web/schema.ex | 1 + lib/mobilizon_web/schema/actors/member.ex | 27 +- ...0190301141830_move_member_role_to_enum.exs | 46 +++ .../20190301143831_actor_group_openness.exs | 18 ++ test/mobilizon/actors/actors_test.exs | 12 +- .../resolvers/group_resolver_test.exs | 8 +- .../resolvers/member_resolver_test.exs | 289 ++++++++++++++++++ test/support/factory.ex | 2 +- 14 files changed, 567 insertions(+), 39 deletions(-) create mode 100644 priv/repo/migrations/20190301141830_move_member_role_to_enum.exs create mode 100644 priv/repo/migrations/20190301143831_actor_group_openness.exs create mode 100644 test/mobilizon_web/resolvers/member_resolver_test.exs diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex index d7bbe8498..b3709829f 100644 --- a/lib/mobilizon/actors/actor.ex +++ b/lib/mobilizon/actors/actor.ex @@ -8,7 +8,7 @@ defenum(Mobilizon.Actors.ActorTypeEnum, :actor_type, [ :Service ]) -defenum(Mobilizon.Actors.ActorOpennesssEnum, :openness, [ +defenum(Mobilizon.Actors.ActorOpennessEnum, :actor_openness, [ :invite_only, :moderated, :open @@ -48,6 +48,7 @@ defmodule Mobilizon.Actors.Actor do field(:preferred_username, :string) field(:keys, :string) field(:manually_approves_followers, :boolean, default: false) + field(:openness, Mobilizon.Actors.ActorOpennessEnum, default: :moderated) field(:suspended, :boolean, default: false) field(:avatar_url, :string) field(:banner_url, :string) diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index e58a8e849..e1ad14461 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/lib/mobilizon/actors/actors.ex @@ -162,6 +162,12 @@ defmodule Mobilizon.Actors do ) end + @doc """ + Get the default member role depending on the actor openness + """ + def get_default_member_role(%Actor{openness: :open}), do: :member + def get_default_member_role(%Actor{}), do: :not_approved + @doc """ Get a group by it's title """ diff --git a/lib/mobilizon/actors/member.ex b/lib/mobilizon/actors/member.ex index 5d6b85075..6580a69df 100644 --- a/lib/mobilizon/actors/member.ex +++ b/lib/mobilizon/actors/member.ex @@ -1,17 +1,29 @@ +import EctoEnum + +defenum(Mobilizon.Actors.MemberRoleEnum, :member_role_type, [ + :not_approved, + :member, + :moderator, + :administrator, + :creator +]) + defmodule Mobilizon.Actors.Member do @moduledoc """ Represents the membership of an actor to a group """ use Ecto.Schema + import Ecto.Changeset + import Ecto.Query, warn: false + import Mobilizon.Ecto + alias Mobilizon.Actors.Member alias Mobilizon.Actors.Actor alias Mobilizon.Repo schema "members" do - field(:approved, :boolean, default: true) - # 0 : Member, 1 : Moderator, 2 : Admin - field(:role, :integer, default: 0) + field(:role, Mobilizon.Actors.MemberRoleEnum, default: :member) belongs_to(:parent, Actor) belongs_to(:actor, Actor) @@ -21,7 +33,7 @@ defmodule Mobilizon.Actors.Member do @doc false def changeset(%Member{} = member, attrs) do member - |> cast(attrs, [:role, :approved, :parent_id, :actor_id]) + |> cast(attrs, [:role, :parent_id, :actor_id]) |> validate_required([:parent_id, :actor_id]) |> unique_constraint(:parent_id, name: :members_actor_parent_unique_index) end @@ -36,11 +48,27 @@ defmodule Mobilizon.Actors.Member do end end - def is_administrator(%Member{role: 2}) do - {:is_admin, true} + @doc """ + Gets a single member of an actor (for example a group) + """ + def can_be_joined(%Actor{type: :Group, openness: :invite_only}), do: false + def can_be_joined(%Actor{type: :Group}), do: true + + @doc """ + Returns the list of administrator members for a group. + """ + def list_administrator_members_for_group(id, page \\ nil, limit \\ nil) do + Repo.all( + from( + m in Member, + where: m.parent_id == ^id and (m.role == ^:creator or m.role == ^:administrator), + preload: [:actor] + ) + |> paginate(page, limit) + ) end - def is_administrator(%Member{}) do - {:is_admin, false} - end + 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} end diff --git a/lib/mobilizon/events/events.ex b/lib/mobilizon/events/events.ex index 1855435f3..61263efc6 100644 --- a/lib/mobilizon/events/events.ex +++ b/lib/mobilizon/events/events.ex @@ -5,8 +5,8 @@ defmodule Mobilizon.Events do import Ecto.Query, warn: false import Mobilizon.Ecto - alias Mobilizon.Repo + alias Mobilizon.Repo alias Mobilizon.Events.{Event, Comment, Participant} alias Mobilizon.Actors.Actor alias Mobilizon.Addresses.Address @@ -607,9 +607,7 @@ defmodule Mobilizon.Events do Repo.all( from( p in Participant, - join: e in Event, - on: p.event_id == e.id, - where: e.id == ^id and p.role == ^:creator, + where: p.event_id == ^id and p.role == ^:creator, preload: [:actor] ) |> paginate(page, limit) diff --git a/lib/mobilizon_web/resolvers/event.ex b/lib/mobilizon_web/resolvers/event.ex index b1ce90944..ac11e1b12 100644 --- a/lib/mobilizon_web/resolvers/event.ex +++ b/lib/mobilizon_web/resolvers/event.ex @@ -32,8 +32,8 @@ defmodule MobilizonWeb.Resolvers.Event do @doc """ List participant for event (separate request) """ - def list_participants_for_event(_parent, %{uuid: uuid}, _resolution) do - {:ok, Mobilizon.Events.list_participants_for_event(uuid)} + def list_participants_for_event(_parent, %{uuid: uuid, page: page, limit: limit}, _resolution) do + {:ok, Mobilizon.Events.list_participants_for_event(uuid, page, limit)} end @doc """ diff --git a/lib/mobilizon_web/resolvers/group.ex b/lib/mobilizon_web/resolvers/group.ex index f104e3d27..edd199e67 100644 --- a/lib/mobilizon_web/resolvers/group.ex +++ b/lib/mobilizon_web/resolvers/group.ex @@ -36,19 +36,30 @@ defmodule MobilizonWeb.Resolvers.Group do _parent, args, %{ - context: %{current_user: _user} + context: %{ + current_user: _user + } } ) do - with {:ok, %Activity{data: %{"object" => %{"type" => "Group"} = object}}} <- + with { + :ok, + %Activity{ + data: %{ + "object" => %{"type" => "Group"} = object + } + } + } <- MobilizonWeb.API.Groups.create_group(args) do - {:ok, - %Actor{ - preferred_username: object["preferredUsername"], - summary: object["summary"], - type: :Group, - # uuid: object["uuid"], - url: object["id"] - }} + { + :ok, + %Actor{ + preferred_username: object["preferredUsername"], + summary: object["summary"], + type: :Group, + # uuid: object["uuid"], + url: object["id"] + } + } end # with %Actor{id: actor_id} <- Actors.get_local_actor_by_name(actor_username), @@ -106,4 +117,111 @@ defmodule MobilizonWeb.Resolvers.Group do def delete_group(_parent, _args, _resolution) do {:error, "You need to be logged-in to delete a group"} end + + @doc """ + Join an existing group + """ + def join_group( + _parent, + %{group_id: group_id, actor_id: actor_id}, + %{ + context: %{ + current_user: user + } + } + ) do + with {:is_owned, true, actor} <- User.owns_actor(user, actor_id), + {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id), + {:error, :member_not_found} <- Member.get_member(actor.id, group.id), + {:is_able_to_join, true} <- {:is_able_to_join, Member.can_be_joined(group)}, + role <- Mobilizon.Actors.get_default_member_role(group), + {:ok, _} <- + Actors.create_member(%{ + parent_id: group.id, + actor_id: actor.id, + role: role + }) do + {:ok, %{parent: group, person: actor, role: role}} + else + {:is_owned, false} -> + {:error, "Actor id is not owned by authenticated user"} + + {:error, :group_not_found} -> + {:error, "Group id not found"} + + {:is_able_to_join, false} -> + {:error, "You cannot join this group"} + + {:ok, %Member{}} -> + {:error, "You are already a member of this group"} + end + end + + def join_group(_parent, _args, _resolution) do + {:error, "You need to be logged-in to join a group"} + end + + @doc """ + Leave a existing group + """ + def leave_group( + _parent, + %{group_id: group_id, actor_id: actor_id}, + %{ + context: %{ + current_user: user + } + } + ) 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)}, + {:ok, _} <- + Mobilizon.Actors.delete_member(member) do + { + :ok, + %{ + parent: %{ + id: group_id + }, + person: %{ + id: actor_id + } + } + } + else + {:is_owned, false} -> + {:error, "Actor id is not owned by authenticated user"} + + {:error, :member_not_found} -> + {:error, "Member not found"} + + {:only_administrator, true} -> + {:error, "You can't leave this group because you are the only administrator"} + end + end + + def leave_group(_parent, _args, _resolution) do + {:error, "You need to be logged-in to leave a group"} + end + + # 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 + with [ + %Member{ + actor: %Actor{ + id: member_actor_id + } + } + ] <- + Member.list_administrator_members_for_group(group_id) do + actor_id == member_actor_id + else + _ -> false + end + end end diff --git a/lib/mobilizon_web/schema.ex b/lib/mobilizon_web/schema.ex index 7d6ff35ae..65cebddc9 100644 --- a/lib/mobilizon_web/schema.ex +++ b/lib/mobilizon_web/schema.ex @@ -143,6 +143,7 @@ defmodule MobilizonWeb.Schema do import_fields(:event_mutations) import_fields(:comment_mutations) import_fields(:participant_mutations) + import_fields(:member_mutations) # @desc "Upload a picture" # field :upload_picture, :picture do diff --git a/lib/mobilizon_web/schema/actors/member.ex b/lib/mobilizon_web/schema/actors/member.ex index 26848f914..86e2f624b 100644 --- a/lib/mobilizon_web/schema/actors/member.ex +++ b/lib/mobilizon_web/schema/actors/member.ex @@ -4,6 +4,8 @@ defmodule MobilizonWeb.Schema.Actors.MemberType do """ use Absinthe.Schema.Notation + alias MobilizonWeb.Resolvers + @desc """ Represents a member of a group """ @@ -11,6 +13,29 @@ defmodule MobilizonWeb.Schema.Actors.MemberType do field(:parent, :group, description: "Of which the profile is member") field(:person, :person, description: "Which profile is member of") field(:role, :integer, description: "The role of this membership") - field(:approved, :boolean, description: "Whether this membership has been approved") + end + + @desc "Represents a deleted member" + object :deleted_member do + field(:parent, :deleted_object) + field(:person, :deleted_object) + end + + object :member_mutations do + @desc "Join a group" + field :join_group, :member do + arg(:group_id, non_null(:integer)) + arg(:actor_id, non_null(:integer)) + + resolve(&Resolvers.Group.join_group/3) + end + + @desc "Leave an event" + field :leave_group, :deleted_member do + arg(:group_id, non_null(:integer)) + arg(:actor_id, non_null(:integer)) + + resolve(&Resolvers.Group.leave_group/3) + end end end diff --git a/priv/repo/migrations/20190301141830_move_member_role_to_enum.exs b/priv/repo/migrations/20190301141830_move_member_role_to_enum.exs new file mode 100644 index 000000000..b0e8dfea1 --- /dev/null +++ b/priv/repo/migrations/20190301141830_move_member_role_to_enum.exs @@ -0,0 +1,46 @@ +defmodule Mobilizon.Repo.Migrations.MoveMemberRoleToEnum do + use Ecto.Migration + alias Mobilizon.Actors.MemberRoleEnum + + def up do + MemberRoleEnum.create_type() + + alter table(:members) do + add(:role_tmp, MemberRoleEnum.type(), default: "member") + end + + execute("UPDATE members set role_tmp = 'member' where role = 0") + execute("UPDATE members set role_tmp = 'moderator' where role = 1") + execute("UPDATE members set role_tmp = 'creator' where role = 2") + + execute("UPDATE members set role_tmp = 'not_approved' where approved is false") + + alter table(:members) do + remove(:role) + remove(:approved) + end + + rename(table(:members), :role_tmp, to: :role) + end + + def down do + alter table(:members) do + add(:role_tmp, :integer, default: 0) + add(:approved, :boolean, default: true) + end + + execute("UPDATE members set approved = false where role = 'not_approved'") + + execute("UPDATE members set role_tmp = 0 where role = 'member' or role = 'not_approved'") + execute("UPDATE members set role_tmp = 1 where role = 'moderator'") + execute("UPDATE members set role_tmp = 2 where role = 'administrator' or role = 'creator'") + + alter table(:members) do + remove(:role) + end + + MemberRoleEnum.drop_type() + + rename(table(:members), :role_tmp, to: :role) + end +end diff --git a/priv/repo/migrations/20190301143831_actor_group_openness.exs b/priv/repo/migrations/20190301143831_actor_group_openness.exs new file mode 100644 index 000000000..34c551734 --- /dev/null +++ b/priv/repo/migrations/20190301143831_actor_group_openness.exs @@ -0,0 +1,18 @@ +defmodule Mobilizon.Repo.Migrations.ActorGroupOpenness do + use Ecto.Migration + alias Mobilizon.Actors.ActorOpennessEnum + + def up do + ActorOpennessEnum.create_type() + + alter table(:actors) do + add(:openness, ActorOpennessEnum.type(), default: "moderated") + end + end + + def down do + alter table(:actors) do + remove(:openness) + end + end +end diff --git a/test/mobilizon/actors/actors_test.exs b/test/mobilizon/actors/actors_test.exs index 70192c111..4b79cf76c 100644 --- a/test/mobilizon/actors/actors_test.exs +++ b/test/mobilizon/actors/actors_test.exs @@ -499,9 +499,9 @@ defmodule Mobilizon.ActorsTest do alias Mobilizon.Actors.Member alias Mobilizon.Actors.Actor - @valid_attrs %{approved: true, role: 0} - @update_attrs %{approved: false, role: 1} - @invalid_attrs %{approved: nil, role: nil} + @valid_attrs %{role: :member} + @update_attrs %{role: :not_approved} + @invalid_attrs %{role: nil} setup do actor = insert(:actor) @@ -528,8 +528,7 @@ defmodule Mobilizon.ActorsTest do |> Map.put(:parent_id, group.id) assert {:ok, %Member{} = member} = Actors.create_member(valid_attrs) - assert member.approved == true - assert member.role == 0 + assert member.role == :member assert [group] = Actor.get_groups_member_of(actor) assert [actor] = Actor.get_members_for_group(group) @@ -562,8 +561,7 @@ defmodule Mobilizon.ActorsTest do member = create_test_member(context) assert {:ok, member} = Actors.update_member(member, @update_attrs) assert %Member{} = member - assert member.approved == false - assert member.role == 1 + assert member.role == :not_approved end # This can't happen, since attrs are optional diff --git a/test/mobilizon_web/resolvers/group_resolver_test.exs b/test/mobilizon_web/resolvers/group_resolver_test.exs index 24469aa8a..8bfa255be 100644 --- a/test/mobilizon_web/resolvers/group_resolver_test.exs +++ b/test/mobilizon_web/resolvers/group_resolver_test.exs @@ -115,7 +115,7 @@ defmodule MobilizonWeb.Resolvers.GroupResolverTest do test "delete_group/3 deletes a group", %{conn: conn, user: user, actor: actor} do group = insert(:group) - insert(:member, parent: group, actor: actor, role: 2) + insert(:member, parent: group, actor: actor, role: :administrator) mutation = """ mutation { @@ -146,7 +146,7 @@ defmodule MobilizonWeb.Resolvers.GroupResolverTest do test "delete_group/3 should check user authentication", %{conn: conn, actor: actor} do group = insert(:group) - insert(:member, parent: group, actor: actor, role: 2) + insert(:member, parent: group, actor: actor, role: :member) mutation = """ mutation { @@ -172,7 +172,7 @@ defmodule MobilizonWeb.Resolvers.GroupResolverTest do actor: actor } do group = insert(:group) - insert(:member, parent: group, actor: actor, role: 2) + insert(:member, parent: group, actor: actor, role: :member) mutation = """ mutation { @@ -225,7 +225,7 @@ defmodule MobilizonWeb.Resolvers.GroupResolverTest do actor: actor } do group = insert(:group) - insert(:member, parent: group, actor: actor, role: 1) + insert(:member, parent: group, actor: actor, role: :member) mutation = """ mutation { diff --git a/test/mobilizon_web/resolvers/member_resolver_test.exs b/test/mobilizon_web/resolvers/member_resolver_test.exs new file mode 100644 index 000000000..fb075ed0d --- /dev/null +++ b/test/mobilizon_web/resolvers/member_resolver_test.exs @@ -0,0 +1,289 @@ +defmodule MobilizonWeb.Resolvers.MemberResolverTest do + use MobilizonWeb.ConnCase + + alias MobilizonWeb.AbsintheHelpers + + import Mobilizon.Factory + + setup %{conn: conn} do + user = insert(:user) + actor = insert(:actor, user: user, preferred_username: "test") + + {:ok, conn: conn, actor: actor, user: user} + end + + describe "Member Resolver" do + test "join_group/3 should create a member", %{conn: conn, user: user, actor: actor} do + group = insert(:group) + + mutation = """ + mutation { + joinGroup( + actor_id: #{actor.id}, + group_id: #{group.id} + ) { + role, + person { + id + }, + parent { + id + } + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["errors"] == nil + assert json_response(res, 200)["data"]["joinGroup"]["role"] == "not_approved" + assert json_response(res, 200)["data"]["joinGroup"]["parent"]["id"] == group.id + assert json_response(res, 200)["data"]["joinGroup"]["person"]["id"] == actor.id + + mutation = """ + mutation { + joinGroup( + actor_id: #{actor.id}, + group_id: #{group.id} + ) { + role + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "already a member" + end + + test "join_group/3 should check the actor is owned by the user", %{ + conn: conn, + user: user + } do + group = insert(:group) + + mutation = """ + mutation { + joinGroup( + actor_id: 1042, + group_id: #{group.id} + ) { + role + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "not owned" + end + + test "join_group/3 should check the group is not invite only", %{ + conn: conn, + actor: actor, + user: user + } do + group = insert(:group, %{openness: :invite_only}) + + mutation = """ + mutation { + joinGroup( + actor_id: #{actor.id}, + group_id: #{group.id} + ) { + role + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "cannot join this group" + end + + test "join_group/3 should check the group exists", %{ + conn: conn, + user: user, + actor: actor + } do + mutation = """ + mutation { + joinGroup( + actor_id: #{actor.id}, + group_id: 1042 + ) { + role + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "Group id not found" + end + + test "leave_group/3 should delete a member from a group", %{ + conn: conn, + user: user, + actor: actor + } do + group = insert(:group) + insert(:member, %{actor: actor, parent: group}) + + mutation = """ + mutation { + leaveGroup( + actor_id: #{actor.id}, + group_id: #{group.id} + ) { + person { + id + }, + parent { + id + } + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["errors"] == nil + assert json_response(res, 200)["data"]["leaveGroup"]["parent"]["id"] == group.id + assert json_response(res, 200)["data"]["leaveGroup"]["person"]["id"] == actor.id + end + + test "leave_group/3 should check if the member is the only administrator", %{ + conn: conn, + actor: actor, + user: user + } do + group = insert(:group) + insert(:member, %{actor: actor, role: :creator, parent: group}) + insert(:member, %{parent: group}) + + mutation = """ + mutation { + leaveGroup( + actor_id: #{actor.id}, + group_id: #{group.id} + ) { + person { + id + }, + parent { + id + } + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "only administrator" + end + + test "leave_group/3 should check the user is logged in", %{conn: conn, actor: actor} do + group = insert(:group) + insert(:member, %{actor: actor, parent: group}) + + mutation = """ + mutation { + leaveGroup( + actor_id: #{actor.id}, + group_id: #{group.id} + ) { + person { + id + } + } + } + """ + + res = + conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "logged-in" + end + + test "leave_group/3 should check the actor is owned by the user", %{ + conn: conn, + user: user, + actor: actor + } do + group = insert(:group) + insert(:member, %{actor: actor, parent: group}) + + mutation = """ + mutation { + leaveGroup( + actor_id: 1042, + group_id: #{group.id} + ) { + person { + id + } + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "not owned" + end + + test "leave_group/3 should check the member exists", %{ + conn: conn, + user: user, + actor: actor + } do + group = insert(:group) + insert(:member, %{actor: actor, parent: group}) + + mutation = """ + mutation { + leaveGroup( + actor_id: #{actor.id}, + group_id: 1042 + ) { + person { + id + } + } + } + """ + + res = + conn + |> auth_conn(user) + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] =~ "Member not found" + end + end +end diff --git a/test/support/factory.ex b/test/support/factory.ex index 57f364677..d34e9d0b9 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -149,7 +149,7 @@ defmodule Mobilizon.Factory do %Mobilizon.Actors.Member{ parent: build(:actor), actor: build(:actor), - role: 0 + role: :not_approved } end end