Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/Command/StartCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use Swagger\Client\ApiException;
use Swagger\Client\Model\TimesheetEditForm;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
Expand All @@ -25,12 +26,15 @@ protected function configure(): void
$this
->setName('start')
->setDescription('Starts a new timesheet')
->setHelp('This command lets you start a new timesheet')
->setHelp('This command lets you start a new timesheet and optionally end it immediately.')
->addOption('customer', 'c', InputOption::VALUE_OPTIONAL, 'The customer to filter the project list, can be an ID or a search term or empty (you will be prompted for a customer).')
->addOption('project', 'p', InputOption::VALUE_OPTIONAL, 'The project to use, can be an ID or a search term or empty. You will be prompted for the project.')
->addOption('activity', 'a', InputOption::VALUE_OPTIONAL, 'The activity ID to use')
->addOption('tags', 't', InputOption::VALUE_OPTIONAL, 'Comma separated list of tag names')
->addOption('description', 'd', InputOption::VALUE_OPTIONAL, 'The timesheet description')
->addOption('begin', 'b', InputOption::VALUE_OPTIONAL, 'The begin (date and) time in a format supported by PHP')
->addOption('end', 'e', InputOption::VALUE_OPTIONAL, 'The end (date and) time in a format supported by PHP')
->addOption('timezone', 'z', InputOption::VALUE_OPTIONAL, 'IANA timezone (e.g. Asia/Kolkata) for interpreting --begin/--end. Falls back to the PHP default timezone if omitted.')
;
}

Expand Down Expand Up @@ -81,6 +85,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$form->setDescription($description);
}

$timezone = $input->getOption('timezone');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


try {
if (null !== ($begin = $input->getOption('begin'))) {
$begin = $this->parseAndRefineDateTime((string) $begin, 'begin', $timezone);
$form->setBegin($begin);
}

if (null !== ($end = $input->getOption('end'))) {
$end = $this->parseAndRefineDateTime((string) $end, 'end', $timezone);
$form->setEnd($end);
}
Comment on lines +96 to +99

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} catch (\InvalidArgumentException $ex) {
$io->error($ex->getMessage());

return Command::FAILURE;
}

try {
$timesheet = $api->postPostTimesheet($form);
} catch (ApiException $ex) {
Expand All @@ -92,6 +114,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$fields = [
'ID' => $timesheet->getId(),
'Begin' => $timesheet->getBegin()->format(\DateTime::ISO8601),
'End' => $timesheet->getEnd()?->format(\DateTime::ISO8601),
'Description' => $timesheet->getDescription(),
'Tags' => implode(PHP_EOL, $timesheet->getTags()),
'Customer' => $customer->getName(),
Expand Down
22 changes: 22 additions & 0 deletions src/Command/TimesheetCommandTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -321,4 +321,26 @@ private function askForActivity(SymfonyStyle $io, array $activities): ?Activity

return null;
}

private function parseAndRefineDateTime(string $value, string $valueName, ?string $timezone = null): \DateTime

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseAndRefineDateTime() should be renamed to parseDateTime()

{
$zone = $this->resolveTimezone($timezone);

try {
return new \DateTime($value, $zone);
} catch (\Exception $e) {
throw new \InvalidArgumentException(\sprintf('Value "%s" for field "%s" is not a valid DateTime.', $value, $valueName));
}
}

private function resolveTimezone(?string $timezone): \DateTimeZone
{
$name = $timezone ?? date_default_timezone_get();

try {
return new \DateTimeZone($name);
} catch (\Exception $e) {
throw new \InvalidArgumentException(\sprintf('Invalid timezone "%s".', $name));
}
}
}