search

sonata-project / SonataPageBundle

This bundle provides a Site and Page management through container and block services
https://docs.sonata-project.org/projects/SonataPageBundle
MIT License
216 stars 209 forks source link

Check why setPosition is set as null #1496

Closed eerison closed 2 years ago

eerison commented 2 years ago

I got this on branch 4.x

Sonata\BlockBundle\Model\BaseBlock::setPosition(): Argument #1 ($position) must be of type int, null given, called in /app/vendor/sonata-project/page-bundle/src/Entity/Transformer.php on line 157

Stack trace

TypeError:
Sonata\BlockBundle\Model\BaseBlock::setPosition(): Argument #1 ($position) must be of type int, null given, called in /app/vendor/sonata-project/page-bundle/src/Entity/Transformer.php on line 157

  at vendor/sonata-project/block-bundle/src/Model/BaseBlock.php:147
  at Sonata\BlockBundle\Model\BaseBlock->setPosition(null)
     (vendor/sonata-project/page-bundle/src/Entity/Transformer.php:157)
  at Sonata\PageBundle\Entity\Transformer->loadBlock(array('id' => 11, 'name' => 'container 1', 'enabled' => true, 'position' => null, 'settings' => array('code' => null, 'layout' => '{{ CONTENT }}', 'class' => null, 'template' => '@SonataPage/Block/block_container.html.twig'), 'type' => 'sonata.block.service.container', 'created_at' => '1656522317', 'updated_at' => '1656522349', 'blocks' => array(array('id' => 9, 'name' => 'Test A', 'enabled' => true, 'position' => 1, 'settings' => array('mode' => 'form.choice_public', 'title' => 'Title B', 'translation_domain' => null, 'icon' => 'fa fa-globe', 'class' => null, 'template' => '@SonataPage/Block/block_pagelist.html.twig'), 'type' => 'sonata.page.block.pagelist', 'created_at' => '1656521962', 'updated_at' => '1656522564', 'blocks' => array()), array('id' => 10, 'name' => 'test s', 'enabled' => true, 'position' => 2, 'settings' => array('content' => 'Insert your custom content here', 'template' => '@SonataBlock/Block/block_core_text.html.twig'), 'type' => 'sonata.block.service.text', 'created_at' => '1656522079', 'updated_at' => '1656522388', 'blocks' => array()))), object(SnapshotPageProxy))
     (vendor/sonata-project/page-bundle/src/Model/SnapshotPageProxy.php:129)
  at Sonata\PageBundle\Model\SnapshotPageProxy->getBlocks()
     (vendor/sonata-project/page-bundle/src/CmsManager/CmsSnapshotManager.php:152)
  at Sonata\PageBundle\CmsManager\CmsSnapshotManager->loadBlocks(object(SnapshotPageProxy))
     (vendor/sonata-project/page-bundle/src/CmsManager/CmsSnapshotManager.php:133)
  at Sonata\PageBundle\CmsManager\CmsSnapshotManager->getPageBy(object(SonataPageSite), 'url', '/')
     (vendor/sonata-project/page-bundle/src/CmsManager/BaseCmsPageManager.php:54)
  at Sonata\PageBundle\CmsManager\BaseCmsPageManager->getPageByUrl(object(SonataPageSite), '/')
     (vendor/sonata-project/page-bundle/src/Route/CmsPageRouter.php:132)
  at Sonata\PageBundle\Route\CmsPageRouter->match('/')
     (vendor/symfony-cmf/routing/src/ChainRouter.php:192)
  at Symfony\Cmf\Component\Routing\ChainRouter->doMatch('/', object(Request))
     (vendor/symfony-cmf/routing/src/ChainRouter.php:158)
  at Symfony\Cmf\Component\Routing\ChainRouter->matchRequest(object(Request))
     (vendor/symfony/http-kernel/EventListener/RouterListener.php:112)
  at Symfony\Component\HttpKernel\EventListener\RouterListener->onKernelRequest(object(RequestEvent), 'kernel.request', object(TraceableEventDispatcher))
     (vendor/symfony/event-dispatcher/Debug/WrappedListener.php:126)
  at Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke(object(RequestEvent), 'kernel.request', object(TraceableEventDispatcher))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:264)
  at Symfony\Component\EventDispatcher\EventDispatcher->doDispatch(array(object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener)), 'kernel.request', object(RequestEvent))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:239)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners(array(object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener)), 'kernel.request', object(RequestEvent))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:73)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch(object(RequestEvent), 'kernel.request')
     (vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:168)
  at Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch(object(RequestEvent), 'kernel.request')
     (vendor/symfony/http-kernel/HttpKernel.php:134)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:80)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:201)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:29)

But all my pages there are position value

Screenshot 2022-07-26 at 13 10 09
Hanmac commented 2 years ago

@eerison i might look into it, do you have a testcase for this to repoduce?

eerison commented 2 years ago

Hey @Hanmac I just updated from 3.x to 4.x using sonata 4, and it crash my page, if you test it in some project you can see this issue!

Hanmac commented 2 years ago

hm curious, you should probably check the content json data from your snapshots there is where the null is coming from but i don't know how it got there

eerison commented 2 years ago

Hey @Hanmac I guess you are right, it's related with snapshot content, the position of container is null

{
   "id":642,
   "name":"Homepage",
   "javascript":null,
   "stylesheet":null,
   "raw_headers":null,
   "title":null,
   "meta_description":null,
   "meta_keyword":null,
   "template_code":"default",
   "request_method":"GET|POST|HEAD|DELETE|PUT",
   "created_at":"1656521887",
   "updated_at":"1656522565",
   "slug":"",
   "parent_id":null,
   "target_id":null,
   "blocks":[
      {
         "id":11,
         "name":"container 1",
         "enabled":true,
         "position":null,
         "settings":{
            "code":null,
            "layout":"{{ CONTENT }}",
            "class":null,
            "template":"@SonataPage\/Block\/block_container.html.twig"
         },
         "type":"sonata.block.service.container",
         "created_at":"1656522317",
         "updated_at":"1656522349",
         "blocks":[
            {
               "id":9,
               "name":"Test A",
               "enabled":true,
               "position":1,
               "settings":{
                  "mode":"form.choice_public",
                  "title":"Title B",
                  "translation_domain":null,
                  "icon":"fa fa-globe",
                  "class":null,
                  "template":"@SonataPage\/Block\/block_pagelist.html.twig"
               },
               "type":"sonata.page.block.pagelist",
               "created_at":"1656521962",
               "updated_at":"1656522564",
               "blocks":[

               ]
            },
            {
               "id":10,
               "name":"test s",
               "enabled":true,
               "position":2,
               "settings":{
                  "content":"Insert your custom content here",
                  "template":"@SonataBlock\/Block\/block_core_text.html.twig"
               },
               "type":"sonata.block.service.text",
               "created_at":"1656522079",
               "updated_at":"1656522388",
               "blocks":[

               ]
            }
         ]
      }
   ]
}

in this case position id should be ?int ?

Hanmac commented 2 years ago

i don't know about BlockBundle to understand what the container is doing there and if the null position is valid there

if it does, then it needs to be fixed in BlockBundle

eerison commented 2 years ago

the container block is like the main container

for example:

header container block
 - menu block
 - ads block
content container block
  - block 1 (assets list)
  - block 2 (assets 2 list)
footer container block
 - menu block
 - sponsors block

I don't know if it was clear, but it's more or less the block to keep others blocks into like sections.

eerison commented 2 years ago

@jordisala1991 BlockBundle requires position as integer

But how do you handle this for container block?

eerison commented 2 years ago

I guess the problem not related with position only!

I have others blocks that there isn't name 

{
   "id":652,
   "name":"contact",
   "javascript":null,
   "stylesheet":null,
   "raw_headers":null,
   "title":null,
   "meta_description":null,
   "meta_keyword":null,
   "template_code":"default",
   "request_method":"GET|POST|HEAD|DELETE|PUT",
   "created_at":"1656522709",
   "updated_at":"1657119067",
   "slug":"contact",
   "parent_id":642,
   "target_id":null,
   "blocks":[
      {
         "id":15,
         "name":"Top content",
         "enabled":true,
         "position":1,
         "settings":{
            "code":"content_top"
         },
         "type":"sonata.page.block.container",
         "created_at":"1656571318",
         "updated_at":"1656571318",
         "blocks":[
            {
               "id":21,
               "name":null,
               "enabled":true,
               "position":1,
               "settings":{
                  "template":"@SonataSeo\/Block\/block_email_share_button.html.twig",
                  "subject":"subject test",
                  "body":"body test"
               },
               "type":"sonata.seo.block.email.share_button",
               "created_at":"1656571654",
               "updated_at":"1656571654",
               "blocks":[

               ]
            },
            {
               "id":22,
               "name":null,
               "enabled":true,
               "position":2,
               "settings":{
                  "mode":"form.choice_public",
                  "title":"ABBB",
                  "translation_domain":null,
                  "icon":"fa fa-globe",
                  "class":null,
                  "template":"@SonataPage\/Block\/block_pagelist.html.twig"
               },
               "type":"sonata.page.block.pagelist",
               "created_at":"1656571709",
               "updated_at":"1656571709",
               "blocks":[

               ]
            }
         ]
      },
      {
         "id":16,
         "name":"Main content",
         "enabled":true,
         "position":1,
         "settings":{
            "code":"content"
         },
         "type":"sonata.page.block.container",
         "created_at":"1656571318",
         "updated_at":"1656571318",
         "blocks":[
            {
               "id":25,
               "name":null,
               "enabled":true,
               "position":0,
               "settings":{
                  "url":"https:\/\/facebook.com",
                  "title":"RSS feed",
                  "translation_domain":null,
                  "icon":"fa fa-rss-square",
                  "class":null,
                  "template":"@SonataBlock\/Block\/block_core_rss.html.twig"
               },
               "type":"sonata.block.service.rss",
               "created_at":"1656574713",
               "updated_at":"1657119037",
               "blocks":[

               ]
            },
            {
               "id":27,
               "name":null,
               "enabled":true,
               "position":1,
               "settings":{
                  "template":"@SonataSeo\/Block\/block_email_share_button.html.twig",
                  "subject":"test 12455",
                  "body":"test 4333"
               },
               "type":"sonata.seo.block.email.share_button",
               "created_at":"1657118800",
               "updated_at":"1657119037",
               "blocks":[

               ]
            },
            {
               "id":19,
               "name":null,
               "enabled":true,
               "position":2,
               "settings":{
                  "content":"SIMPLE TEXT :D",
                  "template":"@SonataBlock\/Block\/block_core_text.html.twig"
               },
               "type":"sonata.block.service.text",
               "created_at":"1656571374",
               "updated_at":"1657119037",
               "blocks":[

               ]
            },
            {
               "id":23,
               "name":"aaa",
               "enabled":true,
               "position":3,
               "settings":{
                  "title":"Breadcrumb (Homepage)",
                  "cache_policy":"public",
                  "template":"@SonataBlock\/Block\/block_core_menu.html.twig",
                  "menu_name":null,
                  "safe_labels":false,
                  "current_class":"active",
                  "first_class":null,
                  "last_class":null,
                  "current_uri":null,
                  "menu_class":"list-group",
                  "children_class":"list-group-item",
                  "menu_template":"@SonataSeo\/Block\/breadcrumb.html.twig",
                  "include_homepage_link":true,
                  "context":false
               },
               "type":"sonata.seo.block.breadcrumb.homepage",
               "created_at":"1656571781",
               "updated_at":"1657119037",
               "blocks":[

               ]
            },
            {
               "id":24,
               "name":null,
               "enabled":true,
               "position":4,
               "settings":{
                  "url":"https:\/\/google.com",
                  "title":"google link",
                  "translation_domain":null,
                  "icon":"fa fa-rss-square",
                  "class":null,
                  "template":"@SonataBlock\/Block\/block_core_rss.html.twig"
               },
               "type":"sonata.block.service.rss",
               "created_at":"1656574614",
               "updated_at":"1657119037",
               "blocks":[

               ]
            },
            {
               "id":26,
               "name":"Instagram",
               "enabled":true,
               "position":5,
               "settings":{
                  "url":"https:\/\/instagram.com",
                  "title":"INSTA PROD 45678",
                  "translation_domain":null,
                  "icon":"fa fa-rss-square",
                  "class":null,
                  "template":"@SonataBlock\/Block\/block_core_rss.html.twig"
               },
               "type":"sonata.block.service.rss",
               "created_at":"1656574756",
               "updated_at":"1657119037",
               "blocks":[

               ]
            }
         ]
      },
      {
         "id":17,
         "name":"Bottom content",
         "enabled":true,
         "position":1,
         "settings":{
            "code":"content_bottom"
         },
         "type":"sonata.page.block.container",
         "created_at":"1656571318",
         "updated_at":"1656571318",
         "blocks":[

         ]
      },
      {
         "id":18,
         "name":"Footer",
         "enabled":true,
         "position":1,
         "settings":{
            "code":"footer"
         },
         "type":"sonata.page.block.container",
         "created_at":"1656571318",
         "updated_at":"1656571318",
         "blocks":[

         ]
      },
      {
         "id":14,
         "name":"Header",
         "enabled":true,
         "position":1,
         "settings":{
            "code":"header"
         },
         "type":"sonata.page.block.container",
         "created_at":"1656571318",
         "updated_at":"1656571318",
         "blocks":[
            {
               "id":20,
               "name":"Menu 1",
               "enabled":true,
               "position":1,
               "settings":{
                  "title":"sonata.block.menu",
                  "cache_policy":"public",
                  "template":"@SonataBlock\/Block\/block_core_menu.html.twig",
                  "menu_name":"sonata_admin_sidebar",
                  "safe_labels":false,
                  "current_class":"active",
                  "first_class":null,
                  "last_class":null,
                  "current_uri":null,
                  "menu_class":"list-group",
                  "children_class":"list-group-item",
                  "menu_template":null
               },
               "type":"sonata.block.service.menu",
               "created_at":"1656571563",
               "updated_at":"1656571563",
               "blocks":[

               ]
            }
         ]
      }
   ]
}

and I got the same issue 

Sonata\BlockBundle\Model\BaseBlock::setName(): Argument #1 ($name) must be of type string, null given, called in /app/vendor/sonata-project/page-bundle/src/Entity/Transformer.php on line 155
jordisala1991 commented 2 years ago

Looks like the Transformer.php could be changed to not assign null properties?

Hanmac commented 2 years ago

Looks like the Transformer.php could be changed to not assign null properties?

i think thats an hotfix for now

i checked BlockBundle code, the property says it can be null, the getter can return null values, but the setter can't set null values for some reason?

should that be fixed in BlockBundle too?

eerison commented 2 years ago

the container block is used in sonata page only?

VincentLanglet commented 2 years ago

i checked BlockBundle code, the property says it can be null, the getter can return null values, but the setter can't set null values for some reason?

should that be fixed in BlockBundle too?

The getter can return null if no value was set. For instance if you call (new Block())->getName(). But I assume the setter are not nullable, because this field were maybe supposed to be non-nullable in database.

I don't know if it makes sens to have a block without a name, a type or a position...

Where come from your data @eerison ?

should that be fixed in BlockBundle too?

The issue is that changing the param typehint will be a BC break...

eerison commented 2 years ago

I don't know if it makes sens to have a block without a name, a type or a position...

  • IMO all blocks should have name too
  • position, I'm not sure because it's container block is no a block that can be updated in the admin, it's more or less a fake block, it's just to group others blocks.

Where come from your data @eerison ?

when we create snapshot, it generates this content with the json

Screenshot 2022-07-26 at 16 34 28

The issue is that changing the param typehint will be a BC break...

what we can do is container position put value 0 and name = '', and we need to fix those blocks.

eerison commented 2 years ago

those blocks are added here

Screenshot 2022-07-26 at 16 47 08

and it's the default blocks into the sonata page, probably we need to fix them to require name, But some of them I would say that could be removed maybe.

Screenshot 2022-07-26 at 16 50 15

But In 4.x branch someone started to remove some blocks 😃

Screenshot 2022-07-26 at 17 00 57
jordisala1991 commented 2 years ago

Can you check with https://github.com/sonata-project/SonataPageBundle/pull/1501 @eerison ?

jordisala1991 commented 2 years ago

This one should fix this problem: https://github.com/sonata-project/SonataPageBundle/pull/1532