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

Update to Symfony 7.1 #1519

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Update to Symfony 7.1 #1519

merged 1 commit into from
Jun 4, 2024

Conversation

javiereguiluz
Copy link
Member

These are the pending indirect deprecations:

Remaining indirect deprecation notices (56)


37x: Since twig/twig 3.10: The "Twig\Extension\EscaperExtension::addSafeClass()" method is deprecated, use the "Twig\Runtime\EscaperRuntime::addSafeClass()" method instead.
Remove Doctine deprecations
4x in BlogControllerTest::testAccessDeniedForRegularUsers from App\Tests\Controller\Admin
4x in BlogControllerTest::testNewComment from App\Tests\Controller
4x in DefaultControllerTest::testSecureUrls from App\Tests\Controller
3x in BlogControllerTest::testAdminNewDuplicatedPost from App\Tests\Controller\Admin
3x in DefaultControllerTest::testPublicUrls from App\Tests\Controller
...

❓ Not sure if this has been fixed upstream


15x: Since twig/twig 3.9: Using the internal "twig_escape_filter" function is deprecated.
9x in DefaultControllerTest::testPublicUrls from App\Tests\Controller
6x in BlogControllerTest::testAjaxSearch from App\Tests\Controller

❓ Not sure if this has been fixed upstream


2x: Since symfony/doctrine-bridge 7.1: Relying on auto-mapping for Doctrine entities is deprecated for argument $post of "App\Controller\BlogController::commentForm": declare the mapping using either the #[MapEntity] attribute or mapped route parameters.
2x in BlogControllerTest::testNewComment from App\Tests\Controller

❌ I don't know how to fix this. It's related thi this: https://github.com/symfony/demo/blob/main/src/Controller/BlogController.php#L154 Should I add a #[MapEntity] attribute? Maybe @stof can help me here. Thanks!


1x: The "Symfony\Component\HttpKernel\DependencyInjection\Extension" class is considered internal since Symfony 7.1, to be deprecated in 8.1; use Symfony\Component\DependencyInjection\Extension\Extension instead. It may change without further notice. You should not use it from "DAMA\DoctrineTestBundle\DependencyInjection\DAMADoctrineTestExtension".
1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

@dmaicher fixed this last week (dmaicher/doctrine-test-bundle@e4995f9) so it'll be ready in the next release


1x: Since symfony/property-info 7.1: The "Symfony\Bridge\Doctrine\PropertyInfo\DoctrineExtractor::getTypes()" method is deprecated, use "Symfony\Bridge\Doctrine\PropertyInfo\DoctrineExtractor::getType()" instead.
1x in BlogControllerTest::testAjaxSearch from App\Tests\Controller

✅ I think this was fixed upstream in 7.1-dev

@nicolas-grekas
Copy link
Member

Here is the diff for the commentForm deprecation:

--- a/templates/blog/post_show.html.twig
+++ b/templates/blog/post_show.html.twig
@@ -22,7 +22,7 @@
         See https://symfony.com/doc/current/security/remember_me.html#forcing-the-user-to-re-authenticate-before-accessing-certain-resources
         #}
         {% if is_granted('IS_AUTHENTICATED_FULLY') %}
-            {{ render(controller('App\\Controller\\BlogController::commentForm', {'id': post.id})) }}
+            {{ render(controller('App\\Controller\\BlogController::commentForm', {'post': post.id})) }}
         {% else %}
             <p>

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 28, 2024

Note that this would work also (and that'd skip the entity resolver entirely):

--- a/templates/blog/post_show.html.twig
+++ b/templates/blog/post_show.html.twig
@@ -22,7 +22,7 @@
         See https://symfony.com/doc/current/security/remember_me.html#forcing-the-user-to-re-authenticate-before-accessing-certain-resources
         #}
         {% if is_granted('IS_AUTHENTICATED_FULLY') %}
-            {{ render(controller('App\\Controller\\BlogController::commentForm', {'id': post.id})) }}
+            {{ render(controller('App\\Controller\\BlogController::commentForm', {'post': post})) }}
         {% else %}
             <p>
                 <a class="btn btn-success" href="{{ path('security_login', {'redirect_to': app.request.pathInfo}) }}">

composer.json Outdated Show resolved Hide resolved
@javiereguiluz
Copy link
Member Author

What would be the best way to solve the issues reported by PHPStan? Thanks

@seb-jean
Copy link
Contributor

@javiereguiluz
Copy link
Member Author

@seb-jean yes, but let's do that change in a separate PR after this one. There might be other new features that we could use too. Thanks!

@javiereguiluz
Copy link
Member Author

javiereguiluz commented May 31, 2024

This is now ready for the final review before merge.

Remaining indirect deprecation notices (52)


37x: Since twig/twig 3.10: The "Twig\Extension\EscaperExtension::addSafeClass()" method is deprecated, use the "Twig\Runtime\EscaperRuntime::addSafeClass()" method instead.
4x in BlogControllerTest::testAccessDeniedForRegularUsers from App\Tests\Controller\Admin
4x in BlogControllerTest::testNewComment from App\Tests\Controller
4x in DefaultControllerTest::testSecureUrls from App\Tests\Controller
3x in BlogControllerTest::testAdminNewDuplicatedPost from App\Tests\Controller\Admin
3x in DefaultControllerTest::testPublicUrls from App\Tests\Controller
...


15x: Since twig/twig 3.9: Using the internal "twig_escape_filter" function is deprecated.
9x in DefaultControllerTest::testPublicUrls from App\Tests\Controller
6x in BlogControllerTest::testAjaxSearch from App\Tests\Controller

composer.json Outdated
@@ -75,7 +86,7 @@
"php": "8.2.0"
},
"preferred-install": {
"*": "dist"
"7.1.*": "dist"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants