Introduce SonataConfiguration class #6640

franmomu · 2020-11-30T17:48:02Z

Subject

Part of #6624

This class (I wasn't sure in which folder should go) will handle configuration for sonata which is currently holded by Pool class.

It adds a global twig sonata_config variable in order to replace the current sonata_admin variable whose name could mislead to think that is some kind of admin.

sonata_admin is registered as global twig variable in:

SonataAdminBundle/src/DependencyInjection/Compiler/GlobalVariablesCompilerPass.php

Lines 27 to 31 in ea9f3a4

    
           public function process(ContainerBuilder $container) 
        
           { 
        
               $container->getDefinition('twig') 
        
                   ->addMethodCall('addGlobal', ['sonata_admin', new Reference('sonata.admin.twig.global')]); 
        
           }

SonataAdminBundle/src/Resources/config/core.php

Lines 216 to 223 in ea9f3a4

    
           ->set('sonata.admin.twig.global', GlobalVariables::class) 
        
               ->public() 
        
               ->args([ 
        
                   new ReferenceConfigurator('sonata.admin.pool'), 
        
                   '%sonata.admin.configuration.mosaic_background%', 
        
               ]) 
        
           ->alias(GlobalVariables::class, 'sonata.admin.twig.global')

All GlobalVariables methods have been deprecated:

url and objectUrl were added in 964853c, I haven't found any use, this is usually done though the AdminInterface::generateUrl and AdminInterface::generateObjectUrl.
getAdminPool shouldn't be used in twig, it still has one usage, I'll comment later.
getMosaicBackground has been replaced by SonataConfiguration::getOption('mosaic_background').

I added a GroupExtension Twig extension in order to handle some logic that was done in twig directly.

I am targeting this branch, because these changes are BC.

Changelog

### Added
- Added `SonataConfiguration` class to handle sonata global configuration.
- Added global Twig `sonata_config` to access global configuration from Twig.
### Deprecated
- Deprecated `Pool::getTitle()` method.
- Deprecated `Pool::getTitleLogo()` method.
- Deprecated `Pool::getOption()` method.
- Deprecated global Twig `sonata_admin` variable.

The only thing left is in dashboard.html.twig where there are a couple of calls:

SonataAdminBundle/src/Resources/views/Core/dashboard.html.twig

Line 53 in ea9f3a4

    
           {{ sonata_block_render_event('sonata.admin.dashboard.top', { 'admin_pool': sonata_admin.adminPool }) }}

SonataAdminBundle/src/Resources/views/Core/dashboard.html.twig

Line 128 in ea9f3a4

    
           {{ sonata_block_render_event('sonata.admin.dashboard.bottom', { 'admin_pool': sonata_admin.adminPool }) }}

which I haven't used, but I guess someone using this event could inject the Pool service in the listener directly.

VincentLanglet

Can we chose another directory ?
There is already too much file in the Admin directory IMHO.

Everything is Admin-related, since it's an Admin bundle.

franmomu · 2020-11-30T19:38:35Z

Can we chose another directory ?

Sure, I said in the description that I didn't know where to put it, so I'm open to suggestions, any idea where?

VincentLanglet · 2020-11-30T19:49:32Z

any idea where?

No. I currently don't find better than admin.
But maybe the problem with this "too big directory" are the others files:

BaseFieldDescription, FieldDescriptionCollection, FieldDescriptionInterface and FieldDescriptionRegistryInterface could be in a specific directory.
BreadcrumbsBuilder, BreadcrumbsBuilderInterface, MenuBuilderInterface could be in the Builder directory.

franmomu · 2020-11-30T20:33:59Z

Maybe just a Config or Configuration directory.

greg0ire · 2020-11-30T21:05:55Z

Admin is indeed probably not the right place since it does not relate to an Admin object such as UserAdmin. Instead, this is the global configuration of the whole administration backend, which means it's common, shared, global, cross-cutting. I'd either put it in a directory named like one of those words, or just drop it next to SonataAdminBundle.php since it's so global. Do you foresee the need to have other classes being grouped with that one?

franmomu · 2020-12-01T09:20:13Z

I've moved it to the root directory, I don't see other possible classes going in the Configuration directory right now.

OskarStark · 2020-12-02T06:22:09Z

Root directory is fine, like @greg0ire explained 👌🏻

core23 · 2020-12-05T11:03:57Z

UPGRADE-3.x.md

@@ -4,12 +4,30 @@ UPGRADE 3.x
 UPGRADE FROM 3.xx to 3.xx
 =========================

+### Deprecated `admin_pool` parameter in `sonata.admin.dashboard.top` and `onata.admin.dashboard.bottom` block events.


Suggested change

### Deprecated `admin_pool` parameter in `sonata.admin.dashboard.top` and `onata.admin.dashboard.bottom` block events.

### Deprecated `admin_pool` parameter in `sonata.admin.dashboard.top` and `sonata.admin.dashboard.bottom` block events.

core23 · 2020-12-05T11:04:56Z

src/DependencyInjection/Compiler/GlobalVariablesCompilerPass.php

@@ -27,6 +27,8 @@ class GlobalVariablesCompilerPass implements CompilerPassInterface
    public function process(ContainerBuilder $container)
    {
        $container->getDefinition('twig')
+            ->addMethodCall('addGlobal', ['sonata_configuration', new Reference('sonata.admin.configuration')])


No blocker, more a personal choice. I would prefer sonata_config

I like it, changed.

core23

There is a typo in the docs

SonataCI · 2020-12-05T13:53:13Z

Could you please rebase your PR and fix merge conflicts?

SonataCI · 2020-12-05T17:03:16Z

Could you please rebase your PR and fix merge conflicts?

SonataCI · 2020-12-05T17:53:15Z

Could you please rebase your PR and fix merge conflicts?

VincentLanglet

The changelog and the commit message should be updated about
sonata_config

This class will handle configuration for sonata which is currently holded by Pool class. It adds a global twig "sonata_config" variable in order to replace the current "sonata_admin" variable which all its methods are deprecated.

VincentLanglet · 2020-12-05T20:58:54Z

Thanks @franmomu

mleko64 · 2020-12-09T08:30:32Z

@franmomu You forgot change variable "sonata_admin" to "sonata_config" in src/Resources/views/Pager/base_links.html.twig template file (line 27).

franmomu · 2020-12-09T08:39:08Z

@franmomu You forgot change variable "sonata_admin" to "sonata_config" in src/Resources/views/Pager/base_links.html.twig template file (line 27).

Thanks for reporting @mleko64! I've created #6677

core23 · 2021-03-02T22:53:56Z

src/Twig/GlobalVariables.php

@@ -87,6 +98,11 @@ public function getAdminPool()
     */
    public function url($code, $action, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH)
    {
+        @trigger_error(sprintf(


Is there any replacement method for this method @VincentLanglet @franmomu ?

Our other bundles are still using this method, e.g. https://github.com/sonata-project/SonataPageBundle/blob/6b80c4a675303b2ad707655150dc02fd0be43efc/src/Resources/views/Block/block_pagelist.html.twig#L26

I would recommend to pass the PageAdmin or the AdminPool in the BlockService https://github.com/sonata-project/SonataPageBundle/blob/3.x/src/Block/PageListBlockService.php

And then calling pageAdmin.generateUrl()

franmomu added the minor label Nov 30, 2020

franmomu mentioned this pull request Nov 30, 2020

[RFC] Use Pool just as a registry #6624

Closed

3 tasks

franmomu force-pushed the create_configuration branch 2 times, most recently from 1834a75 to f58b650 Compare November 30, 2020 18:04

VincentLanglet reviewed Nov 30, 2020

View reviewed changes

franmomu force-pushed the create_configuration branch 2 times, most recently from 1e85057 to 2034bef Compare December 1, 2020 09:14

franmomu force-pushed the create_configuration branch from 2034bef to fac90c9 Compare December 2, 2020 18:24

VincentLanglet previously approved these changes Dec 4, 2020

View reviewed changes

VincentLanglet requested a review from a team December 4, 2020 19:58

OskarStark previously approved these changes Dec 5, 2020

View reviewed changes

core23 reviewed Dec 5, 2020

View reviewed changes

core23 requested changes Dec 5, 2020

View reviewed changes

SonataCI added the pending author label Dec 5, 2020

franmomu dismissed stale reviews from OskarStark and VincentLanglet via 48f7806 December 5, 2020 14:38

franmomu force-pushed the create_configuration branch from fac90c9 to 48f7806 Compare December 5, 2020 14:38

SonataCI removed the pending author label Dec 5, 2020

core23 previously approved these changes Dec 5, 2020

View reviewed changes

franmomu requested a review from a team December 5, 2020 15:31

franmomu mentioned this pull request Dec 5, 2020

3.82.0 #6652

Merged

SonataCI added the pending author label Dec 5, 2020

franmomu dismissed core23’s stale review via ac92837 December 5, 2020 17:09

franmomu force-pushed the create_configuration branch from 48f7806 to ac92837 Compare December 5, 2020 17:09

SonataCI removed the pending author label Dec 5, 2020

SonataCI added the pending author label Dec 5, 2020

franmomu force-pushed the create_configuration branch from ac92837 to 3981973 Compare December 5, 2020 18:08

SonataCI removed the pending author label Dec 5, 2020

franmomu force-pushed the create_configuration branch from 3981973 to 6809da5 Compare December 5, 2020 18:10

VincentLanglet previously approved these changes Dec 5, 2020

View reviewed changes

VincentLanglet reviewed Dec 5, 2020

View reviewed changes

Introduce SonataConfiguration class

d6750fc

This class will handle configuration for sonata which is currently holded by Pool class. It adds a global twig "sonata_config" variable in order to replace the current "sonata_admin" variable which all its methods are deprecated.

franmomu dismissed VincentLanglet’s stale review via d6750fc December 5, 2020 18:20

franmomu force-pushed the create_configuration branch from 6809da5 to d6750fc Compare December 5, 2020 18:20

franmomu requested a review from a team December 5, 2020 20:54

VincentLanglet approved these changes Dec 5, 2020

View reviewed changes

VincentLanglet merged commit ad8a050 into sonata-project:3.x Dec 5, 2020

franmomu deleted the create_configuration branch December 5, 2020 21:15

franmomu mentioned this pull request Dec 7, 2020

Fix trigger_error messages #6660

Merged

franmomu mentioned this pull request Dec 9, 2020

Use sonata_config instead of admin_pool #6677

Merged

core23 reviewed Mar 2, 2021

View reviewed changes

core23 mentioned this pull request Mar 3, 2021

Remove sonata_admin deprecations in twig templates sonata-project/SonataPageBundle#1282

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce SonataConfiguration class #6640

Introduce SonataConfiguration class #6640

franmomu commented Nov 30, 2020 •

edited

Loading

VincentLanglet left a comment

franmomu commented Nov 30, 2020

VincentLanglet commented Nov 30, 2020

franmomu commented Nov 30, 2020

greg0ire commented Nov 30, 2020

franmomu commented Dec 1, 2020

OskarStark commented Dec 2, 2020

core23 Dec 5, 2020

core23 Dec 5, 2020

franmomu Dec 5, 2020

core23 left a comment

SonataCI commented Dec 5, 2020

SonataCI commented Dec 5, 2020

SonataCI commented Dec 5, 2020

VincentLanglet left a comment

VincentLanglet commented Dec 5, 2020

mleko64 commented Dec 9, 2020

franmomu commented Dec 9, 2020

core23 Mar 2, 2021 •

edited

Loading

VincentLanglet Mar 2, 2021

	public function process(ContainerBuilder $container)
	{
	$container->getDefinition('twig')
	->addMethodCall('addGlobal', ['sonata_admin', new Reference('sonata.admin.twig.global')]);
	}

	->set('sonata.admin.twig.global', GlobalVariables::class)
	->public()
	->args([
	new ReferenceConfigurator('sonata.admin.pool'),
	'%sonata.admin.configuration.mosaic_background%',
	])

	->alias(GlobalVariables::class, 'sonata.admin.twig.global')

	### Deprecated `admin_pool` parameter in `sonata.admin.dashboard.top` and `onata.admin.dashboard.bottom` block events.
	### Deprecated `admin_pool` parameter in `sonata.admin.dashboard.top` and `sonata.admin.dashboard.bottom` block events.

Introduce SonataConfiguration class #6640

Introduce SonataConfiguration class #6640

Conversation

franmomu commented Nov 30, 2020 • edited Loading

Subject

Changelog

VincentLanglet left a comment

Choose a reason for hiding this comment

franmomu commented Nov 30, 2020

VincentLanglet commented Nov 30, 2020

franmomu commented Nov 30, 2020

greg0ire commented Nov 30, 2020

franmomu commented Dec 1, 2020

OskarStark commented Dec 2, 2020

core23 Dec 5, 2020

Choose a reason for hiding this comment

core23 Dec 5, 2020

Choose a reason for hiding this comment

franmomu Dec 5, 2020

Choose a reason for hiding this comment

core23 left a comment

Choose a reason for hiding this comment

SonataCI commented Dec 5, 2020

SonataCI commented Dec 5, 2020

SonataCI commented Dec 5, 2020

VincentLanglet left a comment

Choose a reason for hiding this comment

VincentLanglet commented Dec 5, 2020

mleko64 commented Dec 9, 2020

franmomu commented Dec 9, 2020

core23 Mar 2, 2021 • edited Loading

Choose a reason for hiding this comment

VincentLanglet Mar 2, 2021

Choose a reason for hiding this comment

franmomu commented Nov 30, 2020 •

edited

Loading

core23 Mar 2, 2021 •

edited

Loading