Skip to content

chore: convert user to userdetails refactoring #21441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

netroms
Copy link
Contributor

@netroms netroms commented Jul 19, 2025

Summary:

Refactoring the use of User to UserDetails, this PR focuses to eliminate the use of UserDetails.fromUser() method, which usually means UserDetails can be used of properly refactored from the top, typically the controller.

netroms added 15 commits July 20, 2025 00:58
…om:dhis2/dhis2-core into chore/use-userdetails-in-bundles

# Conflicts:
#	dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/user/UserServiceTest.java
…om:dhis2/dhis2-core into chore/use-userdetails-in-bundles
@netroms netroms changed the title Chore/more usercredentials refactor chore: convert user to userdetails refactoring Jul 21, 2025
@netroms netroms marked this pull request as ready for review July 25, 2025 06:12
@@ -550,6 +550,9 @@ Map<String, Optional<Locale>> findNotifiableUsersWithPasswordLastUpdatedBetween(
*/
UserDetails createUserDetails(String userUid) throws NotFoundException;

@CheckForNull
UserDetails createUserDetailsSafe(@Nonnull String userUid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to use the UID type, especially for new methods, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, would be great to do a big refactor just for that. Feels right to do that after UserDetails are mostly done.

Copy link

@@ -495,6 +502,8 @@ public WebMessage replicateUser(@PathVariable String uid, HttpServletRequest req
return conflict(result.getErrorMessage());
}

Session session = entityManager.unwrap(Session.class);
Copy link
Contributor

@david-mackessy david-mackessy Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the entity manager and session at the controller layer feels a bit wrong. It kind of hints that this should be done at the service level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, totally. We should refactor this one.

Copy link
Contributor

@david-mackessy david-mackessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a couple of small comments left

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants