Refactor Bean Scopes: User, Role, Authority, Client, LdapGroup, LdapServer forms #6770

thomaslow · 2025-11-25T16:33:43Z

This PR refactors all forms on the "users" page from @SessionScoped to @ViewScoped. In addition, a few minor cleanup improvements are made.

Related to: #6456

Updated forms:

UserForm
RoleForm
AuthorityForm
ClientForm
LdapGroupForm
LdapServerForm

Changes:

@SessionScoped to @ViewScoped
Splitting form classes into list view and edit view
Splitting user edit form into multiple classes, one for each tab view
Column selection dialog only shows columns of the currently active tab
Various view parameters are encoded as URL query parameters and updated via Javascript
- tabIndex - the currently active tab
- sortField - the field that is used for sorting
- sortOrder - the order of sorting
- firstRow - the first data row shown in the data table (corresponds to a page)
- filter - the active list filter
- referrerListOptions - the list options sortField, sortOrder, firstRow and filter encoded in a single parameter such they can be restored when navigating back to the list after editing a single entity
Various cleanups and refactoring, e.g.
- moving database retrieval calls from getter to bean init methods
- removing unused methods
- adding Javadoc
Fixes Newly saved filters are sometimes missing in the list of saved filters #6826

Due to the replacement of UserForm.java, RoleForm.java, ..., merging with the following PR needs to be done more carefully such that any improvements are not lost:

…ity and role form.

…erver.

… improvements.

…embering row index of referred list view.

…n be restored after navigating back from an edit view.

Kitodo/src/main/java/org/kitodo/production/forms/AuthorityListView.java

Kitodo/src/main/java/org/kitodo/production/forms/ClientListView.java

Kitodo/src/main/java/org/kitodo/production/forms/LdapGroupListView.java

Kitodo/src/main/java/org/kitodo/production/forms/LdapServerListView.java

Kitodo/src/main/java/org/kitodo/production/forms/RoleListView.java

Kitodo/src/main/java/org/kitodo/production/forms/user/UserListView.java

… edit links.

…sts between reloads and when editing a user.

thomaslow · 2025-12-17T14:42:11Z

@solth After refactoring the user list view from @SessionScoped to @ViewScoped I noticed that various state information is missing because of that, in particular:

the current active tab
the current page in the list
sorting criteria for a list
any search filter

I moved these state information into the URL as query parameters, which seems to work fine. I even keep these state information available by forwarding them to the respective edit views and reloading them after navigating back to the source list view (via another query parameter referrerListOptions similar to keepPagination).

In general, I think this approach works fine and now would allow to, e.g., create bookmarks with existing sorting and filter criteria.

However, I see a problem for the task and process list, and their respective filter options. When navigating to the task or process list, the current page and sorting options are reset to their defaults, but the filter option is loaded from the session scope. You can navigate anywhere in Kitodo.Production and still have the same filter option active when returning to the task or process list.

I am not entirely sure, but I think this is most certainly desired behavior.

Anyway, this long-term information about the current active filter in the task and process list cannot be modeled as query parameters (alone). We could try cookies? What do you think?

BartChris · 2025-12-17T18:51:41Z

Anyway, this long-term information about the current active filter in the task and process list cannot be modeled as query parameters (alone). We could try cookies? What do you think?

Would it be an option to store only the filter-related information in a small, dedicated @SessionScoped bean and inject it into the @ViewScoped process and task list views?

thomaslow · 2025-12-18T09:30:36Z

Would it be an option to store only the filter-related information in a small, dedicated @SessionScoped bean and inject it into the @ViewScoped process and task list views?

Yes, of course this would be possible. Other options would be, storing it:

in the database in a field lastProcessListFilter and lastTaskListFilter
in a @SessionScoped bean
in a Cookie with some lifetime (session, one day, one week, forever), or the browser's LocalStorage (without a lifetime)

In any case I would suggest storing all relevant list view options (including the filter) in URL query parameters as well (overwriting any long-term preferences), such that you can easily save various filters via browser bookmarks.

…ons in authority edit view and role list view.

solth

@thomaslow thanks a lot for this pull request. The implementation looks very solid and it's great that we will finally have properly scoped beans in Kitodo! I encountered just a few minor issues (see below). And please do not forget to rebase against/merge with the main branch (if you want to wait for #6803 to be merged that's fine by me as well)

solth · 2026-01-09T10:01:23Z

Kitodo/src/main/java/org/kitodo/production/forms/ClientListView.java

+
+    private static final Logger logger = LogManager.getLogger(ClientListView.class);
+
+    public static final String VIEW_PATH = MessageFormat.format(REDIRECT_PATH, "users") + "&tabIndex=2";


I wonder whether having static tab index values is a good idea here. In case the user does not have the permission to see the user list, for example, the index for all following tabs should be decreased accordingly, shouldn't it (same applies to the other views as well, of course)?

Good point. I was thinking about changing the view parameter to something like tab=users, tab=roles (because it "looks" better), but noticed that the view parameter tabIndex was used in other parts of Kitodo.Production, so I simply reused that instead. I didn't think about users with restricted permissions that will have a different tab order. I'll change it to tab=<view_name>.

solth · 2026-01-09T10:27:51Z

Kitodo/src/main/java/org/kitodo/production/forms/LdapServerListView.java

+
+    public static final String VIEW_PATH = MessageFormat.format(REDIRECT_PATH, "users") + "&tabIndex=5";
+
+    private static final Logger logger = LogManager.getLogger(LdapServerEditView.class);    


Suggested change

private static final Logger logger = LogManager.getLogger(LdapServerEditView.class);

private static final Logger logger = LogManager.getLogger(LdapServerListView.class);

solth · 2026-01-09T11:00:41Z

Kitodo/src/main/java/org/kitodo/production/forms/RoleListView.java

+     * @param role the role to be removed
+     * @return true if role was removed
+     */
+    public static boolean deleteRole(Role role) {


Maybe we can move this method to the RoleService, as it contains quite a bit of business logic and isn't directly called from the template? If it stays in this class, it could probably be made private, since it is only called from within this class itself, isn't it?

Yes, you are right. The view-independent part should be moved to the RoleService. I'll update that.

solth · 2026-01-09T12:15:52Z

Kitodo/src/main/java/org/kitodo/production/forms/user/UserEditViewClientsTab.java

+     * @return true if user clients data was saved
+     */
+    public boolean save() {
+        return true;


I am not quite sure why we need this method. It only ever returns true. Have you added this because some tabs have actual conditions that are checked before the user can be saved?

Yes, I was trying to establish a method "pattern" of common save and load methods for different edit view tabs. Maybe I should refactor this into a base class BaseEditTabView.

solth · 2026-01-09T12:33:56Z

Kitodo/src/main/java/org/kitodo/production/forms/user/UserEditViewDetailsTab.java

+            return Collections.emptySet();
+        }
+        KitodoPassword validPassword = new KitodoPassword(passwordToEncrypt);
+        ValidatorFactory factory = Validation.buildDefaultValidatorFactory();


My IDE tells me that ValidatorFactor is AutoClosable and should be used with try-with-resources. Would something like

try (ValidatorFactory factory = Validation.buildDefaultValidatorFactory()) { Validator validator = factory.getValidator(); return validator.validate(validPassword); }

work?

solth · 2026-01-09T12:38:02Z

Kitodo/src/main/java/org/kitodo/production/forms/user/UserEditViewDetailsTab.java

+     * Check whether the old password matches the password stored in the database.
+     * 
+     * @return true if the old password does not match the password stored in the database
+     */
+    private boolean isOldPasswordInvalid() {
+        if (!ServiceManager.getSecurityAccessService().hasAuthorityToEditUser()) {
+            return !Objects.equals(this.oldPassword, passwordEncoder.decrypt(this.userObject.getPassword()));
+        }
+        return false;


Despite being just a few lines, I find this whole method confusing because of the multiple negations (e.g. "invalid" instead of "valid", if(!ServicManager...), return !Objects.equals etc.). I know you just copied this method from the UserForm and didn't author it, but perhaps now would be a good opportunity to simplify the code?

solth · 2026-01-09T13:14:17Z

Kitodo/src/main/webapp/WEB-INF/resources/js/queryParams.js

+/**
+ * Update the current browser URL to include the selected query parameter and value.
+ * 
+ * @param str key the query parmaeter name


Suggested change

* @param str key the query parmaeter name

* @param str key the query parameter name

solth · 2026-01-09T13:40:56Z

Kitodo/src/main/webapp/WEB-INF/templates/includes/users/userList.xhtml

                        id="yesButton"
                        value="#{msgs.yes}"
-                        action="#{UserForm.resetTasksAndDeleteUser}"
+                        action="#{UserListView.resetTasksAndDeleteUser(confirmResetTasksDialogUser)}"


Suggested change

action="#{UserListView.resetTasksAndDeleteUser(confirmResetTasksDialogUser)}"

action="#{UserListView.resetTasksAndDeleteUser(UserListView.confirmResetTasksDialogUser)}"

I think this needs to reference the bean that contains user object, doesn't it? Without it I get a NullPointerException when trying to delete a user with assigned tasks:

thomaslow · 2026-01-09T14:28:52Z

@solth Thank you for your feedback. I'll update this branch next week.

Refactor bean scopes of user, client, ldap server, ldap group, author…

f12ff8e

…ity and role form.

thomaslow force-pushed the refactor-bean-scopes-user-form branch from 1e46493 to f12ff8e Compare December 9, 2025 16:05

thomaslow added 12 commits December 11, 2025 17:20

Merge branch 'main' into refactor-bean-scopes-user-form

2b44a7f

Replace constructor with init method in AuthorityListView.

a11c17f

Remove LdapGroupEditView constructor.

99b5f3d

Remove constructor in LdapServerListView.

c45878e

Improve javadoc.

8867c42

Navigate to respective list view tab after editing an ldap group or s…

5c42a97

…erver.

Switch to appropriate tab via tabIndex query parameter.

4a92d58

Use Javascript to update tabIndex query parameter when switching tabs.

c2f5555

Update activeTabIndex to use UsersTabView.

8e3f152

Fix codacy errors.

55bbe44

Merge branch 'main' into refactor-bean-scopes-user-form

262469b

Update user edit view metadata tab after merge with pagination editor…

a5000d2

… improvements.

thomaslow force-pushed the refactor-bean-scopes-user-form branch from 1f63b41 to a5000d2 Compare December 16, 2025 11:56

thomaslow added 2 commits December 16, 2025 17:01

Move session scoped keepPagination logic into URL query parameter rem…

7f7637a

…embering row index of referred list view.

Encode list view sort state as URL query parameters such that they ca…

256a4ab

…n be restored after navigating back from an edit view.

github-advanced-security bot found potential problems Dec 16, 2025

View reviewed changes

thomaslow added 5 commits December 16, 2025 23:15

Fix filter menu in user list view.

2424379

Add missing @OverRide annotations.

ae438fe

Fix selenium tests due to addtional list view url query paramaters in…

8f76917

… edit links.

Encode current user list filter as query parameter such that is persi…

1d8eff7

…sts between reloads and when editing a user.

Fix newly saved filters are not listed.

9c28cf9

Add more javadoc.

f241a34

thomaslow marked this pull request as ready for review December 17, 2025 17:17

thomaslow added 2 commits January 2, 2026 10:36

Merge branch 'main' into refactor-bean-scopes-user-form

d0f1cbf

Replace referrerFirstRow view parameter with updated referrerListOpti…

a713588

…ons in authority edit view and role list view.

solth mentioned this pull request Jan 5, 2026

Optimize user list #6803

Open

Merge branch 'main' into refactor-bean-scopes-user-form

d861371

thomaslow mentioned this pull request Jan 7, 2026

Refactor Bean Scopes: Project, Template, Workflow, Docket, Ruleset forms #6847

Draft

solth requested review from BartChris and solth January 7, 2026 17:26

solth requested changes Jan 9, 2026

View reviewed changes


		private static final Logger logger = LogManager.getLogger(ClientListView.class);

		public static final String VIEW_PATH = MessageFormat.format(REDIRECT_PATH, "users") + "&tabIndex=2";

	* @param str key the query parmaeter name
	* @param str key the query parameter name

	action="#{UserListView.resetTasksAndDeleteUser(confirmResetTasksDialogUser)}"
	action="#{UserListView.resetTasksAndDeleteUser(UserListView.confirmResetTasksDialogUser)}"

Refactor Bean Scopes: User, Role, Authority, Client, LdapGroup, LdapServer forms #6770

Are you sure you want to change the base?

Refactor Bean Scopes: User, Role, Authority, Client, LdapGroup, LdapServer forms #6770

Uh oh!

Conversation

thomaslow commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomaslow commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BartChris commented Dec 17, 2025

Uh oh!

thomaslow commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

solth Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomaslow commented Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomaslow commented Nov 25, 2025 •

edited

Loading

thomaslow commented Dec 17, 2025 •

edited

Loading

thomaslow commented Dec 18, 2025 •

edited

Loading

solth Jan 9, 2026 •

edited

Loading