symfony / demo

Symfony Demo Application
https://symfony.com/
MIT License
2.46k stars 1.61k forks source link

Update to Symfony 7.1 #1519

Closed javiereguiluz closed 3 months ago

javiereguiluz commented 3 months ago

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 (https://github.com/dmaicher/doctrine-test-bundle/commit/e4995f9f96592b4235016e190255204782c43987) 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 commented 3 months ago

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 commented 3 months ago

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}) }}">
javiereguiluz commented 3 months ago

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

seb-jean commented 3 months ago

Should we use IsCsrfTokenValid Attribute on the delete function: https://github.com/symfony/demo/blob/main/src/Controller/Admin/BlogController.php#L162?

https://symfony.com/blog/new-in-symfony-7-1-iscsrftokenvalid-attribute

javiereguiluz commented 3 months ago

@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 commented 3 months ago

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