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
Allow extensions to customize the siteaccess configuration #1937
Conversation
262eeb0
to
13533c2
Compare
|
||
namespace eZ\Bundle\EzPublishCoreBundle\SiteAccess; | ||
|
||
interface SiteAccessConfigurationFilter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
phpdoc
d5734cd
to
80ed9af
Compare
After discussing with @lolautruche there are better ways to do this (in the |
b7de9df
to
335f869
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides nitpick, looks good! 👍
@@ -142,7 +148,10 @@ public function load(array $configs, ContainerBuilder $container) | |||
*/ | |||
public function getConfiguration(array $config, ContainerBuilder $container) | |||
{ | |||
return new Configuration($this->getMainConfigParser(), $this->suggestionCollector); | |||
$configuration = new Configuration($this->getMainConfigParser(), $this->suggestionCollector); | |||
$configuration->setSiteAccessConfigurationFilters($this->siteaccessConfigurationFilters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not pass those via constructor instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually what I started with. But since they actually have a default value, I chose to do it that way instead.
Are there advantages to a ctor argument ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, in this context not really.
More generally, it's just that setter injection is optional, so you can't be sure that what's supposed to be injected is really here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is what I did guess, indeed. In that particular case, since we iterate over the property, and it is initialized as an array, I think we're good, and I'd rather leave it as it is.
@bdunogier 5ae654e Might not belong here ;) (feel free to delete this when no longer relevant) |
Oops, mispushed a little bit yesterday evening. Thank you @andrerom :) |
Allows one to filter the siteaccess config before it is applied: // MyBundle.php $extension = $container->getExtension('ezpublish'); $extension->addSiteAccessConfigurationFilter(new MyFilter()); // MyFilter.php function filter(array $configuration) { $configuration['list'][] = 'foo'; $configuration['groups']['site_group'][] = 'foo'; return $configuration; }
5ae654e
to
1752adf
Compare
Removed the misplaced commit, and squashed the whole thing. |
🎉 |
Allows bundles to add a
SiteAccessConfigurationFilter
, that gets to filter the siteaccess configuration array before it is applied:Use-cases
Dynamically create admin siteaccess(es) for the Hybrid UI.
Architecture
The
SiteAccessConfigurationFilter
, added by 3rd party bundles to theezpublish
extension, are provided to the CoreBundle'sConfiguration
. They are iterated on in thebeforeNormalization()
method of thesiteaccess
configuration node.Tasks
EzPublishCoreExtension
.doc/specifications/siteaccess/siteaccess_aware_configuration.md
)