From 89761fc51140ce1b9d5ad277fdab35863eb58344 Mon Sep 17 00:00:00 2001 From: HUGO LOUREIRO <149155946+Lightshadow02@users.noreply.github.com> Date: Fri, 10 Jul 2026 02:38:49 +0200 Subject: [PATCH] fix(cards): do not clear card color when update omits the field Fixes #8131 Signed-off-by: HUGO LOUREIRO <149155946+Lightshadow02@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- lib/Controller/CardApiController.php | 3 +- lib/Controller/CardController.php | 5 +- lib/Controller/CardOcsController.php | 5 +- lib/Service/CardService.php | 7 +- tests/unit/Service/CardServiceTest.php | 101 ++++++++++++++++++++++++- 5 files changed, 115 insertions(+), 6 deletions(-) diff --git a/lib/Controller/CardApiController.php b/lib/Controller/CardApiController.php index 4bd3fdd2cb..b3f3fd173d 100644 --- a/lib/Controller/CardApiController.php +++ b/lib/Controller/CardApiController.php @@ -88,8 +88,9 @@ public function create($title, $type = 'plain', $order = 999, $description = '', #[NoAdminRequired] #[CORS] #[NoCSRFRequired] - public function update(string $title, $type, string $owner, string $description = '', int $order = 0, $duedate = null, $startdate = null, $archived = null, ?string $color = null): DataResponse { + public function update(string $title, $type, string $owner, string $description = '', int $order = 0, $duedate = null, $startdate = null, $archived = null): DataResponse { $done = array_key_exists('done', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('done', null)) : null; + $color = array_key_exists('color', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('color', null)) : null; $card = $this->cardService->update($this->request->getParam('cardId'), $title, $this->request->getParam('stackId'), $type, $owner, $description, $order, $duedate, 0, $archived, $done, $startdate, $color); return new DataResponse($card, HTTP::STATUS_OK); } diff --git a/lib/Controller/CardController.php b/lib/Controller/CardController.php index 58e412665d..129ffed1c7 100644 --- a/lib/Controller/CardController.php +++ b/lib/Controller/CardController.php @@ -64,10 +64,13 @@ public function create(string $title, int $stackId, string $type = 'plain', int * @param $duedate */ #[NoAdminRequired] - public function update(int $id, string $title, int $stackId, string $type, int $order, string $description, $duedate, $deletedAt, $archived = null, $startdate = null, ?string $color = null): Card { + public function update(int $id, string $title, int $stackId, string $type, int $order, string $description, $duedate, $deletedAt, $archived = null, $startdate = null): Card { $done = array_key_exists('done', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('done', null)) : null; + $color = array_key_exists('color', $this->request->getParams()) + ? new OptionalNullableValue($this->request->getParam('color', null)) + : null; return $this->cardService->update($id, $title, $stackId, $type, $this->userId, $description, $order, $duedate, $deletedAt, $archived, $done, $startdate, $color); } diff --git a/lib/Controller/CardOcsController.php b/lib/Controller/CardOcsController.php index 7ca27a0ef6..cd6629d204 100644 --- a/lib/Controller/CardOcsController.php +++ b/lib/Controller/CardOcsController.php @@ -113,10 +113,13 @@ public function removeLabel(?int $boardId, int $cardId, int $labelId): DataRespo #[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, ?string $color = null): DataResponse { + 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 { $done = array_key_exists('done', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('done', null)) : null; + $color = array_key_exists('color', $this->request->getParams()) + ? new OptionalNullableValue($this->request->getParam('color', null)) + : null; if (!$owner) { $owner = $this->userId; } else { diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 5d30ccaafc..325dc95777 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -246,7 +246,7 @@ public function delete(int $id): Card { * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @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, ?string $color = null): Card { + 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->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT, allowDeletedCard: true); @@ -289,7 +289,10 @@ public function update(int $id, string $title, int $stackId, string $type, strin $card->setOrder($order); $card->setDuedate($duedate ? new \DateTime($duedate) : null); $card->setStartdate($startdate ? new \DateTime($startdate) : null); - $card->setColor($color); + if ($color !== null) { + $colorValue = $color->getValue(); + $card->setColor(is_string($colorValue) && $colorValue !== '' ? $colorValue : null); + } $resetDuedateNotification = false; if ( $card->getDuedate() === null diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 840cdd6fc1..58084a1fa4 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -37,6 +37,7 @@ use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; use OCA\Deck\Model\CardDetails; +use OCA\Deck\Model\OptionalNullableValue; use OCA\Deck\Notification\NotificationHelper; use OCA\Deck\StatusException; use OCA\Deck\Validators\CardServiceValidator; @@ -368,7 +369,7 @@ public function testUpdate() { ->method('find') ->with(234) ->willReturn($stack); - $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null, null, null, null, 'ffffff'); + $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null, null, null, null, new OptionalNullableValue('ffffff')); $this->assertEquals('newtitle', $actual->getTitle()); $this->assertEquals(234, $actual->getStackId()); $this->assertEquals('text', $actual->getType()); @@ -378,6 +379,104 @@ public function testUpdate() { $this->assertEquals('ffffff', $actual->getColor()); } + public function testUpdateKeepsColorWhenOmitted() { + $card = Card::fromParams([ + 'title' => 'Card title', + 'archived' => 'false', + 'stackId' => 234, + 'color' => '00ff00', + ]); + $card->setColor('ff0000'); + $stack = Stack::fromParams([ + 'id' => 234, + 'boardId' => 1337, + ]); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { + $c->setId(1); + return $c; + }); + $this->stackMapper->expects($this->once()) + ->method('find') + ->with(234) + ->willReturn($stack); + $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null, null, null, null, null); + $this->assertSame('ff0000', $actual->getColor()); + } + + public function testUpdateClearsColorWhenNullProvided() { + $card = Card::fromParams([ + 'title' => 'Card title', + 'archived' => 'false', + 'stackId' => 234, + 'color' => '00ff00', + ]); + $card->setColor('ff0000'); + $stack = Stack::fromParams([ + 'id' => 234, + 'boardId' => 1337, + ]); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { + $c->setId(1); + return $c; + }); + $this->stackMapper->expects($this->once()) + ->method('find') + ->with(234) + ->willReturn($stack); + $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null, null, null, null, new OptionalNullableValue(null)); + $this->assertNull($actual->getColor()); + } + + public function testUpdateClearsColorWhenEmptyStringProvided() { + $card = Card::fromParams([ + 'title' => 'Card title', + 'archived' => 'false', + 'stackId' => 234, + 'color' => '00ff00', + ]); + $card->setColor('ff0000'); + $stack = Stack::fromParams([ + 'id' => 234, + 'boardId' => 1337, + ]); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { + $c->setId(1); + return $c; + }); + $this->stackMapper->expects($this->once()) + ->method('find') + ->with(234) + ->willReturn($stack); + $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null, null, null, null, new OptionalNullableValue('')); + $this->assertNull($actual->getColor()); + } + + public function testUpdateSetsColor() { + $card = Card::fromParams([ + 'title' => 'Card title', + 'archived' => 'false', + 'stackId' => 234, + ]); + $stack = Stack::fromParams([ + 'id' => 234, + 'boardId' => 1337, + ]); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { + $c->setId(1); + return $c; + }); + $this->stackMapper->expects($this->once()) + ->method('find') + ->with(234) + ->willReturn($stack); + $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null, null, null, null, new OptionalNullableValue('00ff00')); + $this->assertSame('00ff00', $actual->getColor()); + } + public function testUpdateWithStartdate() { $card = Card::fromParams([ 'title' => 'Card title',