Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bulk delete #611

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Bundle/Routing/OperationRouteFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ private function getDefaultRoutePathForOperation(MetadataInterface $metadata, Ht
return sprintf('%s/{id}', $rootPath);
}

if ('bulk_delete' === $shortName) {
return sprintf('%s/bulk_delete', $rootPath);
}

if ('show' === $shortName) {
return sprintf('%s/{id}', $rootPath);
}
Expand Down
21 changes: 21 additions & 0 deletions src/Bundle/spec/Routing/OperationRouteFactorySpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use PhpSpec\ObjectBehavior;
use Sylius\Bundle\ResourceBundle\Routing\OperationRouteFactory;
use Sylius\Component\Resource\Action\PlaceHolderAction;
use Sylius\Component\Resource\Metadata\BulkDelete;
use Sylius\Component\Resource\Metadata\Create;
use Sylius\Component\Resource\Metadata\Delete;
use Sylius\Component\Resource\Metadata\HttpOperation;
Expand Down Expand Up @@ -132,6 +133,26 @@ function it_generates_delete_routes(): void
]);
}

function it_generates_bulk_delete_routes(): void
{
$metadata = Metadata::fromAliasAndConfiguration('app.dummy', ['driver' => 'dummy_driver']);

$route = $this->create(
$metadata,
new Resource('app.dummy'),
new BulkDelete(),
);

$route->getPath()->shouldReturn('/dummies/bulk_delete');
$route->getMethods()->shouldReturn(['DELETE']);
$route->getDefaults()->shouldReturn([
'_controller' => PlaceHolderAction::class,
'_sylius' => [
'resource' => 'app.dummy',
],
]);
}

function it_generates_custom_operations_routes(): void
{
$metadata = Metadata::fromAliasAndConfiguration('app.dummy', ['driver' => 'dummy_driver']);
Expand Down
2 changes: 2 additions & 0 deletions src/Bundle/test/config/packages/test/sylius_grid.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ sylius_grid:
delete: 'grid/action/delete.html.twig'
show: 'grid/action/show.html.twig'
update: 'grid/action/update.html.twig'
bulk_action:
delete: 'grid/bulk_action/delete.html.twig'
12 changes: 6 additions & 6 deletions src/Bundle/test/src/Subscription/Entity/Subscription.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
use App\Subscription\Form\Type\SubscriptionType;
use Doctrine\ORM\Mapping as ORM;
use Sylius\Component\Resource\Metadata\ApplyStateMachineTransition;
use Sylius\Component\Resource\Metadata\BulkDelete;
use Sylius\Component\Resource\Metadata\Create;
use Sylius\Component\Resource\Metadata\Delete;
use Sylius\Component\Resource\Metadata\Index;
use Sylius\Component\Resource\Metadata\Resource;
use Sylius\Component\Resource\Metadata\Show;
use Sylius\Component\Resource\Metadata\Update;
use Sylius\Component\Resource\Model\ResourceInterface;
use Symfony\Component\Uid\Uuid;
use Symfony\Component\Validator\Constraints as Assert;

#[Resource(
Expand All @@ -36,6 +36,7 @@
#[Index(grid: 'app_subscription')]
#[Create]
#[Update]
#[BulkDelete]
#[ApplyStateMachineTransition(stateMachineTransition: 'accept')]
#[ApplyStateMachineTransition(stateMachineTransition: 'reject')]
#[Delete]
Expand All @@ -48,10 +49,9 @@ class Subscription implements ResourceInterface

public function __construct(
#[ORM\Id]
#[ORM\Column(type: 'uuid', unique: true)]
#[ORM\GeneratedValue(strategy: 'CUSTOM')]
#[ORM\CustomIdGenerator(class: 'doctrine.uuid_generator')]
public ?Uuid $id = null,
#[ORM\Column(type: 'integer', unique: true)]
#[ORM\GeneratedValue(strategy: 'AUTO')]
public ?int $id = null,

#[Assert\NotBlank]
#[Assert\Email]
Expand All @@ -60,7 +60,7 @@ public function __construct(
) {
}

public function getId(): ?Uuid
public function getId(): ?int
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've simplified this cause I had some troubles. But it works on a test project with an uuid on a mysql database.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were the troubles?

Copy link
Member Author

@loic425 loic425 Mar 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The findById($ids) did not work with SQLite, but I have to check it that again later.
But if there is a bug with that on SQLite, we already have a bug on current implementation.

{
return $this->id;
}
Expand Down
6 changes: 6 additions & 0 deletions src/Bundle/test/src/Subscription/Grid/SubscriptionGrid.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Sylius\Bundle\GridBundle\Builder\Action\DeleteAction;
use Sylius\Bundle\GridBundle\Builder\Action\ShowAction;
use Sylius\Bundle\GridBundle\Builder\Action\UpdateAction;
use Sylius\Bundle\GridBundle\Builder\ActionGroup\BulkActionGroup;
use Sylius\Bundle\GridBundle\Builder\ActionGroup\ItemActionGroup;
use Sylius\Bundle\GridBundle\Builder\ActionGroup\MainActionGroup;
use Sylius\Bundle\GridBundle\Builder\Field\StringField;
Expand Down Expand Up @@ -67,6 +68,11 @@ public function buildGrid(GridBuilderInterface $gridBuilder): void
]),
),
)
->addActionGroup(
BulkActionGroup::create(
DeleteAction::create(),
),
)
;
}

Expand Down
16 changes: 16 additions & 0 deletions src/Bundle/test/src/Tests/Controller/SubscriptionUiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,22 @@ public function it_allows_deleting_a_subscription(): void
$this->assertEmpty($subscriptions);
}

/** @test */
public function it_allows_deleting_multiple_subscriptions(): void
{
$this->loadFixturesFromFile('subscriptions.yml');

$this->client->request('GET', '/admin/subscriptions');
$this->client->submitForm('Bulk delete');

$this->assertResponseRedirects(null, expectedCode: Response::HTTP_FOUND);
Comment on lines +145 to +148
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It checks that the redirect is ok. (it's a code from @Zales0123 on ScienceBookUiTest)


/** @var Subscription[] $subscriptions */
$subscriptions = static::getContainer()->get('app.repository.subscription')->findAll();

$this->assertEmpty($subscriptions);
}

/** @test */
public function it_allows_accepting_a_subscription(): void
{
Expand Down
6 changes: 6 additions & 0 deletions src/Bundle/test/templates/crud/index.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
{% set grid = resources %}
{% set definition = grid.definition %}

{% if definition.actionGroups.bulk is defined and definition.getEnabledActions('bulk')|length > 0 %}
{% for action in definition.getEnabledActions('bulk') %}
{{ sylius_grid_render_bulk_action(grid, action, null) }}
{% endfor %}
{% endif %}

<table>
<thead>
<tr>
Expand Down
13 changes: 13 additions & 0 deletions src/Bundle/test/templates/grid/bulk_action/delete.html.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{% set path = options.link.url|default(path(options.link.route|default(grid.requestConfiguration.getRouteName('bulk_delete')), options.link.parameters|default({}))) %}

<form action="{{ path }}" method="post">
<input type="hidden" name="_method" value="DELETE">
<button type="submit">
<i class="icon trash"></i> Bulk delete
</button>
<input type="hidden" name="_csrf_token" value="{{ csrf_token('bulk_delete') }}" />

{% for resource in grid.data %}
<input type="hidden" name="ids[]" value="{{ resource.id }}" />
{% endfor %}
</form>
14 changes: 9 additions & 5 deletions src/Component/Doctrine/Common/State/RemoveProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@ public function __construct(private ManagerRegistry $managerRegistry)

public function process(mixed $data, Operation $operation, Context $context): mixed
{
if (!\is_object($data) || !$manager = $this->getManager($data)) {
return null;
}
$data = \is_array($data) ? $data : [$data];

foreach ($data as $row) {
if (!\is_object($row) || !$manager = $this->getManager($row)) {
return null;
}

$manager->remove($data);
$manager->flush();
$manager->remove($row);
$manager->flush();
}

return null;
}
Expand Down
59 changes: 59 additions & 0 deletions src/Component/Metadata/BulkDelete.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* 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;

/**
* @experimental
*/
#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
final class BulkDelete extends HttpOperation implements DeleteOperationInterface, BulkOperationInterface
{
public function __construct(
?array $methods = null,
?string $path = null,
?string $routePrefix = null,
?string $template = null,
?string $shortName = null,
?string $name = null,
string|callable|null $provider = null,
string|callable|null $processor = null,
string|callable|null $responder = null,
string|callable|null $repository = null,
?string $repositoryMethod = null,
?bool $read = null,
?bool $write = null,
?string $formType = null,
?array $formOptions = null,
?string $redirectToRoute = null,
) {
parent::__construct(
methods: $methods ?? ['DELETE'],
path: $path,
routePrefix: $routePrefix,
template: $template,
shortName: $shortName ?? 'bulk_delete',
name: $name,
provider: $provider,
processor: $processor,
responder: $responder,
repository: $repository,
repositoryMethod: $repositoryMethod,
read: $read,
write: $write,
formType: $formType,
formOptions: $formOptions,
redirectToRoute: $redirectToRoute,
);
}
}
23 changes: 23 additions & 0 deletions src/Component/Metadata/BulkOperationInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* 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;

/**
* The Operation returns a collection.
*
* @experimental
*/
interface BulkOperationInterface
{
}
35 changes: 29 additions & 6 deletions src/Component/Symfony/Request/RepositoryArgumentResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,39 @@ final class RepositoryArgumentResolver
{
public function getArguments(Request $request, \ReflectionFunctionAbstract $reflector): array
{
$arguments = array_merge($request->attributes->all('_route_params'), $request->query->all());
$matchedArguments = FunctionArgumentsFilter::filter($reflector, $arguments);
$allArguments = [
$request->attributes->all('_route_params'),
$request->query->all(),
$request->request->all(),
];

if (0 === count($matchedArguments) && $this->hasOnlyOneRequiredArrayParameter($reflector)) {
$arguments = $this->filterPrivateArguments($arguments);
foreach ($allArguments as $arguments) {
$matchedArguments = FunctionArgumentsFilter::filter($reflector, $arguments);

return [$arguments];
if (0 === count($matchedArguments) && $this->hasOnlyOneRequiredArrayParameter($reflector)) {
$arguments = $this->filterPrivateArguments($arguments);

return [$arguments];
}

if ('__call' === $reflector->getName()) {
$arguments = $this->filterPrivateArguments($arguments);

if ([] === $arguments) {
continue;
}

return array_values($arguments);
}

if ([] === $matchedArguments) {
continue;
}

return $matchedArguments;
}

return $matchedArguments;
return [];
}

/**
Expand Down
23 changes: 21 additions & 2 deletions src/Component/Symfony/Request/State/Provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Psr\Container\ContainerInterface;
use Sylius\Component\Resource\Context\Context;
use Sylius\Component\Resource\Context\Option\RequestOption;
use Sylius\Component\Resource\Metadata\BulkOperationInterface;
use Sylius\Component\Resource\Metadata\CollectionOperationInterface;
use Sylius\Component\Resource\Metadata\Operation;
use Sylius\Component\Resource\Reflection\CallableReflection;
Expand All @@ -43,8 +44,15 @@ public function provide(Operation $operation, Context $context): object|iterable
return null;
}

$repositoryInstance = null;

if (\is_string($repository)) {
$defaultMethod = $operation instanceof CollectionOperationInterface ? 'createPaginator' : 'findOneBy';

if ($operation instanceof BulkOperationInterface) {
$defaultMethod = 'findById';
}

$method = $operation->getRepositoryMethod() ?? $defaultMethod;

if (!$this->locator->has($repository)) {
Expand All @@ -58,9 +66,20 @@ public function provide(Operation $operation, Context $context): object|iterable
$repository = [$repositoryInstance, $method];
}

$reflector = CallableReflection::from($repository);
$arguments = $this->argumentResolver->getArguments($request, $reflector);
try {
$reflector = CallableReflection::from($repository);
} catch (\ReflectionException $exception) {
if (null === $repositoryInstance) {
throw $exception;
}

/** @var callable $callable */
$callable = [$repositoryInstance, '__call'];

$reflector = CallableReflection::from($callable);
}

$arguments = $this->argumentResolver->getArguments($request, $reflector);
$data = $repository(...$arguments);

if ($data instanceof Pagerfanta) {
Expand Down
5 changes: 5 additions & 0 deletions src/Component/Tests/Dummy/RepositoryWithCallables.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,9 @@ public static function findOneBy(array $criteria, ?array $orderBy = null): array
{
return [];
}

public function __call(string $method, mixed $arguments): array
{
return [];
}
}
Loading