Merge pull request #6933 from nextcloud/fix/label-card-limit

fix: Ensure consistent label mappings

appinfo/info.xml

@@ -54,6 +54,9 @@
         <live-migration>
             <step>OCA\Deck\Migration\DeletedCircleCleanup</step>
         </live-migration>
+        <post-migration>
+            <step>OCA\Deck\Migration\LabelMismatchCleanup</step>
+        </post-migration>
     </repair-steps>
     <commands>
         <command>OCA\Deck\Command\UserExport</command>

lib/Migration/LabelMismatchCleanup.php

@@ -0,0 +1,78 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\Deck\Migration;
+
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class LabelMismatchCleanup implements IRepairStep {
+
+	public function __construct(
+		private IDBConnection $db,
+	) {
+	}
+
+	public function getName() {
+		return 'Migrate labels with wrong board mapping';
+	}
+
+	public function run(IOutput $output) {
+		// Find assingments where a label of another (wrong) board is used
+		$qb = $this->db->getQueryBuilder();
+		$qb->select('al.id', 'al.label_id', 'al.card_id', 's.board_id as actual_board_id', 'l.board_id as wrong_id', 'l.color', 'l.title')
+			->from('deck_assigned_labels', 'al')
+			->innerJoin('al', 'deck_cards', 'c', 'c.id = al.card_id')
+			->innerJoin('c', 'deck_stacks', 's', 'c.stack_id = s.id')
+			->innerJoin('al', 'deck_labels', 'l', 'l.id = al.label_id')
+			->where($qb->expr()->neq('l.board_id', 's.board_id'));
+
+		$labels = $qb->executeQUery()->fetchAll();
+		if (count($labels) === 0) {
+			return;
+		}
+
+		$output->info('Found ' . count($labels) . ' labels with wrong board mapping');
+
+		foreach ($labels as $label) {
+			// Select existing label on the correct board
+			$qb = $this->db->getQueryBuilder();
+			$qb->select('id')
+				->from('deck_labels')
+				->where($qb->expr()->eq('title', $qb->createNamedParameter($label['title'])))
+				->andWhere($qb->expr()->eq('color', $qb->createNamedParameter($label['color'])))
+				->andWhere($qb->expr()->eq('board_id', $qb->createNamedParameter($label['actual_board_id'])));
+			$result = $qb->executeQuery();
+			$newLabel = $result->fetchOne();
+			$result->closeCursor();
+
+			if (!$newLabel) {
+				// Create a new label with the same title and color on the correct board
+				$qb = $this->db->getQueryBuilder();
+				$qb->insert('deck_labels')
+					->values([
+						'title' => $qb->createNamedParameter($label['title']),
+						'color' => $qb->createNamedParameter($label['color']),
+						'board_id' => $qb->createNamedParameter($label['actual_board_id']),
+					]);
+				$qb->executeStatement();
+				$newLabel = $qb->getLastInsertId();
+				$output->debug('Created new label ' . $label['title'] . ' on board ' . $label['actual_board_id']);
+			} else {
+				$output->debug('Found existing label ' . $label['title'] . ' on board ' . $label['actual_board_id']);
+			}
+
+			// Update the assignment to use the new label
+			$qb = $this->db->getQueryBuilder();
+			$qb->update('deck_assigned_labels')
+				->set('label_id', $qb->createNamedParameter($newLabel))
+				->where($qb->expr()->eq('id', $qb->createNamedParameter($label['id'])));
+			$qb->executeStatement();
+			$output->debug('Updated label assignment ' . $label['id'] . ' to use label ' . $newLabel);
+		}
+	}
+}

lib/Service/CardService.php

@@ -609,8 +609,9 @@ class CardService {
 	public function assignLabel($cardId, $labelId) {
 		$this->cardServiceValidator->check(compact('cardId', 'labelId'));
 
-
 		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
+		$this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ);
+
 		if ($this->boardService->isArchived($this->cardMapper, $cardId)) {
 			throw new StatusException('Operation not allowed. This board is archived.');
 		}
@@ -619,6 +620,9 @@ class CardService {
 			throw new StatusException('Operation not allowed. This card is archived.');
 		}
 		$label = $this->labelMapper->find($labelId);
+		if ($label->getBoardId() !== $this->cardMapper->findBoardId($card->getId())) {
+			throw new StatusException('Operation not allowed. Label does not exist.');
+		}
 		$this->cardMapper->assignLabel($cardId, $labelId);
 		$this->changeHelper->cardChanged($cardId);
 		$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_ASSIGN, ['label' => $label]);
@@ -640,6 +644,8 @@ class CardService {
 
 
 		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
+		$this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ);
+
 		if ($this->boardService->isArchived($this->cardMapper, $cardId)) {
 			throw new StatusException('Operation not allowed. This board is archived.');
 		}
@@ -648,6 +654,9 @@ class CardService {
 			throw new StatusException('Operation not allowed. This card is archived.');
 		}
 		$label = $this->labelMapper->find($labelId);
+		if ($label->getBoardId() !== $this->cardMapper->findBoardId($card->getId())) {
+			throw new StatusException('Operation not allowed. Label does not exist.');
+		}
 		$this->cardMapper->removeLabel($cardId, $labelId);
 		$this->changeHelper->cardChanged($cardId);
 		$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_UNASSING, ['label' => $label]);

tests/unit/Service/CardServiceTest.php

@@ -258,6 +258,17 @@ class CardServiceTest extends TestCase {
 			->method('find')
 			->willReturn($card, $clonedCard);
 
+		$this->cardMapper->expects($this->any())
+			->method('findBoardId')
+			->willReturn(1234);
+
+		$this->labelMapper->expects($this->any())
+			->method('find')
+			->willReturn(Label::fromRow([
+				'id' => 1,
+				'boardId' => 1234,
+			]));
+
 		// check if users are assigned
 		$this->assignmentService->expects($this->once())
 			->method('assignUser')
@@ -433,8 +444,17 @@ class CardServiceTest extends TestCase {
 	public function testAssignLabel() {
 		$card = new Card();
 		$card->setArchived(false);
+		$card->setId(123);
+		$label = new Label();
+		$label->setBoardId(1);
 		$this->cardMapper->expects($this->once())->method('find')->willReturn($card);
 		$this->cardMapper->expects($this->once())->method('assignLabel');
+		$this->cardMapper->expects($this->once())
+			->method('findBoardId')
+			->willReturn(1);
+		$this->labelMapper->expects($this->once())
+			->method('find')
+			->willReturn($label);
 		$this->cardService->assignLabel(123, 999);
 	}
 
@@ -450,8 +470,17 @@ class CardServiceTest extends TestCase {
 	public function testRemoveLabel() {
 		$card = new Card();
 		$card->setArchived(false);
+		$card->setId(123);
+		$label = new Label();
+		$label->setBoardId(1);
 		$this->cardMapper->expects($this->once())->method('find')->willReturn($card);
 		$this->cardMapper->expects($this->once())->method('removeLabel');
+		$this->cardMapper->expects($this->once())
+			->method('findBoardId')
+			->willReturn(1);
+		$this->labelMapper->expects($this->once())
+			->method('find')
+			->willReturn($label);
 		$this->cardService->removeLabel(123, 999);
 	}