Conversation
| return $this->parseAndRefineDateTime( | ||
| $io, | ||
| $io->ask(\sprintf('Value "%s" for field "%s" is not a valid DateTime. Please refine:', $value, $valueName), $value), | ||
| $valueName, | ||
| $timezone, | ||
| ); |
There was a problem hiding this comment.
I know this was in the original PR, but I would propose to use something like this instead (untested code!)
$io->error((\sprintf('Value "%s" for field "%s" is not a valid DateTime.', $value, $valueName));
return Command::ERROR;There was a problem hiding this comment.
Addressed and tested. Thanks for the review.
Allow creating timesheet records with explicit begin/end times, and a -z/--timezone to name the IANA zone used to read them (falls back to the PHP default). Invalid zone fails cleanly without entering the refine prompt. Co-authored-by: webemerge <office@web-emerge.com> Signed-off-by: webemerge <office@web-emerge.com> Signed-off-by: Parthiban Nallathambi <parthiban@linumiz.com>
dd26444 to
6c99e23
Compare
|
@kevinpapst Please revisit and thanks |
kevinpapst
left a comment
kevinpapst
left a comment
There was a problem hiding this comment.
Thanks for owning the PR and taking over!
| $form->setDescription($description); | ||
| } | ||
|
|
||
| $timezone = $input->getOption('timezone'); |
There was a problem hiding this comment.
Timezone: is only validated if either begin or end are passed. always execute the resolveTimezone() - otherwise the user won't recognize some broken value (e.g. --timezone=foo would be silently ignored currently).
Add a try & catch and show an error + Command::FAILURE if parsing fails
| return null; | ||
| } | ||
|
|
||
| private function parseAndRefineDateTime(string $value, string $valueName, ?string $timezone = null): \DateTime |
There was a problem hiding this comment.
parseAndRefineDateTime() should be renamed to parseDateTime()
| 'Activity' => $activity->getName(), | ||
| ]; | ||
|
|
||
| $io->success('Started timesheet'); |
There was a problem hiding this comment.
The message still says "Started timesheet" when the record is already ended.
| if (null !== ($end = $input->getOption('end'))) { | ||
| $end = $this->parseAndRefineDateTime((string) $end, 'end', $timezone); | ||
| $form->setEnd($end); | ||
| } |
There was a problem hiding this comment.
two issues:
- end could be before begin: this is validated in Kimai, but we should catch it early here
- end could be passed without begin, which shouldn't be possible
2 participants
Allow creating timesheet records with explicit begin/end times, and a -z/--timezone to name the IANA zone used to read them (falls back to the PHP default). Invalid zone fails cleanly without entering the refine prompt.
Co-authored-by: webemerge office@web-emerge.com
Signed-off-by: webemerge office@web-emerge.com
Signed-off-by: Parthiban Nallathambi parthiban@linumiz.com