Fix unlisted groups being available in search

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

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",
+}

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";

js/src/views/Group/GroupSettings.vue

@@ -76,7 +76,7 @@
           <b-radio
             v-model="group.visibility"
             name="groupVisibility"
-            :native-value="GroupVisibility.UNLISTED"
+            :native-value="GroupVisibility.PRIVATE"
             >{{ $t("Only accessible through link") }}<br />
             <small>{{
               $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 = {
+  GroupVisibility = GroupVisibility;
-    PUBLIC: "PUBLIC",
-    UNLISTED: "UNLISTED",
-  };
 
   Openness = Openness;
 

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),
+            term,
-            [result_type],
+            [
+              actor_type: [result_type],
+              radius: Map.get(args, :radius),
+              location: Map.get(args, :location),
+              minimum_visibility: :public
+            ],
             page,
             limit
           )

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"

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

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(),
+          String.t(),
-          [ActorType.t()],
+          Keyword.t(),
           integer | nil,
           integer | nil
         ) :: Page.t()
   def build_actors_by_username_or_name_page(
-        %{term: term} = args,
+        term,
-        types,
+        options \\ [],
         page \\ nil,
         limit \\ nil
       ) do
     Actor
     |> actor_by_username_or_name_query(term)
-    |> actors_for_location(args)
+    |> actors_for_location(Keyword.get(options, :location), Keyword.get(options, :radius))
-    |> filter_by_types(types)
+    |> 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()
+  @spec actors_for_location(Ecto.Query.t(), String.t(), integer()) :: Ecto.Query.t()
-  defp actors_for_location(query, %{radius: radius}) when is_nil(radius),
+  defp actors_for_location(query, location, radius)
-    do: query
-
-  defp actors_for_location(query, %{location: location, radius: 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))

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

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