Upgrade rest bundle #167

loic425 · 2020-06-04T18:45:04Z

Q	A
Bug fix?	no
New feature?	no
BC breaks?	not anymore
Deprecations?	no
Related tickets
License	MIT

loic425 · 2020-06-04T19:54:37Z

@pamil we have a lot of work here 🤣

loic425 · 2020-06-09T13:05:54Z

@lchrusciel @pamil
I worked a lot today on this pull request. Tell me if I am in the good direction or not.

src/Bundle/Controller/Parameters.php

src/Bundle/Controller/ViewHandler.php

src/Bundle/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php

src/Bundle/test/app/config/config.yml

…Handler.php Co-authored-by: Kamil Kokot <kamil@kokot.me>

loic425 · 2020-06-10T13:43:39Z

@pamil To copy here what we said in slack.

We can create a recipe for this bundle with this configuration:

fos_rest:
    body_listener:
        enabled: true

src/Bundle/test/app/config/config.yml

UPGRADE-1.7.md

src/Bundle/Controller/ViewHandler.php

src/Bundle/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php

src/Bundle/test/app/config/config.yml

Co-authored-by: Kamil Kokot <kamil@kokot.me>

…Handler.php Co-authored-by: Kamil Kokot <kamil@kokot.me>

lchrusciel

looks good to me

lchrusciel · 2020-06-19T11:22:13Z

src/Bundle/Controller/ResourcesCollectionProvider.php

                $paginationLimits
            ));
-            $paginator->setCurrentPage($request->query->get('page', 1));
+            $currentPage = (int) $request->query->get('page', '1');


Woulndn't it be better?

Suggested change

$currentPage = (int) $request->query->get('page', '1');

$currentPage = (int) $request->query->getInt('page', '1');

@lchrusciel Yes, but, if it returns an integer, we don't need the (int) conversion after :)

I pushed another suggestion to remove the unnecessary int conversion in your suggestion.

loic425 · 2020-06-23T07:20:42Z

src/Bundle/Controller/ResourcesCollectionProvider.php

                $paginationLimits
            ));
-            $paginator->setCurrentPage($request->query->get('page', 1));
+            $currentPage = (int) $request->query->get('page', '1');


@lchrusciel

Suggested change

$currentPage = (int) $request->query->get('page', '1');

$currentPage = $request->query->getInt('page', '1');

pamil · 2020-06-25T12:38:20Z

Thank you, Loïc! 🎉

Upgrade rest bundle

1ce86e6

loic425 requested a review from a team as a code owner June 4, 2020 18:45

Use templating or twig for template rendering

f4fdada

loic425 added 2 commits June 10, 2020 09:46

Fix psalm errors

ce8f902

Fix phpspec errors

90a376f

loic425 mentioned this pull request Jun 10, 2020

The road to Symfony 5 Sylius/Sylius#10928

Closed

loic425 added 2 commits June 10, 2020 10:01

Fix coding standard

61420b7

Enable body listener on fos rest configuration

6f9ef32

pamil reviewed Jun 10, 2020

View reviewed changes

loic425 and others added 4 commits June 10, 2020 13:12

Update src/Bundle/Form/Extension/HttpFoundation/HttpFoundationRequest…

9b33f88

…Handler.php Co-authored-by: Kamil Kokot <kamil@kokot.me>

Remove a bc-break

f988b35

Remove an unnecessary change on fos rest config

ac1e9b5

Add an upgrade guide

2bf3bcd

loic425 commented Jun 10, 2020

View reviewed changes

src/Bundle/test/app/config/config.yml Outdated Show resolved Hide resolved

pamil reviewed Jun 17, 2020

View reviewed changes

loic425 and others added 4 commits June 17, 2020 16:59

Update UPGRADE-1.7.md

ce995cd

Co-authored-by: Kamil Kokot <kamil@kokot.me>

Update UPGRADE-1.7.md

ed03d67

Co-authored-by: Kamil Kokot <kamil@kokot.me>

Update src/Bundle/Form/Extension/HttpFoundation/HttpFoundationRequest…

5ec3938

…Handler.php Co-authored-by: Kamil Kokot <kamil@kokot.me>

Trying to prepend fos rest configuration

2b0c45f

loic425 force-pushed the feature/upgrade-rest-bundle branch from 32279f1 to 2b0c45f Compare June 17, 2020 15:40

loic425 added 2 commits June 17, 2020 17:43

Fix phpstan error

6f8bd46

Remove useless doc for upgrade

9b64585

lchrusciel approved these changes Jun 19, 2020

View reviewed changes

loic425 commented Jun 23, 2020

View reviewed changes

pamil approved these changes Jun 25, 2020

View reviewed changes

pamil merged commit 9196919 into Sylius:master Jun 25, 2020

pamil mentioned this pull request Jun 25, 2020

Allow for FOSRestBundle ^3.0@dev #153

Closed

loic425 deleted the feature/upgrade-rest-bundle branch June 25, 2020 13:23

esserj mentioned this pull request Sep 4, 2020

Allow FOS rest-bundle 2 #180

Closed

	$currentPage = (int) $request->query->get('page', '1');
	$currentPage = (int) $request->query->getInt('page', '1');

	$currentPage = (int) $request->query->get('page', '1');
	$currentPage = $request->query->getInt('page', '1');

Uh oh!

Upgrade rest bundle #167

Upgrade rest bundle #167

Uh oh!

Conversation

loic425 commented Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loic425 commented Jun 4, 2020

Uh oh!

loic425 commented Jun 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

loic425 commented Jun 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lchrusciel left a comment

Choose a reason for hiding this comment

Uh oh!

lchrusciel Jun 19, 2020

Choose a reason for hiding this comment

Uh oh!

loic425 Jun 21, 2020

Choose a reason for hiding this comment

Uh oh!

loic425 Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic425 Jun 23, 2020

Choose a reason for hiding this comment

Uh oh!

pamil commented Jun 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

loic425 commented Jun 4, 2020 •

edited

Loading

loic425 commented Jun 10, 2020 •

edited

Loading

loic425 Jun 23, 2020 •

edited

Loading