Add methods to be used by AdminBundle

[VincentLanglet]

Subject

I am targeting this branch, because BC-break.

The AdminBundle and the DatagridBundle has a lot of classes in common.
I would like that the AdminBundle requires the DatagridBundle and use his classes.

But currently some methods are missing in the interface, so I added them.

Changelog

### Added
- Added hasDisplayableFilters in DatagridInterface and Datagrid
- Added getFieldOption, setFieldOption, getParentAssociationMappings, getFieldMapping, getAssociationMapping in FilterInterface and BaseFilter
- Added getUniqueParameterId, entityJoin in ProxyQueryInterface and BaseProxyQuery

### Changed
- BaseProxyQuery::setSortBy and ProxyQueryInterface::setSortBy signature

[VincentLanglet]

I have only one issue.

SonataAdminBundle Datagrid use in the buildPager methods

$this->formBuilder->get('_sort_by')->addViewTransformer(new CallbackTransformer(
    static function ($value) {
        return $value;
    },
    static function ($value) {
        return $value instanceof FieldDescriptionInterface ? $value->getName() : $value;
    }
));

And

if (isset($this->values['_sort_by'])) {
    if (!$this->values['_sort_by'] instanceof FieldDescriptionInterface) {
        throw new UnexpectedTypeException($this->values['_sort_by'], FieldDescriptionInterface::class);
    }

    if ($this->values['_sort_by']->isSortable()) {
        $this->query->setSortBy($this->values['_sort_by']->getSortParentAssociationMapping(), $this->values['_sort_by']->getSortFieldMapping());

        $this->values['_sort_order'] = $this->values['_sort_order'] ?? 'ASC';
        $this->query->setSortOrder($this->values['_sort_order']);
    }
}

That's why the AdminBundle Datagrid works with FieldDescriptionInterface, the Datagrid from DatagridBundle work with a string value and the ProxyQuery didn't had the same setSortBy signature.

I wouldlike that the DatagridBundle Datagrid works with FieldDescriptionInterface but I don't know how to import this class.

• If I require the admin bundle, I won't be able to require the datagrid bundle in the admin bundle because it will lead to circular references, isn't it ?
• If I move the FieldDescriptionInterface in the DatagridBundle, I don't know if this change makes sens. And some methods of the FieldDescriptionInterface is using the AdminInterface so I would need a way to avoid moving the AdminInterface too 😅. Maybe splitting the FieldDescriptionInterface in two...

Any advice @greg0ire ?

[greg0ire]

Maybe splitting the FieldDescriptionInterface in two...

This should be BC, and sounds good to me. You could have the admin interface extend the datagrid interface.

[VincentLanglet]

Maybe splitting the FieldDescriptionInterface in two...

This should be BC, and sounds good to me. You could have the admin interface extend the datagrid interface.

Let's go with this

interface FieldDescriptionInterface
{
    public function setFieldName($fieldName);
    public function getFieldName();
    public function setName($name);
    public function getName();
    public function getOption($name, $default = null);
    public function setOption($name, $value);
    public function setOptions(array $options);
    public function getOptions();
    public function setTemplate($template);
    public function getTemplate();
    public function setType($type);
    public function getType();
    public function setParent(AdminInterface $parent);
    public function getParent();
    public function setAssociationMapping($associationMapping);
    public function getAssociationMapping();
    public function getTargetEntity();
    public function setFieldMapping($fieldMapping);
    public function getFieldMapping();
    public function setParentAssociationMappings(array $parentAssociationMappings);
    public function getParentAssociationMappings();
    public function setAssociationAdmin(AdminInterface $associationAdmin);
    public function getAssociationAdmin();
    public function isIdentifier();
    public function getValue($object);
    public function setAdmin(AdminInterface $admin);
    public function getAdmin();
    public function mergeOption($name, array $options = []);
    public function mergeOptions(array $options = []);
    public function setMappingType($mappingType);
    public function getMappingType();
    public function getLabel();
    public function getTranslationDomain();
    public function isSortable();
    public function getSortFieldMapping();
    public function getSortParentAssociationMapping();
    public function getFieldValue($object, $fieldName);
}

For the Datagrid, I only need the methods

• getName()
• getSortParentAssociationMapping
• getSortFieldMapping

If I want to add the getSortParameters/getPaginationParameters to the DatagridInterface, I'll need to support

• getName()
• getOption('sortable')

So I can make a smaller interface

interface FieldInterface
{
    public function setName($name);
    public function getName();
    public function getOption($name, $default = null);
    public function setOption($name, $value);
    public function setOptions(array $options);
    public function getOptions();
    public function mergeOption($name, array $options = []);
    public function mergeOptions(array $options = []);
    public function getLabel();
    public function getTranslationDomain();
    public function isSortable();
    public function getSortFieldMapping();
    public function getSortParentAssociationMapping();
}

I just don't know if I should have in this interface

    public function getLabel()
    {
        return $this->getOption('label');
    }

    public function getTranslationDomain()
    {
        return $this->getOption('translation_domain');
    }

Or if this should be in the Admin one. Any idea @greg0ire ?

And I don't think these methods

    public function isSortable(): bool
    {
        return false !== $this->getOption('sortable', false);
    }

    public function getSortFieldMapping()
    {
        return $this->getOption('sort_field_mapping');
    }

    public function getSortParentAssociationMapping()
    {
        return $this->getOption('sort_parent_association_mappings');
    }

Are needed in the interface, since it's just helpers called once ; in the datagrid.

if ($this->values['_sort_by']->isSortable()) {
    $this->query->setSortBy($this->values['_sort_by']->getSortParentAssociationMapping(), $this->values['_sort_by']->getSortFieldMapping());

    $this->values['_sort_order'] = $this->values['_sort_order'] ?? 'ASC';
    $this->query->setSortOrder($this->values['_sort_order']);
}

And I could use getOption instead.
Or I can also create a special Interface SortableFieldInterface extending FieldInterface.
What do you prefer @greg0ire ?

After these decisions, I'll start the PR :)

[greg0ire]

Or if this should be in the Admin one. Any idea @greg0ire ?

I don't know either, it's unclear to me what the scope of datagrid is, and what these methods do. Let's see if other @sonata-project/contributors that also, you know, use Sonata think :P

[VincentLanglet]

I don't know either, it's unclear to me what the scope of datagrid is, and what these methods do. Let's see if other @sonata-project/contributors that also, you know, use Sonata think :P

I feel sad for your company for not using the excellent Sonata bundles !
You should try to convince them :P

@core23 Made some PR on this bundle, maybe he'll know.

[VincentLanglet]

It seems like that every bundle using the DatagridBundle are using it for the PagerInterface, and not the DatagridInterface.

https://github.com/sonata-project/SonataUserBundle/search?q=datagridbundle&unscoped_q=datagridbundle
https://github.com/sonata-project/SonataPageBundle/search?q=datagridbundle&unscoped_q=datagridbundle
https://github.com/sonata-project/SonataNotificationBundle/search?q=datagridbundle&unscoped_q=datagridbundle
https://github.com/sonata-project/SonataNewsBundle/search?q=datagridbundle&unscoped_q=datagridbundle
https://github.com/sonata-project/SonataMediaBundle/search?q=datagridbundle&unscoped_q=datagridbundle
https://github.com/sonata-project/ecommerce/search?q=datagridbundle&unscoped_q=datagridbundle
https://github.com/sonata-project/SonataClassificationBundle/search?p=1&q=datagridbundle&unscoped_q=datagridbundle

This could explain the differences between DatagridBundle and AdminBundle.
The Datagrid was copy paste from the Admin, then the Admin one was updated and not the Datagrid one.

[core23]

I'm not sure if we should add this method. Showing a filter or not is logic for the UI, not the general lib

[VincentLanglet]

What is the purpose of the library ?

hasActiveFilters and hasDisplayableFilters are really similar. I think it's better to have the two methods in the same Interface/class.

Currently we have a loooot of duplicated code/really similar code. I want to deal with this, and I think we'll have to make some small trade-off.

There are some others UI method in this bundle, like Filter::getRenderSettings, or getTranslationDomain. The Datagrid class is also mainly a UI class, and is never use by others bundle. They only use the Pager and the ProxyQuery.

[VincentLanglet]

Closing in favor of #239