OBPIH-6765 Add "Desired Date of Delivery" for E-Requests#4925
OBPIH-6765 Add "Desired Date of Delivery" for E-Requests#4925awalkowiak merged 8 commits intodevelopfrom
Conversation
alannadolny
requested review from
awalkowiak,
drodzewicz,
ewaterman and
kchelstowski
github-actions
bot
added
domain: frontend
| dateCreated : dateCreated?.format("MM/dd/yyyy"), | ||
| dateShipped : dateShipped?.format("MM/dd/yyyy HH:mm XXX"), | ||
| expectedDeliveryDate: expectedDeliveryDate?.format("MM/dd/yyyy HH:mm XXX"), | ||
| desiredDeliveryDate : requisition?.desiredDeliveryDate?.format("MM/dd/yyyy"), |
There was a problem hiding this comment.
why requisition and not the desiredDeliveryDate field you added?
There was a problem hiding this comment.
I think we wanted to get rid of formatting a date on the backend and let the frontend handle the eventual formatting.
How about removing .format here and formatting it with Moment on the frontend?
| Date dateDelivered | ||
| Date dateIssued | ||
| Date dateReceived | ||
| Date desiredDeliveryDate |
There was a problem hiding this comment.
Looks like our convention is date<Something>, but I don't know how this should look like for this one. @jmiranda do you have any preference or suggestion?
There was a problem hiding this comment.
@awalkowiak @alannadolny I will have an answer for you today.
There was a problem hiding this comment.
In the future, let's flag all tickets that require data model changes during the planning poker activity. All data model changes (no matter how small) must go through a design process. And this should almost always be done before we start working on the feature.
There was a problem hiding this comment.
I think dateDeliveryRequested makes the most sense.
Besides the naming, the reason I mentioned "design" was because I would have implemented this a little more generically at the data model and API level. The requested delivery date should also be available at the requisition line item level, even if the UI of the current feature does not support it.
| second part of the sentence. If there is no delivery date, we just finish the sentence after requestCreatedBy | ||
| message. It cannot be included in HTML alone because there will be additional space around the punctuation mark. | ||
| --}% | ||
| <g:set var="punctuationMark" value="${requisition.desiredDeliveryDate ? ',' : '.'}" /> |
There was a problem hiding this comment.
I get the idea here, but this looks odd. Would it not be clearer if it was just a {requisition.desiredDeliveryDate ? ',' : '.'} in between two g:message tags instead of an additional argument?
There was a problem hiding this comment.
or the other option would be always having the
You have received a new request from: {0}, created by: {1}, desired date of delivery: {2}. Click <a href="{2}">here</a> to review your requests pending your approval.
and {2} would get requisition.desiredDeliveryDate ?: 'unknown', or something like that.
There was a problem hiding this comment.
To your second comment: I asked Manon about that before and she answered it would be better to not show the date if it is null.
To your first comment: As you can see I left a comment "It cannot be included in HTML alone because there will be additional space around the punctuation mark."
| <include file="0.9.x/changelog-2024-08-19-1200-alter-table-invoice-item-set-invoice-non-null.xml" /> | ||
| <include file="0.9.x/changelog-2024-08-20-0000-migrate-prepayment-invoices-add-amount-and-inverse-items.groovy" /> | ||
| <include file="0.9.x/changelog-2024-10-04-1610-assign-amount-to-invoice-items.xml" /> | ||
| <include file="0.9.x/changelog-2024-10-28-1330-add-desired-delivery-date-field.xml" /> |
There was a problem hiding this comment.
make sure the prefix here is "add-column-..."
5 participants
No description provided.