Merge pull request #7217 from pmattmann/feature/materialitem-nullable

pmattmann · web-flow · commit 4233aeeca529 · 2025-04-13T12:20:22.000Z
MaterialItem.MaterialList is nullable
diff --git a/api/migrations/schema/Version20250412165829.php b/api/migrations/schema/Version20250412165829.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Auto-generated Migration: Please modify to your needs!
+ */
+final class Version20250412165829 extends AbstractMigration {
+    public function getDescription(): string {
+        return 'MaterialItem.MaterialList is nullable';
+    }
+
+    public function up(Schema $schema): void {
+        // this up() migration is auto-generated, please modify it to your needs
+        $this->addSql(<<<'SQL'
+            ALTER TABLE material_item ALTER materialListId DROP NOT NULL
+        SQL);
+    }
+
+    public function down(Schema $schema): void {
+        // this down() migration is auto-generated, please modify it to your needs
+        $this->addSql(<<<'SQL'
+            ALTER TABLE material_item ALTER materiallistid SET NOT NULL
+        SQL);
+    }
+}
diff --git a/api/src/Entity/ContentNode.php b/api/src/Entity/ContentNode.php
@@ -53,7 +53,7 @@
 #[ORM\InheritanceType('SINGLE_TABLE')]
 #[ORM\DiscriminatorColumn(name: 'strategy', type: 'string')]
 #[ORM\UniqueConstraint(name: 'contentnode_parentid_slot_position_unique', columns: ['parentid', 'slot', 'position'])]
-abstract class ContentNode extends BaseEntity implements BelongsToContentNodeTreeInterface, CopyFromPrototypeInterface, HasParentInterface {
+abstract class ContentNode extends BaseEntity implements BelongsToCampInterface, BelongsToContentNodeTreeInterface, CopyFromPrototypeInterface, HasParentInterface {
     use ClassInfoTrait;
 
     /**
@@ -193,6 +193,17 @@ public function setParent(?ContentNode $parent) {
         $this->root ??= $parent?->root;
     }
 
+    public function getCamp(): ?Camp {
+        if (null == $this->getRoot()) {
+            return null;
+        }
+        if ($this->getRoot()->campRootContentNodes->count() > 0) {
+            return $this->getRoot()->campRootContentNodes[0]->camp;
+        }
+
+        return null;
+    }
+
     /**
      * Holds the actual data of the content node.
      */
diff --git a/api/src/Entity/MaterialItem.php b/api/src/Entity/MaterialItem.php
@@ -23,6 +23,7 @@
 use Doctrine\Common\Collections\Collection;
 use Doctrine\ORM\Mapping as ORM;
 use Symfony\Component\Serializer\Annotation\Groups;
+use Symfony\Component\Serializer\Attribute\SerializedName;
 use Symfony\Component\Validator\Constraints as Assert;
 
 /**
@@ -44,6 +45,7 @@
             security: 'is_authenticated()'
         ),
         new Post(
+            denormalizationContext: ['groups' => ['write', 'create']],
             securityPostDenormalize: 'is_granted("CAMP_MEMBER", object) or is_granted("CAMP_MANAGER", object) or object.materialList === null'
         ),
     ],
@@ -58,11 +60,12 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface, CopyFro
      * The list to which this item belongs. Lists are used to keep track of who is
      * responsible to prepare and bring the item to the camp.
      */
-    #[AssertBelongsToSameCamp(compareToPrevious: true, groups: ['update'])]
+    #[Assert\NotNull]
+    #[AssertBelongsToSameCamp]
     #[ApiProperty(example: '/material_lists/1a2b3c4d')]
     #[Groups(['read', 'write'])]
     #[ORM\ManyToOne(targetEntity: MaterialList::class, inversedBy: 'materialItems')]
-    #[ORM\JoinColumn(nullable: false, onDelete: 'cascade')]
+    #[ORM\JoinColumn(nullable: true, onDelete: 'cascade')]
     public ?MaterialList $materialList = null;
 
     /**
@@ -131,9 +134,13 @@ public function __construct() {
         $this->periodMaterialItems = new ArrayCollection();
     }
 
-    #[ApiProperty(readable: false)]
+    #[ApiProperty]
+    #[SerializedName('camp')]
+    #[Groups(['read'])]
     public function getCamp(): ?Camp {
-        return $this->materialList?->getCamp();
+        return $this->period?->getCamp()
+            ?? $this->materialNode?->getCamp()
+            ?? $this->materialList?->getCamp();
     }
 
     /**
@@ -143,9 +150,14 @@ public function getCamp(): ?Camp {
     public function copyFromPrototype($prototype, $entityMap): void {
         $entityMap->add($prototype, $this);
 
-        /** @var MaterialList $materialList */
-        $materialList = $entityMap->get($prototype->materialList);
-        $materialList->addMaterialItem($this);
+        if (null != $prototype->materialList) {
+            /** @var MaterialList $materialList */
+            $materialList = $entityMap->get($prototype->materialList);
+
+            if ($entityMap->belongsToTargetCamp($materialList)) {
+                $materialList->addMaterialItem($this);
+            }
+        }
 
         $this->article = $prototype->article;
         $this->quantity = $prototype->quantity;
diff --git a/api/src/Repository/MaterialItemRepository.php b/api/src/Repository/MaterialItemRepository.php
@@ -24,7 +24,8 @@ public function __construct(ManagerRegistry $registry) {
     }
 
     public function filterByUser(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, User $user): void {
-        $materialList = QueryBuilderHelper::findOrAddInnerRootJoinAlias($queryBuilder, $queryNameGenerator, 'materialList');
-        $this->filterByCampCollaboration($queryBuilder, $user, "{$materialList}.camp");
+        $periodMaterialItems = QueryBuilderHelper::findOrAddInnerRootJoinAlias($queryBuilder, $queryNameGenerator, 'periodMaterialItems');
+        $period = QueryBuilderHelper::findOrAddInnerJoinAlias($queryBuilder, $queryNameGenerator, $periodMaterialItems, 'period');
+        $this->filterByCampCollaboration($queryBuilder, $user, "{$period}.camp");
     }
 }
diff --git a/api/src/State/ActivityCreateProcessor.php b/api/src/State/ActivityCreateProcessor.php
@@ -34,6 +34,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
             throw new \UnexpectedValueException('Property rootContentNode of provided category is of wrong type. Object of type '.ColumnLayout::class.' expected.');
         }
 
+        $targetCamp = $data->category->camp;
         $data->camp = $data->category->camp;
         $rootContentNodePrototype = $data->category->rootContentNode;
 
@@ -50,7 +51,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
         $data->setRootContentNode($rootContentNode);
 
         // deep copy from category root node
-        $entityMap = new EntityMap();
+        $entityMap = new EntityMap($targetCamp);
         $rootContentNode->copyFromPrototype($rootContentNodePrototype, $entityMap);
 
         return $data;
diff --git a/api/src/State/CampCreateProcessor.php b/api/src/State/CampCreateProcessor.php
@@ -37,7 +37,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
 
         // copy from prototype, if given
         if (isset($data->campPrototype)) {
-            $entityMap = new EntityMap();
+            $entityMap = new EntityMap($data);
             $data->copyFromPrototype($data->campPrototype, $entityMap);
         }
 
diff --git a/api/src/State/CategoryCreateProcessor.php b/api/src/State/CategoryCreateProcessor.php
@@ -38,7 +38,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
 
         if (isset($data->copyCategorySource)) {
             // CopyActivity Source is set -> copy it's content (rootContentNode)
-            $entityMap = new EntityMap();
+            $entityMap = new EntityMap($data->camp);
             $rootContentNode->copyFromPrototype($data->copyCategorySource->getRootContentNode(), $entityMap);
         }
 
diff --git a/api/src/State/ChecklistCreateProcessor.php b/api/src/State/ChecklistCreateProcessor.php
@@ -22,7 +22,7 @@ public function __construct(ProcessorInterface $decorated) {
     public function onBefore($data, Operation $operation, array $uriVariables = [], array $context = []): Checklist {
         if (isset($data->copyChecklistSource)) {
             // CopyChecklist Source is set -> copy it's content
-            $entityMap = new EntityMap();
+            $entityMap = new EntityMap($data->camp);
             $data->copyFromPrototype($data->copyChecklistSource, $entityMap);
         }
 
diff --git a/api/src/Util/EntityMap.php b/api/src/Util/EntityMap.php
@@ -3,12 +3,16 @@
 namespace App\Util;
 
 use App\Entity\BaseEntity;
+use App\Entity\BelongsToCampInterface;
+use App\Entity\Camp;
 
 class EntityMap {
     use ClassInfoTrait;
 
     private $map = [];
 
+    public function __construct(private Camp $targetCamp) {}
+
     public function add(BaseEntity $prototype, BaseEntity $entity) {
         $key = $this->getObjectClass($prototype).'#'.$prototype->getId();
         $this->map[$key] = $entity;
@@ -20,4 +24,8 @@ public function get(BaseEntity $prototype): BaseEntity {
 
         return $keyExists ? $this->map[$key] : $prototype;
     }
+
+    public function belongsToTargetCamp(BelongsToCampInterface $entity) {
+        return $entity->getCamp() == $this->targetCamp;
+    }
 }
diff --git a/api/tests/Api/Activities/CreateActivityTest.php b/api/tests/Api/Activities/CreateActivityTest.php
@@ -519,7 +519,7 @@ public function testCreateActivityFromCopySourceWithinSameCamp() {
     }
 
     public function testCreateActivityFromCopySourceAcrossCamp() {
-        static::createClientWithCredentials()->request(
+        $response = static::createClientWithCredentials()->request(
             'POST',
             '/activities',
             ['json' => $this->getExampleWritePayload(
@@ -540,6 +540,21 @@ public function testCreateActivityFromCopySourceAcrossCamp() {
 
         // Activity created
         $this->assertResponseStatusCodeSame(201);
+
+        // Embedded MaterialNode -> MaterialItems
+        // Test MaterialList is nulled
+        $responseArray = $response->toArray();
+        $contentNodes = $responseArray['_embedded']['contentNodes'];
+
+        $materialNodes = array_filter($contentNodes, fn ($cn) => 'Material' == $cn['contentTypeName']);
+        $this->assertCount(1, $materialNodes);
+
+        $materialNode = reset($materialNodes);
+        $materialItems = $materialNode['_embedded']['materialItems'];
+        $this->assertCount(1, $materialItems);
+
+        $materailItem = reset($materialItems);
+        $this->assertNull($materailItem['_links']['materialList']);
     }
 
     /**
diff --git a/api/tests/Api/MaterialItems/CreateMaterialItemTest.php b/api/tests/Api/MaterialItems/CreateMaterialItemTest.php
@@ -205,7 +205,7 @@ public function testCreateMaterialItemValidatesPeriodFromDifferentCamp() {
         $this->assertJsonContains([
             'violations' => [
                 [
-                    'propertyPath' => 'period',
+                    'propertyPath' => 'materialList',
                     'message' => 'Must belong to the same camp.',
                 ],
             ],
@@ -222,7 +222,7 @@ public function testCreateMaterialItemValidatesMaterialNodeFromDifferentCamp() {
         $this->assertJsonContains([
             'violations' => [
                 [
-                    'propertyPath' => 'materialNode',
+                    'propertyPath' => 'materialList',
                     'message' => 'Must belong to the same camp.',
                 ],
             ],
diff --git a/api/tests/Api/MaterialItems/UpdateMaterialItemTest.php b/api/tests/Api/MaterialItems/UpdateMaterialItemTest.php
@@ -151,9 +151,9 @@ public function testPatchMaterialItemValidatesMissingMaterialList() {
             'materialList' => null,
         ], 'headers' => ['Content-Type' => 'application/merge-patch+json']]);
 
-        $this->assertResponseStatusCodeSame(400);
+        $this->assertResponseStatusCodeSame(422);
         $this->assertJsonContains([
-            'detail' => 'The type of the "materialList" attribute must be "array" (nested document) or "string" (IRI), "NULL" given.',
+            'detail' => 'materialList: This value should not be null.',
         ]);
     }
 
@@ -244,7 +244,7 @@ public function testPatchMaterialItemValidatesPeriodFromDifferentCamp() {
         $this->assertJsonContains([
             'violations' => [
                 [
-                    'propertyPath' => 'period',
+                    'propertyPath' => 'materialList',
                     'message' => 'Must belong to the same camp.',
                 ],
             ],
@@ -261,7 +261,7 @@ public function testPatchMaterialItemValidatesMaterialNodeFromDifferentCamp() {
         $this->assertJsonContains([
             'violations' => [
                 [
-                    'propertyPath' => 'materialNode',
+                    'propertyPath' => 'materialList',
                     'message' => 'Must belong to the same camp.',
                 ],
             ],
diff --git a/api/tests/Api/SnapshotTests/EndpointPerformanceTest.php b/api/tests/Api/SnapshotTests/EndpointPerformanceTest.php
@@ -186,9 +186,9 @@ protected function getSnapshotId(): string {
 
     private static function getContentNodeEndpointQueryCountRanges(): array {
         return [
-            '/content_nodes' => [8, 11],
+            '/content_nodes' => [10, 12],
             '/content_node/column_layouts' => [6, 6],
-            '/content_node/column_layouts/item' => [10, 10],
+            '/content_node/column_layouts/item' => [9, 9],
             '/content_node/checklist_nodes' => [6, 7],
             '/content_node/checklist_nodes/item' => [9, 9],
             '/content_node/material_nodes' => [6, 7],
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetCollectionMatchesStructure with data set content_nodematerial_nodes__1.json b/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetCollectionMatchesStructure with data set content_nodematerial_nodes__1.json
@@ -35,6 +35,9 @@
                     "materialItems": [
                         {
                             "_links": {
+                                "camp": {
+                                    "href": "escaped_value"
+                                },
                                 "materialList": {
                                     "href": "escaped_value"
                                 },
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetCollectionMatchesStructure with data set content_nodes__1.json b/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetCollectionMatchesStructure with data set content_nodes__1.json
@@ -35,6 +35,9 @@
                     "materialItems": [
                         {
                             "_links": {
+                                "camp": {
+                                    "href": "escaped_value"
+                                },
                                 "materialList": {
                                     "href": "escaped_value"
                                 },
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetCollectionMatchesStructure with data set material_items__1.json b/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetCollectionMatchesStructure with data set material_items__1.json
@@ -3,6 +3,9 @@
         "items": [
             {
                 "_links": {
+                    "camp": {
+                        "href": "escaped_value"
+                    },
                     "materialList": {
                         "href": "escaped_value"
                     },
@@ -21,6 +24,9 @@
             },
             {
                 "_links": {
+                    "camp": {
+                        "href": "escaped_value"
+                    },
                     "materialList": {
                         "href": "escaped_value"
                     },
@@ -39,6 +45,9 @@
             },
             {
                 "_links": {
+                    "camp": {
+                        "href": "escaped_value"
+                    },
                     "materialList": {
                         "href": "escaped_value"
                     },
@@ -57,6 +66,9 @@
             },
             {
                 "_links": {
+                    "camp": {
+                        "href": "escaped_value"
+                    },
                     "materialList": {
                         "href": "escaped_value"
                     },
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetItemMatchesStructure with data set activities__1.json b/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetItemMatchesStructure with data set activities__1.json
@@ -46,6 +46,9 @@
                     "materialItems": [
                         {
                             "_links": {
+                                "camp": {
+                                    "href": "escaped_value"
+                                },
                                 "materialList": {
                                     "href": "escaped_value"
                                 },
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetItemMatchesStructure with data set material_items__1.json b/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testGetItemMatchesStructure with data set material_items__1.json
@@ -1,5 +1,8 @@
 {
     "_links": {
+        "camp": {
+            "href": "escaped_value"
+        },
         "materialList": {
             "href": "escaped_value"
         },
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testOpenApiSpecMatchesSnapshot__1.yml b/api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testOpenApiSpecMatchesSnapshot__1.yml
diff --git a/api/tests/Api/SnapshotTests/__snapshots__/test_EndpointPerformanceTest__testPerformanceDidNotChangeForStableEndpoints__1.yml b/api/tests/Api/SnapshotTests/__snapshots__/test_EndpointPerformanceTest__testPerformanceDidNotChangeForStableEndpoints__1.yml
diff --git a/api/tests/Entity/CampTest.php b/api/tests/Entity/CampTest.php
diff --git a/api/tests/Security/Voter/CampIsPrototypeVoterTest.php b/api/tests/Security/Voter/CampIsPrototypeVoterTest.php
diff --git a/api/tests/Security/Voter/CampRoleVoterTest.php b/api/tests/Security/Voter/CampRoleVoterTest.php
diff --git a/frontend/src/components/material/DialogMaterialItemEdit.vue b/frontend/src/components/material/DialogMaterialItemEdit.vue
diff --git a/frontend/src/components/material/MaterialTable.vue b/frontend/src/components/material/MaterialTable.vue
diff --git a/pdf/src/campPrint/scheduleEntry/contentNode/Material.vue b/pdf/src/campPrint/scheduleEntry/contentNode/Material.vue
diff --git a/print/components/scheduleEntry/contentNode/Material.vue b/print/components/scheduleEntry/contentNode/Material.vue

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,8 @@ public function __construct(ManagerRegistry $registry) {`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`public function filterByUser(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, User $user): void {`
`27`		`- $materialList = QueryBuilderHelper::findOrAddInnerRootJoinAlias($queryBuilder, $queryNameGenerator, 'materialList');`
`28`		`- $this->filterByCampCollaboration($queryBuilder, $user, "{$materialList}.camp");`
	`27`	`+ $periodMaterialItems = QueryBuilderHelper::findOrAddInnerRootJoinAlias($queryBuilder, $queryNameGenerator, 'periodMaterialItems');`
	`28`	`+ $period = QueryBuilderHelper::findOrAddInnerJoinAlias($queryBuilder, $queryNameGenerator, $periodMaterialItems, 'period');`
	`29`	`+ $this->filterByCampCollaboration($queryBuilder, $user, "{$period}.camp");`
`29`	`30`	`}`
`30`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],`
`37`	`37`
`38`	`38`	`// copy from prototype, if given`
`39`	`39`	`if (isset($data->campPrototype)) {`
`40`		`- $entityMap = new EntityMap();`
	`40`	`+ $entityMap = new EntityMap($data);`
`41`	`41`	`$data->copyFromPrototype($data->campPrototype, $entityMap);`
`42`	`42`	`}`
`43`	`43`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],`
`38`	`38`
`39`	`39`	`if (isset($data->copyCategorySource)) {`
`40`	`40`	`// CopyActivity Source is set -> copy it's content (rootContentNode)`
`41`		`- $entityMap = new EntityMap();`
	`41`	`+ $entityMap = new EntityMap($data->camp);`
`42`	`42`	`$rootContentNode->copyFromPrototype($data->copyCategorySource->getRootContentNode(), $entityMap);`
`43`	`43`	`}`
`44`	`44`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ public function __construct(ProcessorInterface $decorated) {`
`22`	`22`	`public function onBefore($data, Operation $operation, array $uriVariables = [], array $context = []): Checklist {`
`23`	`23`	`if (isset($data->copyChecklistSource)) {`
`24`	`24`	`// CopyChecklist Source is set -> copy it's content`
`25`		`- $entityMap = new EntityMap();`
	`25`	`+ $entityMap = new EntityMap($data->camp);`
`26`	`26`	`$data->copyFromPrototype($data->copyChecklistSource, $entityMap);`
`27`	`27`	`}`
`28`	`28`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,8 @@`
`1`	`1`	`{`
`2`	`2`	`"_links": {`
	`3`	`+ "camp": {`
	`4`	`+ "href": "escaped_value"`
	`5`	`+ },`
`3`	`6`	`"materialList": {`
`4`	`7`	`"href": "escaped_value"`
`5`	`8`	`},`