feat: implemented update logic for deliveries #1004
Conversation
There was a problem hiding this comment.
Thanks, great contribution and will support the incorporation of the feature!
I did some freehand testing to ensure validations are fine and found a few issues (also in my "specification").
Further, I've forgot to mention that we please raise the pr against this branch so that we merge it as a bundle of features: feat/edit-modals
| ### Added | ||
|
|
||
| - / | ||
| - Implemented update logic for delivery information ([#1004](https://github.com/eclipse-tractusx/puris/pull/1004)) |
There was a problem hiding this comment.
Changelog version outdated.
I would consider this a minor change instead of bugfix.
| label="Departure Time*" | ||
| placeholder="Pick Departure Date" | ||
| locale="de" | ||
| error={ |
There was a problem hiding this comment.
Smaller bug: departure time is not highlighted as an error on save during update:
- Create delivery with departure type = estimated, departure time = tomorrow 11 am
- Update delivery with departure type = actual
No update feasible, no highlighting of departure time or depature type. If I now update departure time, it will be shown as an error.
| label="Arrival Time*" | ||
| placeholder="Pick Arrival Date" | ||
| locale="de" | ||
| error={ |
There was a problem hiding this comment.
Same as for departure:
Smaller bug: departure time is not highlighted as an error on save during update:
- Create delivery with departure type = estimated, departure time = tomorrow 11 am
- Update delivery with departure type = actual
No update feasible, no highlighting of departure time or depature type. If I now update departure time, it will be shown as an error.
Same behavior for setting arrival time < departure time even when updating arrival time
There was a problem hiding this comment.
Minor and maybe ergonomics (shorter), but wouldn't it be more conssistent to name it DeliveryInformationCreationModal?
There was a problem hiding this comment.
Strictly speaking it would be better to differentiate the names even more by using DeliveryInformationListModal and DeliveryInformationCreationModal. I didn't opt to use it, because I had issues with git recognizing either of the modals as simple renaming of the original DeliveryInformationModal, which would lead to a lot more changed lines.
I'll adjust the name for the creation modal for now and see if I can rename the list modal correctly.
| method(temporaryDelivery) | ||
| .then((d) => { | ||
| onSave(d); | ||
| notify({ |
There was a problem hiding this comment.
The notification is not shown on the correctly layer:
Material Details View > DeliveryInformationModal > DeliveryCreationModal
It's shown in the gray back of the Material Details View.
| })) | ||
| } | ||
| sx={{ marginTop: '.5rem' }} | ||
| data-testid="delivery-quantity-field" |
There was a problem hiding this comment.
| data-testid="delivery-quantity-field" | |
| data-testid="delivery-quantity-field" | |
| disabled={delivery?.departureType === 'actual-departure' && delivery?.arrivalType === 'actual-arrival'} | |
Currently the quantity is updateable if both arrivals have been set. Obviously missed that during defintion of the rules for #1007
I would additionally to disable the edit button (gray out + tooltip) the edit pencil in the deliveries list modal.
Note: you can check with int data for todays delivery for semiconductor as customer
There was a problem hiding this comment.
Thank you for the feedback. Makes sense. Due to the nature of the constraints that only allow actual-arrival when actual-departure is set, I'll simplify the condition to just check for arrivalType === 'actual-arrival'.
ReneSchroederLJ
force-pushed
the
feat/delivery-update
branch
from
aa9df19 to
2a13eff
Compare
Description
resolves #1007
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: