Cherry pick changes from the 0.8.23 Hotfix 1 to the obgm feature branch #4411
Conversation
…ED to not be sent to the requestor (#4398) * OBPIH-5984 Fix issue causing approval email for status update to ISSUED to not be sent to the requestor * OBPIH-5984 Hydrate mostRecentEvent value before building the mail template to avoid lazyInitializationException * OBPIH-5984 Create a service method which fetches requisition with events
…o stock movement (#4400)
…4404) * OBPIH-5990 Add validation for deleting stock movement with associated invoice to api controller * OBPIH-5990 Remove unused import
awalkowiak
added
the
warn: do not squash
awalkowiak
requested review from
jmiranda,
drodzewicz,
alannadolny and
kchelstowski
| @@ -837,6 +838,14 @@ class RequisitionService { | |||
| } | |||
| } | |||
|
|
|||
| // TODO in Grails 3 move this method to @Service RequisitionDataService | |||
There was a problem hiding this comment.
@drodzewicz I see here is a TODO in Grails 3. Is it something that will be fairly quick? Could you add it to this PR?
There was a problem hiding this comment.
Yes, this should be rather quick.
It requires for me to create a RequisitionDataService which would implement a getRequisitionWithEvents with @Join("events")
|
@awalkowiak the changes that were mentioned in the TODO comment have been pushed |
| @@ -16,6 +16,7 @@ import org.grails.plugins.web.taglib.ApplicationTagLib | |||
| import org.joda.time.LocalDate | |||
| import org.pih.warehouse.DateUtil | |||
| import javassist.NotFoundException | |||
| import org.hibernate.criterion.CriteriaSpecification | |||
| @@ -263,8 +265,7 @@ class StockMovementService { | |||
| void updateRequisitionStatus(String id, RequisitionStatus status, Comment comment = null) { | |||
|
|
|||
| log.info "Update status ${id} " + status | |||
| // TODO: In Grails the get below should be replaced by the data service get that joins the Events | |||
| Requisition requisition = Requisition.get(id) | |||
| Requisition requisition = requisitionDataService.getRequisitionWithEvents(id) | |||
There was a problem hiding this comment.
ok, since we joined the events here, shouldn't we remove the additional fetch from somewhere, that was added in order to prevent the LIE?
There was a problem hiding this comment.
@drodzewicz hm, I guess nevermind, but it's weird that here, I see the Requisition.get, but when I go to your commit directly, I can see this:
which would be an answer to what I wrote, but I got mistaken by the change I can actually see in "Changed files"
There was a problem hiding this comment.
there was not additional fetch before the hotfix, everything is correct.
The additional fetch was in the first PR for this hotfix which added the Requisition.get(event.requsistion.id) in onApplicationEvent(RequisitionStatusTransitionEvent event)
drodzewicz
force-pushed
the
obgm-0.8.23-hotfix1
branch
from
27d0a49
to
8a24aa5
Compare
No description provided.