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

Add Support for Symfony 5.4 #13339

Merged
merged 5 commits into from
Dec 3, 2021
Merged

Conversation

AdamKasp
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
License MIT

@AdamKasp AdamKasp added the Maintenance CI configurations, READMEs, releases, etc. label Nov 29, 2021
@AdamKasp AdamKasp requested a review from a team as a code owner November 29, 2021 09:27
@probot-autolabeler probot-autolabeler bot added Admin AdminBundle related issues and PRs. API APIs related issues and PRs. Shop ShopBundle related issues and PRs. labels Nov 29, 2021
@AdamKasp AdamKasp force-pushed the bump-up-symfony-5.4 branch 2 times, most recently from 37c87b5 to d50d4dd Compare November 29, 2021 14:43
@AdamKasp AdamKasp force-pushed the bump-up-symfony-5.4 branch 5 times, most recently from d5f3ef4 to 2f541d5 Compare November 30, 2021 10:03
@AdamKasp AdamKasp force-pushed the bump-up-symfony-5.4 branch from 2f541d5 to fef6243 Compare November 30, 2021 10:16
config/packages/security.yaml Outdated Show resolved Hide resolved
@@ -57,7 +57,7 @@ public function __invoke(Request $request): ImageInterface
$image->setFile($file);

/** @var string $ownerIri */
$ownerIri = $request->get('owner');
$ownerIri = $request->request->get('owner');
Copy link
Member

Choose a reason for hiding this comment

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

This could be considered as BC-Break, but I'm fine with that. Can we add a note about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have @experimental on the entire apiBundle so IMO it isn't BC-break, but ofc I will add a note to apiUpgrade file

@@ -38,7 +38,10 @@ public function __construct(
public function __invoke(Request $request): JsonResponse
{
/** @var PaymentInterface|null $payment */
$payment = $this->paymentRepository->findOneByOrderToken($request->get('paymentId'), $request->get('id'));
$payment = $this->paymentRepository->findOneByOrderToken(
$request->attributes->get('paymentId'),
Copy link
Member

Choose a reason for hiding this comment

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

This could be considered as BC-Break, but I'm fine with that. Can we add a note about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have @experimental on the entire apiBundle so IMO it isn't BC-break, but ofc I will add a note to apiUpgrade file

}

if ($request->query->has($key)) {
return $request->query->all()[$key];
Copy link
Member

Choose a reason for hiding this comment

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

Why not this way?

Suggested change
return $request->query->all()[$key];
return $request->query->get($key);

* @deprecated This function will be removed in Sylius 2.0, since Symfony 5.4, use explicit input sources instead
* based on Symfony\Component\HttpFoundation\Request::get
*/
private function getParameterFromRequest(Request $request, string $key)
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we shouldn't port it down to ResourceBundle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is temporary logic, I am not sure if we need to make bigger refactor of it.

Copy link
Member

Choose a reason for hiding this comment

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

this "temporary" logic will be with us for a long time

Copy link
Member

Choose a reason for hiding this comment

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

👍 for moving it to ResourceBundle. However, to not stop this PR for too long, I suggest we merge it and proceed with ResourceBundle changes parallelly 🖖

@AdamKasp AdamKasp force-pushed the bump-up-symfony-5.4 branch from eb3b56a to d8827c4 Compare November 30, 2021 13:38
@AdamKasp AdamKasp force-pushed the bump-up-symfony-5.4 branch from d8827c4 to fcc5abe Compare December 1, 2021 07:54
@AdamKasp AdamKasp changed the title Prepare Sylius for support Symfony 5.4 Add Support for Symfony 5.4 Dec 1, 2021
* @deprecated This function will be removed in Sylius 2.0, since Symfony 5.4, use explicit input sources instead
* based on Symfony\Component\HttpFoundation\Request::get
*/
private function getParameterFromRequest(Request $request, string $key)
Copy link
Member

Choose a reason for hiding this comment

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

this "temporary" logic will be with us for a long time

src/Sylius/Behat/Service/SecurityService.php Outdated Show resolved Hide resolved
src/Sylius/Bundle/CoreBundle/Security/UserImpersonator.php Outdated Show resolved Hide resolved
Comment on lines +24 to +33
private ?SlugGeneratorInterface $slugGenerator;

public function __construct(?SlugGeneratorInterface $slugGenerator = null)
{
$this->slugGenerator = $slugGenerator;

if ($this->slugGenerator === null) {
@trigger_error(sprintf('Not passing a $slugGenerator to %s constructor is deprecated since Sylius 1.11 and will be prohibited in Sylius 2.0.', self::class), \E_USER_DEPRECATED);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add this service in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because
Screenshot 2021-12-02 at 10 39 50
in Symfony 5.4 function get is deprecated

@AdamKasp AdamKasp force-pushed the bump-up-symfony-5.4 branch from fcc5abe to 2964e0e Compare December 2, 2021 11:12
@GSadee GSadee merged commit 6191f36 into Sylius:master Dec 3, 2021
@GSadee
Copy link
Member

GSadee commented Dec 3, 2021

Thank you, Adam! 🎉

lchrusciel pushed a commit to lchrusciel/Sylius that referenced this pull request Dec 6, 2021
This PR was merged into the 1.11-dev branch.

Discussion
----------

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

<!--
 - Bug fixes must be submitted against the 1.10 branch
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

Commits
-------

28a02a6 Prepare Sylius for support Symfony 5.4
d2c39e4 remove usage of symfony internal method
fef6243 bump symofny verion from 5.4.0-RC1 to 5.4
6339528 update argument in UsernamePasswordToken
2964e0e make workorounds over deprecated method in symofny 5.4
lchrusciel pushed a commit to lchrusciel/Sylius that referenced this pull request Dec 6, 2021
This PR was merged into the 1.11-dev branch.

Discussion
----------

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

<!--
 - Bug fixes must be submitted against the 1.10 branch
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

Commits
-------

28a02a6 Prepare Sylius for support Symfony 5.4
d2c39e4 remove usage of symfony internal method
fef6243 bump symofny verion from 5.4.0-RC1 to 5.4
6339528 update argument in UsernamePasswordToken
2964e0e make workorounds over deprecated method in symofny 5.4
lchrusciel pushed a commit to lchrusciel/Sylius that referenced this pull request Dec 6, 2021
This PR was merged into the 1.11-dev branch.

Discussion
----------

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

<!--
 - Bug fixes must be submitted against the 1.10 branch
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

Commits
-------

28a02a6 Prepare Sylius for support Symfony 5.4
d2c39e4 remove usage of symfony internal method
fef6243 bump symofny verion from 5.4.0-RC1 to 5.4
6339528 update argument in UsernamePasswordToken
2964e0e make workorounds over deprecated method in symofny 5.4
lchrusciel pushed a commit to lchrusciel/Sylius that referenced this pull request Dec 6, 2021
This PR was merged into the 1.11-dev branch.

Discussion
----------

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

<!--
 - Bug fixes must be submitted against the 1.10 branch
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

Commits
-------

28a02a6 Prepare Sylius for support Symfony 5.4
d2c39e4 remove usage of symfony internal method
fef6243 bump symofny verion from 5.4.0-RC1 to 5.4
6339528 update argument in UsernamePasswordToken
2964e0e make workorounds over deprecated method in symofny 5.4
lchrusciel pushed a commit to lchrusciel/Sylius that referenced this pull request Dec 6, 2021
This PR was merged into the 1.11-dev branch.

Discussion
----------

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

<!--
 - Bug fixes must be submitted against the 1.10 branch
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

Commits
-------

28a02a6 Prepare Sylius for support Symfony 5.4
d2c39e4 remove usage of symfony internal method
fef6243 bump symofny verion from 5.4.0-RC1 to 5.4
6339528 update argument in UsernamePasswordToken
2964e0e make workorounds over deprecated method in symofny 5.4
AdamKasp added a commit that referenced this pull request Dec 6, 2021
…e, lchrusciel)

This PR was merged into the 1.10 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.10
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | Backport of #13339 and #13358, fixes Sylius/Sylius-Standard#630
| License         | MIT

Let's bring Sf5.4 support to Sylius 1.10. This way, current users will benefit from Symfony 5.4 with current installations. I'm considering it as a bug-fix, as it is already possible to install this version according to our composer.json. What is more, it will make it possible to migrate first to Sf5.4 and then upgrade to Sylius 1.11.

<!--
 - Bug fixes must be submitted against the 1.10 branch
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

860c3e8 [Maintanance] add conflict
71e4fd6 feature #13339 Add Support for Symfony 5.4 (AdamKasp)
860399f [Minor] Fix CONFLICT.md lining
5937ebb [Maintenance] Conflict with Symfony ^6.0
Zales0123 added a commit that referenced this pull request Dec 7, 2021
* 1.10:
  Minimize number of build for packages
  [Maintenance] Conflict with Symfony ^6.0
  [Minor] Fix CONFLICT.md lining
  feature #13339 Add Support for Symfony 5.4 (AdamKasp)
  [Maintanance] add conflict
  Corrections for adding a field to the response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin AdminBundle related issues and PRs. API APIs related issues and PRs. Maintenance CI configurations, READMEs, releases, etc. Shop ShopBundle related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants