-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EAM-1124] Shift scheduled end date with the start date #108
base: master
Are you sure you want to change the base?
Conversation
src/ui/pages/work/Workorder.js
Outdated
const initialStartDate = (scheduledStartDate || value).replace('00:00', '').trim(); | ||
const initialEndDate = scheduledEndDate.replace('00:00', '').trim(); | ||
|
||
const dateFormat = 'dd-MMM-yyyy'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have constants for date formats and datetime formats in eam-components, that can be reused for this.
https://github.com/cern-eam/eam-components/blob/master/src/enums/Constants.js
src/ui/pages/work/Workorder.js
Outdated
}; | ||
} | ||
|
||
const initialStartDate = (scheduledStartDate || value).replace('00:00', '').trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing time part instead of processing the date with a datetime format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The datepicker gives date format, having time in the format will cause an error. Not sure if the error comes from date-fns format/parse or the datepicker. The datepicker handles dates, not datetimes, the time itself is then not needed. I guess the time values come from the backend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Activity dialog seems to be giving current time in the value, will change the hardcoded 00:00 to a regex.
@@ -136,6 +138,40 @@ class Workorder extends Entity { | |||
} | |||
} | |||
|
|||
updateScheduleProperty = (key, value) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this implementation can be used for the scheduled start/end dates of a Work Order, we still need the same behavior for the 'Add Activity' dialog start/end dates.
I believe the logic to add the days difference could be extracted in order to be reused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separated the logic to a utils file, it's now used in both places
8e32eed
to
2d80128
Compare
This pull request shifts the scheduled end date according to the date difference to the start date, when scheduled start date is changed