From a646d4a40a79770ee763b77ab19f892281d05e84 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 17 Dec 2020 15:25:58 +0100 Subject: [PATCH] Fix unlisted groups being available in search Signed-off-by: Thomas Citharel --- js/src/types/enums.ts | 6 +++++ js/src/views/Group/Create.vue | 2 +- js/src/views/Group/GroupSettings.vue | 9 +++---- lib/graphql/api/search.ex | 9 +++++-- lib/graphql/schema/actors/group.ex | 6 +++++ lib/mobilizon/actors/actor.ex | 9 ++++++- lib/mobilizon/actors/actors.ex | 37 ++++++++++++++++++--------- test/graphql/api/search_test.exs | 9 +++++-- test/mobilizon/actors/actors_test.exs | 7 +++-- 9 files changed, 68 insertions(+), 26 deletions(-) diff --git a/js/src/types/enums.ts b/js/src/types/enums.ts index a081a3463..18c5d7da6 100644 --- a/js/src/types/enums.ts +++ b/js/src/types/enums.ts @@ -172,3 +172,9 @@ export enum RoutingTransportationType { TRANSIT = "TRANSIT", CAR = "CAR", } + +export enum GroupVisibility { + PUBLIC = "PUBLIC", + UNLISTED = "UNLISTED", + PRIVATE = "PRIVATE", +} diff --git a/js/src/views/Group/Create.vue b/js/src/views/Group/Create.vue index b0517fe97..438bc6b96 100644 --- a/js/src/views/Group/Create.vue +++ b/js/src/views/Group/Create.vue @@ -80,7 +80,7 @@ import { CURRENT_ACTOR_CLIENT, PERSON_MEMBERSHIPS } from "@/graphql/actor"; import { CREATE_GROUP } from "@/graphql/group"; import { mixins } from "vue-class-component"; import IdentityEditionMixin from "@/mixins/identityEdition"; -import { MemberRole } from "@/types/enums"; +import { MemberRole, Openness } from "@/types/enums"; import RouteName from "../../router/name"; import { convertToUsername } from "../../utils/username"; import PictureUpload from "../../components/PictureUpload.vue"; diff --git a/js/src/views/Group/GroupSettings.vue b/js/src/views/Group/GroupSettings.vue index 819600908..54bfbb9ed 100644 --- a/js/src/views/Group/GroupSettings.vue +++ b/js/src/views/Group/GroupSettings.vue @@ -76,7 +76,7 @@ {{ $t("Only accessible through link") }}
{{ $t( @@ -163,7 +163,7 @@ import { Route } from "vue-router"; import PictureUpload from "@/components/PictureUpload.vue"; import { mixins } from "vue-class-component"; import GroupMixin from "@/mixins/group"; -import { Openness } from "@/types/enums"; +import { GroupVisibility, Openness } from "@/types/enums"; import RouteName from "../../router/name"; import { UPDATE_GROUP, DELETE_GROUP } from "../../graphql/group"; import { IGroup, usernameWithDomain } from "../../types/actor"; @@ -189,10 +189,7 @@ export default class GroupSettings extends mixins(GroupMixin) { usernameWithDomain = usernameWithDomain; - GroupVisibility = { - PUBLIC: "PUBLIC", - UNLISTED: "UNLISTED", - }; + GroupVisibility = GroupVisibility; Openness = Openness; diff --git a/lib/graphql/api/search.ex b/lib/graphql/api/search.ex index 637a5a7d6..f0f1ec83c 100644 --- a/lib/graphql/api/search.ex +++ b/lib/graphql/api/search.ex @@ -40,8 +40,13 @@ defmodule Mobilizon.GraphQL.API.Search do true -> page = Actors.build_actors_by_username_or_name_page( - Map.put(args, :term, term), - [result_type], + term, + [ + actor_type: [result_type], + radius: Map.get(args, :radius), + location: Map.get(args, :location), + minimum_visibility: :public + ], page, limit ) diff --git a/lib/graphql/schema/actors/group.ex b/lib/graphql/schema/actors/group.ex index 4a41b8f71..ad1fc9de9 100644 --- a/lib/graphql/schema/actors/group.ex +++ b/lib/graphql/schema/actors/group.ex @@ -149,6 +149,7 @@ defmodule Mobilizon.GraphQL.Schema.Actors.GroupType do enum :group_visibility do value(:public, description: "Publicly listed and federated") value(:unlisted, description: "Visible only to people with the link - or invited") + value(:private, description: "Visible only to people with the link - or invited") end object :group_queries do @@ -198,6 +199,11 @@ defmodule Mobilizon.GraphQL.Schema.Actors.GroupType do default_value: :public ) + arg(:openness, :openness, + default_value: :invite_only, + description: "Whether the group can be join freely, with approval or is invite-only." + ) + arg(:avatar, :media_input, description: "The avatar for the group, either as an object or directly the ID of an existing media" diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex index 38727afc1..49316bc89 100644 --- a/lib/mobilizon/actors/actor.ex +++ b/lib/mobilizon/actors/actor.ex @@ -135,7 +135,14 @@ defmodule Mobilizon.Actors.Actor do :preferred_username, :members_url ] - @group_creation_optional_attrs [:shared_inbox_url, :name, :domain, :summary, :visibility] + @group_creation_optional_attrs [ + :shared_inbox_url, + :name, + :domain, + :summary, + :visibility, + :openness + ] @group_creation_attrs @group_creation_required_attrs ++ @group_creation_optional_attrs schema "actors" do diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index 36ebf9d81..d5e88d4df 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/lib/mobilizon/actors/actors.ex @@ -512,21 +512,22 @@ defmodule Mobilizon.Actors do Builds a page struct for actors by their name or displayed name. """ @spec build_actors_by_username_or_name_page( - map(), - [ActorType.t()], + String.t(), + Keyword.t(), integer | nil, integer | nil ) :: Page.t() def build_actors_by_username_or_name_page( - %{term: term} = args, - types, + term, + options \\ [], page \\ nil, limit \\ nil ) do Actor |> actor_by_username_or_name_query(term) - |> actors_for_location(args) - |> filter_by_types(types) + |> actors_for_location(Keyword.get(options, :location), Keyword.get(options, :radius)) + |> filter_by_types(Keyword.get(options, :actor_type, :Group)) + |> filter_by_minimum_visibility(Keyword.get(options, :minimum_visibility, :public)) |> filter_suspended(false) |> Page.build_page(page, limit) end @@ -1395,11 +1396,8 @@ defmodule Mobilizon.Actors do ) end - @spec actors_for_location(Ecto.Query.t(), map()) :: Ecto.Query.t() - defp actors_for_location(query, %{radius: radius}) when is_nil(radius), - do: query - - defp actors_for_location(query, %{location: location, radius: radius}) + @spec actors_for_location(Ecto.Query.t(), String.t(), integer()) :: Ecto.Query.t() + defp actors_for_location(query, location, radius) when is_valid_string?(location) and not is_nil(radius) do with {lon, lat} <- Geohax.decode(location), point <- Geo.WKT.decode!("SRID=4326;POINT(#{lon} #{lat})") do @@ -1414,7 +1412,7 @@ defmodule Mobilizon.Actors do end end - defp actors_for_location(query, _args), do: query + defp actors_for_location(query, _location, _radius), do: query @spec person_query :: Ecto.Query.t() defp person_query do @@ -1660,6 +1658,21 @@ defmodule Mobilizon.Actors do from(a in query, where: a.type in ^types) end + @spec filter_by_minimum_visibility(Ecto.Query.t(), atom()) :: Ecto.Query.t() + defp filter_by_minimum_visibility(query, :private), do: query + + defp filter_by_minimum_visibility(query, :restricted) do + from(a in query, where: a.visibility in ^[:public, :unlisted, :restricted]) + end + + defp filter_by_minimum_visibility(query, :unlisted) do + from(a in query, where: a.visibility in ^[:public, :unlisted]) + end + + defp filter_by_minimum_visibility(query, :public) do + from(a in query, where: a.visibility == ^:public) + end + @spec filter_by_name(Ecto.Query.t(), [String.t()]) :: Ecto.Query.t() defp filter_by_name(query, [name]) do from(a in query, where: a.preferred_username == ^name and is_nil(a.domain)) diff --git a/test/graphql/api/search_test.exs b/test/graphql/api/search_test.exs index c9f095071..74e8a65ea 100644 --- a/test/graphql/api/search_test.exs +++ b/test/graphql/api/search_test.exs @@ -35,14 +35,19 @@ defmodule Mobilizon.GraphQL.API.SearchTest do test "search actors" do with_mock Actors, - build_actors_by_username_or_name_page: fn %{term: "toto"}, _type, 1, 10 -> + build_actors_by_username_or_name_page: fn "toto", _options, 1, 10 -> %Page{total: 1, elements: [%Actor{id: 42}]} end do assert {:ok, %{total: 1, elements: [%Actor{id: 42}]}} = Search.search_actors(%{term: "toto"}, 1, 10, :Person) assert_called( - Actors.build_actors_by_username_or_name_page(%{term: "toto"}, [:Person], 1, 10) + Actors.build_actors_by_username_or_name_page( + "toto", + [actor_type: [:Person], radius: nil, location: nil, minimum_visibility: :public], + 1, + 10 + ) ) end end diff --git a/test/mobilizon/actors/actors_test.exs b/test/mobilizon/actors/actors_test.exs index 55928eb08..ca4ef2ba7 100644 --- a/test/mobilizon/actors/actors_test.exs +++ b/test/mobilizon/actors/actors_test.exs @@ -188,7 +188,10 @@ defmodule Mobilizon.ActorsTest do with {:ok, %Actor{id: actor2_id}} <- ActivityPub.get_or_fetch_actor_by_url(@remote_account_url) do %Page{total: 2, elements: actors} = - Actors.build_actors_by_username_or_name_page(%{term: "tcit"}, [:Person]) + Actors.build_actors_by_username_or_name_page("tcit", + actor_type: [:Person], + minimum_visibility: :private + ) actors_ids = actors |> Enum.map(& &1.id) @@ -199,7 +202,7 @@ defmodule Mobilizon.ActorsTest do test "test build_actors_by_username_or_name_page/4 returns actors with similar names" do %{total: 0, elements: actors} = - Actors.build_actors_by_username_or_name_page(%{term: "ohno"}, [:Person]) + Actors.build_actors_by_username_or_name_page("ohno", actor_type: [:Person]) assert actors == [] end