diff --git a/appinfo/info.xml b/appinfo/info.xml index 8f696b737b..dd1b8aac91 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -63,6 +63,7 @@ OCA\Deck\Command\BoardImport OCA\Deck\Command\TransferOwnership OCA\Deck\Command\CalendarToggle + OCA\Deck\Command\FixCardDate diff --git a/docs/API.md b/docs/API.md index c3d40d265d..2de077d75d 100644 --- a/docs/API.md +++ b/docs/API.md @@ -48,7 +48,19 @@ In any case a user doesn't have access to a requested entity, a 403 error will b ### Date -Datetime values in request data need to be provided in ISO-8601. Example: 2020-01-20T09:52:43+00:00 +Datetime values in request data (`duedate`, `startdate`, `done`) need to be provided in one of the following formats: + +| Format | Example | +|-------------------------------------------|----------------------------------------------------------------| +| RFC 3339 / ISO-8601 with timezone | `2020-01-20T09:52:43+00:00`, `2020-01-20T09:52:43Z` | +| … with optional seconds and fraction (up to 9 digits) | `2020-01-20T09:52Z`, `2020-01-20T09:52:43.123456789+01:00` | +| Date only | `2020-01-20` | +| Date and time without timezone | `2020-01-20 09:52:43` | + +Values without a timezone are interpreted in the server's timezone, so providing a timezone is recommended. +`null` or an empty string unsets the value. Any other value is rejected with `400 Bad Request`, e.g. `duedate must be datetime`. + +Since Deck 2.0 these values are validated. Before that, anything PHP's `DateTime` could parse was accepted, including relative values like `+1 day` and ISO-8601 values without a timezone like `2020-01-20T09:52`. ## Headers @@ -633,7 +645,8 @@ The board list endpoint supports setting an `If-Modified-Since` header to limit | type | String | Type of the card (for later use) use 'plain' for now | | order | Integer | Order for sorting the stacks | | description | String | _(optional)_ The markdown description of the card | -| duedate | timestamp | _(optional)_ The duedate of the card or null | +| duedate | timestamp | _(optional)_ The duedate of the card or null, see [Date](#date) for accepted formats | +| startdate | timestamp | _(optional)_ The start date of the card or null, see [Date](#date) for accepted formats | #### Response @@ -682,9 +695,10 @@ The board list endpoint supports setting an `If-Modified-Since` header to limit | type | String | Type of the card (for later use) use 'plain' for now | | owner | String | The user that owns the card | | order | Integer | Order for sorting the stacks | -| duedate | timestamp | The ISO-8601 formatted duedate of the card or null | +| duedate | timestamp | The duedate of the card or null, see [Date](#date) for accepted formats | +| startdate | timestamp | _(optional)_ The start date of the card or null, see [Date](#date) for accepted formats | | archived | bool | Whether the card is archived or not | -| done | timestamp\|null | The ISO-8601 formatted date when the card is marked as done (optional, null indicates undone state) | +| done | timestamp\|null | The date when the card is marked as done (optional, null indicates undone state), see [Date](#date) | ``` diff --git a/lib/Command/FixCardDate.php b/lib/Command/FixCardDate.php new file mode 100644 index 0000000000..bbca1409ac --- /dev/null +++ b/lib/Command/FixCardDate.php @@ -0,0 +1,148 @@ +setName('deck:fix-card-date') + ->setDescription('Fixes the date fields of cards. In some DBs, the date fields may have been stored incorrectly, causing issues with date handling. This command will attempt to fix those dates.') + ->addOption( + 'default-year', + null, + InputOption::VALUE_OPTIONAL, + 'Default year to use for cards with invalid dates. If not provided, the large year number will be trimmed to first 4 digits.', + ) + ->addOption( + 'dry-run', + null, + InputOption::VALUE_NONE, + 'Dry run, do not make any changes.', + ) + ; + } + + /** + * @throws Exception + */ + protected function execute(InputInterface $input, OutputInterface $output): int { + $isDryRun = (bool)$input->getOption('dry-run'); + $defaultYear = $input->getOption('default-year'); + + // Validate default year if provided + if ($defaultYear !== null && !preg_match('/^\d{4}$/', $defaultYear)) { + $output->writeln('Invalid default year provided. It must be a 4-digit year.'); + return 1; + } + + // Fetch cards with date values and validate invalid years in PHP for DB portability. + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'duedate', 'startdate', 'done') + ->from('deck_cards') + ->where($qb->expr()->orX( + $qb->expr()->isNotNull('duedate'), + $qb->expr()->isNotNull('startdate'), + $qb->expr()->isNotNull('done'), + )); + $result = $qb->executeQuery(); + + $hasInvalidCards = false; + + while (($card = $result->fetchAssociative()) !== false) { + $cardId = $card['id']; + $duedate = $card['duedate']; + $startdate = $card['startdate']; + $done = $card['done']; + $hasInvalidDueDate = $duedate && $this->isInvalidDate($duedate); + $hasInvalidStartDate = $startdate && $this->isInvalidDate($startdate); + $hasInvalidDoneDate = $done && $this->isInvalidDate($done); + + if (!$hasInvalidDueDate && !$hasInvalidStartDate && !$hasInvalidDoneDate) { + continue; + } + + $hasInvalidCards = true; + + $output->writeln("Card ID: $cardId"); + + if ($hasInvalidDueDate) { + $output->writeln(" Original Due Date: $duedate"); + $duedate = $this->fixDate($duedate, $defaultYear); + + if (!$isDryRun) { + $this->updateCardDate($cardId, 'duedate', $duedate); + } + $output->writeln(" Fixed Due Date: $duedate"); + } + + if ($hasInvalidStartDate) { + $output->writeln(" Original Start Date: $startdate"); + $startdate = $this->fixDate($startdate, $defaultYear); + + if (!$isDryRun) { + $this->updateCardDate($cardId, 'startdate', $startdate); + } + $output->writeln(" Fixed Start Date: $startdate"); + } + + if ($hasInvalidDoneDate) { + $output->writeln(" Original Done Date: $done"); + $done = $this->fixDate($done, $defaultYear); + + if (!$isDryRun) { + $this->updateCardDate($cardId, 'done', $done); + } + $output->writeln(" Fixed Done Date: $done"); + } + } + + if (!$hasInvalidCards) { + $output->writeln('No cards with invalid dates found.'); + } + + return 0; + } + + protected function isInvalidDate(string $date): bool { + $parts = explode('-', $date); + $year = $parts[0]; + return $year > 9999; + } + + protected function fixDate(string $date, ?string $defaultYear): string { + $parts = explode('-', $date); + $year = $parts[0]; + $fixedYear = $defaultYear; + if (!$defaultYear) { + $fixedYear = substr($year, 0, 4); + } + return str_replace($year . '-', $fixedYear . '-', $date); + } + + protected function updateCardDate(int $cardId, string $field, string $newDate): void { + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_cards') + ->set($field, $qb->createNamedParameter($newDate)) + ->set('last_modified', $qb->createNamedParameter(time())) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($cardId))); + $qb->executeStatement(); + } +} diff --git a/lib/Controller/CardApiController.php b/lib/Controller/CardApiController.php index b3f3fd173d..f491666473 100644 --- a/lib/Controller/CardApiController.php +++ b/lib/Controller/CardApiController.php @@ -65,6 +65,8 @@ public function get() { * @params $type * @params $order * @params $description + * @param string|null $duedate Due date, see docs/API.md#date for accepted formats, null or empty string to unset + * @param string|null $startdate Start date, see docs/API.md#date for accepted formats, null or empty string to unset * * Get a specific card. */ @@ -84,6 +86,9 @@ public function create($title, $type = 'plain', $order = 999, $description = '', /** * Update a card + * + * @param string|null $duedate Due date, see docs/API.md#date for accepted formats, null or empty string to unset + * @param string|null $startdate Start date, see docs/API.md#date for accepted formats, null or empty string to unset */ #[NoAdminRequired] #[CORS] diff --git a/lib/Controller/CardOcsController.php b/lib/Controller/CardOcsController.php index 6dbdb28138..e6dad7a7dd 100644 --- a/lib/Controller/CardOcsController.php +++ b/lib/Controller/CardOcsController.php @@ -40,6 +40,10 @@ public function __construct( parent::__construct($appName, $request); } + /** + * @param string|null $duedate Due date, see docs/API.md#date for accepted formats, null or empty string to unset + * @param string|null $startdate Start date, see docs/API.md#date for accepted formats, null or empty string to unset + */ #[NoAdminRequired] #[PublicPage] public function create(string $title, int $stackId, ?int $boardId = null, ?string $type = 'plain', ?string $owner = null, ?int $order = self::DEFAULT_ORDER, ?string $description = '', $duedate = null, $startdate = null, ?array $labels = [], ?array $users = [], ?string $color = null) { @@ -118,6 +122,10 @@ public function removeLabel(?int $boardId, int $cardId, int $labelId): DataRespo return new DataResponse($this->cardService->removeLabel($cardId, $labelId)); } + /** + * @param string|null $duedate Due date, see docs/API.md#date for accepted formats, null or empty string to unset + * @param string|null $startdate Start date, see docs/API.md#date for accepted formats, null or empty string to unset + */ #[NoAdminRequired] #[PublicPage] public function update(int $id, string $title, int $stackId, string $type, int $order, string $description, $duedate, $deletedAt, ?int $boardId, array|string|null $owner = null, $archived = null, $startdate = null): DataResponse { diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 695a249d68..2f158d966f 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -190,7 +190,7 @@ public function findCalendarEntries(int $boardId): array { * @throws BadrequestException */ public function create(string $title, int $stackId, string $type, int $order, string $owner, string $description = '', $duedate = null, $startdate = null, ?string $color = null, bool $insertAtPosition = false): Card { - $this->cardServiceValidator->check(compact('title', 'stackId', 'type', 'order', 'owner')); + $this->cardServiceValidator->check(compact('title', 'stackId', 'type', 'order', 'owner', 'startdate', 'duedate')); $this->permissionService->checkPermission($this->stackMapper, $stackId, Acl::PERMISSION_EDIT); if ($this->boardService->isArchived($this->stackMapper, $stackId)) { @@ -203,8 +203,8 @@ public function create(string $title, int $stackId, string $type, int $order, st $card->setOrder($order); $card->setOwner($owner); $card->setDescription($description); - $card->setDuedate($duedate); - $card->setStartdate($startdate); + $card->setDuedate($duedate ? new \DateTime($duedate) : null); + $card->setStartdate($startdate ? new \DateTime($startdate) : null); $card->setColor($color); if (!$insertAtPosition) { @@ -263,7 +263,7 @@ public function delete(int $id): Card { * @throws BadRequestException */ public function update(int $id, string $title, int $stackId, string $type, string $owner, string $description = '', int $order = 0, ?string $duedate = null, ?int $deletedAt = null, ?bool $archived = null, ?OptionalNullableValue $done = null, ?string $startdate = null, ?OptionalNullableValue $color = null): Card { - $this->cardServiceValidator->check(compact('id', 'title', 'stackId', 'type', 'owner', 'order')); + $this->cardServiceValidator->check(compact('id', 'title', 'stackId', 'type', 'owner', 'order', 'startdate', 'duedate', 'done')); $this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT, allowDeletedCard: true); $this->permissionService->checkPermission($this->stackMapper, $stackId, Acl::PERMISSION_EDIT); diff --git a/lib/Validators/BaseValidator.php b/lib/Validators/BaseValidator.php index 8b1350442e..74fdc510f7 100644 --- a/lib/Validators/BaseValidator.php +++ b/lib/Validators/BaseValidator.php @@ -11,6 +11,7 @@ use Exception; use OCA\Deck\BadRequestException; +use OCA\Deck\Model\OptionalNullableValue; abstract class BaseValidator { @@ -31,9 +32,22 @@ private function validate($data) { foreach ($data as $field => $value) { $field_rule = $rules[$field]; + $isOptional = is_array($field_rule) && in_array('optional', $field_rule, true); + + if ($isOptional) { + if ($value instanceof OptionalNullableValue) { + $value = $value->getValue(); + } + if ($value === null || $value === '') { + continue; + } + } if (is_array($field_rule)) { foreach ($field_rule as $rule) { + if ($rule === 'optional') { + continue; + } // The format for specifying validation rules and parameters follows an // easy {rule}:{parameters} formatting convention. For instance the // rule "Max:3" states that the value may only be three letters. @@ -41,12 +55,12 @@ private function validate($data) { [$rule, $parameter] = explode(':', $rule, 2); if (!$this->{$rule}($value, $parameter)) { throw new BadRequestException( - $this->getErrorMessage($rule, $field, $parameter)); + $this->getErrorMessage($rule, $field, $parameter, $isOptional)); } } else { if (!$this->{$rule}($value)) { throw new BadRequestException( - $field . ' must be provided and must be ' . str_replace('_', ' ', $rule)); + $this->getErrorMessage($rule, $field, null, $isOptional)); } } } @@ -135,6 +149,61 @@ private function min($value, $limit): bool { return $this->getSize($value) >= $limit; } + /** + * @throws Exception + */ + private function datetime(mixed $value): bool { + if (!is_string($value) || $value === '') { + return false; + } + + // Java/Android commonly send ISO_OFFSET_DATE_TIME and ISO_INSTANT. + if (preg_match('/^(?\d{4}-\d{2}-\d{2})T(?\d{2}):(?\d{2})(?::(?\d{2})(?:\.(?\d{1,9}))?)?(?Z|[+\-]\d{2}:\d{2})$/', $value, $matches)) { + $second = ($matches['second'] ?? '') !== '' ? $matches['second'] : '00'; + $fraction = $matches['fraction'] ?? ''; + + if ($fraction !== '') { + // PHP supports microseconds, so normalize Java nanoseconds to 6 digits. + $fraction = str_pad(substr($fraction, 0, 6), 6, '0'); + } else { + $fraction = '000000'; + } + + $timezone = $matches['timezone'] === 'Z' ? '+00:00' : $matches['timezone']; + $normalizedValue = sprintf( + '%sT%s:%s:%s.%s%s', + $matches['date'], + $matches['hour'], + $matches['minute'], + $second, + $fraction, + $timezone + ); + + $datetime = \DateTime::createFromFormat('Y-m-d\TH:i:s.uP', $normalizedValue); + if ($datetime && $datetime->format('Y-m-d\TH:i:s.uP') === $normalizedValue) { + return true; + } + } + + $allowedFormats = [ + 'Y-m-d', + 'Y-m-d H:i:s', + 'Y-m-d\TH:i:sP', + 'Y-m-d\TH:i:s\Z', + 'Y-m-d\TH:i:s.v\Z', + ]; + + foreach ($allowedFormats as $format) { + $datetime = \DateTime::createFromFormat($format, $value); + if ($datetime && $datetime->format($format) === $value) { + return true; + } + } + + return false; + } + /** * Get the size of an attribute. * @@ -159,13 +228,17 @@ protected function getSize($value): int { * @param $parameter * @return string */ - protected function getErrorMessage(string $rule, $field, $parameter = null): string { + protected function getErrorMessage(string $rule, $field, $parameter = null, $isOptional = false): string { if (in_array($rule, ['max', 'min'], true)) { return $rule === 'max' ? $field . ' cannot be longer than ' . $parameter . ' characters ' : $field . ' must be at least ' . $parameter . ' characters long '; } + if ($isOptional) { + return $field . ' must be ' . str_replace('_', ' ', $rule); + } + return $field . ' must be provided and must be ' . str_replace('_', ' ', $rule); } } diff --git a/lib/Validators/CardServiceValidator.php b/lib/Validators/CardServiceValidator.php index ddbcf8738c..faa0af366d 100644 --- a/lib/Validators/CardServiceValidator.php +++ b/lib/Validators/CardServiceValidator.php @@ -21,6 +21,9 @@ public function rules() { 'type' => ['not_empty', 'not_null', 'not_false', 'max:64'], 'order' => ['numeric'], 'owner' => ['not_empty', 'not_null', 'not_false', 'max:64'], + 'startdate' => ['optional', 'datetime'], + 'duedate' => ['optional', 'datetime'], + 'done' => ['optional', 'datetime'], ]; } } diff --git a/src/components/card/DueDateSelector.vue b/src/components/card/DueDateSelector.vue index a531c9de03..4f7dcfa599 100644 --- a/src/components/card/DueDateSelector.vue +++ b/src/components/card/DueDateSelector.vue @@ -180,6 +180,9 @@ export default defineComponent({ }, set(val) { + if (!this.isValidDate(val)) { + return + } this.$emit('input', val ? new Date(val) : null) }, }, diff --git a/src/components/card/StartDateSelector.vue b/src/components/card/StartDateSelector.vue index 3cacc97057..77e6146aae 100644 --- a/src/components/card/StartDateSelector.vue +++ b/src/components/card/StartDateSelector.vue @@ -95,6 +95,9 @@ export default defineComponent({ }, set(val) { + if (!this.isValidDate(val)) { + return + } this.$emit('input', val ? new Date(val) : null) }, }, diff --git a/src/mixins/readableDate.js b/src/mixins/readableDate.js index df30fa8c4b..1f7eb7940f 100644 --- a/src/mixins/readableDate.js +++ b/src/mixins/readableDate.js @@ -5,6 +5,8 @@ import moment from '@nextcloud/moment' +const maxYear = 9999 + export default { computed: { formatReadableDate() { @@ -13,4 +15,11 @@ export default { } }, }, + methods: { + isValidDate(val) { + // Prevent setting a date with a year greater than 9999, as this can cause issues with some date libraries and databases. + const date = val ? new Date(val) : null + return !(date && date.getFullYear() > maxYear) + }, + }, } diff --git a/tests/unit/Validators/CardServiceValidatorTest.php b/tests/unit/Validators/CardServiceValidatorTest.php new file mode 100644 index 0000000000..a43394efd2 --- /dev/null +++ b/tests/unit/Validators/CardServiceValidatorTest.php @@ -0,0 +1,93 @@ +. + */ + +namespace OCA\Deck\Validators; + +use OCA\Deck\Tests\unit\Validators\ValidatorTestBase; + +class CardServiceValidatorTest extends ValidatorTestBase { + public function setUp(): void { + parent::setUpValidatorTest(CardServiceValidator::class); + } + + public function testInvalidDueDateIsRejected(): void { + $this->assertFail([ + 'duedate' => 'not a date', + ]); + $this->assertFail([ + 'duedate' => '99999-01-01 00:00:00', + ]); + $this->assertFail([ + 'duedate' => '+1 day', + ]); + $this->assertFail([ + 'duedate' => '99999-01-01T00:00:00Z', + ]); + $this->assertFail([ + 'duedate' => '99999-01-01T00:00Z', + ]); + $this->assertFail([ + 'duedate' => '99999-01-01', + ]); + } + + public function testValidDueDateIsAccepted(): void { + $this->assertPass([ + 'duedate' => '2026-01-01T01:00:00.000Z', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01 00:00:00', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T00:00:00+00:00', + ]); + // Java/Android ISO_INSTANT samples + $this->assertPass([ + 'duedate' => '2026-01-01T00:00:00Z', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T00:00:00.123Z', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T00:00:00.123456789Z', + ]); + // Java/Android ISO_OFFSET_DATE_TIME samples + $this->assertPass([ + 'duedate' => '2026-01-01T07:00:00+07:00', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T07:00+07:00', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T00:00Z', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T07:00:00.123+07:00', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01T07:00:00.123456789+07:00', + ]); + $this->assertPass([ + 'duedate' => '2026-01-01', + ]); + } +}