fix: handle null employee role in leave admin check#1082
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a null pointer exception bug by adding null safety checks when accessing the leaveRole field from EmployeeRole objects. The fix prevents potential crashes when an employee's leave role is not set.
Changes:
- Added null checks before calling
getLeaveRole().equals()in three locations across two service implementation files - Changed from direct equality check to a two-step check: first verifying the role is not null, then performing the equality comparison
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| LeaveServiceImpl.java | Added null check for leaveRole in getResourceAvailabilityCalendar method |
| LeaveAnalyticsServiceImpl.java | Added null checks for leaveRole in getEmployeesOnLeave and getOrganizationalAbsenceRate methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @param requestedAnalytics requested analytics | ||
| */ | ||
| private void isEmployeeUnderCurrentUserSupervision(User currentUser, Employee employee, String requestedAnalytics) { | ||
| String currentUserRole = currentUser.getEmployee().getEmployeeRole().getLeaveRole().toString(); |
There was a problem hiding this comment.
The getLeaveRole() method is called without null safety check. This can lead to a NullPointerException if leaveRole is null. Consider adding a null check similar to the pattern used in the fixed locations.
There was a problem hiding this comment.
this null scenario comes only for guest user apis
| * @param requestedAnalytics requested analytics | ||
| */ | ||
| private void isTeamUnderCurrentUserSupervision(User currentUser, Team team, String requestedAnalytics) { | ||
| String currentUserRole = currentUser.getEmployee().getEmployeeRole().getLeaveRole().toString(); |
There was a problem hiding this comment.
The getLeaveRole() method is called without null safety check. This can lead to a NullPointerException if leaveRole is null. Consider adding a null check similar to the pattern used in the fixed locations.
There was a problem hiding this comment.
this null scenario comes only for guest user apis
| throw new EntityNotFoundException(PeopleMessageConstant.PEOPLE_ERROR_EMPLOYEE_NOT_FOUND); | ||
| } | ||
|
|
||
| Role leaveRole = currentUser.getEmployee().getEmployeeRole().getLeaveRole(); |
There was a problem hiding this comment.
The getLeaveRole() method is called without null safety check. This can lead to a NullPointerException if leaveRole is null. Consider adding a null check or using Optional.ofNullable() pattern as done in AuthorityServiceImpl.
There was a problem hiding this comment.
this null scenario comes only for guest user apis
|
|
||
| private boolean isUserSuperAdminOrLeaveAdmin(User user) { | ||
| EmployeeRole role = user.getEmployee().getEmployeeRole(); | ||
| return role.getIsSuperAdmin() || Role.LEAVE_ADMIN.equals(role.getLeaveRole()); |
There was a problem hiding this comment.
The getLeaveRole() method is called without null safety check. This can lead to a NullPointerException if leaveRole is null. Consider adding a null check similar to the pattern used elsewhere in this file at line 730.
There was a problem hiding this comment.
this null scenario comes only for guest user apis
2 participants
PR checklist
TaskId: (https://github.com/SkappHQ/skapp/issues/[id])
Summary
How to test
Project Checklist
npm run formatnpm run check-lintOther
PR Checklist
ready-for-code-review)Additional Information