[cioan]

I applied the fix to EncounterWithBedPatientAssignmentSaveHandler.handle and removed both calls to the @transactional bed-management service that were forcing the premature flush:

• Void branch — instead of bedManagementService.deleteBedPatientAssignment(bpa, ...), the handler now sets the void flags directly on the attached bpa (setVoided/setDateVoided/setVoidReason/setVoidedBy), mirroring exactly what the soft-void method did. It uses the handler's user/date params for the void metadata.
• Start-datetime branch — dropped the explicit saveBedPatientAssignment(bpa); just mutating the attached entity is enough.

Both bpa instances come from a readOnly query so they're persistent and attached; their changes now flush with the surrounding transaction, after the unrelated VisitAttribute has had its valueReference populated — so the DataIntegrityViolationException no longer fires.

The bedManagementService field is retained because it's still used for the read-only getBedPatientAssignmentByEncounter lookup, which doesn't trigger a flush.

[sonarqubecloud]

Quality Gate passed

Issues
5 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

[chibongho]

Looks good. Thanks for the fix!

[mogoodrich]

One small question about the date voided but generally LGTM. So interesting that Claude points out our somewhat misuse of how Hibernate works.

[mogoodrich]

Not sure what date is passed in here, I would think this should always be the current datetime? (maybe it already is)

[cioan]

You are correct @mogoodrich :

Yes, for a SaveHandler the date is always the current datetime

SaveHandlers are driven by RequiredDataAdvice (the MethodBeforeAdvice on save* service methods). The save path goes through the 3-arg recursivelyHandle(Class, OpenmrsObject, String) overload, whose bytecode is unambiguous:

2: invokestatic Context.getAuthenticatedUser() // -> user
5: new java/util/Date
9: invokespecial java/util/Date."":()V // -> new Date() == "now"
17: invokestatic recursivelyHandle(Class, OpenmrsObject, User, Date, String, Set)

That new Date() is then handed straight to each handler via RequiredDataHandler.handle(object, user, date, reason) (the #150 interface call at offset 71). So the date your handler receives is a fresh new Date() captured at the start of the save operation — i.e. the current datetime.

That makes bpa.setDateVoided(date) equivalent to the original deleteBedPatientAssignment's bpa.setDateVoided(new Date()) — just sourced from the timestamp the framework already created for this save, so the void instant lines up with the rest of that operation's audit metadata. No behavior change, and no risk of it being null.

One nuance worth knowing (doesn't affect us)

The other branches in RequiredDataAdvice — the Voidable/Retireable paths that drive VoidHandler/RetireHandler — behave differently: they pass the object's existing getDateVoided() / getDateRetired() if it's already set, and only fall back to new Date() otherwise (visible at offsets 253–303 and 384–434). So for those handler types, the date is "whenever it was voided," not necessarily now.

But our class is a SaveHandler, which is only ever invoked through the save path → always new Date(). So your instinct is correct: for this handler it's always the current datetime.

No code change needed — the date parameter is the right thing to use here.

[cioan]

Thanks @chibongho and @mogoodrich !