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.

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

Make SonataAdminBundle dependency optional #1503

Merged
merged 10 commits into from
Feb 17, 2022

Conversation

supersmile2009
Copy link
Contributor

@supersmile2009 supersmile2009 commented Feb 16, 2022

Subject

This PR makes SonataAdminBundle dependency optional.

I am targeting 5.x branch, because v5 hasn't got to a final release yet and BC breaks can potentially be still accepted and there were no objections to that in the referenced issue.

Closes #1499

Changelog

### Changed
- SonataAdminBundle dependency is now optional. Trying to access `sonata_user.userAdmin` variable in Twig templates will throw a `\LogicException` if SonataAdminBundle is not installed.

To do

  • Update the documentation;
  • Update the tests;
  • Add an upgrade note.

Notes:

  1. Do you think nullable admin Pool in GlobalVariables is OK or it would be better to conditionally register pool as another Twig variable (e. g. sonata_user_admin)?
  2. I've checked the Media and Classification bundles (referenced in the linked issue as bundles with optional Admin dependency) and there seems to be no full-blown integration tests with running separate test-suits with and without Admin. So I took a modest approach with simply unit-testing the extension with a different config.

@supersmile2009
Copy link
Contributor Author

Failing tests are not caused by the changes in this PR. See #1504

@supersmile2009
Copy link
Contributor Author

Disregard my last comment. It appears that some Symfony FrameworkBundle configs are cleverly applied depending on whether a certain dependency is expected to be available in non-dev install. I had to add framework.assets config to force activate AssetExtension for twig. Otherwise FrameworkBundle de-registers the service, because symfony/asset is not a prod dependency anymore (unlike it was before as a dependency of the admin bundle)

composer.json Show resolved Hide resolved
@jordisala1991 jordisala1991 merged commit 2041eb5 into sonata-project:5.x Feb 17, 2022
@jordisala1991
Copy link
Member

Thanks @supersmile2009 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using this bundle without SonataAdminBundle
3 participants