From 226c09f031db06d91dc57298e0e6e93fcc261e01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Chmielowski?= <pchmielowski@process-one.net>
Date: Fri, 24 May 2019 14:02:10 +0200
Subject: [PATCH] Make mod_admin_extra add/delete_rosteritem reuse code from
 mod_roster

---
 src/mod_admin_extra.erl | 29 +++++++++---------------
 src/mod_roster.erl      | 49 ++++++++++++++++++++++++-----------------
 2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/src/mod_admin_extra.erl b/src/mod_admin_extra.erl
index d86e50258..f61699a75 100644
--- a/src/mod_admin_extra.erl
+++ b/src/mod_admin_extra.erl
@@ -1233,34 +1233,25 @@ update_vcard_els(Data, ContentList, Els1) ->
 %%%
 
 add_rosteritem(LocalUser, LocalServer, User, Server, Nick, Group, Subs) ->
-    case add_rosteritem(LocalUser, LocalServer, User, Server, Nick, Group, Subs, []) of
-	{atomic, ok} ->
-	    push_roster_item(LocalUser, LocalServer, User, Server, {add, Nick, Subs, Group}),
-	    ok;
-	_ ->
-	    error
+    Jid = jid:make(LocalUser, LocalServer),
+    RosterItem = build_roster_item(User, Server, {add, Nick, Subs, Group}),
+    case mod_roster:set_item_and_notify_clients(Jid, RosterItem, true) of
+	ok -> ok;
+	_ -> error
     end.
 
-add_rosteritem(LU, LS, User, Server, Nick, Group, Subscription, Xattrs) ->
-    subscribe(LU, LS, User, Server, Nick, Group, Subscription, Xattrs).
-
 subscribe(LU, LS, User, Server, Nick, Group, Subscription, _Xattrs) ->
     ItemEl = build_roster_item(User, Server, {add, Nick, Subscription, Group}),
     mod_roster:set_items(LU, LS, #roster_query{items = [ItemEl]}).
 
 delete_rosteritem(LocalUser, LocalServer, User, Server) ->
-    case unsubscribe(LocalUser, LocalServer, User, Server) of
-	{atomic, ok} ->
-	    push_roster_item(LocalUser, LocalServer, User, Server, remove),
-	    ok;
-	_  ->
-	    error
+    Jid = jid:make(LocalUser, LocalServer),
+    RosterItem = build_roster_item(User, Server, remove),
+    case mod_roster:set_item_and_notify_clients(Jid, RosterItem, true) of
+	ok -> ok;
+	_ -> error
     end.
 
-unsubscribe(LU, LS, User, Server) ->
-    ItemEl = build_roster_item(User, Server, remove),
-    mod_roster:set_items(LU, LS, #roster_query{items = [ItemEl]}).
-
 %% -----------------------------
 %% Get Roster
 %% -----------------------------
diff --git a/src/mod_roster.erl b/src/mod_roster.erl
index 6d8e45512..a9bafd4fe 100644
--- a/src/mod_roster.erl
+++ b/src/mod_roster.erl
@@ -51,7 +51,7 @@
 	 roster_versioning_enabled/1, roster_version/2,
 	 mod_opt_type/1, mod_options/1, set_roster/1, del_roster/3,
 	 process_rosteritems/5,
-	 depends/2]).
+	 depends/2, set_item_and_notify_clients/3]).
 
 -include("logger.hrl").
 -include("xmpp.hrl").
@@ -441,23 +441,34 @@ decode_item(Item, R, Managed) ->
 
 process_iq_set(#iq{from = _From, to = To,
 		   sub_els = [#roster_query{items = [QueryItem]}]} = IQ) ->
+    case set_item_and_notify_clients(To, QueryItem, false) of
+	ok ->
+	    xmpp:make_iq_result(IQ);
+	E ->
+	    ?ERROR_MSG("roster set failed:~nIQ = ~s~nError = ~p",
+		       [xmpp:pp(IQ), E]),
+	    xmpp:make_error(IQ, xmpp:err_internal_server_error())
+    end.
+
+-spec set_item_and_notify_clients(jid(), #roster_item{}, boolean()) -> ok | error.
+set_item_and_notify_clients(To, #roster_item{jid = LJID} = RosterItem,
+			    OverrideSubscription) ->
     #jid{luser = LUser, lserver = LServer} = To,
-    LJID = jid:tolower(QueryItem#roster_item.jid),
     F = fun () ->
-		Item = get_roster_item(LUser, LServer, LJID),
-		Item2 = decode_item(QueryItem, Item, false),
-		Item3 = ejabberd_hooks:run_fold(roster_process_item,
-						LServer, Item2,
-						[LServer]),
-		case Item3#roster.subscription of
-		    remove -> del_roster_t(LUser, LServer, LJID);
-		    _ -> update_roster_t(LUser, LServer, LJID, Item3)
-		end,
-		case roster_version_on_db(LServer) of
-		    true -> write_roster_version_t(LUser, LServer);
-		    false -> ok
-		end,
-		{Item, Item3}
+	Item = get_roster_item(LUser, LServer, LJID),
+	Item2 = decode_item(RosterItem, Item, OverrideSubscription),
+	Item3 = ejabberd_hooks:run_fold(roster_process_item,
+					LServer, Item2,
+					[LServer]),
+	case Item3#roster.subscription of
+	    remove -> del_roster_t(LUser, LServer, LJID);
+	    _ -> update_roster_t(LUser, LServer, LJID, Item3)
+	end,
+	case roster_version_on_db(LServer) of
+	    true -> write_roster_version_t(LUser, LServer);
+	    false -> ok
+	end,
+	{Item, Item3}
 	end,
     case transaction(LUser, LServer, [LJID], F) of
 	{atomic, {OldItem, Item}} ->
@@ -468,11 +479,9 @@ process_iq_set(#iq{from = _From, to = To,
 		_ ->
 		    ok
 	    end,
-	    xmpp:make_iq_result(IQ);
+	    ok;
 	E ->
-	    ?ERROR_MSG("roster set failed:~nIQ = ~s~nError = ~p",
-		       [xmpp:pp(IQ), E]),
-	    xmpp:make_error(IQ, xmpp:err_internal_server_error())
+	    E
     end.
 
 push_item(To, OldItem, NewItem) ->