Skip to content

Commit db225ad

Browse files
GSadeeGSadee
authored
bug #918 Fix grid limits on request grid provider (loic425)
This PR was merged into the 1.11 branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT Missing this part from https://github.com/Sylius/SyliusResourceBundle/blob/1.12/src/Bundle/Controller/ResourcesCollectionProvider.php#L76 We do not have "limit" or "paginate" attribute on operation for now, only paginated resources is used with grids for now. Commits ------- de4a0cb Fix grid limits on request grid provider 3f64c5f Fix assert with selector count 277ae28 Fix indentation 2263e37 Add a constant for default max per page
2 parents 5eb1c35 + 2263e37 commit db225ad

File tree

8 files changed

+206
-27
lines changed

8 files changed

+206
-27
lines changed

‎composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
"twig/twig": "^2.12 || ^3.0",
8383
"vimeo/psalm": "^5.20",
8484
"rector/rector": "^0.18.2",
85+
"symfony/css-selector": "^5.4 || ^6.4",
8586
"symfony/messenger": "^5.4 || ^6.4",
8687
"symfony/serializer": "^5.4 || ^6.4",
8788
"symfony/security-bundle": "^5.4 || ^6.4"

‎src/Component/spec/Grid/State/RequestGridProviderSpec.php‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,101 @@ function it_sets_current_page_from_request(
8080
$request->query = new InputBag(['page' => 42]);
8181

8282
$gridProvider->get('app_book')->willReturn($gridDefinition);
83+
$gridDefinition->getLimits()->willReturn([]);
8384
$gridDefinition->getDriverConfiguration()->willReturn([]);
8485

8586
$gridViewFactory->create($gridDefinition, $context, new Parameters(['page' => 42]), [])->willReturn($gridView);
8687

8788
$gridView->getData()->willReturn($pagerfanta);
8889
$pagerfanta->setCurrentPage(42)->willReturn($pagerfanta)->shouldBeCalled();
90+
$pagerfanta->setMaxPerPage(10)->willReturn($pagerfanta)->shouldBeCalled();
91+
92+
$this->provide($operation, $context)
93+
->shouldReturn($gridView)
94+
;
95+
}
96+
97+
function it_sets_max_per_page_from_request(
98+
Request $request,
99+
GridViewFactoryInterface $gridViewFactory,
100+
GridProviderInterface $gridProvider,
101+
Grid $gridDefinition,
102+
GridView $gridView,
103+
Pagerfanta $pagerfanta,
104+
): void {
105+
$context = new Context(new RequestOption($request->getWrappedObject()));
106+
107+
$operation = new Index(grid: 'app_book');
108+
109+
$request->query = new InputBag(['limit' => 25]);
110+
111+
$gridProvider->get('app_book')->willReturn($gridDefinition);
112+
$gridDefinition->getDriverConfiguration()->willReturn([]);
113+
$gridDefinition->getLimits()->willReturn([10, 25]);
114+
115+
$gridViewFactory->create($gridDefinition, $context, new Parameters(['limit' => 25]), [])->willReturn($gridView);
116+
117+
$gridView->getData()->willReturn($pagerfanta);
118+
$pagerfanta->setCurrentPage(1)->willReturn($pagerfanta)->shouldBeCalled();
119+
$pagerfanta->setMaxPerPage(25)->willReturn($pagerfanta)->shouldBeCalled();
120+
121+
$this->provide($operation, $context)
122+
->shouldReturn($gridView)
123+
;
124+
}
125+
126+
function it_sets_max_per_page_from_grid_configuration(
127+
Request $request,
128+
GridViewFactoryInterface $gridViewFactory,
129+
GridProviderInterface $gridProvider,
130+
Grid $gridDefinition,
131+
GridView $gridView,
132+
Pagerfanta $pagerfanta,
133+
): void {
134+
$context = new Context(new RequestOption($request->getWrappedObject()));
135+
136+
$operation = new Index(grid: 'app_book');
137+
138+
$request->query = new InputBag();
139+
140+
$gridProvider->get('app_book')->willReturn($gridDefinition);
141+
$gridDefinition->getDriverConfiguration()->willReturn([]);
142+
$gridDefinition->getLimits()->willReturn([15, 30]);
143+
144+
$gridViewFactory->create($gridDefinition, $context, new Parameters([]), [])->willReturn($gridView);
145+
146+
$gridView->getData()->willReturn($pagerfanta);
147+
$pagerfanta->setCurrentPage(1)->willReturn($pagerfanta)->shouldBeCalled();
148+
$pagerfanta->setMaxPerPage(15)->willReturn($pagerfanta)->shouldBeCalled();
149+
150+
$this->provide($operation, $context)
151+
->shouldReturn($gridView)
152+
;
153+
}
154+
155+
function it_limits_max_per_page_with_max_grid_configuration_limit(
156+
Request $request,
157+
GridViewFactoryInterface $gridViewFactory,
158+
GridProviderInterface $gridProvider,
159+
Grid $gridDefinition,
160+
GridView $gridView,
161+
Pagerfanta $pagerfanta,
162+
): void {
163+
$context = new Context(new RequestOption($request->getWrappedObject()));
164+
165+
$operation = new Index(grid: 'app_book');
166+
167+
$request->query = new InputBag(['limit' => 40]);
168+
169+
$gridProvider->get('app_book')->willReturn($gridDefinition);
170+
$gridDefinition->getDriverConfiguration()->willReturn([]);
171+
$gridDefinition->getLimits()->willReturn([15, 30]);
172+
173+
$gridViewFactory->create($gridDefinition, $context, new Parameters(['limit' => 40]), [])->willReturn($gridView);
174+
175+
$gridView->getData()->willReturn($pagerfanta);
176+
$pagerfanta->setCurrentPage(1)->willReturn($pagerfanta)->shouldBeCalled();
177+
$pagerfanta->setMaxPerPage(30)->willReturn($pagerfanta)->shouldBeCalled();
89178

90179
$this->provide($operation, $context)
91180
->shouldReturn($gridView)

‎src/Component/src/Grid/State/RequestGridProvider.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
final class RequestGridProvider implements ProviderInterface
2727
{
28+
private const DEFAULT_MAX_PER_PAGE = 10;
29+
2830
public function __construct(
2931
private ?GridViewFactoryInterface $gridViewFactory = null,
3032
private ?GridProviderInterface $gridProvider = null,
@@ -64,8 +66,32 @@ public function provide(Operation $operation, Context $context): object|array|nu
6466
if ($data instanceof Pagerfanta) {
6567
$currentPage = $request->query->getInt('page', 1);
6668
$data->setCurrentPage($currentPage);
69+
70+
$maxPerPage = $this->resolveMaxPerPage(
71+
$request->query->has('limit') ? $request->query->getInt('limit') : null,
72+
$gridDefinition->getLimits(),
73+
);
74+
$data->setMaxPerPage($maxPerPage);
6775
}
6876

6977
return $gridView;
7078
}
79+
80+
private function resolveMaxPerPage(?int $requestLimit, array $gridLimits = []): int
81+
{
82+
if (null === $requestLimit) {
83+
$firstGridLimit = reset($gridLimits);
84+
85+
return false === $firstGridLimit ? self::DEFAULT_MAX_PER_PAGE : $firstGridLimit;
86+
}
87+
88+
if (!empty($gridLimits)) {
89+
$maxGridLimit = max($gridLimits);
90+
91+
// Cannot retrieve more items than configured in the grid
92+
return min($requestLimit, $maxGridLimit);
93+
}
94+
95+
return $requestLimit;
96+
}
7197
}

‎tests/Application/src/Subscription/Grid/SubscriptionGrid.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public function buildGrid(GridBuilderInterface $gridBuilder): void
3838
{
3939
$gridBuilder
4040
->orderBy('email', 'asc')
41+
->setLimits([5, 3, 10])
4142
->addField(
4243
StringField::create('email')
4344
->setLabel('Email')

‎tests/Application/src/Tests/Controller/SubscriptionUiTest.php‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ public function it_allows_browsing_subscriptions(): void
4747
$this->assertResponseCode($response, Response::HTTP_OK);
4848
$content = $response->getContent();
4949

50-
$this->assertStringContainsString('<td>[email protected]</td>', $content);
51-
$this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s">Show</a>', $subscriptions['subscription_marty']->getId()), $content);
52-
$this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s/edit">Edit</a>', $subscriptions['subscription_marty']->getId()), $content);
53-
$this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s/delete" method="post">', $subscriptions['subscription_marty']->getId()), $content);
50+
// only 5 subscriptions
51+
if (method_exists($this, 'assertSelectorCount')) {
52+
$this->assertSelectorCount(5, 'tbody tr');
53+
}
5454

5555
$this->assertStringContainsString('<td>[email protected]</td>', $content);
5656
$this->assertStringContainsString(sprintf('<a href="/admin/subscriptions/%s">Show</a>', $subscriptions['subscription_doc']->getId()), $content);
@@ -63,6 +63,38 @@ public function it_allows_browsing_subscriptions(): void
6363
$this->assertStringContainsString(sprintf('<form action="/admin/subscriptions/%s/delete" method="post">', $subscriptions['subscription_biff']->getId()), $content);
6464
}
6565

66+
/** @test */
67+
public function it_allows_browsing_subscriptions_with_page_limit(): void
68+
{
69+
$this->loadFixturesFromFile('subscriptions.yml');
70+
71+
$this->client->request('GET', '/admin/subscriptions?limit=3');
72+
$response = $this->client->getResponse();
73+
74+
$this->assertResponseCode($response, Response::HTTP_OK);
75+
76+
// only 2 subscriptions
77+
if (method_exists($this, 'assertSelectorCount')) {
78+
$this->assertSelectorCount(3, 'tbody tr');
79+
}
80+
}
81+
82+
/** @test */
83+
public function it_allows_browsing_subscriptions_with_grid_limits(): void
84+
{
85+
$this->loadFixturesFromFile('subscriptions.yml');
86+
87+
$this->client->request('GET', '/admin/subscriptions');
88+
$response = $this->client->getResponse();
89+
90+
$this->assertResponseCode($response, Response::HTTP_OK);
91+
92+
// only 5 subscriptions
93+
if (method_exists($this, 'assertSelectorCount')) {
94+
$this->assertSelectorCount(5, 'tbody tr');
95+
}
96+
}
97+
6698
/** @test */
6799
public function it_allows_creating_a_subscription(): void
68100
{
@@ -154,7 +186,7 @@ public function it_allows_deleting_multiple_subscriptions(): void
154186
{
155187
$this->loadFixturesFromFile('subscriptions.yml');
156188

157-
$this->client->request('GET', '/admin/subscriptions');
189+
$this->client->request('GET', '/admin/subscriptions?limit=10');
158190
$this->client->submitForm('Bulk delete');
159191

160192
$this->assertResponseRedirects(null, expectedCode: Response::HTTP_FOUND);
@@ -187,7 +219,7 @@ public function it_allows_accepting_multiple_subscription(): void
187219
{
188220
$this->loadFixturesFromFile('subscriptions.yml');
189221

190-
$this->client->request('GET', '/admin/subscriptions');
222+
$this->client->request('GET', '/admin/subscriptions?limit=10');
191223
$this->client->submitForm('Bulk accept');
192224

193225
$this->assertResponseRedirects(null, expectedCode: Response::HTTP_FOUND);

‎tests/Application/src/Tests/DataFixtures/ORM/subscriptions.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,9 @@ App\Subscription\Entity\Subscription:
66
subscription_biff:
77
email: '[email protected]'
88
state: accepted
9+
subscription_lorraine:
10+
email: '[email protected]'
11+
subscription_george:
12+
email: '[email protected]'
13+
subscription_jennifer:
14+
email: '[email protected]'

‎tests/Application/src/Tests/Responses/subscriptions/index_response.json‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,26 @@
1111
{
1212
"state": "accepted",
1313
"email": "[email protected]"
14+
},
15+
{
16+
"state": "new",
17+
"email": "[email protected]"
18+
},
19+
{
20+
"state": "new",
21+
"email": "[email protected]"
22+
},
23+
{
24+
"state": "new",
25+
"email": "[email protected]"
1426
}
1527
],
1628
"pagination": {
1729
"current_page": 1,
1830
"has_previous_page": false,
1931
"has_next_page": false,
2032
"per_page": 10,
21-
"total_items": 3,
33+
"total_items": 6,
2234
"total_pages": 1
2335
}
2436
}

‎tests/Application/src/Tests/Responses/subscriptions/index_response.xml‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,35 @@
11
<?xml version="1.0"?>
22
<response>
3-
<items>
4-
<state>new</state>
5-
<email>[email protected]</email>
6-
</items>
7-
<items>
8-
<state>new</state>
9-
<email>[email protected]</email>
10-
</items>
11-
<items>
12-
<state>accepted</state>
13-
<email>[email protected]</email>
14-
</items>
15-
<pagination>
16-
<current_page>1</current_page>
17-
<has_previous_page>0</has_previous_page>
18-
<has_next_page>0</has_next_page>
19-
<per_page>10</per_page>
20-
<total_items>3</total_items>
21-
<total_pages>1</total_pages>
22-
</pagination>
3+
<items>
4+
<state>new</state>
5+
<email>[email protected]</email>
6+
</items>
7+
<items>
8+
<state>new</state>
9+
<email>[email protected]</email>
10+
</items>
11+
<items>
12+
<state>accepted</state>
13+
<email>[email protected]</email>
14+
</items>
15+
<items>
16+
<state>new</state>
17+
<email>[email protected]</email>
18+
</items>
19+
<items>
20+
<state>new</state>
21+
<email>[email protected]</email>
22+
</items>
23+
<items>
24+
<state>new</state>
25+
<email>[email protected]</email>
26+
</items>
27+
<pagination>
28+
<current_page>1</current_page>
29+
<has_previous_page>0</has_previous_page>
30+
<has_next_page>0</has_next_page>
31+
<per_page>10</per_page>
32+
<total_items>6</total_items>
33+
<total_pages>1</total_pages>
34+
</pagination>
2335
</response>

0 commit comments

Comments
 (0)