From 22220546a709117a2a39fa39ee6d059faf26cfc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Fr=C3=A9mont?= <lc.fremont@gmail.com>
Date: Fri, 18 Aug 2023 14:10:22 +0200
Subject: [PATCH 1/2] Update delete path name to avoid route conflicts

---
 .../Metadata/Api/ApiOperationInterface.php    | 21 +++++++++++++++++++
 src/Component/Metadata/Api/Delete.php         |  2 +-
 src/Component/Metadata/Api/Get.php            |  2 +-
 src/Component/Metadata/Api/GetCollection.php  |  2 +-
 src/Component/Metadata/Api/Patch.php          |  2 +-
 src/Component/Metadata/Api/Post.php           |  2 +-
 src/Component/Metadata/Api/Put.php            |  2 +-
 .../DeleteOperationRoutePathFactory.php       |  6 ++----
 .../DeleteOperationRoutePathFactorySpec.php   | 11 ++++++++--
 .../src/Subscription/Entity/Subscription.php  |  2 +-
 10 files changed, 39 insertions(+), 13 deletions(-)
 create mode 100644 src/Component/Metadata/Api/ApiOperationInterface.php

diff --git a/src/Component/Metadata/Api/ApiOperationInterface.php b/src/Component/Metadata/Api/ApiOperationInterface.php
new file mode 100644
index 000000000..61fa8f2c4
--- /dev/null
+++ b/src/Component/Metadata/Api/ApiOperationInterface.php
@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Sylius Sp. z o.o.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Component\Resource\Metadata\Api;
+
+/**
+ * @experimental
+ */
+interface ApiOperationInterface
+{
+}
diff --git a/src/Component/Metadata/Api/Delete.php b/src/Component/Metadata/Api/Delete.php
index d14252dcf..0f6aa2c15 100644
--- a/src/Component/Metadata/Api/Delete.php
+++ b/src/Component/Metadata/Api/Delete.php
@@ -20,7 +20,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
-final class Delete extends HttpOperation implements DeleteOperationInterface
+final class Delete extends HttpOperation implements DeleteOperationInterface, ApiOperationInterface
 {
     public function __construct(
         ?string $path = null,
diff --git a/src/Component/Metadata/Api/Get.php b/src/Component/Metadata/Api/Get.php
index 4368f2ff2..3a123d33f 100644
--- a/src/Component/Metadata/Api/Get.php
+++ b/src/Component/Metadata/Api/Get.php
@@ -20,7 +20,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
-final class Get extends HttpOperation implements ShowOperationInterface
+final class Get extends HttpOperation implements ShowOperationInterface, ApiOperationInterface
 {
     public function __construct(
         ?string $path = null,
diff --git a/src/Component/Metadata/Api/GetCollection.php b/src/Component/Metadata/Api/GetCollection.php
index 35850ff94..d10d1ddc0 100644
--- a/src/Component/Metadata/Api/GetCollection.php
+++ b/src/Component/Metadata/Api/GetCollection.php
@@ -20,7 +20,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
-final class GetCollection extends HttpOperation implements CollectionOperationInterface
+final class GetCollection extends HttpOperation implements CollectionOperationInterface, ApiOperationInterface
 {
     public function __construct(
         ?string $path = null,
diff --git a/src/Component/Metadata/Api/Patch.php b/src/Component/Metadata/Api/Patch.php
index d00770f59..bb7c2150f 100644
--- a/src/Component/Metadata/Api/Patch.php
+++ b/src/Component/Metadata/Api/Patch.php
@@ -20,7 +20,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
-final class Patch extends HttpOperation implements UpdateOperationInterface
+final class Patch extends HttpOperation implements UpdateOperationInterface, ApiOperationInterface
 {
     public function __construct(
         ?string $path = null,
diff --git a/src/Component/Metadata/Api/Post.php b/src/Component/Metadata/Api/Post.php
index 29179f739..342082d9b 100644
--- a/src/Component/Metadata/Api/Post.php
+++ b/src/Component/Metadata/Api/Post.php
@@ -20,7 +20,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
-final class Post extends HttpOperation implements CreateOperationInterface
+final class Post extends HttpOperation implements CreateOperationInterface, ApiOperationInterface
 {
     public function __construct(
         ?string $path = null,
diff --git a/src/Component/Metadata/Api/Put.php b/src/Component/Metadata/Api/Put.php
index 4326b5bf7..c342f015d 100644
--- a/src/Component/Metadata/Api/Put.php
+++ b/src/Component/Metadata/Api/Put.php
@@ -20,7 +20,7 @@
  * @experimental
  */
 #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
-final class Put extends HttpOperation implements UpdateOperationInterface
+final class Put extends HttpOperation implements UpdateOperationInterface, ApiOperationInterface
 {
     public function __construct(
         ?string $path = null,
diff --git a/src/Component/Symfony/Routing/Factory/DeleteOperationRoutePathFactory.php b/src/Component/Symfony/Routing/Factory/DeleteOperationRoutePathFactory.php
index 8db2d45b0..0526d3975 100644
--- a/src/Component/Symfony/Routing/Factory/DeleteOperationRoutePathFactory.php
+++ b/src/Component/Symfony/Routing/Factory/DeleteOperationRoutePathFactory.php
@@ -13,6 +13,7 @@
 
 namespace Sylius\Component\Resource\Symfony\Routing\Factory;
 
+use Sylius\Component\Resource\Metadata\Api\ApiOperationInterface;
 use Sylius\Component\Resource\Metadata\DeleteOperationInterface;
 use Sylius\Component\Resource\Metadata\HttpOperation;
 
@@ -28,10 +29,7 @@ public function createRoutePath(HttpOperation $operation, string $rootPath): str
         $identifier = $operation->getResource()?->getIdentifier() ?? 'id';
 
         if ($operation instanceof DeleteOperationInterface) {
-            $path = match ($shortName) {
-                'delete' => '',
-                default => '/' . $shortName,
-            };
+            $path = $operation instanceof ApiOperationInterface && 'delete' === $shortName ? '' : '/' . $shortName;
 
             return sprintf('%s/{%s}%s', $rootPath, $identifier, $path);
         }
diff --git a/src/Component/spec/Symfony/Routing/Factory/DeleteOperationRoutePathFactorySpec.php b/src/Component/spec/Symfony/Routing/Factory/DeleteOperationRoutePathFactorySpec.php
index 7061c6880..ed5599857 100644
--- a/src/Component/spec/Symfony/Routing/Factory/DeleteOperationRoutePathFactorySpec.php
+++ b/src/Component/spec/Symfony/Routing/Factory/DeleteOperationRoutePathFactorySpec.php
@@ -36,14 +36,14 @@ function it_generates_route_path_for_delete_operations(): void
     {
         $operation = new Delete();
 
-        $this->createRoutePath($operation, '/dummies')->shouldReturn('/dummies/{id}');
+        $this->createRoutePath($operation, '/dummies')->shouldReturn('/dummies/{id}/delete');
     }
 
     function it_generates_route_path_for_delete_operations_with_custom_identifier(): void
     {
         $operation = (new Delete())->withResource(new Resource(identifier: 'code'));
 
-        $this->createRoutePath($operation, '/dummies')->shouldReturn('/dummies/{code}');
+        $this->createRoutePath($operation, '/dummies')->shouldReturn('/dummies/{code}/delete');
     }
 
     function it_generates_route_path_for_update_operations_with_custom_short_name(): void
@@ -59,4 +59,11 @@ function it_generates_route_path_for_api_delete_operations(): void
 
         $this->createRoutePath($operation, '/dummies')->shouldReturn('/dummies/{id}');
     }
+
+    function it_generates_route_path_for_api_delete_operations_with_custom_short_name(): void
+    {
+        $operation = new Api\Delete(shortName: 'remove');
+
+        $this->createRoutePath($operation, '/dummies')->shouldReturn('/dummies/{id}/remove');
+    }
 }
diff --git a/tests/Application/src/Subscription/Entity/Subscription.php b/tests/Application/src/Subscription/Entity/Subscription.php
index 0d68249a9..7f9d6b8c7 100644
--- a/tests/Application/src/Subscription/Entity/Subscription.php
+++ b/tests/Application/src/Subscription/Entity/Subscription.php
@@ -39,6 +39,7 @@
 #[Index(grid: 'app_subscription')]
 #[Create]
 #[Update]
+#[Delete]
 #[BulkDelete]
 #[ApplyStateMachineTransition(stateMachineTransition: 'accept')]
 #[ApplyStateMachineTransition(stateMachineTransition: 'reject')]
@@ -46,7 +47,6 @@
     shortName: 'bulk_accept',
     stateMachineTransition: 'accept',
 )]
-#[Delete]
 #[Show(
     template: 'subscription/show.html.twig',
     twigContextFactory: ShowSubscriptionContextFactory::class,

From c59467d660a7845d2ced23d43c90aa105e94299f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Fr=C3=A9mont?= <lc.fremont@gmail.com>
Date: Fri, 18 Aug 2023 14:19:33 +0200
Subject: [PATCH 2/2] Fix PHPUnit tests

---
 tests/Application/src/Tests/Controller/BoardGameUiTest.php  | 4 ++--
 .../Application/src/Tests/Controller/SubscriptionUiTest.php | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/Application/src/Tests/Controller/BoardGameUiTest.php b/tests/Application/src/Tests/Controller/BoardGameUiTest.php
index 354c80d1d..29fe7ed2e 100644
--- a/tests/Application/src/Tests/Controller/BoardGameUiTest.php
+++ b/tests/Application/src/Tests/Controller/BoardGameUiTest.php
@@ -50,12 +50,12 @@ public function it_allows_browsing_board_games(): void
         $this->assertStringContainsString('<td>Stone Age</td>', $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/board-games/%s">Show</a>', $boardGames['stone_age']->id()), $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/board-games/%s/edit">Edit</a>', $boardGames['stone_age']->id()), $content);
-        $this->assertStringContainsString(sprintf('<form action="/admin/board-games/%s" method="post">', $boardGames['stone_age']->id()), $content);
+        $this->assertStringContainsString(sprintf('<form action="/admin/board-games/%s/delete" method="post">', $boardGames['stone_age']->id()), $content);
 
         $this->assertStringContainsString('<td>Ticket to Ride</td>', $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/board-games/%s">Show</a>', $boardGames['ticket_to_ride']->id()), $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/board-games/%s/edit">Edit</a>', $boardGames['ticket_to_ride']->id()), $content);
-        $this->assertStringContainsString(sprintf('<form action="/admin/board-games/%s" method="post">', $boardGames['ticket_to_ride']->id()), $content);
+        $this->assertStringContainsString(sprintf('<form action="/admin/board-games/%s/delete" method="post">', $boardGames['ticket_to_ride']->id()), $content);
     }
 
     /** @test */
diff --git a/tests/Application/src/Tests/Controller/SubscriptionUiTest.php b/tests/Application/src/Tests/Controller/SubscriptionUiTest.php
index ccdcb93ff..50e395015 100644
--- a/tests/Application/src/Tests/Controller/SubscriptionUiTest.php
+++ b/tests/Application/src/Tests/Controller/SubscriptionUiTest.php
@@ -50,17 +50,17 @@ public function it_allows_browsing_subscriptions(): void
         $this->assertStringContainsString('<td>marty.mcfly@bttf.com</td>', $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s">Show</a>', $subscriptions['subscription_marty']->getId()), $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s/edit">Edit</a>', $subscriptions['subscription_marty']->getId()), $content);
-        $this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s" method="post">', $subscriptions['subscription_marty']->getId()), $content);
+        $this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s/delete" method="post">', $subscriptions['subscription_marty']->getId()), $content);
 
         $this->assertStringContainsString('<td>doc.brown@bttf.com</td>', $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s">Show</a>', $subscriptions['subscription_doc']->getId()), $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s/edit">Edit</a>', $subscriptions['subscription_doc']->getId()), $content);
-        $this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s" method="post">', $subscriptions['subscription_doc']->getId()), $content);
+        $this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s/delete" method="post">', $subscriptions['subscription_doc']->getId()), $content);
 
         $this->assertStringContainsString('<td>biff.tannen@bttf.com</td>', $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s">Show</a>', $subscriptions['subscription_biff']->getId()), $content);
         $this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s/edit">Edit</a>', $subscriptions['subscription_biff']->getId()), $content);
-        $this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s" method="post">', $subscriptions['subscription_biff']->getId()), $content);
+        $this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s/delete" method="post">', $subscriptions['subscription_biff']->getId()), $content);
     }
 
     /** @test */