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

Decouple AdminBundle from the Core #14918

Merged
merged 21 commits into from
Apr 19, 2023
Merged

Decouple AdminBundle from the Core #14918

merged 21 commits into from
Apr 19, 2023

Conversation

dvesh3
Copy link
Contributor

@dvesh3 dvesh3 commented Apr 13, 2023

Changes in this pull request

Related to #12531

@dvesh3 dvesh3 added the Task label Apr 13, 2023
@dvesh3 dvesh3 added this to the 10.6.0 milestone Apr 13, 2023
@github-actions
Copy link

github-actions bot commented Apr 13, 2023

Review Checklist

  • Target branch (10.5 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@dvesh3 dvesh3 changed the title Decouple AdminBundle from the Core - … Decouple AdminBundle from the Core Apr 13, 2023
@dvesh3 dvesh3 marked this pull request as ready for review April 13, 2023 12:57
@dvesh3 dvesh3 requested a review from brusch April 13, 2023 12:57
Copy link
Contributor Author

@dvesh3 dvesh3 left a comment

Choose a reason for hiding this comment

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

We have another blocker on Pimcore 11: \Pimcore\Bundle\AdminBundle\System\Config is used in the core to get the config based on LocationAwareConfigRepository. If these can't be refactored then changes in this PR are not relevant.

Also need to consider:

  1. Bundles relying on adminjson calls in AdminController which uses AdminSerializer. (they can't simply switch from AdminController to UserAwareController).
  2. BundleManagerEvents used in PimcoreBundleManager
  3. IndexActionSettingsEvent
  4. Tool::getLanguageFile using AdminBundle\Tool::getLanguageFile
  5. Core tests relying on admin bundle
    use Pimcore\Bundle\AdminBundle\Helper\GridHelperService;
  6. Admin events triggered from the core e.g.
    \Pimcore::getEventDispatcher()->dispatch($event, AdminEvents::ELEMENT_PERMISSION_IS_ALLOWED);

@dpfaffenbauer
Copy link
Contributor

can't that config be moved to the core?

@dvesh3
Copy link
Contributor Author

dvesh3 commented Apr 13, 2023

can't that config be moved to the core?

@dpfaffenbauer yes, we are working on separating the configs belongs to the core and admin bundle so the correct config can be read from their respective config service. There will be a follow up to #14867

@dvesh3 dvesh3 requested a review from brusch April 14, 2023 11:33
@dvesh3 dvesh3 requested a review from mattamon April 17, 2023 13:24
Copy link
Contributor Author

@dvesh3 dvesh3 left a comment

Choose a reason for hiding this comment

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

This needs to be done before merging this PR

@dvesh3 dvesh3 requested a review from mattamon April 19, 2023 13:34
Copy link
Contributor

@mattamon mattamon left a comment

Choose a reason for hiding this comment

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

LGTM

@mattamon mattamon merged commit c1791bb into 10.6 Apr 19, 2023
@mattamon mattamon deleted the decouple_admin_bundle branch April 19, 2023 14:08
dvesh3 added a commit to pimcore/admin-ui-classic-bundle that referenced this pull request Apr 19, 2023
dvesh3 added a commit that referenced this pull request Apr 20, 2023
@dvesh3 dvesh3 mentioned this pull request Apr 20, 2023
dvesh3 added a commit that referenced this pull request Apr 21, 2023
mattamon added a commit that referenced this pull request Apr 24, 2023
* [Pimcore 11] Decouple AdminBundle from the Core - follow up to #14918

* Fix Core Bundles & Security classes

* require dev branch (temproray) of admin ui classic bundle

* Make Core bundles implement PimcoreBundleAdminClassicInterface

* Further decoupling

* Fix tests

* Further decoupling

* Further decoupling

* check disabling admin bundle

* Fix stan issues

* Move Admin Listeners to admin-classic.yaml

* Add ignore again and void to method

* Extend phpstand-baseline.neon and phpstan-parameters.neon

* Revert unrelated changes

* Refactor initializing system settings for tests

* Adding void return type

* Remove line from baseline.neon

* Add interface

* Extract AdminBundle tests from core.

* extract testAddGridFeatureJoinsWithTwoFilters into AdminBundle.

* Fix tests

* Fix tests

* Fix phpstan baseline - remove ignored errors

* cleanup phpstan baseline

* review changes

* fix phpstan baseline

* Fix docs related to bundle support interface & trait

---------

Co-authored-by: mattamon <[email protected]>
Co-authored-by: Martin Eiber <[email protected]>
mattamon pushed a commit to pimcore/admin-ui-classic-bundle that referenced this pull request Apr 24, 2023
* Decouple from the core - changes related to pimcore/pimcore#14918 & pimcore/pimcore#14971

* Adapt more controllers to use new abstract

* fix typo

* Further decoupling

* Fix Stan

* Use ElementService::getCustomViewById

* review changes

* review changes

* Remove unrelated

* Remove unused classes
dvesh3 added a commit that referenced this pull request May 9, 2023
* Decouple AdminBundle from the Core - related to #12531

* Re-add AdminController

* Re-add services with deprecations

* Use Pimcore JsonResponse

* Fix typo

* Move back EditmodeListener, Move deprecated methods to AdminController and add isolated serializer for Core

* naming optimizations

* TokenUsageResolver: Return correct user proxy from deprecated class

* resgister new UserProvider service

* fix tests

* review changes

* required injections

* Review changes & move password hasher to the core

* Replace deprecated usages of AdminController

* Fix Stan

* Revert Serializer DI

* Fix bundle manager DI

* Fix tests

* Ecommerce controllers extends UserAwareController

* Fix Stan

* Fix Stan
dvesh3 added a commit that referenced this pull request May 9, 2023
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.

4 participants