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.

Sign up for GitHub

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

Jump to bottom

Remove usage of sonata_admin twig global variable #1297

Merged
merged 1 commit into from
Apr 14, 2021
Merged

Remove usage of sonata_admin twig global variable #1297

merged 1 commit into from
Apr 14, 2021

Conversation

franmomu
Copy link
Member

@franmomu franmomu commented Apr 13, 2021 •
edited
Loading

Subject

In order to fix #1282, I've added a sonata_page_admin variable which holds the sonata.page.admin.page service.

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

Closes #1282 and #1299.

Changelog

### Added
- Added `sonata_page_admin` Twig global variable which holds `sonata.page.admin.page` service
### Removed
- Removed deprecations from `sonata-project/admin-bundle` using `sonata_admin` Twig global variable

@franmomu franmomu added the minor label Apr 13, 2021
VincentLanglet
VincentLanglet previously approved these changes Apr 13, 2021
View reviewed changes
@VincentLanglet VincentLanglet requested a review from a team April 13, 2021 07:33
@franmomu franmomu force-pushed the sonata_admin_deprecations branch from 8d5dc23 to d298161 Compare April 13, 2021 11:30
VincentLanglet
VincentLanglet previously approved these changes Apr 13, 2021
View reviewed changes
core23
core23 previously approved these changes Apr 13, 2021
View reviewed changes
@VincentLanglet VincentLanglet requested a review from phansys April 13, 2021 12:36
jordisala1991
jordisala1991 previously approved these changes Apr 13, 2021
View reviewed changes
@franmomu franmomu dismissed stale reviews from jordisala1991, core23, and VincentLanglet via 2379846 April 13, 2021 14:48
@franmomu franmomu force-pushed the sonata_admin_deprecations branch 2 times, most recently from 2379846 to cb8376e Compare April 13, 2021 15:28
@franmomu franmomu force-pushed the sonata_admin_deprecations branch from cb8376e to a50d56f Compare April 13, 2021 15:32
@phansys phansys requested review from jordisala1991 and core23 April 14, 2021 06:56
@VincentLanglet VincentLanglet merged commit 8fdb795 into sonata-project:3.x Apr 14, 2021
@franmomu franmomu deleted the sonata_admin_deprecations branch April 15, 2021 10:58
core23
core23 reviewed Apr 30, 2021
View reviewed changes
src/DependencyInjection/Compiler/GlobalVariablesCompilerPass.php
$container->getDefinition('twig')
->addMethodCall('addGlobal', ['sonata_page', new Reference('sonata.page.twig.global')]);
->addMethodCall('addGlobal', ['sonata_page', new Reference('sonata.page.twig.global')])
->addMethodCall('addGlobal', ['sonata_page_admin', new Reference('sonata.page.admin.page')]);
Copy link
Member

Choose a reason for hiding this comment

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

The SonataAdminBundle is an optional dependency, so this can be null @VincentLanglet

Copy link
Member

Choose a reason for hiding this comment

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

Why is it optional ? It's in the require
https://github.com/sonata-project/SonataPageBundle/blob/3.x/composer.json#L28

Copy link
Member

Choose a reason for hiding this comment

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

cc @franmomu

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in #1310, the services are loaded if SonataAdminBundle is available:

SonataPageBundle/src/DependencyInjection/SonataPageExtension.php

Lines 77 to 85 in 654fc11

if (isset($bundles['SonataAdminBundle'])) {
$loader->load('admin.xml');
if (!$config['direct_publication']) {
$container->removeDefinition('sonata.page.admin.extension.snapshot');
}
$this->configureTemplatesAdmin($container, $config);
}

but I guess another PR is needed to make it really optional.

@franmomu franmomu mentioned this pull request Apr 30, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@core23 core23 core23 approved these changes

@VincentLanglet VincentLanglet VincentLanglet approved these changes

@phansys phansys phansys approved these changes

@jordisala1991 jordisala1991 Awaiting requested review from jordisala1991

Assignees
No one assigned
Labels
minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove sonata_admin deprecations in twig templates
5 participants
@franmomu @jordisala1991 @phansys @core23 @VincentLanglet