OBPIH-5822 email alert to requestor approvals email alerts #4299
Conversation
| Comment getRecentComment() { | ||
| return comments?.sort({ a, b -> | ||
| b.dateCreated <=> a.dateCreated | ||
| }).find() |
There was a problem hiding this comment.
wouldn't .first() fit better here?
I believe find() should take some closure as an argument? I'm surprised that it works without anything
There was a problem hiding this comment.
?.first() even with null-safety operator throwns an exception.
I came across this stack post which suggests using find().
When calling find without any arguments it acts the same way as .find(true) which will resolve at first iteration
There was a problem hiding this comment.
@drodzewicz I think you still need to have here a null-safety operator with find() too.
grails-app/i18n/messages.properties
Outdated
| @@ -691,6 +691,8 @@ email.contactAdmin.message=Please contact your local OpenBoxes administrator wit | |||
| email.contactAdminReceiving.message=Please contact the receiver listed above or your local OpenBoxes administrator with any questions you have about the status of this item. If you see a quantity in the cancelled column, this means that the warehouse received fewer items than expected, possibly as a result of a data error or a loss during shipment. The receiver can explain the situation. If you have an OpenBoxes account, you can view the shipment via | |||
| email.thisLink.label=this link | |||
| email.requestReceived.message=You have received a new request from: {0}, created by: {1}. Click <a href="{2}">here</a> to review your requests pending your approval. | |||
| email.statusChange.message=Your request status has been updated to: {0}. Click <a href="{1}">here</a> to review your requests. | |||
| email.withComment.message=With comment: | |||
There was a problem hiding this comment.
I know @jmiranda was not a big fan or adding some : or commas in the message itself, but I'm fine with that.
There was a problem hiding this comment.
You have a point, this should probably not be in the message label
| </div> | ||
| <div> | ||
| <g:message code="email.withComment.message" /> | ||
| <g:set var="comment" value="${requisition.recentComment}" /> |
There was a problem hiding this comment.
i assume we can't run into a NullPointerException here, because it would not be rendered if we don't have requisitoin?
| if (requisition.status == RequisitionStatus.PENDING_APPROVAL) { | ||
| List<Person> recipients = resolveNotificationRecipients(requisition) | ||
| publishRequisitionPendingApprovalNotifications(requisition, recipients) | ||
| } else if (requisition.status in [RequisitionStatus.APPROVED, RequisitionStatus.REJECTED]) { | ||
| publishRequisitionStatusUpdateNotifications(requisition, requisition.requestedBy) |
There was a problem hiding this comment.
I am not a big fan of using else, maybe this approach would be cleaner:
void publishRequisitionStatusTransitionNotifications(Requisition requisition) {
if (requisition.status == RequisitionStatus.PENDING_APPROVAL) {
List<Person> recipients = resolveNotificationRecipients(requisition)
publishRequisitionPendingApprovalNotifications(requisition, recipients)
return
}
if (requisition.status in [RequisitionStatus.APPROVED, RequisitionStatus.REJECTED]) {
publishRequisitionStatusUpdateNotifications(requisition, requisition.requestedBy)
}
}but I have nothing against leaving it as is
There was a problem hiding this comment.
instead I went with an approach of switch/case
drodzewicz
force-pushed
the
OBPIH-5822-email-alert-to-requestor-approvals-email-alerts
branch
from
8e79d2a
to
f3a8f2d
Compare
| Comment getRecentComment() { | ||
| return comments?.sort({ a, b -> | ||
| b.dateCreated <=> a.dateCreated | ||
| }).find() |
There was a problem hiding this comment.
@drodzewicz I think you still need to have here a null-safety operator with find() too.
drodzewicz
force-pushed
the
OBPIH-5822-email-alert-to-requestor-approvals-email-alerts
branch
from
f3a8f2d
to
6c57dcf
Compare
…approve/reject request
drodzewicz
force-pushed
the
OBPIH-5822-email-alert-to-requestor-approvals-email-alerts
branch
from
6c57dcf
to
264d486
Compare
awalkowiak
deleted the
OBPIH-5822-email-alert-to-requestor-approvals-email-alerts
branch
* OBPIH-5822 Added notificatin to requestor on request status change
* OBPIH-5822 Add recent comment to the status update mail notification
* OBPIH-5822 use find{true} instead of find() to get first element of a list
* OBPIH-5822 Change requisition id to identifier of success message on approve/reject request
No description provided.