Merge pull request #213 from os2display/feature/2314-save-playlist-and-regions-in-screen-processor

sinejespersen · web-flow · commit 0d667c787bc4 · 2024-10-23T12:22:59.000+02:00
Feature/2314 save playlist and regions in screen processor
diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php
@@ -16,6 +16,7 @@
         'phpdoc_align' => false,
         'no_superfluous_phpdoc_tags' => false,
         'array_syntax' => ['syntax' => 'short'],
+        'phpdoc_to_comment' => false,
         'declare_strict_types' => true,
         ConstructorEmptyBracesFixer::name() => true,
         MultilinePromotedPropertiesFixer::name() => true,
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,18 @@ All notable changes to this project will be documented in this file.
 
 - [#214](https://github.com/os2display/display-api-service/pull/214)
   - Updated endSessionUrl to be nullable.
+
+- [#213](https://github.com/os2display/display-api-service/pull/213)
+  - Set `phpdoc_to_comment` to `false`in `.php-cs-fixer.dist.php` to avoid breaking psalm ignore
+  - Add regions and groups to `ScreenInput.php`
+  - Add "cascade: persist remove" to PlaylistScreenRegion
+  - Save playlist/regions in `ScreenProcessor.php` and in `src/entity/ScreenLayoutRegions` (as an alternative to sending
+    multiple requests)
+  - Save groups in `ScreenProcessor.php` and in `src/entity/tenant/Screen.php`
+  - Update psalm baseline
+  - Add regions/playlists and groups to POST screen test
+  - `composer update symfony/* --with-dependencies`
+
 - [#193](https://github.com/os2display/display-api-service/pull/193)
   - Adds support for interactive slides.
   - Adds interactivity for creating quick bookings from a slide through Microsoft Graph.
@@ -69,8 +81,8 @@ All notable changes to this project will be documented in this file.
   - Changed route prefix to v2.
 - [#197](https://github.com/os2display/display-api-service/pull/197)
   - Fixed weight issue when assigning slides to playlist.
-- [#194](https://github.com/os2display/display-api-service/pull/194)
-  Updated test run documentation and added test for `rrule` in playlist.
+- [#194](https://github.com/os2display/display-api-service/pull/194) Updated test run documentation and added test for
+  `rrule` in playlist.
 - Fixed issue with PlaylistSlide transaction.
 - Fixed issues with feed following api platform upgrade.
 - [#192](https://github.com/os2display/display-api-service/pull/192)
@@ -92,8 +104,8 @@ All notable changes to this project will be documented in this file.
 - [#184](https://github.com/os2display/display-api-service/pull/184)
   - Added RelationsModifiedTrait to serialization groups.
 - [#182](https://github.com/os2display/display-api-service/pull/182)
-  - Changed "Theme" api output to have "Logo" embedded to avoid 404 errors when fetching logo from other shared slide
-  w. foreign tenant.
+  - Changed "Theme" api output to have "Logo" embedded to avoid 404 errors when fetching logo from other shared slide w.
+  foreign tenant.
 - [#181](https://github.com/os2display/display-api-service/pull/181)
   - Update minimum PHP version to 8.2 to support trait constants
   - Add 'relationsModified' timestamps on relevant entities and API resources.
@@ -114,16 +126,11 @@ All notable changes to this project will be documented in this file.
   - Switch from doctrine annotations to attributes
   - Add rector as dev dependency and apply rules
   - Handle doctrine deprecations
-- [#173](https://github.com/os2display/display-api-service/pull/173)
-  Upgraded to API Platform 3
-- [#172](https://github.com/os2display/display-api-service/pull/172)
-  Linted YAML API resources
-- [#171](https://github.com/os2display/display-api-service/pull/171)
-  Fixed slide playlists collection operation.
-- [#170](https://github.com/os2display/display-api-service/pull/170)
-  Updated Symfony development packages.
-- [#165](https://github.com/os2display/display-api-service/pull/165)
-  Symfony 6.3
+- [#173](https://github.com/os2display/display-api-service/pull/173) Upgraded to API Platform 3
+- [#172](https://github.com/os2display/display-api-service/pull/172) Linted YAML API resources
+- [#171](https://github.com/os2display/display-api-service/pull/171) Fixed slide playlists collection operation.
+- [#170](https://github.com/os2display/display-api-service/pull/170) Updated Symfony development packages.
+- [#165](https://github.com/os2display/display-api-service/pull/165) Symfony 6.3
 - [#162](https://github.com/os2display/display-api-service/pull/162)
   - Adds "external" openid-connect provider.
   - Renamed "oidc" openid-connect provider to "internal".
@@ -133,9 +140,9 @@ All notable changes to this project will be documented in this file.
   - Upgrades openid-connect bundle to 3.1 to support multiple providers.
   - Changes php requirement in composer.json to >= 8.1.
   - Removed PHP Upgrade coding standards github actions check.
-  - Changed user identifier from email to providerId. Made email nullable. Copied value from email to providerId in migration.
-- [#161](https://github.com/os2display/display-api-service/pull/161)
-  Fixed non-entity related psalm errors.
+  - Changed user identifier from email to providerId. Made email nullable. Copied value from email to providerId in
+    migration.
+- [#161](https://github.com/os2display/display-api-service/pull/161) Fixed non-entity related psalm errors.
 
 ## [1.5.0] - 2023-10-26
 
@@ -212,7 +219,8 @@ All notable changes to this project will be documented in this file.
 - [#138](https://github.com/os2display/display-api-service/pull/138)
   - Fixed Tenant and command to allow for empty fallbackImageUrl.
 - [#139](https://github.com/os2display/display-api-service/pull/139)
-  - Changed from service decoration to event listeners to re-enable setting `tenants` on the response from `/v1/authentication/token`.
+  - Changed from service decoration to event listeners to re-enable setting `tenants` on the response from
+    `/v1/authentication/token`.
   - Ensure same response data from both `/v1/authentication/token` and `/v1/authentication/token/refresh`endpoints.
   - Added `user` and `tenants` to JWT payload.
 
diff --git a/README.md b/README.md
@@ -258,16 +258,16 @@ the coding standard for the project.
 
 - PHP files [PHP Coding Standards Fixer](https://cs.symfony.com/)
 
-    ```shell
-    docker compose exec phpfpm composer coding-standards-check
-    ```
+```shell
+docker compose exec phpfpm composer coding-standards-check
+```
 
 - Markdown files (markdownlint standard rules)
 
-    ```shell
-    docker run --rm -v .:/app --workdir=/app node:20 npm install
-    docker run --rm -v .:/app --workdir=/app node:20 npm run coding-standards-check
-    ```
+```shell
+docker run --rm -v .:/app --workdir=/app node:20 npm install
+docker run --rm -v .:/app --workdir=/app node:20 npm run coding-standards-check
+```
 
 #### YAML
 
@@ -281,23 +281,24 @@ To attempt to automatically fix coding style issues
 
 - PHP files [PHP Coding Standards Fixer](https://cs.symfony.com/)
 
-    ```sh
-    docker compose exec phpfpm composer coding-standards-apply
-    ```
+```sh
+docker compose exec phpfpm composer coding-standards-apply
+```
 
 - Markdown files (markdownlint standard rules)
 
-    ```shell
-    docker run --rm -v .:/app --workdir=/app node:18 npm install
-    docker run --rm -v .:/app --workdir=/app node:18 npm run coding-standards-apply
-    ```
+```shell
+docker run --rm -v .:/app --workdir=/app node:18 npm install
+docker run --rm -v .:/app --workdir=/app node:18 npm run coding-standards-apply
+```
 
 ## Tests
 
 Run automated tests:
 
 ```shell
-docker compose exec phpfpm composer tests
+docker compose exec phpfpm composer test setup
+docker compose exec phpfpm composer test
 ```
 
 Disable or hide deprecation warnings using the [`SYMFONY_DEPRECATIONS_HELPER` environment
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -685,9 +685,6 @@
       <code><![CDATA[setSize]]></code>
       <code><![CDATA[setTitle]]></code>
     </PossiblyNullReference>
-    <UndefinedClass>
-      <code><![CDATA[InvalidArgumentException]]></code>
-    </UndefinedClass>
   </file>
   <file src="src/State/ScreenProvider.php">
     <PossiblyNullPropertyAssignmentValue>
@@ -746,10 +743,10 @@
     </NullableReturnStatement>
     <PossiblyNullReference>
       <code><![CDATA[removeLogo]]></code>
-      <code><![CDATA[setLogo]]></code>
       <code><![CDATA[setCreatedBy]]></code>
       <code><![CDATA[setCssStyles]]></code>
       <code><![CDATA[setDescription]]></code>
+      <code><![CDATA[setLogo]]></code>
       <code><![CDATA[setModifiedBy]]></code>
       <code><![CDATA[setTitle]]></code>
     </PossiblyNullReference>
diff --git a/public/api-spec-v2.json b/public/api-spec-v2.json
@@ -11119,6 +11119,24 @@
                             "boolean",
                             "null"
                         ]
+                    },
+                    "regions": {
+                        "type": [
+                            "array",
+                            "null"
+                        ],
+                        "items": {
+                            "type": "string"
+                        }
+                    },
+                    "groups": {
+                        "type": [
+                            "array",
+                            "null"
+                        ],
+                        "items": {
+                            "type": "string"
+                        }
                     }
                 }
             },
@@ -11153,6 +11171,24 @@
                             "boolean",
                             "null"
                         ]
+                    },
+                    "regions": {
+                        "type": [
+                            "array",
+                            "null"
+                        ],
+                        "items": {
+                            "type": "string"
+                        }
+                    },
+                    "groups": {
+                        "type": [
+                            "array",
+                            "null"
+                        ],
+                        "items": {
+                            "type": "string"
+                        }
                     }
                 }
             },
diff --git a/public/api-spec-v2.yaml b/public/api-spec-v2.yaml
@@ -7735,6 +7735,18 @@ components:
           type:
             - boolean
             - 'null'
+        regions:
+          type:
+            - array
+            - 'null'
+          items:
+            type: string
+        groups:
+          type:
+            - array
+            - 'null'
+          items:
+            type: string
     Screen.ScreenInput.jsonld:
       type: object
       description: ''
@@ -7758,6 +7770,18 @@ components:
           type:
             - boolean
             - 'null'
+        regions:
+          type:
+            - array
+            - 'null'
+          items:
+            type: string
+        groups:
+          type:
+            - array
+            - 'null'
+          items:
+            type: string
     Screen.jsonld:
       type: object
       description: ''
diff --git a/src/Controller/PlaylistScreenRegionDeleteController.php b/src/Controller/PlaylistScreenRegionDeleteController.php
@@ -23,7 +23,7 @@ public function __invoke(string $id, string $regionId, string $playlistId): Json
         $regionUlid = $this->validationUtils->validateUlid($regionId);
         $playlistUlid = $this->validationUtils->validateUlid($playlistId);
 
-        $this->playlistScreenRegionRepository->deleteRelations($screenUlid, $regionUlid, $playlistUlid, $this->getActiveTenant());
+        $this->playlistScreenRegionRepository->deleteRelations($screenUlid, $regionUlid, $playlistUlid);
 
         return new JsonResponse(null, \Symfony\Component\HttpFoundation\Response::HTTP_NO_CONTENT);
     }
diff --git a/src/Dto/ScreenInput.php b/src/Dto/ScreenInput.php
@@ -16,4 +16,6 @@ class ScreenInput
     public string $orientation = '';
 
     public ?bool $enableColorSchemeChange = null;
+    public ?array $regions = null;
+    public ?array $groups = null;
 }
diff --git a/src/Entity/ScreenLayoutRegions.php b/src/Entity/ScreenLayoutRegions.php
@@ -41,7 +41,7 @@ class ScreenLayoutRegions extends AbstractBaseEntity implements MultiTenantInter
     /**
      * @var \Doctrine\Common\Collections\Collection<int, \App\Entity\Tenant\PlaylistScreenRegion>|\App\Entity\Tenant\PlaylistScreenRegion[]
      */
-    #[ORM\OneToMany(targetEntity: PlaylistScreenRegion::class, mappedBy: 'region', orphanRemoval: true)]
+    #[ORM\OneToMany(targetEntity: PlaylistScreenRegion::class, cascade: ['persist', 'remove'], mappedBy: 'region', orphanRemoval: true)]
     private Collection $playlistScreenRegions;
 
     public function __construct()
@@ -82,6 +82,18 @@ public function getPlaylistScreenRegions(): Collection
         return $this->playlistScreenRegions;
     }
 
+    public function setPlaylistScreenRegions(Collection $playlistScreenRegions): void
+    {
+        foreach ($this->playlistScreenRegions as $playlistScreenRegion) {
+            if (false === $playlistScreenRegions->contains($playlistScreenRegion)) {
+                $this->removePlaylistScreenRegion($playlistScreenRegion);
+            }
+        }
+        foreach ($playlistScreenRegions as $playlistScreenRegion) {
+            $this->addPlaylistScreenRegion($playlistScreenRegion);
+        }
+    }
+
     public function addPlaylistScreenRegion(PlaylistScreenRegion $playlistScreenRegion): self
     {
         if (!$this->playlistScreenRegions->contains($playlistScreenRegion)) {
diff --git a/src/Entity/Tenant/Screen.php b/src/Entity/Tenant/Screen.php
@@ -52,7 +52,7 @@ class Screen extends AbstractTenantScopedEntity implements RelationsChecksumInte
     /**
      * @var \Doctrine\Common\Collections\Collection<int, \App\Entity\Tenant\PlaylistScreenRegion>|\App\Entity\Tenant\PlaylistScreenRegion[]
      */
-    #[ORM\OneToMany(mappedBy: 'screen', targetEntity: PlaylistScreenRegion::class, orphanRemoval: true)]
+    #[ORM\OneToMany(mappedBy: 'screen', targetEntity: PlaylistScreenRegion::class, cascade: ['persist', 'remove'], orphanRemoval: true)]
     #[ORM\OrderBy(['weight' => \Doctrine\Common\Collections\Order::Ascending->value])]
     private Collection $playlistScreenRegions;
 
@@ -161,7 +161,7 @@ public function removePlaylistScreenRegion(PlaylistScreenRegion $playlistScreenR
 
     public function removeAllPlaylistScreenRegions(): self
     {
-        foreach ($this->playlistScreenRegions as $playlistScreenRegion) {
+        foreach ($this->getPlaylistScreenRegions() as $playlistScreenRegion) {
             // set the owning side to null (unless already changed)
             if ($playlistScreenRegion->getScreen() === $this) {
                 $playlistScreenRegion->removeScreen();
@@ -173,6 +173,18 @@ public function removeAllPlaylistScreenRegions(): self
         return $this;
     }
 
+    public function setScreenGroups(Collection $screenGroups): void
+    {
+        foreach ($this->screenGroups as $screenGroup) {
+            if (false === $screenGroups->contains($screenGroup)) {
+                $this->removeScreenGroup($screenGroup);
+            }
+        }
+        foreach ($screenGroups as $screenGroup) {
+            $this->addScreenGroup($screenGroup);
+        }
+    }
+
     /**
      * @return Collection
      */
diff --git a/src/Repository/PlaylistScreenRegionRepository.php b/src/Repository/PlaylistScreenRegionRepository.php
@@ -119,13 +119,12 @@ public function updateRelations(Ulid $screenUlid, Ulid $regionUlid, ArrayCollect
      * @throws \Doctrine\ORM\ORMException
      * @throws \Doctrine\ORM\OptimisticLockException
      */
-    public function deleteRelations(Ulid $screenUlid, Ulid $regionUid, Ulid $playlistUlid, Tenant $tenant): void
+    public function deleteRelations(Ulid $screenUlid, Ulid $regionUid, Ulid $playlistUlid): void
     {
         $playlistScreenRegion = $this->findOneBy([
             'screen' => $screenUlid,
             'region' => $regionUid,
             'playlist' => $playlistUlid,
-            'tenant' => $tenant,
         ]);
 
         if (is_null($playlistScreenRegion)) {
diff --git a/src/State/ScreenProcessor.php b/src/State/ScreenProcessor.php
diff --git a/tests/Api/ScreensTest.php b/tests/Api/ScreensTest.php

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public function __invoke(string $id, string $regionId, string $playlistId): Json`
`23`	`23`	`$regionUlid = $this->validationUtils->validateUlid($regionId);`
`24`	`24`	`$playlistUlid = $this->validationUtils->validateUlid($playlistId);`
`25`	`25`
`26`		`- $this->playlistScreenRegionRepository->deleteRelations($screenUlid, $regionUlid, $playlistUlid, $this->getActiveTenant());`
	`26`	`+ $this->playlistScreenRegionRepository->deleteRelations($screenUlid, $regionUlid, $playlistUlid);`
`27`	`27`
`28`	`28`	`return new JsonResponse(null, \Symfony\Component\HttpFoundation\Response::HTTP_NO_CONTENT);`
`29`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,4 +16,6 @@ class ScreenInput`
`16`	`16`	`public string $orientation = '';`
`17`	`17`
`18`	`18`	`public ?bool $enableColorSchemeChange = null;`
	`19`	`+ public ?array $regions = null;`
	`20`	`+ public ?array $groups = null;`
`19`	`21`	`}`