fixes according to review

ecamp · Jul 10, 2024 · d080c5f · d080c5f
1 parent 6af1f65
commit d080c5f
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 50 deletions.
diff --git a/api/migrations/schema/Version20240620104153.php b/api/migrations/schema/Version20240620104153.php
@@ -17,16 +17,18 @@ public function getDescription(): string {
 
     public function up(Schema $schema): void {
         // this up() migration is auto-generated, please modify it to your needs
+        $this->addSql('CREATE UNIQUE INDEX checklistitem_checklistid_parentid_position_unique ON checklist_item (checklistid, parentid, position)');
         $this->addSql('CREATE TABLE checklistnode_checklistitem (checklistnode_id VARCHAR(16) NOT NULL, checklistitem_id VARCHAR(16) NOT NULL, PRIMARY KEY(checklistnode_id, checklistitem_id))');
         $this->addSql('CREATE INDEX IDX_5A2B5B31DE6B6F00 ON checklistnode_checklistitem (checklistnode_id)');
         $this->addSql('CREATE INDEX IDX_5A2B5B318A09A289 ON checklistnode_checklistitem (checklistitem_id)');
-        $this->addSql('ALTER TABLE checklistnode_checklistitem ADD CONSTRAINT FK_5A2B5B31DE6B6F00 FOREIGN KEY (checklistnode_id) REFERENCES content_node (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
-        $this->addSql('ALTER TABLE checklistnode_checklistitem ADD CONSTRAINT FK_5A2B5B318A09A289 FOREIGN KEY (checklistitem_id) REFERENCES checklist_item (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
+        $this->addSql('ALTER TABLE checklistnode_checklistitem ADD CONSTRAINT FK_5A2B5B31DE6B6F00 FOREIGN KEY (checklistnode_id) REFERENCES content_node (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE');
+        $this->addSql('ALTER TABLE checklistnode_checklistitem ADD CONSTRAINT FK_5A2B5B318A09A289 FOREIGN KEY (checklistitem_id) REFERENCES checklist_item (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE');
     }
 
     public function down(Schema $schema): void {
         // this down() migration is auto-generated, please modify it to your needs
         $this->addSql('CREATE SCHEMA public');
+        $this->addSql('DROP INDEX checklistitem_checklistid_parentid_position_unique');
         $this->addSql('ALTER TABLE checklistnode_checklistitem DROP CONSTRAINT FK_5A2B5B31DE6B6F00');
         $this->addSql('ALTER TABLE checklistnode_checklistitem DROP CONSTRAINT FK_5A2B5B318A09A289');
         $this->addSql('DROP TABLE checklistnode_checklistitem');

diff --git a/api/migrations/schema/Version20240621195713.php b/api/migrations/schema/Version20240621195713.php
diff --git a/api/src/Entity/ChecklistItem.php b/api/src/Entity/ChecklistItem.php
@@ -67,13 +67,15 @@
 )]
 #[ApiFilter(filterClass: SearchFilter::class, properties: ['checklist'])]
 #[ORM\Entity(repositoryClass: ChecklistItemRepository::class)]
+#[ORM\UniqueConstraint(name: 'checklistitem_checklistid_parentid_position_unique', columns: ['checklistid', 'parentid', 'position'])]
 class ChecklistItem extends BaseEntity implements BelongsToCampInterface, CopyFromPrototypeInterface, HasParentInterface {
     public const CHECKLIST_SUBRESOURCE_URI_TEMPLATE = '/checklists/{checklistId}/checklist_items.{_format}';
 
     /**
      * The Checklist this Item belongs to.
      */
     #[ApiProperty(example: '/checklists/1a2b3c4d')]
+    #[Gedmo\SortableGroup]
     #[Groups(['read', 'create'])]
     #[ORM\ManyToOne(targetEntity: Checklist::class, inversedBy: 'checklistItems')]
     #[ORM\JoinColumn(nullable: false, onDelete: 'cascade')]

diff --git a/api/src/State/ContentNode/ChecklistNodePersistProcessor.php b/api/src/State/ContentNode/ChecklistNodePersistProcessor.php
@@ -25,13 +25,19 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
         if (null !== $data->addChecklistItemIds) {
             foreach ($data->addChecklistItemIds as $checklistItemId) {
                 $checklistItem = $this->checklistItemRepository->find($checklistItemId);
-                $data->addChecklistItem($checklistItem);
+                if (null != $checklistItem) {
+                    // if a checklistItem does not exists, do not add it
+                    $data->addChecklistItem($checklistItem);
+                }
             }
         }
         if (null !== $data->removeChecklistItemIds) {
             foreach ($data->removeChecklistItemIds as $checklistItemId) {
                 $checklistItem = $this->checklistItemRepository->find($checklistItemId);
-                $data->removeChecklistItem($checklistItem);
+                if (null != $checklistItem) {
+                    // if a checklistItem no longer exists, it does not have to be removed
+                    $data->removeChecklistItem($checklistItem);
+                }
             }
         }
 

diff --git a/api/tests/Api/ChecklistItems/CreateChecklistItemTest.php b/api/tests/Api/ChecklistItems/CreateChecklistItemTest.php
@@ -66,14 +66,14 @@ public function testCreateChecklistItemIsAllowedForMember() {
         ;
 
         $this->assertResponseStatusCodeSame(201);
-        $this->assertJsonContains($this->getExampleReadPayload(['position' => 5]));
+        $this->assertJsonContains($this->getExampleReadPayload(['position' => 2]));
     }
 
     public function testCreateChecklistItemIsAllowedForManager() {
         static::createClientWithCredentials()->request('POST', '/checklist_items', ['json' => $this->getExampleWritePayload()]);
 
         $this->assertResponseStatusCodeSame(201);
-        $this->assertJsonContains($this->getExampleReadPayload(['position' => 5]));
+        $this->assertJsonContains($this->getExampleReadPayload(['position' => 2]));
     }
 
     public function testCreateChecklistItemInCampPrototypeIsDeniedForUnrelatedUser() {
@@ -181,7 +181,7 @@ public function testCreateChecklistItemTrimsText() {
         $this->assertJsonContains($this->getExampleReadPayload(
             [
                 'text' => 'Ziel 1',
-                'position' => 5,
+                'position' => 2,
             ]
         ));
     }
@@ -203,7 +203,7 @@ public function testCreateChecklistItemCleansForbiddenCharactersFromText() {
         $this->assertJsonContains($this->getExampleReadPayload(
             [
                 'text' => '<b>Ziel 1',
-                'position' => 5,
+                'position' => 2,
             ]
         ));
     }

diff --git a/api/tests/Api/ContentNodes/ChecklistNode/CreateChecklistNodeTest.php b/api/tests/Api/ContentNodes/ChecklistNode/CreateChecklistNodeTest.php
@@ -22,14 +22,8 @@ public function setUp(): void {
      */
     public function getExampleWritePayload($attributes = [], $except = []) {
         return parent::getExampleWritePayload(
-            array_merge(
-                [
-                    'addChecklistItemIds' => null,
-                    'removeChecklistItemIds' => null,
-                ],
-                $attributes
-            ),
-            $except
+            $attributes,
+            array_merge(['addChecklistItemIds', 'removeChecklistItemIds'], $except)
         );
     }
 }