Skip to content

TRUNK-6298: Validate required fields (person, username) during user creation#6132

Open
RohithVangalla1 wants to merge 2 commits into
openmrs:masterfrom
RohithVangalla1:fix/validate-required-user-fields-on-create
Open

TRUNK-6298: Validate required fields (person, username) during user creation#6132
RohithVangalla1 wants to merge 2 commits into
openmrs:masterfrom
RohithVangalla1:fix/validate-required-user-fields-on-create

Conversation

@RohithVangalla1

@RohithVangalla1 RohithVangalla1 commented May 27, 2026

Copy link
Copy Markdown

Description of what I changed

Adds missing required field validation to UserServiceImpl.createUser() as indicated by the existing TODO comment.

The createUser method had a TODO: "Check required fields for user!!" Without these checks, a User could be created without an associated Person or without a username/systemId, leading to NullPointerExceptions or data integrity issues in downstream operations (e.g., displaying user names, login attempts).

Added validation before password validation and persistence:

  • Person required: user.getPerson() == null → throws APIException with key User.creating.person.required
  • Username/SystemId required: Both blank → throws APIException with key User.creating.username.required

Both follow the same pattern as the existing password validation (User.creating.password.required).

Message keys added to messages.properties:

  • User.creating.person.required=A person must be associated with the user when creating a user
  • User.creating.username.required=A username or system id is required when creating a user

Addresses the TODO in UserServiceImpl.java line 123: "Check required fields for user!!"

Issue I worked on

No existing JIRA issue — this addresses a long-standing TODO in the codebase. Happy to create a ticket if needed.

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.
  • I have added tests to cover my changes.
  • I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

The createUser method had a TODO to check required fields before
persisting a new User. Without these checks, a User could be created
without an associated Person or without a username/systemId, leading
to NullPointerExceptions or data integrity issues downstream.

This change adds validation that:
- User must have an associated Person (required for name, demographics)
- User must have either a username or systemId (required for login)

Both checks throw APIException with internationalized message keys,
consistent with the existing password validation pattern.

Added message keys:
- User.creating.person.required
- User.creating.username.required

Addresses the TODO:
  'Check required fields for user!!'
@RohithVangalla1 RohithVangalla1 changed the title Validate required fields (person, username) during user creation TRUNK-6298: Validate required fields (person, username) during user creation May 27, 2026

@jwnasambu jwnasambu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this @RohithVangalla1 Would you mind creating a ticket in the JIRA issue tracker and attach on this PR please?

@sonarqubecloud

Copy link
Copy Markdown

@RohithVangalla1

Copy link
Copy Markdown
Author

HI @jwnasambu , Could you please guide me where can I find that.

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.

2 participants