[stable33] fix(cards): correctly copy label and description when cloning cards

Signed-off-by: Luka Trovic <luka@nextcloud.com>

Author: luka-nextcloud

lib/Service/CardService.php

@@ -374,8 +374,11 @@ public function cloneCard(int $id, ?int $targetStackId = null):Card {
 			}
 			$this->assignmentService->assignUser($newCard->getId(), $assignement->getParticipant());
 		}
-		$newCard->setDescription($originCard->getDescription());
-		$card = $this->enrichCards([$this->cardMapper->update($newCard)]);
+
+		$freshCard = $this->cardMapper->find($newCard->getId());
+		$freshCard->setDescription($originCard->getDescription());
+		$card = $this->enrichCards([$this->cardMapper->update($freshCard)]);
+
 		return $card[0];
 	}
 

lib/Service/LabelService.php

@@ -17,29 +17,13 @@
 
 class LabelService {
 
-	/** @var LabelMapper */
-	private $labelMapper;
-	/** @var PermissionService */
-	private $permissionService;
-	/** @var BoardService */
-	private $boardService;
-	/** @var ChangeHelper */
-	private $changeHelper;
-	/** @var LabelServiceValidator */
-	private LabelServiceValidator $labelServiceValidator;
-
 	public function __construct(
-		LabelMapper $labelMapper,
-		PermissionService $permissionService,
-		BoardService $boardService,
-		ChangeHelper $changeHelper,
-		LabelServiceValidator $labelServiceValidator,
+		private LabelMapper $labelMapper,
+		private PermissionService $permissionService,
+		private BoardService $boardService,
+		private ChangeHelper $changeHelper,
+		private LabelServiceValidator $labelServiceValidator,
 	) {
-		$this->labelMapper = $labelMapper;
-		$this->permissionService = $permissionService;
-		$this->boardService = $boardService;
-		$this->changeHelper = $changeHelper;
-		$this->labelServiceValidator = $labelServiceValidator;
 	}
 
 	/**
@@ -90,10 +74,10 @@ public function cloneLabelIfNotExists(int $labelId, int $targetBoardId): Label {
 		$originLabel = $this->find($labelId);
 		$filteredValues = array_values(array_filter($boardLabels, fn ($item) => $item->getTitle() === $originLabel->getTitle()));
 		if (empty($filteredValues)) {
-			$label = $this->create($originLabel->getTitle(), $originLabel->getColor(), $targetBoardId);
-			return $label;
+			return $this->create($originLabel->getTitle(), $originLabel->getColor(), $targetBoardId);
 		}
-		return $originLabel;
+
+		return $filteredValues[0];
 	}
 
 	/**

tests/unit/Service/CardServiceTest.php

@@ -250,18 +250,22 @@ public function testClone() {
 		$card->setOrder(0);
 		$card->setOwner('admin');
 		$card->setStackId(12345);
+		$card->setDescription('A test description');
+
 		$clonedCard = clone $card;
 		$clonedCard->setId(2);
 		$clonedCard->setStackId(1234);
+
 		$this->cardMapper->expects($this->exactly(2))
 			->method('insert')
 			->willReturn($card, $clonedCard);
 
 		$this->cardMapper->expects($this->once())
 			->method('update')->willReturn($clonedCard);
-		$this->cardMapper->expects($this->exactly(2))
+
+		$this->cardMapper->expects($this->exactly(3))
 			->method('find')
-			->willReturn($card, $clonedCard);
+			->willReturn($card, $clonedCard, $clonedCard);
 
 		$this->cardMapper->expects($this->any())
 			->method('findBoardId')
@@ -287,6 +291,10 @@ public function testClone() {
 			->with(1)
 			->willReturn([$a1]);
 
+		$this->assignedUsersMapper->expects($this->any())
+			->method('findIn')
+			->willReturn([]);
+
 		// check if labels get cloned
 		$label = new Label();
 		$label->setId(1);
@@ -297,16 +305,31 @@ public function testClone() {
 			->method('assignLabel')
 			->with($clonedCard->getId(), $label->getId());
 
+		$labelForClone = Label::fromRow([
+			'id' => 1,
+			'boardId' => 1234,
+			'cardId' => 2,
+		]);
+		$this->labelMapper->expects($this->any())
+			->method('findAssignedLabelsForCards')
+			->willReturn([$labelForClone]);
+
 		$stackMock = new Stack();
 		$stackMock->setBoardId(1234);
 		$this->stackMapper->expects($this->any())
 			->method('find')
 			->willReturn($stackMock);
+
 		$b = $this->cardService->create('Card title', 123, 'text', 999, 'admin');
 		$c = $this->cardService->cloneCard($b->getId(), 1234);
 		$this->assertEquals($b->getTitle(), $c->getTitle());
 		$this->assertEquals($b->getOwner(), $c->getOwner());
 		$this->assertNotEquals($b->getStackId(), $c->getStackId());
+
+		$this->assertEquals('A test description', $c->getDescription());
+
+		$this->assertCount(1, $c->getLabels());
+		$this->assertEquals($label->getId(), $c->getLabels()[0]->getId());
 	}
 
 	public function testDelete() {