Skip to content

Mod invites web admin#4563

Open
sstrigler wants to merge 1 commit intoprocessone:masterfrom
sstrigler:mod_invites-web-admin
Open

Mod invites web admin#4563
sstrigler wants to merge 1 commit intoprocessone:masterfrom
sstrigler:mod_invites-web-admin

Conversation

@sstrigler
Copy link
Copy Markdown
Contributor

Adds web-admin pane for mod_invites. Adds a new type of table 'action_table' that lets you select multiple rows and execute a command (action_button) on it. Also allows to filter results.

@sstrigler sstrigler force-pushed the mod_invites-web-admin branch from 8b8e3fe to 8a29a4f Compare April 13, 2026 18:36
@sstrigler sstrigler marked this pull request as ready for review April 13, 2026 18:36
@sstrigler sstrigler marked this pull request as draft April 13, 2026 18:52
@sstrigler sstrigler force-pushed the mod_invites-web-admin branch 2 times, most recently from 85d1117 to 93ad99c Compare April 14, 2026 10:16
@sstrigler sstrigler marked this pull request as ready for review April 14, 2026 10:22
@badlop
Copy link
Copy Markdown
Member

badlop commented Apr 17, 2026

It looks great!

I just have some usability remarks:

A) I propose to highlight somehow the filtering information and button, because that filtering substantially modifies what the table is actually showing.

Example code for that idea:

Details
From 6a1716dddea93f834472ac3df69d0ccebe3382b7 Mon Sep 17 00:00:00 2001
From: Badlop <badlop@process-one.net>
Date: Tue, 14 Apr 2026 13:27:24 +0200
Subject: [PATCH 1/3] Add some style to the filters display

---
 priv/css/admin.css         |  6 ++++++
 src/ejabberd_web_admin.erl | 12 +++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/priv/css/admin.css b/priv/css/admin.css
index 12fa97e22..fcbfd4e98 100644
--- a/priv/css/admin.css
+++ b/priv/css/admin.css
@@ -292,6 +292,12 @@ select {
   margin-right: 1em;
   background: #FFE3C9;
 }
+.filters {
+  border-style: dashed;
+  border-color: #FFE3C9;
+  padding: 0 1em;
+  background: #FFF4E9;
+}
 *.alignright {
   text-align: right;
 }
diff --git a/src/ejabberd_web_admin.erl b/src/ejabberd_web_admin.erl
index 9e88faa0f..7c073c031 100644
--- a/src/ejabberd_web_admin.erl
+++ b/src/ejabberd_web_admin.erl
@@ -2286,13 +2286,19 @@ make_command_result(Value,
                     end,
                 case to_filters_bin(to_filter_query(get_filters_from_query(Query, get_result_fields(ResultFormatApi))), <<>>) of
                     <<>> ->
-                        [?C(<<ResNameBin/binary, ": ">>), ResultValueEl];
+                        [?XC(<<"h4">>, <<ResNameBin/binary, ": ">>), ResultValueEl];
                     FiltersBin ->
                         ResNameEls =
                             ?XAE(<<"form">>,
                                  [{<<"class">>, <<"reset-filter-form">>}],
-                                 lists:flatten([?C(<<ResNameBin/binary, ": ">>),
-                                                ?P, ?CT(<<"Filters:">>), ?C(<<" ">>), ?C(FiltersBin), ?C(<<" ">>), ?P, ?INPUTT(<<"submit">>, <<"">>, <<"Reset Filters">>)])),
+                                 lists:flatten([?XAE(<<"blockquote">>,
+                                                     [{<<"class">>, <<"filters">>}],
+                                                     [?XCT(<<"h4">>, <<"Filters:">>),
+                                                      ?C(FiltersBin),
+                                                      ?P,
+                                                      ?INPUTT(<<"submit">>, <<"">>, <<"Reset Filters">>)]),
+                                                ?XC(<<"h4">>, <<ResNameBin/binary, ": ">>)
+                                               ])),
                         [ResNameEls, ResultValueEl]
                 end;
             false ->
-- 
2.50.1

B) Checkbox in the header

Update: Nevermind, the checkbox works great after clearing the browser cache :)


C) Doubts about the table getting automatically filtered when clicking a button

usually a button performs an action, and shows some result message

In http://localhost:5280/admin/purge/
the "Clear Cache" button shows result message: ok

In http://localhost:5280/admin/server/localhost/users/
the "Register" button shows result message: User user1@localhost successfully registered
and the new account is shown in the list of registered users.

Clicking the buttons next to the table in http://localhost:5280/admin/server/localhost/invites/
have three consequences:

  1. performs the corresponding action :)
  2. does not show any result message !!
  3. instead, it enables a filter in the table !!

regarding 2, as the action is performed to a list of elements, it would seem that the response would be a list of results, for example: [ok, ok, {error, xxxx}, ok]

regarding 3, the current behavior is surprising for a regular user:

Let's see how a user experiences the table: a table has 5 elements, user select two elements, clicks a button that says "Delete", and then it shows a table with zero elements!! A button has appeared, user clicks on it, "Reset Filters"... ok, now finally the table shows correctly the 3 remaining elements.

I didn't read the source code for the feature, but from its behavior it seems that filtering the table is mandatory to perform the action to the selected elements.

The question is: is it mandatory that the final table shown to the user has the filter still applied?

@sstrigler sstrigler force-pushed the mod_invites-web-admin branch from 93ad99c to 99d7f16 Compare April 19, 2026 06:23
@sstrigler
Copy link
Copy Markdown
Contributor Author

About A) this looks very nice! I’ve change blockquote to div though and instead applied some margin. blockquote carries semantics, it’s for when you actually quite someone, should not be abused for formatting reasons.

@sstrigler
Copy link
Copy Markdown
Contributor Author

As for C)

The way things are implemented (to remain compatible) currently the result of execute_commands is just the result of the last query. Also execution is stopped the moment on query returns an error. That’s so things don’t get too complicated and the PR remains within reasonable scope.

Since the way this is used right now is commands that only return ok, I saw no value in implementing anything that goes beyond that because what would it tell a user if we showed ok as a result. Or even more complicated, as you suggest, a list of ok’s like [ok,ok,ok,ok], possible a hundred times.

In the case of the expires button, to me it made a lot of sense to show the filtered result. I see myself how an empty result after pressing delete might be confusing though.

But in any case there is a feedback that something happened, I don’t see if we’d show an ok instead would make anything any better or different.

To not apply the filter is really difficult because the command arguments as well as the filters are both extracted from what’s passed into the top-level function as QS. The ejabberd_http framework I guess is what merges query parameters as wells as form parameters into the same datastructure.

To take things apart we’d have to know if a command has been executed after a manual (post?) request and then remove the command arguments from QS.

I can look into that, but within a reasonable cost-benefit calculation it didn’t quite vibe with me.

@sstrigler sstrigler force-pushed the mod_invites-web-admin branch from 99d7f16 to 1cc69be Compare April 19, 2026 07:04
@sstrigler
Copy link
Copy Markdown
Contributor Author

I asked people to try out this new feature to get some more opinions on this but unfortunately no one so far took the effort.

I could imagine proceeding like that: we merge as is and collect feedback and then think about potential solutions. The way it is now admittedly isn’t perfect but functional.

@sstrigler sstrigler force-pushed the mod_invites-web-admin branch from 1cc69be to a644c60 Compare April 20, 2026 09:36
@badlop badlop added the Component:Invites Invite-based account registration, implemented in mod_invites label May 8, 2026
@badlop badlop added this to the ejabberd 26.xx milestone May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component:Invites Invite-based account registration, implemented in mod_invites

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants