Skip to content

Commit 105de4f

Browse files
pmattmannpmattmann
committed
MaterialItem.MaterialList is nullable
1 parent d196dbf commit 105de4f

18 files changed

+172
-53
lines changed

Diff for: ‎api/migrations/schema/Version20250412165829.php

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DoctrineMigrations;
6+
7+
use Doctrine\DBAL\Schema\Schema;
8+
use Doctrine\Migrations\AbstractMigration;
9+
10+
/**
11+
* Auto-generated Migration: Please modify to your needs!
12+
*/
13+
final class Version20250412165829 extends AbstractMigration {
14+
public function getDescription(): string {
15+
return 'MaterialItem.MaterialList is nullable';
16+
}
17+
18+
public function up(Schema $schema): void {
19+
// this up() migration is auto-generated, please modify it to your needs
20+
$this->addSql(<<<'SQL'
21+
ALTER TABLE material_item ALTER materialListId DROP NOT NULL
22+
SQL);
23+
}
24+
25+
public function down(Schema $schema): void {
26+
// this down() migration is auto-generated, please modify it to your needs
27+
$this->addSql(<<<'SQL'
28+
ALTER TABLE material_item ALTER materiallistid SET NOT NULL
29+
SQL);
30+
}
31+
}

Diff for: ‎api/src/Entity/ContentNode.php

+9-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
#[ORM\InheritanceType('SINGLE_TABLE')]
5252
#[ORM\DiscriminatorColumn(name: 'strategy', type: 'string')]
5353
#[ORM\UniqueConstraint(name: 'contentnode_parentid_slot_position_unique', columns: ['parentid', 'slot', 'position'])]
54-
abstract class ContentNode extends BaseEntity implements BelongsToContentNodeTreeInterface, CopyFromPrototypeInterface, HasParentInterface {
54+
abstract class ContentNode extends BaseEntity implements BelongsToCampInterface, BelongsToContentNodeTreeInterface, CopyFromPrototypeInterface, HasParentInterface {
5555
use ClassInfoTrait;
5656

5757
/**
@@ -191,6 +191,14 @@ public function setParent(?ContentNode $parent) {
191191
$this->root ??= $parent?->root;
192192
}
193193

194+
public function getCamp(): ?Camp {
195+
if ($this->getRoot()->campRootContentNodes?->count() > 0) {
196+
return $this->getRoot()->campRootContentNodes[0]->camp;
197+
}
198+
199+
return null;
200+
}
201+
194202
/**
195203
* Holds the actual data of the content node.
196204
*/

Diff for: ‎api/src/Entity/MaterialItem.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
security: 'is_authenticated()'
4545
),
4646
new Post(
47+
denormalizationContext: ['groups' => ['write', 'create']],
4748
securityPostDenormalize: 'is_granted("CAMP_MEMBER", object) or is_granted("CAMP_MANAGER", object) or object.materialList === null'
4849
),
4950
],
@@ -58,11 +59,12 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface, CopyFro
5859
* The list to which this item belongs. Lists are used to keep track of who is
5960
* responsible to prepare and bring the item to the camp.
6061
*/
61-
#[AssertBelongsToSameCamp(compareToPrevious: true, groups: ['update'])]
62+
#[Assert\NotNull]
63+
#[AssertBelongsToSameCamp]
6264
#[ApiProperty(example: '/material_lists/1a2b3c4d')]
6365
#[Groups(['read', 'write'])]
6466
#[ORM\ManyToOne(targetEntity: MaterialList::class, inversedBy: 'materialItems')]
65-
#[ORM\JoinColumn(nullable: false, onDelete: 'cascade')]
67+
#[ORM\JoinColumn(nullable: true, onDelete: 'cascade')]
6668
public ?MaterialList $materialList = null;
6769

6870
/**
@@ -133,7 +135,10 @@ public function __construct() {
133135

134136
#[ApiProperty(readable: false)]
135137
public function getCamp(): ?Camp {
136-
return $this->materialList?->getCamp();
138+
return $this->period?->getCamp()
139+
?? $this->materialNode?->getCamp()
140+
?? $this->materailList?->getCamp()
141+
;
137142
}
138143

139144
/**
@@ -145,7 +150,10 @@ public function copyFromPrototype($prototype, $entityMap): void {
145150

146151
/** @var MaterialList $materialList */
147152
$materialList = $entityMap->get($prototype->materialList);
148-
$materialList->addMaterialItem($this);
153+
154+
if ($entityMap->belongsToTargetCamp($materialList)) {
155+
$materialList->addMaterialItem($this);
156+
}
149157

150158
$this->article = $prototype->article;
151159
$this->quantity = $prototype->quantity;

Diff for: ‎api/src/Repository/MaterialItemRepository.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ public function __construct(ManagerRegistry $registry) {
2424
}
2525

2626
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");
2930
}
3031
}

Diff for: ‎api/src/State/ActivityCreateProcessor.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
3434
throw new \UnexpectedValueException('Property rootContentNode of provided category is of wrong type. Object of type '.ColumnLayout::class.' expected.');
3535
}
3636

37+
$targetCamp = $data->category->camp;
3738
$data->camp = $data->category->camp;
3839
$rootContentNodePrototype = $data->category->rootContentNode;
3940

@@ -50,7 +51,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
5051
$data->setRootContentNode($rootContentNode);
5152

5253
// deep copy from category root node
53-
$entityMap = new EntityMap();
54+
$entityMap = new EntityMap($targetCamp);
5455
$rootContentNode->copyFromPrototype($rootContentNodePrototype, $entityMap);
5556

5657
return $data;

Diff for: ‎api/src/State/CampCreateProcessor.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
3737

3838
// copy from prototype, if given
3939
if (isset($data->campPrototype)) {
40-
$entityMap = new EntityMap();
40+
$entityMap = new EntityMap($data);
4141
$data->copyFromPrototype($data->campPrototype, $entityMap);
4242
}
4343

Diff for: ‎api/src/State/CategoryCreateProcessor.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function onBefore($data, Operation $operation, array $uriVariables = [],
3838

3939
if (isset($data->copyCategorySource)) {
4040
// CopyActivity Source is set -> copy it's content (rootContentNode)
41-
$entityMap = new EntityMap();
41+
$entityMap = new EntityMap($data->camp);
4242
$rootContentNode->copyFromPrototype($data->copyCategorySource->getRootContentNode(), $entityMap);
4343
}
4444

Diff for: ‎api/src/State/ChecklistCreateProcessor.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public function __construct(ProcessorInterface $decorated) {
2222
public function onBefore($data, Operation $operation, array $uriVariables = [], array $context = []): Checklist {
2323
if (isset($data->copyChecklistSource)) {
2424
// CopyChecklist Source is set -> copy it's content
25-
$entityMap = new EntityMap();
25+
$entityMap = new EntityMap($data->camp);
2626
$data->copyFromPrototype($data->copyChecklistSource, $entityMap);
2727
}
2828

Diff for: ‎api/src/Util/EntityMap.php

+8
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,16 @@
33
namespace App\Util;
44

55
use App\Entity\BaseEntity;
6+
use App\Entity\BelongsToCampInterface;
7+
use App\Entity\Camp;
68

79
class EntityMap {
810
use ClassInfoTrait;
911

1012
private $map = [];
1113

14+
public function __construct(private Camp $targetCamp) {}
15+
1216
public function add(BaseEntity $prototype, BaseEntity $entity) {
1317
$key = $this->getObjectClass($prototype).'#'.$prototype->getId();
1418
$this->map[$key] = $entity;
@@ -20,4 +24,8 @@ public function get(BaseEntity $prototype): BaseEntity {
2024

2125
return $keyExists ? $this->map[$key] : $prototype;
2226
}
27+
28+
public function belongsToTargetCamp(BelongsToCampInterface $entity) {
29+
return $entity->getCamp() == $this->targetCamp;
30+
}
2331
}

Diff for: ‎api/tests/Api/MaterialItems/CreateMaterialItemTest.php

+5-13
Original file line numberDiff line numberDiff line change
@@ -159,18 +159,10 @@ public function testCreateMaterialItemWithPeriodFromQueryParameter() {
159159
public function testCreateMaterialItemValidatesMissingPeriodAndMaterialNode() {
160160
static::createClientWithCredentials()->request('POST', '/material_items', ['json' => $this->getExampleWritePayload([], ['period', 'materialNode'])]);
161161

162-
$this->assertResponseStatusCodeSame(422);
162+
$this->assertResponseStatusCodeSame(403);
163163
$this->assertJsonContains([
164-
'violations' => [
165-
[
166-
'propertyPath' => 'period',
167-
'message' => 'Either this value or materialNode should not be null.',
168-
],
169-
[
170-
'propertyPath' => 'materialNode',
171-
'message' => 'Either this value or period should not be null.',
172-
],
173-
],
164+
'title' => 'An error occurred',
165+
'detail' => 'Access Denied.',
174166
]);
175167
}
176168

@@ -205,7 +197,7 @@ public function testCreateMaterialItemValidatesPeriodFromDifferentCamp() {
205197
$this->assertJsonContains([
206198
'violations' => [
207199
[
208-
'propertyPath' => 'period',
200+
'propertyPath' => 'materialList',
209201
'message' => 'Must belong to the same camp.',
210202
],
211203
],
@@ -222,7 +214,7 @@ public function testCreateMaterialItemValidatesMaterialNodeFromDifferentCamp() {
222214
$this->assertJsonContains([
223215
'violations' => [
224216
[
225-
'propertyPath' => 'materialNode',
217+
'propertyPath' => 'materialList',
226218
'message' => 'Must belong to the same camp.',
227219
],
228220
],

Diff for: ‎api/tests/Api/MaterialItems/UpdateMaterialItemTest.php

+6-10
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,9 @@ public function testPatchMaterialItemValidatesMissingMaterialList() {
151151
'materialList' => null,
152152
], 'headers' => ['Content-Type' => 'application/merge-patch+json']]);
153153

154-
$this->assertResponseStatusCodeSame(400);
154+
$this->assertResponseStatusCodeSame(422);
155155
$this->assertJsonContains([
156-
'detail' => 'The type of the "materialList" attribute must be "array" (nested document) or "string" (IRI), "NULL" given.',
156+
'detail' => 'materialList: This value should not be null.',
157157
]);
158158
}
159159

@@ -201,12 +201,8 @@ public function testPatchMaterialItemValidatesMissingPeriodAndMaterialNode() {
201201
$this->assertJsonContains([
202202
'violations' => [
203203
[
204-
'propertyPath' => 'period',
205-
'message' => 'Either this value or materialNode should not be null.',
206-
],
207-
[
208-
'propertyPath' => 'materialNode',
209-
'message' => 'Either this value or period should not be null.',
204+
'propertyPath' => 'materialList',
205+
'message' => 'Must belong to the same camp.',
210206
],
211207
],
212208
]);
@@ -244,7 +240,7 @@ public function testPatchMaterialItemValidatesPeriodFromDifferentCamp() {
244240
$this->assertJsonContains([
245241
'violations' => [
246242
[
247-
'propertyPath' => 'period',
243+
'propertyPath' => 'materialList',
248244
'message' => 'Must belong to the same camp.',
249245
],
250246
],
@@ -261,7 +257,7 @@ public function testPatchMaterialItemValidatesMaterialNodeFromDifferentCamp() {
261257
$this->assertJsonContains([
262258
'violations' => [
263259
[
264-
'propertyPath' => 'materialNode',
260+
'propertyPath' => 'materialList',
265261
'message' => 'Must belong to the same camp.',
266262
],
267263
],

Diff for: ‎api/tests/Api/SnapshotTests/EndpointPerformanceTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ private static function getContentNodeEndpointQueryCountRanges(): array {
188188
return [
189189
'/content_nodes' => [8, 11],
190190
'/content_node/column_layouts' => [6, 6],
191-
'/content_node/column_layouts/item' => [10, 10],
191+
'/content_node/column_layouts/item' => [9, 9],
192192
'/content_node/checklist_nodes' => [6, 7],
193193
'/content_node/checklist_nodes/item' => [9, 9],
194194
'/content_node/material_nodes' => [6, 7],

0 commit comments

Comments
 (0)