Implement Audit Event Recording for Person Entity

[OsaVS]

Issue: #17234

Files Changed:

• StaffController
• DoctorController
• ConsultantController
• PersonService

---

- PersonService -> defined **personToAuditMap()** method to create map with person data for audit recording - Controllers' `saveSelected()` method modified to initiate audit recording - **changesStaff()** method used to create before JSON when selecting a staff member from lists in `hr_staff_without_doctors_admin`, `doctors_including_consultants`, etc. - `hr_staff_without_doctors_admin` **Save** button modified because the previous version used wrong person reference when trying to add new staff

---

Details included:

• personId
• name
• nic
• title
• full name
• name with initials
• surname, lastname
• dob
• address
• phone, mobile, email, fax
• gender

entityType - Person
eventTrigger - createPerson & updatePerson
objectId - person.Id

After:

  {"lastName":"",
    "address":"Galle",
    "surName":"",
    "gender":"Male",
    "mobile":"0112000000",
    "nic":"0000000001",
    "fullName":"",
    "title":"Dr",
    "nameWithInitials":"",
    "phone":"0306666666"
    ,"dob":"1976-02-10",
    "name":"A D M Jayasekara"
    ,"personId":6,
    "fax":"0112456563",
  "email":"" }

Before:

  {"address":"Galle",
  "gender":"Male",
  "mobile":"0406666666",
  "title":"Dr",
  "phone":"0306666666",
  "name":"A D M Jayasekara",
  "personId":6,
  "fax":"0112456563"}

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 17234-implement-audit-event-recording-for-person-entity-update-operations-namedob-changes

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[buddhika75]

Overall Assessment

The PR adds audit logging for Person entity create/update operations across StaffController, DoctorController, and
ConsultantController. The approach is reasonable — capture a "before" snapshot when a staff member is selected, and
compare it with an "after" snapshot on save. However, there are several issues to address.

---

Bugs

1. StaffController.saveSelected() — redundant assignment on create path (line ~1300 in diff)
   initialPerson = new HashMap<>();
   initialPerson = editedPerson;
2. The first line creates a new HashMap that is immediately thrown away. This is dead code — should just be
   initialPerson = editedPerson;.
3. Comment copy-paste error — In all three controllers, the else branch (person edit) has the comment:
   // Person edited, log the creation event
4. Should say "update event", not "creation event".
5. System.out.println left in prepareAdd() (StaffController ~line 1196):
   System.out.println("prepareAdd: current.getPerson().getId() = " + current.getPerson().getId());
   System.out.println(current.getPerson() == null);
6. These are debug statements that should be removed before merge. The second line will also always print false since
   getId() was already called on the line before (it would have NPE'd if null).
7. DoctorController — initialPerson set to null after save (line ~367), while ConsultantController sets initialPerson
   = editedPerson. The behavior is inconsistent. If the user saves and then edits + saves again without reselecting from
   the list, the Doctor path will log null as "before" (appearing as a create), while Consultant will correctly log the
   previous state as "before".
8. ConsultantController — missing initialPerson reset for new consultant flow: When prepareAdd() or equivalent is
   called for a brand new consultant, initialPerson may still hold data from a previously selected person. The
   changeStaff() listener on the list selection only fires when selecting from the list, not when clicking "Add".

Design Issues

6. PersonService is an EJB (@stateless) but has no EJB functionality — personToAuditMap() is a pure utility method
   that doesn't use any injected resources, JPA, or transactions. It should either be:
   - A static utility method (simplest), or
   - Part of the existing AuditService (since it's audit-specific logic)

Making it a stateless EJB adds unnecessary overhead (EJB proxy, pooling).
7. personToAuditMap mutates a passed-in Map — A cleaner API would be to return a new Map:
public static Map<String, Object> personToAuditMap(Person person) {
Map<String, Object> map = new HashMap<>();
// ... populate ...
return map;
}
7. This avoids the caller needing to create and pass an empty map every time, and removes the possibility of passing
null map.
8. gender field mapped from person.getSex() — If getSex() returns an enum, map.put("gender", person.getSex()) will
serialize the enum's toString(). Verify the output is what's intended (the PR description shows "gender":"Male" which
looks okay, but confirm the Title field too — person.getTitle() likely returns an enum).
9. Missing newline at end of PersonService.java — The diff shows \ No newline at end of file.

XHTML Changes

10. hr_staff_without_doctors_admin.xhtml Save button changed from ajax="false" to AJAX — This is actually a good
    improvement, but it's a behavioral change. The old button did a full page postback; the new one uses AJAX with
    process="btnSave gpDetail". Make sure this was tested — if any fields outside gpDetail need to be processed, they'll
    be missed.
11. Stray > removed from admin_doctor_consultant.xhtml — Good cleanup, but confirms the existing code had a rendering
    bug (dangling >). Worth noting this is a separate fix.
12. p:focus changed from context to for in hr_staff_without_doctors_admin.xhtml — context and for behave differently
    in PrimeFaces. context focuses the first focusable element within the container; for focuses a specific component.
    Verify this change doesn't break the focus behavior.

Missing Pieces

13. No audit on saveCurrentStaff() create path — In StaffController.saveCurrentStaff(), after creating a new person,
    initialPerson is set to a new empty HashMap but never populated with the just-created data, so subsequent edits will
    compare against an empty map rather than the created state.
14. No import java.util.HashMap or import java.util.Map** verified** — The diff adds HashMap and Map usage. These
    imports exist already in StaffController, but verify they're present in ConsultantController and DoctorController.

---

Summary

The core idea is sound, but the PR needs:

• Remove debug System.out.println statements
• Fix copy-paste comment errors ("creation" → "update")
• Fix the dead code initialPerson = new HashMap<>(); initialPerson = editedPerson;
• Make initialPerson handling consistent across all three controllers
• Consider moving personToAuditMap to a static utility or into AuditService instead of a separate EJB
• Add missing newline at EOF in PersonService.java
• Test the AJAX save button change in hr_staff_without_doctors_admin.xhtml