[6.6.x] fix: system config webhook check

src/Webhook/Registration/WebhookSubscriber.php

@@ -63,37 +63,47 @@ public function removeSalesChannelWebhookConfiguration(EntityDeletedEvent $event
 
     /**
      * system-config should be written by {@see SettingsSaver} only, which checks the webhook on its own.
-     * Just in case new/changed credentials will be saved via the normal system config save route.
+     * Just in case new/changed credentials will be saved via the normal system config.
      */
     public function checkWebhookBefore(BeforeSystemConfigMultipleChangedEvent $event): void
     {
-        $routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route');
-
-        if (!\str_contains($routeName, 'api.action.core.save.system-config')) {
+        if (!Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
             return;
         }
 
-        if (Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
-            /** @var array<string, array<string, mixed>> $config */
-            $config = $event->getConfig();
-            $this->webhookSystemConfigHelper->checkWebhookBefore($config);
+        if ($config = $this->getConfigToCheck($event)) {
+            $this->webhookSystemConfigHelper->checkWebhookBefore([$event->getSalesChannelId() => $config]);
         }
     }
 
     /**
      * system-config should be written by {@see SettingsSaver} only, which checks the webhook on its own.
-     * Just in case new/changed credentials will be saved via the normal system config save route.
+     * Just in case new/changed credentials will be saved via the normal system config.
      */
     public function checkWebhookAfter(SystemConfigMultipleChangedEvent $event): void
     {
-        $routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route');
-
-        if (!\str_contains($routeName, 'api.action.core.save.system-config')) {
+        if (!Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
             return;
         }
 
-        if (Feature::isActive('PAYPAL_SETTINGS_TWEAKS')) {
-            $this->webhookSystemConfigHelper->checkWebhookAfter(\array_keys($event->getConfig()));
+        if ($this->getConfigToCheck($event)) {
+            $this->webhookSystemConfigHelper->checkWebhookAfter([$event->getSalesChannelId()]);
         }
     }
+
+    /**
+     * @return array<string, mixed>|null
+     */
+    private function getConfigToCheck(BeforeSystemConfigMultipleChangedEvent|SystemConfigMultipleChangedEvent $event): ?array
+    {
+        /** @var array<string, mixed> $config */
+        $config = $event->getConfig();
+        $routeName = (string) $this->requestStack->getMainRequest()?->attributes->getString('_route');
+
+        if (\str_contains($routeName, 'api.action.paypal.settings.save') || !$this->webhookSystemConfigHelper->needsCheck($config)) {
+            return null;
+        }
+
+        return $config;
+    }
 }

src/Webhook/Registration/WebhookSystemConfigHelper.php

@@ -59,7 +59,7 @@ public function checkWebhookBefore(array $newData): array
         $errors = [];
 
         foreach ($newData as $salesChannelId => $newSettings) {
-            if ($salesChannelId === 'null') {
+            if (!$salesChannelId || $salesChannelId === 'null') {
                 $salesChannelId = null;
             }
 
@@ -102,7 +102,7 @@ public function checkWebhookAfter(array $salesChannelIds): array
         $errors = [];
 
         foreach ($salesChannelIds as $salesChannelId) {
-            if ($salesChannelId === 'null') {
+            if (!$salesChannelId || $salesChannelId === 'null') {
                 $salesChannelId = null;
             }
 
@@ -129,6 +129,14 @@ public function checkWebhookAfter(array $salesChannelIds): array
         return $errors;
     }
 
+    /**
+     * @param array<string, mixed> $config
+     */
+    public function needsCheck(array $config): bool
+    {
+        return !empty($this->filterSettings($config));
+    }
+
     private function fetchSettings(?string $salesChannelId, bool $inherit = false): array
     {
         $settings = [];

tests/Webhook/Registration/WebhookSubscriberTest.php

@@ -7,12 +7,14 @@
 
 namespace Swag\PayPal\Test\Webhook\Registration;
 
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\NullLogger;
 use Shopware\Core\Framework\Context;
 use Shopware\Core\Framework\DataAbstractionLayer\EntityWriteResult;
 use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityDeletedEvent;
 use Shopware\Core\Framework\DataAbstractionLayer\Write\EntityExistence;
+use Shopware\Core\Framework\Feature;
 use Shopware\Core\Framework\Log\Package;
 use Shopware\Core\System\SalesChannel\SalesChannelDefinition;
 use Shopware\Core\System\SalesChannel\SalesChannelEvents;
@@ -29,6 +31,7 @@
 use Swag\PayPal\Webhook\Registration\WebhookSystemConfigHelper;
 use Swag\PayPal\Webhook\WebhookRegistry;
 use Swag\PayPal\Webhook\WebhookService;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Routing\RouterInterface;
 
@@ -44,9 +47,15 @@ class WebhookSubscriberTest extends TestCase
 
     private SystemConfigService $systemConfigService;
 
+    private WebhookSystemConfigHelper&MockObject $webhookSystemConfigHelper;
+
+    private RequestStack $requestStack;
+
     protected function setUp(): void
     {
         $this->systemConfigService = SystemConfigServiceMock::createWithCredentials();
+        $this->webhookSystemConfigHelper = $this->createMock(WebhookSystemConfigHelper::class);
+        $this->requestStack = new RequestStack();
     }
 
     public function testRemoveWebhookWithInheritedConfiguration(): void
@@ -73,6 +82,172 @@ public function testRemoveWebhookWithNoConfiguration(): void
         static::assertEmpty($this->systemConfigService->getString(Settings::WEBHOOK_ID, TestDefaults::SALES_CHANNEL));
     }
 
+    public function testCheckWebhookBefore(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], null);
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('checkWebhookBefore')
+            ->with(['' => ['some-key' => 'some-value']]);
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('needsCheck')
+            ->with(['some-key' => 'some-value'])
+            ->willReturn(true);
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookBefore($event);
+    }
+
+    public function testCheckWebhookBeforeWithSalesChannelId(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('checkWebhookBefore')
+            ->with(['some-sales-channel-id' => ['some-key' => 'some-value']]);
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('needsCheck')
+            ->with(['some-key' => 'some-value'])
+            ->willReturn(true);
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookBefore($event);
+    }
+
+    public function testCheckWebhookBeforeWithSaveRoute(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $request = new Request(attributes: ['_route' => 'api.action.paypal.settings.save']);
+        $this->requestStack->push($request);
+
+        $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::never())
+            ->method('checkWebhookBefore');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::never())
+            ->method('needsCheck');
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookBefore($event);
+    }
+
+    public function testCheckWebhookBeforeWithNoCheckNeeded(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $event = new BeforeSystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::never())
+            ->method('checkWebhookBefore');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('needsCheck')
+            ->with(['some-key' => 'some-value'])
+            ->willReturn(false);
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookBefore($event);
+    }
+
+    public function testCheckWebhookAfter(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], null);
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('checkWebhookAfter')
+            ->with(['']);
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('needsCheck')
+            ->with(['some-key' => 'some-value'])
+            ->willReturn(true);
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookAfter($event);
+    }
+
+    public function testCheckWebhookAfterWithSalesChannelId(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('checkWebhookAfter')
+            ->with(['some-sales-channel-id']);
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('needsCheck')
+            ->with(['some-key' => 'some-value'])
+            ->willReturn(true);
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookAfter($event);
+    }
+
+    public function testCheckWebhookAfterWithSaveRoute(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $request = new Request(attributes: ['_route' => 'api.action.paypal.settings.save']);
+        $this->requestStack->push($request);
+
+        $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::never())
+            ->method('checkWebhookAfter');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::never())
+            ->method('needsCheck');
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookAfter($event);
+    }
+
+    public function testCheckWebhookAfterWithNoCheckNeeded(): void
+    {
+        Feature::skipTestIfInActive('PAYPAL_SETTINGS_TWEAKS', $this);
+
+        $event = new SystemConfigMultipleChangedEvent(['some-key' => 'some-value'], 'some-sales-channel-id');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::never())
+            ->method('checkWebhookAfter');
+
+        $this->webhookSystemConfigHelper
+            ->expects(static::once())
+            ->method('needsCheck')
+            ->with(['some-key' => 'some-value'])
+            ->willReturn(false);
+
+        $this->createWebhookSubscriber(['' => null, TestDefaults::SALES_CHANNEL => null])
+            ->checkWebhookAfter($event);
+    }
+
     public function testSubscribedEvents(): void
     {
         static::assertEqualsCanonicalizing([
@@ -105,8 +280,8 @@ private function createWebhookSubscriber(array $configuration): WebhookSubscribe
             new NullLogger(),
             $this->systemConfigService,
             $webhookService,
-            $this->createMock(WebhookSystemConfigHelper::class),
-            new RequestStack(),
+            $this->webhookSystemConfigHelper,
+            $this->requestStack,
         );
     }