Allow to disable the global search by admin

[phansys]

Subject

Allow to disable the global search by admin.

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

Closes #6102.

Changelog

### Added
- `AdminSearchCompilerPass` in order to configure which admins must support search.
- Support for `global_search` attribute in `sonata.admin` tags, which admits boolean values.

To do

• Add tests;
• Update the documentation.

[VincentLanglet]

Suggested change

	private const TAG_DMIN = 'sonata.admin';
	private const TAG_ADMIN = 'sonata.admin';

[VincentLanglet]

And this tag is used in multiple compiler pass. It should be the same constant, isn't it ?

[phansys]

Yes, I see this tag is used in other classes, but since we haven't defined a global place where it could be declared, I decided to use this only for the purposes of this class.

[VincentLanglet]

Should we declare this in the AdminInterface ?

[phansys]

IMO, we should not tie the classes outside the Symfony specific DI scope with something related to that configuration.

[VincentLanglet]

Then I think the tag should be in AddDependencyCallsCompilerPass which should mention Admin in the name (and maybe be splitted ?).

[phansys]

I'll add the constant at AddDependencyCallsCompilerPass in a new pedantic PR.

[VincentLanglet]

I have added it here: #6614

WDYT ?

[phansys]

Do you know how to fix the PHPStan issue without widening the allowed types in the docblock? I would not like to add an ignore rule.

[VincentLanglet]

Do you know how to fix the PHPStan issue without widening the allowed types in the docblock? I would not like to add an ignore rule.

I would say you cant since we're trusting the Phpdoc.

Either the phpdoc say that it should be an array of bool, either there is a check inside ; but phpstan doesnt allow to do both.

[franmomu]

I don't know if we should mark as @internal these methods that are just intended to be called by, in this case, a compiler pass and then trust our input.

[phansys]

Thanks for the feedback.
Given this situation and your answers, I think we should treat phpdocs as they were type declarations (regardless the @internal annotation).
By instance, a method which comes with its corresponding docblocks introduced in the same commit where the method is declared, should be able to add a type check in future commits and throw an exception if the types are not considered valid in the docblock. That addition should be treated as a BC compatible change, since we always told that the expected type was the one declared in the docblock.
I think this decision will helps us a lot narrowing the scope of our BC promise.
WDYT?

[franmomu]

What about using AbstractCompilerPassTestCase?

[franmomu]

Sorry, I thought I had written something more in the previous comment. What I meant was to use that class to make the test simpler and to use real objects, something like:

$adminFooDefinition = new Definition(PostAdmin::class);
$adminFooDefinition->addTag('sonata.admin', ['global_search' => true]);
$this->setDefinition('admin.foo', $adminFooDefinition);

$adminBarDefinition = new Definition(PostAdmin::class);
$adminBarDefinition->addTag('sonata.admin', ['global_search' => false]);
$this->setDefinition('admin.bar', $adminBarDefinition);

$adminBazDefinition = new Definition(PostAdmin::class);
$adminBazDefinition->addTag('sonata.admin', ['some_attribute' => 42]);
$this->setDefinition('admin.baz', $adminBazDefinition);

$searchHandlerDefinition = new Definition();
$this->setDefinition('sonata.admin.search.handler', $searchHandlerDefinition);

$this->compile();

$this->assertContainerBuilderHasServiceDefinitionWithMethodCall(
    'sonata.admin.search.handler',
    'configureAdminSearch',
    [['admin.foo' => true, 'admin.bar' => false]]
);

[phansys]

Thank you. I've applied your suggestion.

[core23]

Can you try to extract some smaller methods to reduce complexity?

[VincentLanglet]

Thanks @phansys