OBPIH-7448 prevent creating a PO if logged into different org#5647
Conversation
github-actions
Bot
added
type: bug
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5647 +/- ##
============================================
+ Coverage 9.12% 9.72% +0.60%
- Complexity 1170 1323 +153
============================================
Files 701 717 +16
Lines 45281 45785 +504
Branches 10851 10926 +75
============================================
+ Hits 4131 4453 +322
- Misses 40497 40642 +145
- Partials 653 690 +37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| Order saveOrder(Order order) { | ||
| if (order.destinationParty?.id != sessionManager.getCurrentLocation().organizationId) { |
There was a problem hiding this comment.
Won't it be better to have a convenience method getCurrentOrganizationId method in the session manager to avoid breaking the law of demeter?
There was a problem hiding this comment.
I wanted session manager to be a simple class for setting/getting objects to/from the session so I feel a bit weird about adding logic around operating on the objects themselves (even if it's just a simple getter).
I feel like we break that law constantly since we often need to fetch a domain, then use that domain to fetch their relationships, then operate on those relationship entities.
I don't know if I see much advantage of adding a getCurrentOrganizationId method but I hadn't heard of the law of demeter before so I haven't given it much thought before. I'm willing to be convinced otherwise.
| } | ||
|
|
||
| Order saveOrder(Order order) { | ||
| if (order.destinationParty?.id != sessionManager.getCurrentLocation().organizationId) { |
There was a problem hiding this comment.
I'm fine with the way you've done it, but I believe this could be also achievable in the Order's destinationParty validator
There was a problem hiding this comment.
hmm interesting idea. I'd be scared of that impacting other flows though since the Order domain is used everywhere. Maybe we want to always fail if the user is in a different org but that would probably require a lot more changes to support in other flows. I'm going to keep it this way for now but if we encounter the issue again, we can revisit.
It does still feel a little weird to be querying the session inside of a domain validator but maybe I just need to get over my idea that the validator should be only for simple validations on the domain/command fields itself. If we wanted to keep the logic out of the domain, we could always create XValidator components that can wire in whatever other components we need. So maybe I am okay with the idea.
Something like:
static constraints = {
destinationParty(validator: { Party destinationParty, Order obj -> return OrderValidator.validateDestinationParty(...)})
}
and then OrderValidator can access the session manager and whatever other components we need.
3 participants
✨ Description of Change
Link to GitHub issue or Jira ticket: https://pihemr.atlassian.net/browse/OBPIH-7448
Description: Prevent you from creating a PO with a destination party that is different than the currently logged in organization.
📷 Screenshots & Recordings (optional)
2025-12-01_14-51-35.mp4