Skip to content

Commit 7f2d27e

Browse files
JaggobJaggob
committed
fix(dav): limit auto-created labels from categories
Signed-off-by: Jaggob <37583151+Jaggob@users.noreply.github.com>
1 parent 7ed31bd commit 7f2d27e

2 files changed

Lines changed: 194 additions & 5 deletions

File tree

‎lib/DAV/DeckCalendarBackend.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
namespace OCA\Deck\DAV;
1212

13+
use OCA\Deck\Db\Acl;
1314
use OCA\Deck\Db\Board;
1415
use OCA\Deck\Db\BoardMapper;
1516
use OCA\Deck\Db\Card;
@@ -28,6 +29,8 @@
2829

2930
class DeckCalendarBackend {
3031

32+
private const MAX_AUTO_CREATED_LABELS_PER_SYNC = 5;
33+
3134
/** @var BoardService */
3235
private $boardService;
3336
/** @var StackService */
@@ -701,6 +704,7 @@ private function syncCardCategories(int $cardId, array $categories): void {
701704
$card = $this->cardService->find($cardId);
702705
$boardId = $this->getBoardIdForCard($card);
703706
$board = $this->boardMapper->find($boardId, true, false);
707+
$canCreateLabels = $this->checkBoardPermission($boardId, Acl::PERMISSION_MANAGE);
704708
$boardLabels = $board->getLabels() ?? [];
705709

706710
$boardLabelsByTitle = [];
@@ -712,13 +716,17 @@ private function syncCardCategories(int $cardId, array $categories): void {
712716
}
713717

714718
$targetLabelIds = [];
719+
$createdLabels = 0;
715720
foreach ($categories as $category) {
716721
$title = trim($category);
717722
$key = mb_strtolower($title);
718723
if ($key === '' || !isset($boardLabelsByTitle[$key])) {
719-
$createdLabel = $this->createLabelForCategory($boardId, $title);
720-
if ($createdLabel !== null) {
721-
$boardLabelsByTitle[$key] = $createdLabel;
724+
if ($canCreateLabels && $createdLabels < self::MAX_AUTO_CREATED_LABELS_PER_SYNC) {
725+
$createdLabel = $this->createLabelForCategory($boardId, $title);
726+
if ($createdLabel !== null) {
727+
$boardLabelsByTitle[$key] = $createdLabel;
728+
$createdLabels++;
729+
}
722730
}
723731
}
724732
if (!isset($boardLabelsByTitle[$key])) {

‎tests/unit/DAV/DeckCalendarBackendTest.php‎

Lines changed: 183 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,14 @@ class DeckCalendarBackendTest extends TestCase {
2828
private StackService $stackService;
2929
private BoardMapper $boardMapper;
3030
private LabelService $labelService;
31+
private PermissionService $permissionService;
3132

3233
public function setUp(): void {
3334
parent::setUp();
3435
$boardService = $this->createMock(BoardService::class);
3536
$this->stackService = $this->createMock(StackService::class);
3637
$this->cardService = $this->createMock(CardService::class);
37-
$permissionService = $this->createMock(PermissionService::class);
38+
$this->permissionService = $this->createMock(PermissionService::class);
3839
$this->boardMapper = $this->createMock(BoardMapper::class);
3940
$this->labelService = $this->createMock(LabelService::class);
4041
$configService = $this->createMock(ConfigService::class);
@@ -45,7 +46,7 @@ public function setUp(): void {
4546
$boardService,
4647
$this->stackService,
4748
$this->cardService,
48-
$permissionService,
49+
$this->permissionService,
4950
$this->boardMapper,
5051
$this->labelService,
5152
$configService
@@ -821,6 +822,12 @@ public function testUpdateCardSyncsAppleTagsToLabels(): void {
821822
->method('removeLabel');
822823
$this->labelService->expects($this->never())
823824
->method('create');
825+
$this->permissionService->expects($this->once())
826+
->method('getPermissions')
827+
->with(12)
828+
->willReturn([
829+
\OCA\Deck\Db\Acl::PERMISSION_MANAGE => true,
830+
]);
824831

825832
$calendarData = <<<ICS
826833
BEGIN:VCALENDAR
@@ -832,6 +839,180 @@ public function testUpdateCardSyncsAppleTagsToLabels(): void {
832839
X-APPLE-TAGS:Test
833840
END:VTODO
834841
END:VCALENDAR
842+
ICS;
843+
844+
$this->backend->updateCalendarObject($sourceCard, $calendarData);
845+
}
846+
847+
public function testUpdateCardDoesNotAutoCreateLabelsWithoutManagePermission(): void {
848+
$sourceCard = new Card();
849+
$sourceCard->setId(123);
850+
851+
$existingCard = new Card();
852+
$existingCard->setId(123);
853+
$existingCard->setTitle('Card');
854+
$existingCard->setDescription('Old description');
855+
$existingCard->setStackId(42);
856+
$existingCard->setType('plain');
857+
$existingCard->setOrder(0);
858+
$existingCard->setOwner('admin');
859+
$existingCard->setDeletedAt(0);
860+
$existingCard->setArchived(false);
861+
$existingCard->setDone(null);
862+
$existingCard->setLabels([]);
863+
864+
$updatedCard = new Card();
865+
$updatedCard->setId(123);
866+
867+
$this->cardService->expects($this->exactly(2))
868+
->method('find')
869+
->with(123)
870+
->willReturnOnConsecutiveCalls($existingCard, $existingCard);
871+
872+
$currentStack = new Stack();
873+
$currentStack->setId(42);
874+
$currentStack->setBoardId(12);
875+
$this->stackService->expects($this->exactly(2))
876+
->method('find')
877+
->with(42)
878+
->willReturn($currentStack);
879+
880+
$this->cardService->expects($this->once())
881+
->method('update')
882+
->willReturn($updatedCard);
883+
884+
$board = new Board();
885+
$board->setId(12);
886+
$board->setLabels([]);
887+
888+
$this->boardMapper->expects($this->once())
889+
->method('find')
890+
->with(12, true, false)
891+
->willReturn($board);
892+
893+
$this->permissionService->expects($this->once())
894+
->method('getPermissions')
895+
->with(12)
896+
->willReturn([
897+
\OCA\Deck\Db\Acl::PERMISSION_MANAGE => false,
898+
]);
899+
900+
$this->labelService->expects($this->never())
901+
->method('create');
902+
$this->cardService->expects($this->never())
903+
->method('assignLabel');
904+
$this->cardService->expects($this->never())
905+
->method('removeLabel');
906+
907+
$calendarData = <<<ICS
908+
BEGIN:VCALENDAR
909+
VERSION:2.0
910+
BEGIN:VTODO
911+
UID:deck-card-123
912+
SUMMARY:Card
913+
DESCRIPTION:Updated description
914+
CATEGORIES:Alpha,Beta
915+
END:VTODO
916+
END:VCALENDAR
917+
ICS;
918+
919+
$this->backend->updateCalendarObject($sourceCard, $calendarData);
920+
}
921+
922+
public function testUpdateCardLimitsAutoCreatedLabelsPerSync(): void {
923+
$sourceCard = new Card();
924+
$sourceCard->setId(123);
925+
926+
$existingCard = new Card();
927+
$existingCard->setId(123);
928+
$existingCard->setTitle('Card');
929+
$existingCard->setDescription('Old description');
930+
$existingCard->setStackId(42);
931+
$existingCard->setType('plain');
932+
$existingCard->setOrder(0);
933+
$existingCard->setOwner('admin');
934+
$existingCard->setDeletedAt(0);
935+
$existingCard->setArchived(false);
936+
$existingCard->setDone(null);
937+
$existingCard->setLabels([]);
938+
939+
$updatedCard = new Card();
940+
$updatedCard->setId(123);
941+
942+
$this->cardService->expects($this->exactly(2))
943+
->method('find')
944+
->with(123)
945+
->willReturnOnConsecutiveCalls($existingCard, $existingCard);
946+
947+
$currentStack = new Stack();
948+
$currentStack->setId(42);
949+
$currentStack->setBoardId(12);
950+
$this->stackService->expects($this->exactly(2))
951+
->method('find')
952+
->with(42)
953+
->willReturn($currentStack);
954+
955+
$this->cardService->expects($this->once())
956+
->method('update')
957+
->willReturn($updatedCard);
958+
959+
$board = new Board();
960+
$board->setId(12);
961+
$board->setLabels([]);
962+
963+
$this->boardMapper->expects($this->once())
964+
->method('find')
965+
->with(12, true, false)
966+
->willReturn($board);
967+
968+
$this->permissionService->expects($this->once())
969+
->method('getPermissions')
970+
->with(12)
971+
->willReturn([
972+
\OCA\Deck\Db\Acl::PERMISSION_MANAGE => true,
973+
]);
974+
975+
$labels = [];
976+
for ($i = 1; $i <= 5; $i++) {
977+
$label = new Label();
978+
$label->setId($i);
979+
$label->setTitle('Tag ' . $i);
980+
$labels[] = $label;
981+
}
982+
983+
$this->labelService->expects($this->exactly(5))
984+
->method('create')
985+
->withConsecutive(
986+
['Tag 1', '31CC7C', 12],
987+
['Tag 2', '31CC7C', 12],
988+
['Tag 3', '31CC7C', 12],
989+
['Tag 4', '31CC7C', 12],
990+
['Tag 5', '31CC7C', 12],
991+
)
992+
->willReturnOnConsecutiveCalls(...$labels);
993+
994+
$this->cardService->expects($this->exactly(5))
995+
->method('assignLabel')
996+
->withConsecutive(
997+
[123, 1],
998+
[123, 2],
999+
[123, 3],
1000+
[123, 4],
1001+
[123, 5],
1002+
);
1003+
$this->cardService->expects($this->never())
1004+
->method('removeLabel');
1005+
1006+
$calendarData = <<<ICS
1007+
BEGIN:VCALENDAR
1008+
VERSION:2.0
1009+
BEGIN:VTODO
1010+
UID:deck-card-123
1011+
SUMMARY:Card
1012+
DESCRIPTION:Updated description
1013+
CATEGORIES:Tag 1,Tag 2,Tag 3,Tag 4,Tag 5,Tag 6
1014+
END:VTODO
1015+
END:VCALENDAR
8351016
ICS;
8361017

8371018
$this->backend->updateCalendarObject($sourceCard, $calendarData);

0 commit comments

Comments
 (0)