Need ability to declare bind params for grids and forms

[Vinai]

This issue is intended as a design discussion.

There is a problem I’m trying to find a good solution to. I would appreciate any ideas or suggestions.

The problem exists for grids and forms, but for forms it’s more obvious. Often a grid is embedded in a page with additional data, e.g. on a customer detail page all their pending reviews could be shown, or on a product page I want to show all orders.

In a nutshell, there needs to be some default limitation of the displayed data, that is, I want to show the orders related to the current product.

For forms problem is more obvious because it occurs every time an existing entity should be loaded to populate the form (that is, editing the entity).

Because hyva grids (and forms in future, too) are defined in XML only, there is no obvious way I can see where some kind of assignment can be made. For grids there are currently two workarounds:

• A specific data provider can be created, i.e. a repo or collection class that only returns the desired data based on the query parameters or some other global state. I don’t like this, because my base desire is that I want it to be simple and easy to use existing classes to get my job done of creating a grid or form.
• Currently two events are dispatched before grid data is loaded: hyva_grid_source_prefetch and 'hyva_grid_repository_source_prefetch_' . $this->getGridNameEventSuffix($gridName). These events could be used to set additional criteria on the search criteria that is then applied to load the grid data. I like this better than the first option, but it still requires creating a new class (the observer) and an events.xml file only to apply the default criteria.

What is needed is some simple way to declare a mapping from global state to query conditions in XML. How can this be accomplished?

[Vinai]

My current idea to only support mapping of query arguments to fields, either with an eq or an in type condition. This would probably be good enough for most cases. More complex scenarios would then require the use of an event observer.

Here is a first idea for the XML

<source>
    <repositoryListMethod>\Magento\Catalog\Api\ProductRepositoryInterface::getList</repositoryListMethod>
    <defaultSearchCriteriaBindings>
        <requestParam name="id" field="entity_id"/>
        <requestParam name="cust" field="customer_ids"/>
    </defaultSearchCriteriaBindings>
</source>

This would also allow adding additional bind params that would be bound to additional method parameters.

<paramBindings>
    <requestParam name="store" argument="storeId"/>
</paramBindings>

This would make the value of the request param store the argument value for the repository method argument $storeId.

These are both thoughts on how to pass request arguments to the source data providers. Ideally it would be possible to pass other state, too, but I don’t have ideas that are simple enough (according to the design principles).

[Vinai]

Comment via Slack from @schmengler:

What about something like this for arbitrary state:

<defaultSearchCriteriaBindings>
  <!-- 1. useful predefined accessors, like request params -->
  <field name="entity_id">
    <requestParam name="id">
  </field>
  <field name="store_id">
    <currentStoreId />
  </field>
  <!-- 2. generic accessor -->
  <field name="customer_id">
    <method class="Magento\Customer\Model\Session" method="getCustomerId">
      <!-- optionally pass arguments here with di.xml syntax or the same elements that are allowed in `<field>` -->
      <arguments></arguments>
    </method>
  </field>
</defaultSearchCriteriaBindings>

[Vinai]

I like that the most common case is quite simple and elegant. I also like a lot that both search criteria mappings and also method arguments could be covered, that is field and maybe something like an argument node, that then could be passed in addition to the search criteria. The wrapping node then should be something like queryBindings or such.

The method <field><arguments> node looks dangerously recursive (those arguments might need to be dynamic, too). I don’t want to turn this into another almost touring-complete dsl :slightly_smiling_face: I think you are on a good track though.

[schmengler]

If the recursive nature makes it too complex, 1 level is probably enough. In cases where it isn't you could still create a custom class. You wanted to prevent this for typical cases, not for every case I guess

[Vinai]

It shouldn't be possible to bind multiple values to the same field, so I think it would make sense to use field attributes instead. Here is what I thought could work instead:

<source>
  <defaultSearchCriteriaBindings>
    <field name="entity_id" requestParam="id">
    <field name="entity_id" class="Magento\Framework\App\RequestInterface" method="getParam" param="id"/>
    <field name="store_id" class="Magento\Store\Model\StoreManagerInterface" method="getStore" property="id"/>
    <field name="customer_ids" condition="finset" class="Magento\Customer\Model\Session" method="getCustomerId"/>
  </defaultSearchCriteriaBindings>
</source>

The first and second declarations are equivalent and would both map to

$objectManager->get('Magento\Framework\App\RequestInterface')->getParam('id');

The requestParam attribute is for convenience.

The third declaration would map to

$objectManager->get('Magento\Store\Model\StoreManagerInterface')->getStore()->getId();

This would be accomplished with the help of reflection. The field to access would be fetched with a matching getter (if it is an object and a getter exists), or getData($property) (if it's an object and that method exists. If it's an array it would default to array access (.e.g. ->getStore()['id']). Note: only a string type param value is supported.

The fourth (without param or property attribute) would map to

$objectManager->get('Magento\Customer\Model\Session')->getCustomerId()`

---

If more complex mappings with deeper nestings are required, they can always be wrapped in a simpler class method. All in all I prefer this approach over the existing event/observer idea because it will work in many cases without having to create additional classes. Even if a new class is required for a complex case, no additional configuration files are needed.

[Vinai]

To apply the bindings to the SearchCriteria instance an eq condition should be used by default. Other conditions, for example in or finset could be specified, too, using an condition attribute. I've updated the example above to include one such attribute.

[Vinai]

A fist implementation can be found in the bindparams branch: https://github.com/hyva-themes/magento2-hyva-admin/blob/bindparams/Model/GridSource/SearchCriteriaBindings.php

Tests: https://github.com/hyva-themes/magento2-hyva-admin/blob/bindparams/Test/Integration/Model/GridSource/SearchCriteriaBindingsTest.php

[Vinai]

Implemented and merged into main.

[Vinai]

Quick follow up in case someone stumbles over this thread. I refactored the syntax before releasing the feature so the class and method are specified in a single attribute instead of separate class and method. This avoids the issue where one is overridden through XML merging and the other is not.

<field name="customer_id" method="Magento\Customer\Model\Session::getCustomerId"/>