sonata-project / sandbox

[Abandoned] Sonata Project's sandbox
https://sonata-project.org
MIT License
268 stars 198 forks source link

Unable to add block container in global composer #590

Closed MaraScott closed 4 years ago

MaraScott commented 8 years ago

Hi,

I m trying to allow admin to add block container within a block container like in the footer of the global page on sonata demo, but it doesn t work.

The block is created without parent assignment, when I refresh the composer zone for the footer block i got this ajax response

"No parent found, unable to preview an orphan block"

In the same topic, when I try to add a block to an empty block container through this url /admin/core/append-form-field-element?code=sonata.page.admin.block&elementId=s582267c897d8c_children&objectId=95&uniqid=s582267c897d8c

I got this error

"The block service `` does not exist"

How can I fix it ?

MaraScott commented 8 years ago

Another example is this error : Admin service "sonata.block.service.container" not found in admin pool.

MaraScott commented 7 years ago

I found a way to add block container for administration but so far I have to override some files in the vendor/sonata-project/admin-bundle, vendor/sonata-project/block-bundle & vendor/sonata-project/page-bundle directories.

I had to override the vendor/sonata-project/admin-bundle/Admin/Pool.php which is called in a lots of files within the different bundles from the sonata project and I can t simply override it (except misunderstanding on my side) throught the sonata extend feature.

What can I do to make it more sonata friendly ? @core23 , @OskarStark , @rande

Below the files involved in my overriding

sonota-block-admin

OskarStark commented 7 years ago

Can you provide this as a PR, its much easier to "talk" about code.

Thank you @davask đź‘Ť

MaraScott commented 7 years ago

How do you want me to do that as my modifications are in the vendor directory it is ignored from any commit. I could make a PR per bundle but we will loose the overall view as my modifications are across bundles.

Otherwise I can push in src the files with respected tree structure but that will not be best practices.

Whta is you suggestion @OskarStark ?

OskarStark commented 7 years ago

Puh, you are right, any idea @greg0ire ?

greg0ire commented 7 years ago

Uuuuh… something like this :

for vendor in some/vendor some/othervendor
do
    pushd vendor/$vendor
    git --no-pager diff
    popd
done

You can list modified vendor with composer status

MaraScott commented 7 years ago

Thanks @greg0ire,

However, the admin-bundle and block-bundle have no .git repository (How is it possible ?) and the composer status return a git diff.

Do you want a git diff file or a PR ?

About admin-bundle and block-bundle, I m using :

do I git init && git fetch https://github.com/sonata-project/SonataAdminBundle/tree/3.x and https://github.com/sonata-project/SonataBlockBundle/tree/3.x to comply with your bash script ?

greg0ire commented 7 years ago

the admin-bundle and block-bundle have no .git repository (How is it possible ?)

It happens when composer chooses to install from a tarball instead of a git, which is the default for stable versions AFAIK

So basically, you can't have a diff, sorry… unless you reinstall composer with --prefer-source (I think), and then copy your changes again over that.

Do you want a git diff file or a PR ?

I don't think you can make a PR, you can only make several of them… so git diff it is. Unless you want to make several PRs of course.

MaraScott commented 7 years ago

Below the result from greg0ire script :


~/files/vendor/sonata-project/admin-bundle ~/files
diff --git a/Admin/Pool.php b/Admin/Pool.php
index 63856d2..d8f69e8 100644
--- a/Admin/Pool.php
+++ b/Admin/Pool.php
@@ -194,6 +194,9 @@ class Pool
         }

         if (count($this->adminClasses[$class]) > 1) {
+            if (in_array($class, array("Sonata\PageBundle\Entity\Block","Application\Sonata\PageBundle\Entity\Block","AppBundle\Entity\Page\Block"))) {
+                return $this->getInstance("sonata.page.admin.block");
+            }
             throw new \RuntimeException(sprintf('Unable to find a valid admin for the class: %s, there are too many registered: %s', $class, implode(',', $this->adminClasses[$class])));
         }

~/files
~/files/vendor/sonata-project/block-bundle ~/files
diff --git a/Block/Service/ContainerBlockService.php b/Block/Service/ContainerBlockService.php
index 71efcce..2502a2c 100644
--- a/Block/Service/ContainerBlockService.php
+++ b/Block/Service/ContainerBlockService.php
@@ -31,6 +31,8 @@ class ContainerBlockService extends AbstractAdminBlockService
     public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
     {
         $formMapper->add('enabled');
+        $formMapper->add('page');
+        $formMapper->add('parent');

         $formMapper->add('settings', 'sonata_type_immutable_array', array(
             'keys' => array(
@@ -41,12 +43,14 @@ class ContainerBlockService extends AbstractAdminBlockService
             ),
         ));

-        $formMapper->add('children', 'sonata_type_collection', array(), array(
-            'admin_code' => 'sonata.page.admin.block',
-            'edit' => 'inline',
-            'inline' => 'table',
-            'sortable' => 'position',
-        ));
+        if(!in_array($block->getType(), array("sonata.page.block.container", "sonata.block.service.container"))) {
+            $formMapper->add('children', 'sonata_type_collection', array(), array(
+                'admin_code' => 'sonata.page.admin.block',
+                'edit' => 'inline',
+                'inline' => 'table',
+                'sortable' => 'position',
+            ));
+        }
     }

     /**
~/files
~/files/vendor/sonata-project/page-bundle ~/files
diff --git a/Admin/BaseBlockAdmin.php b/Admin/BaseBlockAdmin.php
index d3fbc74..b54188f 100644
--- a/Admin/BaseBlockAdmin.php
+++ b/Admin/BaseBlockAdmin.php
@@ -71,6 +71,10 @@ abstract class BaseBlockAdmin extends AbstractAdmin
     {
         $block = parent::getNewInstance();
         $block->setType($this->getPersistentParameter('type'));
+        if(is_null($block->getType())) {
+            $block->setType('sonata.page.block.container');
+
+        }

         return $this->loadBlockDefaults($block);
     }
diff --git a/Admin/BlockAdmin.php b/Admin/BlockAdmin.php
index ce37f78..71b21a0 100644
--- a/Admin/BlockAdmin.php
+++ b/Admin/BlockAdmin.php
@@ -137,7 +137,8 @@ class BlockAdmin extends BaseBlockAdmin
         $formMapper->end();

         $isContainerRoot = $block && in_array($blockType, array('sonata.page.block.container', 'sonata.block.service.container')) && !$this->hasParentFieldDescription();
-        $isStandardBlock = $block && !in_array($blockType, array('sonata.page.block.container', 'sonata.block.service.container')) && !$this->hasParentFieldDescription();
+        // $isStandardBlock = $block && !in_array($blockType, array('sonata.page.block.container', 'sonata.block.service.container')) && !$this->hasParentFieldDescription();
+        $isStandardBlock = true;

         if ($isContainerRoot || $isStandardBlock) {
             $formMapper->with('form.field_group_general', $generalGroupOptions);
@@ -209,6 +210,7 @@ class BlockAdmin extends BaseBlockAdmin
                 ->with('form.field_group_options', $optionsGroupOptions)
                 ->add('type', 'sonata_block_service_choice', array(
                         'context' => 'sonata_page_bundle',
+                        'include_containers' => true,
                     ))
                 ->add('enabled')
                 ->add('position', 'integer')
diff --git a/Controller/BlockAdminController.php b/Controller/BlockAdminController.php
index eb6febd..4236a4a 100644
--- a/Controller/BlockAdminController.php
+++ b/Controller/BlockAdminController.php
@@ -88,7 +88,7 @@ class BlockAdminController extends Controller

         if (!$parameters['type']) {
             return $this->render('SonataPageBundle:BlockAdmin:select_type.html.twig', array(
-                'services' => $this->get('sonata.block.manager')->getServicesByContext('sonata_page_bundle'),
+                'services' => $this->get('sonata.block.manager')->getServicesByContext('sonata_page_bundle', true),
                 'base_template' => $this->getBaseTemplate(),
                 'admin' => $this->admin,
                 'action' => 'create',
@@ -154,7 +154,7 @@ class BlockAdminController extends Controller
             throw new PageNotFoundException('No parent found, unable to preview an orphan block');
         }

-        $blockServices = $this->get('sonata.block.manager')->getServicesByContext('sonata_page_bundle', false);
+        $blockServices = $this->get('sonata.block.manager')->getServicesByContext('sonata_page_bundle', true);

         return $this->render('SonataPageBundle:BlockAdmin:compose_preview.html.twig', array(
             'container' => $container,
diff --git a/Controller/PageAdminController.php b/Controller/PageAdminController.php
index 688203f..d87059d 100644
--- a/Controller/PageAdminController.php
+++ b/Controller/PageAdminController.php
@@ -238,7 +238,7 @@ class PageAdminController extends Controller
             throw new NotFoundHttpException(sprintf('unable to find the block with id : %s', $id));
         }

-        $blockServices = $this->get('sonata.block.manager')->getServicesByContext('sonata_page_bundle', false);
+        $blockServices = $this->get('sonata.block.manager')->getServicesByContext('sonata_page_bundle', true);

         // filter service using the template configuration
         if ($page = $block->getPage()) {
diff --git a/Resources/views/Block/block_core_children_pages.html.twig b/Resources/views/Block/block_core_children_pages.html.twig
index 6840544..b62fb5b 100644
--- a/Resources/views/Block/block_core_children_pages.html.twig
+++ b/Resources/views/Block/block_core_children_pages.html.twig
@@ -18,7 +18,9 @@ file that was distributed with this source code.
         <ul class="sonata-page-menu-chilren-list">
             {% if page %}
                 {% for child in page.children %}
+                    {% if child.token is defined %}
                     <li class="sonata-page-menu-children-element"><a href="{{ path(child) }}" title="{{ child.name|escape }}">{{ child.name }}</a></li>
+                    {% endif %}
                 {% endfor %}
             {% endif %}
         </ul>
diff --git a/Resources/views/BlockAdmin/compose_preview.html.twig b/Resources/views/BlockAdmin/compose_preview.html.twig
index 7099ec8..6eebac9 100644
--- a/Resources/views/BlockAdmin/compose_preview.html.twig
+++ b/Resources/views/BlockAdmin/compose_preview.html.twig
@@ -9,6 +9,7 @@
     >
         {% set service = attribute(blockServices, child.type) %}
         <h4 class="page-composer__container__child__name">{{ child.name|default(service.name) }}</h4>
+        <div>[#{{child.id}}]</div>
         {% if not service.blockMetadata.image %}
             <i class="{{ service.blockMetadata.option('class') }}" ></i>
         {% else %}
~/files
greg0ire commented 7 years ago

The first diff references the block bundle, we're never going to merge this. You may disambiguise by providing the admin_code.

MaraScott commented 7 years ago

@greg0ire, @OskarStark What do you mean by _providing the admin_code_ ? Which doc do I follow for that ?

greg0ire commented 7 years ago

Here is some documentation about it : https://github.com/sonata-project/SonataAdminBundle/blob/240ecbd02d453f9acaf81e00112ff5d8dae4459b/Resources/doc/reference/form_types.rst#fielddescription-options

MaraScott commented 7 years ago

Thanks for that !

What about overriding the vendor/sonata-project/admin-bundle/Admin/Pool.php in src/Application/Sonata/AdminBundle/Admin/Pool.php ?

The overriding doesn't work as use Sonata\AdminBundle\Admin\Pool is called in lots of vendor/sonata-project/*.

Is there s a way to tell symfony to identify src/Application/Sonata/AdminBundle as Sonata\AdminBundle\Admin\Pool and if a file is missing in src/Application/Sonata/AdminBundle then it check in vendor/sonata-project/admin-bundle ?

Or Am I missing something ?

FYI : I m not trying to override sonata bundles through some AppBundle but through Application\Sonata, as far as I know it doesn t matter, but I just wanted to mention it.

greg0ire commented 7 years ago

Well you can fork the bundles if you want to override, and use inline-aliases to refer to your fork(s). Once it works, consider contributing your fixes back.

MaraScott commented 7 years ago

Okay, I ll try to do that and get back to you. It might take some time as I m less advanced than you on symfony. Thanks for the help

greg0ire commented 7 years ago

No problem, keep up the positive attitude, here is how you can use your forks in case you're wondering : https://getcomposer.org/doc/articles/aliases.md#require-inline-alias

MaraScott commented 7 years ago

I got one more question before digging, Why did you (as sonata project) not allow to manage 'sonata.page.block.container' and 'sonata.block.service.container' through the admin but put some of them in the demo ?

Is there a reason am I missing ?

greg0ire commented 7 years ago

I don't understand your question.

MaraScott commented 7 years ago

Why is the 'sonata.page.block.container' block is not available by default in the pageAdmin compose ?

greg0ire commented 7 years ago

I really don't know, I don't use the page bundle. @OskarStark , do you know?

ebrost commented 7 years ago

really interested by this feature. I did @davask steps and i can add a block container but can't put any child in it

MaraScott commented 7 years ago

@ebrost, it is a tricky feature, I haven t fix it yet, and I will not be able to do so before a few months unfortunatly. Some times, you have to create an independant block, through the block admin. Once created, you can add a child to a parent. this is definitly not perfect and not prod ready but it is a start.

However if @OskarStark, can help us on the purpose of not having this feature by default, I ll be interested to know why before suggesting a PR for this feature.

ebrost commented 7 years ago

yes, it's tricky indeed, specially the part in Pool.php, but you're right, this feature should be available by default. As i'm a newbie on Symfony and Sonata, i'm not able to do a PR by myself. I will wait ....

RSalo commented 5 years ago

faced the same bug. so sandbox so sonatapages throwing up error

mrpdean commented 5 years ago

Yes, facing the same bug too.

github-actions[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.