Cleanup configuration shipping method after channel deletion (#18512)

| Q               | A
|-----------------|-----
| Branch?         | 2.1
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets |
| License         | MIT

Ref:
```
After having deleted a channel in the admin view, I got form errors when editing the shipping methods.

I found out that the “configuration” column of the “sylius_shipping_method” table still contains the channel data as json, e.g.

{"WEB_EUR": {"amount": 960}, "FASHION_WEB": {"amount": 782}}

although both channels are deleted.

The users of our software need to delete the channel without errors.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
  * Find shipping methods by channel-specific configuration.
  * Automatically remove channel configuration entries from shipping methods when a channel is deleted.
  * Added a channel-aware shipping-method updater and wired it into the channel deletion workflow.

* **Tests**
  * Added tests for shipping-method lookup by channel code.
  * Added tests for channel-configuration removal (including flush behavior and edge cases).
  * Added fixtures for shipping methods and zones used by tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rafał Jaskulski 2025-11-21 13:24:12 +01:00 committed by GitHub
commit 5539dae490
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 574 additions and 7 deletions

View file

@ -98,4 +98,4 @@ jobs:
chmod +x robo.phar && sudo mv robo.phar /tmp/robo
- name: "Run pipeline"
run: find src/Sylius -mindepth 3 -maxdepth 3 -type f -name composer.json -exec dirname '{}' \; | sed -e 's/src\/Sylius\///g' | sort | jq --raw-input . | jq --slurp . | jq -c . | xargs -0 /tmp/robo ci:packages
run: find src/Sylius -mindepth 3 -maxdepth 3 -type f -name composer.json -exec dirname '{}' \; | sed -e 's/src\/Sylius\///g' | sort | jq --raw-input . | jq --slurp . | jq -c . | xargs /tmp/robo ci:packages

View file

@ -54,6 +54,17 @@ class ShippingMethodRepository extends BaseShippingMethodRepository implements S
;
}
public function findByChannelCodeInConfiguration(string $channelCode): array
{
$shippingMethods = $this->findAll();
return array_filter($shippingMethods, function (ShippingMethodInterface $method) use ($channelCode) {
$configuration = $method->getConfiguration();
return array_key_exists($channelCode, $configuration);
});
}
protected function createEnabledForChannelQueryBuilder(ChannelInterface $channel): QueryBuilder
{
return $this->createQueryBuilder('o')

View file

@ -15,19 +15,43 @@ namespace Sylius\Bundle\CoreBundle\EventListener;
use Sylius\Component\Channel\Checker\ChannelDeletionCheckerInterface;
use Sylius\Component\Channel\Model\ChannelInterface;
use Sylius\Component\Core\ShippingMethod\Updater\ChannelAwareShippingMethodUpdaterInterface;
use Sylius\Resource\Exception\UnexpectedTypeException;
use Sylius\Resource\Symfony\EventDispatcher\GenericEvent;
final class ChannelDeletionListener
final readonly class ChannelDeletionListener
{
public function __construct(private ChannelDeletionCheckerInterface $channelDeletionChecker)
{
public function __construct(
private ChannelDeletionCheckerInterface $channelDeletionChecker,
private ChannelAwareShippingMethodUpdaterInterface $shippingMethodUpdater,
) {
}
/**
* Prevent channel deletion if no more channels enabled.
*/
public function onChannelPreDelete(GenericEvent $event): void
{
$channel = $this->getChannel($event);
if (!$this->channelDeletionChecker->isDeletable($channel)) {
$event->stop('sylius.channel.delete_error');
}
}
public function removeChannelConfigurationFromShippingMethods(GenericEvent $event): void
{
$channel = $this->getChannel($event);
$channelCode = $channel->getCode();
if ($channelCode === null) {
return;
}
$this->shippingMethodUpdater->removeChannelConfigurationFromShippingMethods($channelCode);
}
private function getChannel(GenericEvent $event): ChannelInterface
{
$channel = $event->getSubject();
@ -38,8 +62,6 @@ final class ChannelDeletionListener
);
}
if (!$this->channelDeletionChecker->isDeletable($channel)) {
$event->stop('sylius.channel.delete_error');
}
return $channel;
}
}

View file

@ -296,5 +296,13 @@
<argument type="service" id="security.firewall.map" />
<tag name="security.voter" />
</service>
<service id="sylius.updater.shipping_method"
class="Sylius\Bundle\CoreBundle\ShippingMethod\Updater\ShippingMethodUpdater">
<argument type="service" id="sylius.repository.shipping_method" />
<argument type="service" id="doctrine.orm.entity_manager" />
</service>
<service id="Sylius\Component\Core\ShippingMethod\Updater\ChannelAwareShippingMethodUpdaterInterface"
alias="sylius.updater.shipping_method" />
</services>
</container>

View file

@ -25,7 +25,9 @@
<service id="sylius.listener.channel_deletion" class="Sylius\Bundle\CoreBundle\EventListener\ChannelDeletionListener">
<argument type="service" id="sylius.checker.channel_deletion" />
<argument type="service" id="sylius.updater.shipping_method" />
<tag name="kernel.event_listener" event="sylius.channel.pre_delete" method="onChannelPreDelete" />
<tag name="kernel.event_listener" event="sylius.channel.post_delete" method="removeChannelConfigurationFromShippingMethods" />
</service>
<service id="sylius.listener.images_upload" class="Sylius\Bundle\CoreBundle\EventListener\ImagesUploadListener">

View file

@ -0,0 +1,40 @@
<?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\Bundle\CoreBundle\ShippingMethod\Updater;
use Doctrine\ORM\EntityManagerInterface;
use Sylius\Component\Core\Repository\ShippingMethodRepositoryInterface;
use Sylius\Component\Core\ShippingMethod\Updater\ChannelAwareShippingMethodUpdaterInterface;
readonly class ShippingMethodUpdater implements ChannelAwareShippingMethodUpdaterInterface
{
public function __construct(
private ShippingMethodRepositoryInterface $shippingMethodRepository,
private EntityManagerInterface $entityManager,
) {
}
public function removeChannelConfigurationFromShippingMethods(string $channelCode): void
{
$shippingMethods = $this->shippingMethodRepository->findByChannelCodeInConfiguration($channelCode);
foreach ($shippingMethods as $shippingMethod) {
$configuration = $shippingMethod->getConfiguration();
unset($configuration[$channelCode]);
$shippingMethod->setConfiguration($configuration);
}
$this->entityManager->flush();
}
}

View file

@ -0,0 +1,100 @@
<?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 Tests\Sylius\Bundle\CoreBundle\Doctrine\ORM;
use Doctrine\Common\DataFixtures\Purger\ORMPurger;
use Doctrine\ORM\EntityManagerInterface;
use Fidry\AliceDataFixtures\LoaderInterface;
use Fidry\AliceDataFixtures\Persistence\PurgeMode;
use PHPUnit\Framework\Attributes\Test;
use Sylius\Component\Core\Model\ShippingMethodInterface;
use Sylius\Component\Core\Repository\ShippingMethodRepositoryInterface;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
final class ShippingMethodRepositoryTest extends KernelTestCase
{
/** @var ShippingMethodRepositoryInterface<ShippingMethodInterface> */
private ShippingMethodRepositoryInterface $repository;
protected function setUp(): void
{
parent::setUp();
$this->loadFixtures();
/** @var ShippingMethodRepositoryInterface<ShippingMethodInterface> $repository */
$repository = self::getContainer()->get('sylius.repository.shipping_method');
$this->repository = $repository;
}
#[Test]
public function it_finds_shipping_methods_by_channel_code_in_configuration(): void
{
$shippingMethods = $this->repository->findByChannelCodeInConfiguration('WEB_US');
self::assertCount(2, $shippingMethods);
self::assertEqualsCanonicalizing(
['dhl_express', 'ups'],
$this->getShippingMethodCodes($shippingMethods),
);
}
#[Test]
public function it_returns_empty_array_when_channel_code_not_found(): void
{
$shippingMethods = $this->repository->findByChannelCodeInConfiguration('NON_EXISTENT_CHANNEL');
self::assertCount(0, $shippingMethods);
}
#[Test]
public function it_finds_multiple_shipping_methods_with_same_channel_code(): void
{
$shippingMethods = $this->repository->findByChannelCodeInConfiguration('WEB_EU');
self::assertCount(2, $shippingMethods);
self::assertEqualsCanonicalizing(
['dhl_express', 'fedex'],
$this->getShippingMethodCodes($shippingMethods),
);
}
/**
* @param ShippingMethodInterface[] $shippingMethods
*
* @return string[]
*/
private function getShippingMethodCodes(array $shippingMethods): array
{
return array_map(
static fn (ShippingMethodInterface $method): string => $method->getCode(),
$shippingMethods,
);
}
private function loadFixtures(): void
{
/** @var LoaderInterface $fixtureLoader */
$fixtureLoader = self::getContainer()->get('fidry_alice_data_fixtures.loader.doctrine');
/** @var EntityManagerInterface $manager */
$manager = self::getContainer()->get('doctrine.orm.default_entity_manager');
(new ORMPurger($manager))->purge();
$fixtureLoader->load([
__DIR__ . '/ShippingMethodRepositoryTest/fixtures.yaml',
], [], [], PurgeMode::createDeleteMode());
}
}

View file

@ -0,0 +1,49 @@
Sylius\Component\Addressing\Model\Zone:
zone_us:
code: US
name: United States
type: country
zone_eu:
code: EU
name: European Union
type: country
Sylius\Component\Core\Model\ShippingMethod:
shipping_method_dhl:
code: dhl_express
calculator: flat_rate
configuration:
WEB_US:
amount: 1000
WEB_EU:
amount: 1500
zone: "@zone_us"
enabled: true
shipping_method_ups:
code: ups
calculator: flat_rate
configuration:
WEB_US:
amount: 1200
zone: "@zone_us"
enabled: true
shipping_method_fedex:
code: fedex
calculator: flat_rate
configuration:
WEB_EU:
amount: 2000
zone: "@zone_eu"
enabled: true
shipping_method_free:
code: free_shipping
calculator: flat_rate
configuration:
OTHER_CHANNEL:
amount: 0
zone: "@zone_us"
enabled: true

View file

@ -0,0 +1,137 @@
<?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 Tests\Sylius\Bundle\CoreBundle\EventListener;
use InvalidArgumentException;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Sylius\Bundle\CoreBundle\EventListener\ChannelDeletionListener;
use Sylius\Component\Channel\Checker\ChannelDeletionCheckerInterface;
use Sylius\Component\Channel\Model\ChannelInterface;
use Sylius\Component\Core\ShippingMethod\Updater\ChannelAwareShippingMethodUpdaterInterface;
use Sylius\Resource\Symfony\EventDispatcher\GenericEvent;
final class ChannelDeletionListenerTest extends TestCase
{
private ChannelDeletionCheckerInterface&MockObject $channelDeletionChecker;
private ChannelAwareShippingMethodUpdaterInterface&MockObject $shippingMethodUpdater;
private ChannelDeletionListener $channelDeletionListener;
protected function setUp(): void
{
$this->channelDeletionChecker = $this->createMock(ChannelDeletionCheckerInterface::class);
$this->shippingMethodUpdater = $this->createMock(ChannelAwareShippingMethodUpdaterInterface::class);
$this->channelDeletionListener = new ChannelDeletionListener(
$this->channelDeletionChecker,
$this->shippingMethodUpdater,
);
}
public function testThrowsAnExceptionWhenSubjectIsNotAChannelOnPreDelete(): void
{
$event = $this->createEventWithSubject('invalid_subject');
$this->expectException(InvalidArgumentException::class);
$this->channelDeletionListener->onChannelPreDelete($event);
}
public function testAllowsChannelDeletionWhenChannelIsDeletable(): void
{
$channel = $this->createMock(ChannelInterface::class);
$event = $this->createEventWithSubject($channel);
$this->channelDeletionChecker
->expects(self::once())
->method('isDeletable')
->with($channel)
->willReturn(true)
;
$event->expects(self::never())->method('stop');
$this->channelDeletionListener->onChannelPreDelete($event);
}
public function testPreventsChannelDeletionWhenChannelIsNotDeletable(): void
{
$channel = $this->createMock(ChannelInterface::class);
$event = $this->createEventWithSubject($channel);
$this->channelDeletionChecker
->expects(self::once())
->method('isDeletable')
->with($channel)
->willReturn(false)
;
$event->expects(self::once())->method('stop')->with('sylius.channel.delete_error');
$this->channelDeletionListener->onChannelPreDelete($event);
}
public function testThrowsAnExceptionWhenSubjectIsNotAChannelOnPostDelete(): void
{
$event = $this->createEventWithSubject('invalid_subject');
$this->expectException(InvalidArgumentException::class);
$this->channelDeletionListener->removeChannelConfigurationFromShippingMethods($event);
}
public function testDoesNotRemoveConfigurationWhenChannelHasNoCode(): void
{
$channel = $this->createChannelWithCode(null);
$event = $this->createEventWithSubject($channel);
$this->shippingMethodUpdater
->expects(self::never())
->method('removeChannelConfigurationFromShippingMethods')
;
$this->channelDeletionListener->removeChannelConfigurationFromShippingMethods($event);
}
public function testRemovesChannelCodeFromShippingMethodConfiguration(): void
{
$channel = $this->createChannelWithCode('DELETED_CHANNEL');
$event = $this->createEventWithSubject($channel);
$this->shippingMethodUpdater
->expects(self::once())
->method('removeChannelConfigurationFromShippingMethods')
->with('DELETED_CHANNEL')
;
$this->channelDeletionListener->removeChannelConfigurationFromShippingMethods($event);
}
private function createEventWithSubject(mixed $subject): GenericEvent&MockObject
{
$event = $this->createMock(GenericEvent::class);
$event->expects(self::once())->method('getSubject')->willReturn($subject);
return $event;
}
private function createChannelWithCode(?string $code): ChannelInterface&MockObject
{
$channel = $this->createMock(ChannelInterface::class);
$channel->expects(self::once())->method('getCode')->willReturn($code);
return $channel;
}
}

View file

@ -0,0 +1,174 @@
<?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 Tests\Sylius\Bundle\CoreBundle\ShippingMethod\Updater;
use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Sylius\Bundle\CoreBundle\ShippingMethod\Updater\ShippingMethodUpdater;
use Sylius\Component\Core\Model\ShippingMethodInterface;
use Sylius\Component\Core\Repository\ShippingMethodRepositoryInterface;
final class ShippingMethodUpdaterTest extends TestCase
{
private MockObject&ShippingMethodRepositoryInterface $shippingMethodRepository;
private EntityManagerInterface&MockObject $entityManager;
private ShippingMethodUpdater $shippingMethodUpdater;
protected function setUp(): void
{
$this->shippingMethodRepository = $this->createMock(ShippingMethodRepositoryInterface::class);
$this->entityManager = $this->createMock(EntityManagerInterface::class);
$this->shippingMethodUpdater = new ShippingMethodUpdater(
$this->shippingMethodRepository,
$this->entityManager,
);
}
public function testRemovesChannelCodeFromShippingMethodConfiguration(): void
{
$shippingMethod1 = $this->createShippingMethodWithConfiguration([
'DELETED_CHANNEL' => ['amount' => 100],
'OTHER_CHANNEL' => ['amount' => 200],
]);
$shippingMethod2 = $this->createShippingMethodWithConfiguration([
'DELETED_CHANNEL' => ['amount' => 300],
]);
$this->shippingMethodRepository
->expects(self::once())
->method('findByChannelCodeInConfiguration')
->with('DELETED_CHANNEL')
->willReturn([$shippingMethod1, $shippingMethod2])
;
$shippingMethod1
->expects(self::once())
->method('setConfiguration')
->with(['OTHER_CHANNEL' => ['amount' => 200]])
;
$shippingMethod2
->expects(self::once())
->method('setConfiguration')
->with([])
;
$this->entityManager->expects(self::once())->method('flush');
$this->shippingMethodUpdater->removeChannelConfigurationFromShippingMethods('DELETED_CHANNEL');
}
public function testFlushesChangesEvenWhenNoShippingMethodsFound(): void
{
$this->shippingMethodRepository
->expects(self::once())
->method('findByChannelCodeInConfiguration')
->with('DELETED_CHANNEL')
->willReturn([])
;
$this->entityManager->expects(self::once())->method('flush');
$this->shippingMethodUpdater->removeChannelConfigurationFromShippingMethods('DELETED_CHANNEL');
}
public function testRemovesOnlySpecifiedChannelCodeFromConfiguration(): void
{
$shippingMethod = $this->createShippingMethodWithConfiguration([
'CHANNEL_A' => ['amount' => 100],
'CHANNEL_B' => ['amount' => 200],
'CHANNEL_C' => ['amount' => 300],
]);
$this->shippingMethodRepository
->expects(self::once())
->method('findByChannelCodeInConfiguration')
->with('CHANNEL_B')
->willReturn([$shippingMethod])
;
$shippingMethod
->expects(self::once())
->method('setConfiguration')
->with([
'CHANNEL_A' => ['amount' => 100],
'CHANNEL_C' => ['amount' => 300],
])
;
$this->entityManager->expects(self::once())->method('flush');
$this->shippingMethodUpdater->removeChannelConfigurationFromShippingMethods('CHANNEL_B');
}
public function testHandlesMultipleShippingMethodsWithDifferentConfigurations(): void
{
$shippingMethod1 = $this->createShippingMethodWithConfiguration([
'DELETED_CHANNEL' => ['amount' => 100],
]);
$shippingMethod2 = $this->createShippingMethodWithConfiguration([
'DELETED_CHANNEL' => ['amount' => 200, 'tax_rate' => 0.23],
'ANOTHER_CHANNEL' => ['amount' => 300],
]);
$shippingMethod3 = $this->createShippingMethodWithConfiguration([
'DELETED_CHANNEL' => ['custom_config' => 'value'],
'CHANNEL_X' => ['amount' => 400],
'CHANNEL_Y' => ['amount' => 500],
]);
$this->shippingMethodRepository
->expects(self::once())
->method('findByChannelCodeInConfiguration')
->with('DELETED_CHANNEL')
->willReturn([$shippingMethod1, $shippingMethod2, $shippingMethod3])
;
$shippingMethod1
->expects(self::once())
->method('setConfiguration')
->with([])
;
$shippingMethod2
->expects(self::once())
->method('setConfiguration')
->with(['ANOTHER_CHANNEL' => ['amount' => 300]])
;
$shippingMethod3
->expects(self::once())
->method('setConfiguration')
->with([
'CHANNEL_X' => ['amount' => 400],
'CHANNEL_Y' => ['amount' => 500],
])
;
$this->entityManager->expects(self::once())->method('flush');
$this->shippingMethodUpdater->removeChannelConfigurationFromShippingMethods('DELETED_CHANNEL');
}
/** @param array<string, mixed> $configuration */
private function createShippingMethodWithConfiguration(array $configuration): MockObject&ShippingMethodInterface
{
$shippingMethod = $this->createMock(ShippingMethodInterface::class);
$shippingMethod->expects(self::once())->method('getConfiguration')->willReturn($configuration);
return $shippingMethod;
}
}

View file

@ -38,4 +38,9 @@ interface ShippingMethodRepositoryInterface extends BaseShippingMethodRepository
* @return T[]
*/
public function findEnabledForZonesAndChannel(array $zones, ChannelInterface $channel): array;
/**
* @return T[]
*/
public function findByChannelCodeInConfiguration(string $channelCode): array;
}

View file

@ -0,0 +1,19 @@
<?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\Core\ShippingMethod\Updater;
interface ChannelAwareShippingMethodUpdaterInterface
{
public function removeChannelConfigurationFromShippingMethods(string $channelCode): void;
}