fix: return correct HTTP status codes and remove patient data logging in GeneralOPDController

[Aarti-panchal01]

📋 Description

Proof of concept fix for the HTTP status code and patient data logging issues identified in PSMRI/AMRIT#153.

Problem 1: Always HTTP 200
Every controller method returned String instead of ResponseEntity, meaning the HTTP status was always 200 — even on failures. The error details were buried in the JSON body, forcing clients to parse the body to detect failures instead of using standard HTTP semantics. The fix already existed — OutputResponse.toStringWithHttpStatus() was implemented but never used. This PR uses it.

Problem 2: Patient data in logs
Every controller logged the full raw requestObj at INFO level. These request bodies contain patient vitals, medical history, examination details and prescriptions. INFO logs are always-on in production — meaning sensitive patient data was being stored in plain text logs. Replaced with a generic log message containing no patient data.

Fixes: PSMRI/AMRIT#153

---

✅ Type of Change

• 🐞 Bug fix (non-breaking change which resolves an issue)
• 🛠 Refactor (change that is neither a fix nor a new feature)

---

ℹ️ Additional Information

Scope: One method in GeneralOPDController as proof of concept.

Changes made:

• Return type: String → ResponseEntity<String>
• response.toString() → response.toStringWithHttpStatus()
• Raw requestObj logging removed, replaced with generic message

Next steps if approved:

• Add @ControllerAdvice global exception handler
• Extend to remaining 20+ controllers
• Fix BAD_REQUEST mapped to 404 instead of 400 in OutputResponse.java

Summary by CodeRabbit

• Bug Fixes
  • Fixed nurse OPD data endpoint to properly return appropriate HTTP status codes for requests, ensuring accurate API response status handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

The saveBenGenOPDNurseData endpoint in GeneralOPDController is updated to return ResponseEntity<String> instead of plain String. The return statement now uses response.toStringWithHttpStatus() to include HTTP status information. Request logging is simplified to a generic message instead of echoing the request payload.

Changes

Nurse OPD Save Endpoint

Layer / File(s)	Summary
Nurse save endpoint return type and HTTP response handling src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java	The ResponseEntity import is added, the saveBenGenOPDNurseData method signature is updated to return ResponseEntity<String>, request logging is simplified to "Request received", and the return statement is updated to use response.toStringWithHttpStatus() for HTTP-aware serialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• [HWC-API] API always returns HTTP 200 even on failure + patient data getting logged at INFO level AMRIT#153: Directly addresses the same nurse save endpoint update—changing the return type to ResponseEntity, using OutputResponse.toStringWithHttpStatus(), and reducing sensitive request-body logging.

Poem

🐰 A nurse's response finds its proper place,
ResponseEntity wraps the data with grace,
HTTP status codes now shine so bright,
Logging less, security's tight,
One endpoint refined, requests treated right.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the two main changes in the PR: returning correct HTTP status codes (via ResponseEntity and toStringWithHttpStatus) and removing patient data logging (replacing raw requestObj logging with generic messages).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java (1)

84-109: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Move JSON parsing inside the try-catch block.

requestObj is parsed at lines 91–92 before the try block (which starts at line 94), so malformed JSON or non-object payloads throw JsonSyntaxException or IllegalStateException before reaching the error handler. These exceptions propagate uncaught and return a default 500 error instead of the structured error response. Wrap the parse and getAsJsonObject() calls inside the try-catch so exceptions are caught and handled via response.setError(), then properly serialized by toStringWithHttpStatus().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java`
around lines 84 - 109, The JSON parsing (JsonParser/JsonElement and jsnOBJ =
jsnElmnt.getAsJsonObject()) must be moved inside the existing try block in
saveBenGenOPDNurseData so JsonSyntaxException/IllegalStateException are caught
and turned into a structured error via response.setError(...) before returning
response.toStringWithHttpStatus(); ensure jsnOBJ is declared outside the try so
it can still be passed to generalOPDServiceImpl.deleteVisitDetails(jsnOBJ) in
the catch, and do not let parsing occur prior to the try that would bypass the
error handler.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java`:
- Around line 84-109: The JSON parsing (JsonParser/JsonElement and jsnOBJ =
jsnElmnt.getAsJsonObject()) must be moved inside the existing try block in
saveBenGenOPDNurseData so JsonSyntaxException/IllegalStateException are caught
and turned into a structured error via response.setError(...) before returning
response.toStringWithHttpStatus(); ensure jsnOBJ is declared outside the try so
it can still be passed to generalOPDServiceImpl.deleteVisitDetails(jsnOBJ) in
the catch, and do not let parsing occur prior to the try that would bypass the
error handler.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 723aa9f4-e80d-4aa5-8a6e-b48def05add8

📥 Commits

Reviewing files that changed from the base of the PR and between 3861dd1 and e3250b8.

📒 Files selected for processing (1)

• src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java

[Aarti-panchal01]

Hi @drtechie and @sharma-sugurthi
Implemented the proof of concept for #153 in GeneralOPDController.saveBenGenOPDNurseData():

1. Return type: String → ResponseEntity
2. Used existing toStringWithHttpStatus() (was already there, just unused!)
3. Removed raw requestObj logging — patient vitals/history were being
   logged at INFO level in production

This is intentionally scoped to one method as a proof of concept.
Happy to extend to other controllers if the approach is approved.