Fix types in subscribeAllToAll roster API

esl · Sep 12, 2022 · 0da283d · 0da283d
1 parent 77951fc
commit 0da283d
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 4 deletions.
diff --git a/big_tests/tests/graphql_helper.erl b/big_tests/tests/graphql_helper.erl
@@ -106,7 +106,7 @@ init_domain_admin_handler(Config) ->
             Password = base16:encode(crypto:strong_rand_bytes(8)),
             Creds = {<<"admin@", Domain/binary>>, Password},
             ok = domain_helper:set_domain_password(mim(), Domain, Password),
-            add_specs([{protocol, http}, {domain_admin, Creds}, {schema_endpoint, domain_admin} 
+            add_specs([{protocol, http}, {domain_admin, Creds}, {schema_endpoint, domain_admin}
                       | Config]);
         false -> {skip, require_rdbms}
     end.
@@ -143,6 +143,11 @@ get_unauthorized({Code, #{<<"errors">> := Errors}}) ->
     assert_response_code(unauthorized, Code),
     ?assertEqual(<<"no_permissions">>, ErrorCode).
 
+get_bad_request({Code, #{<<"errors">> := [_Error]}}) ->
+    assert_response_code(bad_request, Code);
+get_bad_request({Code, _Msg}) ->
+    assert_response_code(bad_request, Code).
+
 get_coercion_err_msg({Code, #{<<"errors">> := [Error]}}) ->
     assert_response_code(bad_request, Code),
     ?assertEqual(<<"input_coercion">>, get_value([extensions, code], Error)),

diff --git a/big_tests/tests/graphql_roster_SUITE.erl b/big_tests/tests/graphql_roster_SUITE.erl
@@ -5,7 +5,8 @@
 -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]).
 -import(graphql_helper, [execute_user_command/5, execute_command/4, get_listener_port/1,
                          get_listener_config/1, get_ok_value/2, get_err_value/2, get_err_msg/1,
-                         get_err_msg/2, user_to_jid/1, user_to_bin/1, get_unauthorized/1]).
+                         get_err_msg/2, get_bad_request/1, user_to_jid/1, user_to_bin/1,
+                         get_unauthorized/1]).
 
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("../../include/mod_roster.hrl").
@@ -55,8 +56,12 @@ admin_roster_tests() ->
      admin_set_mutual_subscription_try_disconnect_nonexistent_users,
      admin_subscribe_to_all,
      admin_subscribe_to_all_with_wrong_user,
+     admin_subscribe_to_all_no_groups,
+     admin_subscribe_to_all_without_arguments,
      admin_subscribe_all_to_all,
      admin_subscribe_all_to_all_with_wrong_user,
+     admin_subscribe_all_to_all_no_groups,
+     admin_subscribe_all_to_all_without_arguments,
      admin_list_contacts,
      admin_list_contacts_wrong_user,
      admin_get_contact,
@@ -81,9 +86,13 @@ domain_admin_tests() ->
      domain_admin_subscribe_to_all_no_permission,
      admin_subscribe_to_all,
      domain_admin_subscribe_to_all_with_wrong_user,
+     admin_subscribe_to_all_no_groups,
+     admin_subscribe_to_all_without_arguments,
      domain_admin_subscribe_all_to_all_no_permission,
      admin_subscribe_all_to_all,
      domain_admin_subscribe_all_to_all_with_wrong_user,
+     admin_subscribe_all_to_all_no_groups,
+     admin_subscribe_all_to_all_without_arguments,
      admin_list_contacts,
      domain_admin_list_contacts_wrong_user,
      domain_admin_list_contacts_no_permission,
@@ -314,6 +323,24 @@ admin_subscribe_to_all_with_wrong_user_story(Config, Alice, Bob) ->
     check_contacts([Bob], Alice),
     check_contacts([Alice], Bob).
 
+
+admin_subscribe_to_all_no_groups(Config) ->
+    escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}, {kate, 1}],
+                                    fun admin_subscribe_to_all_no_groups_story/4).
+
+admin_subscribe_to_all_no_groups_story(Config, Alice, Bob, Kate) ->
+    EmptyGroups = [],
+    Res = admin_subscribe_to_all(Alice, [Bob, Kate], null, Config),
+    check_if_created_succ(?SUBSCRIBE_TO_ALL_PATH, Res),
+
+    check_contacts([Bob, Kate], Alice, EmptyGroups),
+    check_contacts([Alice], Bob, EmptyGroups),
+    check_contacts([Alice], Kate, EmptyGroups).
+
+admin_subscribe_to_all_without_arguments(Config) ->
+    Res = admin_subscribe_to_all_no_args(Config),
+    get_bad_request(Res).
+
 admin_subscribe_all_to_all(Config) ->
     escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}, {kate, 1}],
                                     fun admin_subscribe_all_to_all_story/4).
@@ -340,6 +367,23 @@ admin_subscribe_all_to_all_with_wrong_user_story(Config, Alice, Bob) ->
     check_contacts([Bob], Alice),
     check_contacts([Alice], Bob).
 
+admin_subscribe_all_to_all_no_groups(Config) ->
+    escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}, {kate, 1}],
+                                    fun admin_subscribe_all_to_all_no_groups_story/4).
+
+admin_subscribe_all_to_all_no_groups_story(Config, Alice, Bob, Kate) ->
+    EmptyGroups = [],
+    Res = admin_subscribe_all_to_all([Alice, Bob, Kate], null, Config),
+    check_if_created_succ(?SUBSCRIBE_ALL_TO_ALL_PATH, Res),
+
+    check_contacts([Bob, Kate], Alice, EmptyGroups),
+    check_contacts([Alice, Kate], Bob, EmptyGroups),
+    check_contacts([Alice, Bob], Kate, EmptyGroups).
+
+admin_subscribe_all_to_all_without_arguments(Config) ->
+    Res = admin_subscribe_all_to_no_args(Config),
+    get_bad_request(Res).
+
 admin_list_contacts(Config) ->
     escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}],
                                     fun admin_list_contacts_story/3).
@@ -665,6 +709,17 @@ check_contacts(ContactClients, User) ->
     ?assertEqual(lists:sort(ExpectedNames), lists:sort(ActualNames)),
     [?assertEqual(?DEFAULT_GROUPS, Groups) || #roster{groups = Groups} <- ActualContacts].
 
+check_contacts(ContactClients, User, ContactGroups) ->
+    Expected = [escalus_utils:jid_to_lower(escalus_client:short_jid(Client))
+                || Client <- ContactClients],
+    ExpectedNames = [escalus_client:username(Client) || Client <- ContactClients],
+    ActualContacts = get_roster(User),
+    Actual = [ jid:to_binary(JID) || #roster{jid = JID} <- ActualContacts],
+    ActualNames = [ Name || #roster{name = Name} <- ActualContacts],
+    ?assertEqual(lists:sort(Expected), lists:sort(Actual)),
+    ?assertEqual(lists:sort(ExpectedNames), lists:sort(ActualNames)),
+    [?assertEqual(ContactGroups, Groups) || #roster{groups = Groups} <- ActualContacts].
+
 check_if_created_succ(Path, Res) ->
     OkList = get_ok_value(Path, Res),
     lists:foreach(fun check_created_msg/1, OkList).
@@ -694,9 +749,15 @@ get_roster(User, Contact) ->
 make_contacts(Users) ->
     [make_contact(U) || U <- Users].
 
+make_contacts(Users, Groups) ->
+    [make_contact(U, Groups) || U <- Users].
+
 make_contact(U) ->
     #{jid => user_to_bin(U), name => escalus_utils:get_username(U), groups => ?DEFAULT_GROUPS}.
 
+make_contact(U, Groups) ->
+    #{jid => user_to_bin(U), name => escalus_utils:get_username(U), groups => Groups}.
+
 %% Commands
 
 admin_add_contact(User, Contact, Name, Groups, Config) ->
@@ -728,10 +789,24 @@ admin_subscribe_to_all(User, Contacts, Config) ->
     Vars = #{user => make_contact(User), contacts => make_contacts(Contacts)},
     execute_command(<<"roster">>, <<"subscribeToAll">>, Vars, Config).
 
+admin_subscribe_to_all(User, Contacts, Groups, Config) ->
+    Vars = #{user => make_contact(User, Groups), contacts => make_contacts(Contacts, Groups)},
+    execute_command(<<"roster">>, <<"subscribeToAll">>, Vars, Config).
+
+admin_subscribe_to_all_no_args(Config) ->
+    execute_command(<<"roster">>, <<"subscribeToAll">>, #{}, Config).
+
 admin_subscribe_all_to_all(Users, Config) ->
     Vars = #{contacts => make_contacts(Users)},
     execute_command(<<"roster">>, <<"subscribeAllToAll">>, Vars, Config).
 
+admin_subscribe_all_to_no_args(Config) ->
+    execute_command(<<"roster">>, <<"subscribeAllToAll">>, #{}, Config).
+
+admin_subscribe_all_to_all(Users, Groups, Config) ->
+    Vars = #{contacts => make_contacts(Users, Groups)},
+    execute_command(<<"roster">>, <<"subscribeAllToAll">>, Vars, Config).
+
 admin_list_contacts(User, Config) ->
     Vars = #{user => user_to_bin(User)},
     execute_command(<<"roster">>, <<"listContacts">>, Vars, Config).

diff --git a/priv/graphql/schemas/admin/roster.gql b/priv/graphql/schemas/admin/roster.gql
@@ -24,7 +24,7 @@ type RosterAdminMutation @protected{
   subscribeToAll(user: ContactInput!, contacts: [ContactInput!]!): [String]!
     @protected(type: DOMAIN, args: ["user.jid"])
   "Set mutual subscriptions between all of the given contacts"
-  subscribeAllToAll(contacts: [ContactInput!]): [String]!
+  subscribeAllToAll(contacts: [ContactInput!]!): [String]!
     @protected(type: DOMAIN, args: ["contacts.jid"])
 }
 

diff --git a/src/graphql/admin/mongoose_graphql_roster_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_roster_admin_mutation.erl
@@ -87,4 +87,5 @@ do_subscribe_all_to_all([User | Contacts]) ->
     do_subscribe_to_all(User, Contacts) ++ do_subscribe_all_to_all(Contacts).
 
 contact_input_map_to_tuple(#{<<"jid">> := JID, <<"name">> := Name, <<"groups">> := Groups}) ->
-    {JID, Name, Groups}.
+    Groups1 = null_to_default(Groups, []),
+    {JID, Name, Groups1}.