Rename opts->name to label, to avoid confusion with the group name (#3214)

Also updated WebAdmin to show more meaningful explanations
Also fixed a bug that break support for group@host in Displayed
This commit is contained in:
Badlop 2020-04-14 13:45:44 +02:00
parent b02506eaaf
commit e197b25e82
2 changed files with 96 additions and 55 deletions

View File

@ -188,12 +188,12 @@ get_user_roster(Items, US) ->
{U, S} = US, {U, S} = US,
DisplayedGroups = get_user_displayed_groups(US), DisplayedGroups = get_user_displayed_groups(US),
SRUsers = lists:foldl(fun (Group, Acc1) -> SRUsers = lists:foldl(fun (Group, Acc1) ->
GroupName = get_group_name(S, Group), GroupLabel = get_group_label(S, Group), %++
lists:foldl(fun (User, Acc2) -> lists:foldl(fun (User, Acc2) ->
if User == US -> Acc2; if User == US -> Acc2;
true -> true ->
dict:append(User, dict:append(User,
GroupName, GroupLabel,
Acc2) Acc2)
end end
end, end,
@ -208,12 +208,12 @@ get_user_roster(Items, US) ->
case dict:find(US1, case dict:find(US1,
SRUsers1) SRUsers1)
of of
{ok, GroupNames} -> {ok, GroupLabels} ->
{Item#roster{subscription {Item#roster{subscription
= =
both, both,
groups = groups =
Item#roster.groups ++ GroupNames, Item#roster.groups ++ GroupLabels,
ask = ask =
none}, none},
dict:erase(US1, dict:erase(US1,
@ -226,8 +226,8 @@ get_user_roster(Items, US) ->
SRItems = [#roster{usj = {U, S, {U1, S1, <<"">>}}, SRItems = [#roster{usj = {U, S, {U1, S1, <<"">>}},
us = US, jid = {U1, S1, <<"">>}, us = US, jid = {U1, S1, <<"">>},
name = get_rosteritem_name(U1, S1), name = get_rosteritem_name(U1, S1),
subscription = both, ask = none, groups = GroupNames} subscription = both, ask = none, groups = GroupLabels}
|| {{U1, S1}, GroupNames} <- dict:to_list(SRUsersRest)], || {{U1, S1}, GroupLabels} <- dict:to_list(SRUsersRest)],
SRItems ++ NewItems1. SRItems ++ NewItems1.
get_rosteritem_name(U, S) -> get_rosteritem_name(U, S) ->
@ -268,12 +268,12 @@ process_item(RosterItem, Host) ->
[] -> RosterItem; [] -> RosterItem;
%% Roster item cannot be removed: We simply reset the original groups: %% Roster item cannot be removed: We simply reset the original groups:
_ when RosterItem#roster.subscription == remove -> _ when RosterItem#roster.subscription == remove ->
GroupNames = lists:map(fun (Group) -> GroupLabels = lists:map(fun (Group) ->
get_group_name(Host, Group) get_group_label(Host, Group)
end, end,
CommonGroups), CommonGroups),
RosterItem#roster{subscription = both, ask = none, RosterItem#roster{subscription = both, ask = none,
groups = GroupNames}; groups = GroupLabels};
%% Both users have at least a common shared group, %% Both users have at least a common shared group,
%% So each user can see the other %% So each user can see the other
_ -> _ ->
@ -352,10 +352,10 @@ get_jid_info({Subscription, Ask, Groups}, User, Server,
US1 = {U1, S1}, US1 = {U1, S1},
DisplayedGroups = get_user_displayed_groups(US), DisplayedGroups = get_user_displayed_groups(US),
SRUsers = lists:foldl(fun (Group, Acc1) -> SRUsers = lists:foldl(fun (Group, Acc1) ->
GroupName = get_group_name(LServer, Group), GroupLabel = get_group_label(LServer, Group), %++
lists:foldl(fun (User1, Acc2) -> lists:foldl(fun (User1, Acc2) ->
dict:append(User1, dict:append(User1,
GroupName, GroupLabel,
Acc2) Acc2)
end, end,
Acc1, Acc1,
@ -363,8 +363,8 @@ get_jid_info({Subscription, Ask, Groups}, User, Server,
end, end,
dict:new(), DisplayedGroups), dict:new(), DisplayedGroups),
case dict:find(US1, SRUsers) of case dict:find(US1, SRUsers) of
{ok, GroupNames} -> {ok, GroupLabels} ->
NewGroups = if Groups == [] -> GroupNames; NewGroups = if Groups == [] -> GroupLabels;
true -> Groups true -> Groups
end, end,
{both, none, NewGroups}; {both, none, NewGroups};
@ -445,7 +445,8 @@ delete_group(Host, Group) ->
end, end,
Mod:delete_group(Host, Group). Mod:delete_group(Host, Group).
get_group_opts(Host, Group) -> get_group_opts(Host1, Group1) ->
{Host, Group} = split_grouphost(Host1, Group1),
Mod = gen_mod:db_mod(Host, ?MODULE), Mod = gen_mod:db_mod(Host, ?MODULE),
case use_cache(Mod, Host) of case use_cache(Mod, Host) of
true -> true ->
@ -539,9 +540,9 @@ get_group_explicit_users(Host, Group) ->
Mod:get_group_explicit_users(Host, Group) Mod:get_group_explicit_users(Host, Group)
end. end.
get_group_name(Host1, Group1) -> get_group_label(Host1, Group1) ->
{Host, Group} = split_grouphost(Host1, Group1), {Host, Group} = split_grouphost(Host1, Group1),
get_group_opt(Host, Group, name, Group). get_group_opt(Host, Group, label, Group).
%% Get list of names of groups that have @all@/@online@/etc in the memberlist %% Get list of names of groups that have @all@/@online@/etc in the memberlist
get_special_users_groups(Host) -> get_special_users_groups(Host) ->
@ -746,10 +747,10 @@ remove_user_from_group(Host, US, Group) ->
push_members_to_user(LUser, LServer, Group, Host, push_members_to_user(LUser, LServer, Group, Host,
Subscription) -> Subscription) ->
GroupOpts = get_group_opts(LServer, Group), GroupOpts = get_group_opts(LServer, Group),
GroupName = proplists:get_value(name, GroupOpts, Group), GroupLabel = proplists:get_value(label, GroupOpts, Group), %++
Members = get_group_users(Host, Group), Members = get_group_users(Host, Group),
lists:foreach(fun ({U, S}) -> lists:foreach(fun ({U, S}) ->
push_roster_item(LUser, LServer, U, S, GroupName, push_roster_item(LUser, LServer, U, S, GroupLabel,
Subscription) Subscription)
end, end,
Members). Members).
@ -779,12 +780,12 @@ push_user_to_members(User, Server, Subscription) ->
Group), Group),
GroupOpts = proplists:get_value(Group, GroupsOpts, GroupOpts = proplists:get_value(Group, GroupsOpts,
[]), []),
GroupName = proplists:get_value(name, GroupOpts, GroupLabel = proplists:get_value(label, GroupOpts,
Group), Group),
lists:foreach(fun ({U, S}) -> lists:foreach(fun ({U, S}) ->
push_roster_item(U, S, LUser, push_roster_item(U, S, LUser,
LServer, LServer,
GroupName, GroupLabel,
Subscription) Subscription)
end, end,
get_group_users(LServer, Group, get_group_users(LServer, Group,
@ -793,20 +794,20 @@ push_user_to_members(User, Server, Subscription) ->
lists:usort(SpecialGroups ++ UserGroups)). lists:usort(SpecialGroups ++ UserGroups)).
push_user_to_displayed(LUser, LServer, Group, Host, Subscription, DisplayedToGroupsOpts) -> push_user_to_displayed(LUser, LServer, Group, Host, Subscription, DisplayedToGroupsOpts) ->
GroupName = get_group_opt(Host, Group, name, Group), GroupLabel = get_group_opt(Host, Group, label, Group), %++
[push_user_to_group(LUser, LServer, GroupD, Host, [push_user_to_group(LUser, LServer, GroupD, Host,
GroupName, Subscription) GroupLabel, Subscription)
|| GroupD <- DisplayedToGroupsOpts]. || GroupD <- DisplayedToGroupsOpts].
push_user_to_group(LUser, LServer, Group, Host, push_user_to_group(LUser, LServer, Group, Host,
GroupName, Subscription) -> GroupLabel, Subscription) ->
lists:foreach(fun ({U, S}) lists:foreach(fun ({U, S})
when (U == LUser) and (S == LServer) -> when (U == LUser) and (S == LServer) ->
ok; ok;
({U, S}) -> ({U, S}) ->
case lists:member(S, ejabberd_option:hosts()) of case lists:member(S, ejabberd_option:hosts()) of
true -> true ->
push_roster_item(U, S, LUser, LServer, GroupName, push_roster_item(U, S, LUser, LServer, GroupLabel,
Subscription); Subscription);
_ -> _ ->
ok ok
@ -831,12 +832,12 @@ push_item(User, Server, Item) ->
Item). Item).
push_roster_item(User, Server, ContactU, ContactS, push_roster_item(User, Server, ContactU, ContactS,
GroupName, Subscription) -> GroupLabel, Subscription) ->
Item = #roster{usj = Item = #roster{usj =
{User, Server, {ContactU, ContactS, <<"">>}}, {User, Server, {ContactU, ContactS, <<"">>}},
us = {User, Server}, jid = {ContactU, ContactS, <<"">>}, us = {User, Server}, jid = {ContactU, ContactS, <<"">>},
name = <<"">>, subscription = Subscription, ask = none, name = <<"">>, subscription = Subscription, ask = none,
groups = [GroupName]}, groups = [GroupLabel]},
push_item(User, Server, Item). push_item(User, Server, Item).
-spec c2s_self_presence({presence(), ejabberd_c2s:state()}) -spec c2s_self_presence({presence(), ejabberd_c2s:state()})
@ -893,6 +894,10 @@ list_shared_roster_groups(Host, Query, Lang) ->
SRGroups = list_groups(Host), SRGroups = list_groups(Host),
FGroups = (?XAE(<<"table">>, [], FGroups = (?XAE(<<"table">>, [],
[?XE(<<"tbody">>, [?XE(<<"tbody">>,
[?XE(<<"tr">>,
[?X(<<"td">>),
?XE(<<"td">>, [?CT(?T("Name:"))])
])]++
(lists:map(fun (Group) -> (lists:map(fun (Group) ->
?XE(<<"tr">>, ?XE(<<"tr">>,
[?XE(<<"td">>, [?XE(<<"td">>,
@ -909,12 +914,12 @@ list_shared_roster_groups(Host, Query, Lang) ->
[?X(<<"td">>), [?X(<<"td">>),
?XE(<<"td">>, ?XE(<<"td">>,
[?INPUT(<<"text">>, <<"namenew">>, [?INPUT(<<"text">>, <<"namenew">>,
<<"">>)]), <<"">>),
?XE(<<"td">>, ?C(<<" ">>),
[?INPUTT(<<"submit">>, <<"addnew">>, ?INPUTT(<<"submit">>, <<"addnew">>,
?T("Add New"))])])]))])), ?T("Add New"))])])]))])),
(?H1GL((translate:translate(Lang, ?T("Shared Roster Groups"))), (?H1GL((translate:translate(Lang, ?T("Shared Roster Groups"))),
<<"mod-shared-roster">>, <<"mod_shared_roster">>)) <<"modules/#mod-shared-roster">>, <<"mod_shared_roster">>))
++ ++
case Res of case Res of
ok -> [?XREST(?T("Submitted"))]; ok -> [?XREST(?T("Submitted"))];
@ -962,7 +967,7 @@ shared_roster_group(Host, Group, Query, Lang) ->
Res = shared_roster_group_parse_query(Host, Group, Res = shared_roster_group_parse_query(Host, Group,
Query), Query),
GroupOpts = get_group_opts(Host, Group), GroupOpts = get_group_opts(Host, Group),
Name = get_opt(GroupOpts, name, <<"">>), Label = get_opt(GroupOpts, label, <<"">>), %%++
Description = get_opt(GroupOpts, description, <<"">>), Description = get_opt(GroupOpts, description, <<"">>),
AllUsers = get_opt(GroupOpts, all_users, false), AllUsers = get_opt(GroupOpts, all_users, false),
OnlineUsers = get_opt(GroupOpts, online_users, false), OnlineUsers = get_opt(GroupOpts, online_users, false),
@ -986,41 +991,51 @@ shared_roster_group(Host, Group, Query, Lang) ->
[?XE(<<"tbody">>, [?XE(<<"tbody">>,
[?XE(<<"tr">>, [?XE(<<"tr">>,
[?XCT(<<"td">>, ?T("Name:")), [?XCT(<<"td">>, ?T("Name:")),
?XE(<<"td">>, [?C(Group)]),
?XE(<<"td">>, [?C(<<"">>)])]),
?XE(<<"tr">>,
[?XCT(<<"td">>, ?T("Label:")),
?XE(<<"td">>, ?XE(<<"td">>,
[?INPUT(<<"text">>, <<"name">>, Name)])]), [?INPUT(<<"text">>, <<"label">>, Label)]),
?XE(<<"td">>, [?C(<<"Name in the rosters where this group will be displayed">>)])]),
?XE(<<"tr">>, ?XE(<<"tr">>,
[?XCT(<<"td">>, ?T("Description:")), [?XCT(<<"td">>, ?T("Description:")),
?XE(<<"td">>, ?XE(<<"td">>,
[?TEXTAREA(<<"description">>, [?TEXTAREA(<<"description">>,
integer_to_binary(lists:max([3, integer_to_binary(lists:max([3,
DescNL])), DescNL])),
<<"20">>, Description)])]), <<"20">>, Description)]),
?XE(<<"td">>, [?C(<<"Only admins can see this">>)])
]),
?XE(<<"tr">>, ?XE(<<"tr">>,
[?XCT(<<"td">>, ?T("Members:")), [?XCT(<<"td">>, ?T("Members:")),
?XE(<<"td">>, ?XE(<<"td">>,
[?TEXTAREA(<<"members">>, [?TEXTAREA(<<"members">>,
integer_to_binary(lists:max([3, integer_to_binary(lists:max([3,
length(Members)+3])), length(Members)+3])),
<<"20">>, FMembers)])]), <<"20">>, FMembers)]),
?XE(<<"td">>, [?C(<<"JIDs, @all@, @online@">>)])
]),
?XE(<<"tr">>, ?XE(<<"tr">>,
[?XCT(<<"td">>, ?T("Displayed Groups:")), [?XCT(<<"td">>, ?T("Displayed:")),
?XE(<<"td">>, ?XE(<<"td">>,
[?TEXTAREA(<<"dispgroups">>, [?TEXTAREA(<<"dispgroups">>,
integer_to_binary(lists:max([3, length(FDisplayedGroups)])), integer_to_binary(lists:max([3, length(FDisplayedGroups)])),
<<"20">>, <<"20">>,
list_to_binary(FDisplayedGroups))])])])])), list_to_binary(FDisplayedGroups))]),
?XE(<<"td">>, [?C(<<"Groups that will be displayed to the members">>)])
])])])),
(?H1GL((translate:translate(Lang, ?T("Shared Roster Groups"))), (?H1GL((translate:translate(Lang, ?T("Shared Roster Groups"))),
<<"mod-shared-roster">>, <<"mod_shared_roster">>)) <<"modules/#mod-shared-roster">>, <<"mod_shared_roster">>))
++ ++
[?XC(<<"h2">>, <<(translate:translate(Lang, ?T("Group ")))/binary, Group/binary>>)] ++ [?XC(<<"h2">>, translate:translate(Lang, ?T("Group")))] ++
case Res of case Res of
ok -> [?XREST(?T("Submitted"))]; ok -> [?XREST(?T("Submitted"))];
{error_jids, NonAddedList1} -> {error_elements, NonAddedList1, NG1} ->
NonAddedList2 = [jid:encode({U,S,<<>>}) || {U,S} <- NonAddedList1], make_error_el(Lang,
NonAddedList3 = str:join(NonAddedList2, <<", ">>), ?T("Members not added (inexistent vhost!): "),
NonAddedText1 = translate:translate(Lang, ?T("Members not added (inexistent vhost): ")), [jid:encode({U,S,<<>>}) || {U,S} <- NonAddedList1])
NonAddedText2 = str:concat(NonAddedText1, NonAddedList3), ++ make_error_el(Lang, ?T("'Displayed groups' not added (they do not exist!): "), NG1);
[?XRES(NonAddedText2)];
error -> [?XREST(?T("Bad format"))]; error -> [?XREST(?T("Bad format"))];
nothing -> [] nothing -> []
end end
@ -1030,24 +1045,33 @@ shared_roster_group(Host, Group, Query, Lang) ->
[FGroup, ?BR, [FGroup, ?BR,
?INPUTT(<<"submit">>, <<"submit">>, ?T("Submit"))])]. ?INPUTT(<<"submit">>, <<"submit">>, ?T("Submit"))])].
make_error_el(_, _, []) ->
[];
make_error_el(Lang, Message, BinList) ->
NG2 = str:join(BinList, <<", ">>),
NG3 = translate:translate(Lang, Message),
NG4 = str:concat(NG3, NG2),
[?XRES(NG4)].
shared_roster_group_parse_query(Host, Group, Query) -> shared_roster_group_parse_query(Host, Group, Query) ->
case lists:keysearch(<<"submit">>, 1, Query) of case lists:keysearch(<<"submit">>, 1, Query) of
{value, _} -> {value, _} ->
{value, {_, Name}} = lists:keysearch(<<"name">>, 1, {value, {_, Label}} = lists:keysearch(<<"label">>, 1,
Query), Query), %++
{value, {_, Description}} = {value, {_, Description}} =
lists:keysearch(<<"description">>, 1, Query), lists:keysearch(<<"description">>, 1, Query),
{value, {_, SMembers}} = lists:keysearch(<<"members">>, {value, {_, SMembers}} = lists:keysearch(<<"members">>,
1, Query), 1, Query),
{value, {_, SDispGroups}} = {value, {_, SDispGroups}} =
lists:keysearch(<<"dispgroups">>, 1, Query), lists:keysearch(<<"dispgroups">>, 1, Query),
NameOpt = if Name == <<"">> -> []; LabelOpt = if Label == <<"">> -> [];
true -> [{name, Name}] true -> [{label, Label}] %++
end, end,
DescriptionOpt = if Description == <<"">> -> []; DescriptionOpt = if Description == <<"">> -> [];
true -> [{description, Description}] true -> [{description, Description}]
end, end,
DispGroups = str:tokens(SDispGroups, <<"\r\n">>), DispGroups1 = str:tokens(SDispGroups, <<"\r\n">>),
{DispGroups, WrongDispGroups} = filter_groups_existence(Host, DispGroups1),
DispGroupsOpt = if DispGroups == [] -> []; DispGroupsOpt = if DispGroups == [] -> [];
true -> [{displayed_groups, DispGroups}] true -> [{displayed_groups, DispGroups}]
end, end,
@ -1087,7 +1111,7 @@ shared_roster_group_parse_query(Host, Group, Query) ->
displayed_groups_update(OldMembers, RemovedDisplayedGroups, remove), displayed_groups_update(OldMembers, RemovedDisplayedGroups, remove),
displayed_groups_update(OldMembers, AddedDisplayedGroups, both), displayed_groups_update(OldMembers, AddedDisplayedGroups, both),
set_group_opts(Host, Group, set_group_opts(Host, Group,
NameOpt ++ LabelOpt ++
DispGroupsOpt ++ DispGroupsOpt ++
DescriptionOpt ++ DescriptionOpt ++
AllUsersOpt ++ OnlineUsersOpt), AllUsersOpt ++ OnlineUsersOpt),
@ -1108,9 +1132,9 @@ shared_roster_group_parse_query(Host, Group, Query) ->
Group) Group)
end, end,
AddedMembers), AddedMembers),
case NonAddedMembers of case (NonAddedMembers /= []) or (WrongDispGroups /= []) of
[] -> ok; true -> {error_elements, NonAddedMembers, WrongDispGroups};
_ -> {error_jids, NonAddedMembers} false -> ok
end end
end; end;
_ -> nothing _ -> nothing
@ -1131,6 +1155,11 @@ split_grouphost(Host, Group) ->
[_] -> {Host, Group} [_] -> {Host, Group}
end. end.
filter_groups_existence(Host, Groups) ->
lists:partition(
fun(Group) -> error /= get_group_opts(Host, Group) end,
Groups).
displayed_groups_update(Members, DisplayedGroups, Subscription) -> displayed_groups_update(Members, DisplayedGroups, Subscription) ->
lists:foreach( lists:foreach(
fun({U, S}) -> fun({U, S}) ->
@ -1139,8 +1168,10 @@ displayed_groups_update(Members, DisplayedGroups, Subscription) ->
opts_to_binary(Opts) -> opts_to_binary(Opts) ->
lists:map( lists:map(
fun({name, Name}) -> fun({label, Label}) ->
{name, iolist_to_binary(Name)}; {label, iolist_to_binary(Label)};
({name, Label}) -> % For SQL backwards compat with ejabberd 20.03 and older
{label, iolist_to_binary(Label)};
({description, Desc}) -> ({description, Desc}) ->
{description, iolist_to_binary(Desc)}; {description, iolist_to_binary(Desc)};
({displayed_groups, Gs}) -> ({displayed_groups, Gs}) ->

View File

@ -156,9 +156,19 @@ need_transform({sr_user, {U, S}, {G, H}})
when is_list(U) orelse is_list(S) orelse is_list(G) orelse is_list(H) -> when is_list(U) orelse is_list(S) orelse is_list(G) orelse is_list(H) ->
?INFO_MSG("Mnesia table 'sr_user' will be converted to binary", []), ?INFO_MSG("Mnesia table 'sr_user' will be converted to binary", []),
true; true;
need_transform({sr_group, {G, H}, [{name, _} | _]}) ->
?INFO_MSG("Mnesia table 'sr_group' will be converted from option Name to Label", []),
true;
need_transform(_) -> need_transform(_) ->
false. false.
transform(#sr_group{group_host = {G, H}, opts = Opts} = R)
when is_binary(G) ->
Opts2 = case proplists:get_value(name, Opts, false) of
false -> Opts;
Name -> [{label, Name} | proplists:delete(name, Opts)]
end,
R#sr_group{opts = Opts2};
transform(#sr_group{group_host = {G, H}, opts = Opts} = R) -> transform(#sr_group{group_host = {G, H}, opts = Opts} = R) ->
R#sr_group{group_host = {iolist_to_binary(G), iolist_to_binary(H)}, R#sr_group{group_host = {iolist_to_binary(G), iolist_to_binary(H)},
opts = mod_shared_roster:opts_to_binary(Opts)}; opts = mod_shared_roster:opts_to_binary(Opts)};