Commerce: Specific Products

Working on it.

sayan_k_dutta → changed the visibility of the branch 3478960-commerce-specific-products to hidden.

sayan_k_dutta → changed the visibility of the branch 2.0.x to hidden.

sayan_k_dutta → changed the visibility of the branch 2.0.x to active.

jurgenhaas → changed the visibility of the branch 2.0.x to hidden.

jurgenhaas → changed the visibility of the branch 2.1.x to hidden.

Here is what's happening: when commerce conditions use a commerce_entity_select or a entity_autocomplete field type in a configuration form, ECA changes that to a text field which accepts a comma separated list of product IDs.

The commerce product trait, however, contains a form submit handler that expects an array with each item containing the key target_id with the product IDs. From those IDs, they lookup the UUIDs of each product and store that as configuration.

We can't have that configuration as such because ECA needs to store the comma separated ID list as this is required again next time the ECA model is being edited.

However, we need to adjust the plugin configuration right before execution/evaluation in the same way to provide the UUIDs instead of IDs.

This is certainly a bigger task and then also needs someone for testing. Not sure, if I can get to it soon.

@sayan using the issue queue for questions is better.

No problem opening am issue fork, if you don't add to it someone else will!

The scope here is slightly bigger than what's described in the issue title and summary, as suggested in #9. In short, commerce condition plugins are not correctly handled due to form decoration at the eca deriver level. I am bumping the severity to Major as reusing commerce Condition Plugins is one of the biggest potential value of using ECA with Commerce.

Backstory: I am working on recipes and want to react to the add-to-cart event (that part works fine) but only if a certain condition is met - order type is X (doesn't work). The issue is that bundles are stored as an array, but the eca_commerce_commerce plugin expects them to be string

TBH, I'm not sure we could or should try and work around this in ECA Commerce. In an ideal world, I'd suggest a refactor of commerce conditions such that they keep arbitrary config values when config forms get submitted and only process them when processed. The result will be the same, but we all prevent the need to reverse convert config values when editing configuration afterwards.

Not sure if there's any chance to get buy-in for that idea from commerce maintainers?

I'd suggest a refactor of commerce conditions such that they keep arbitrary config values when config forms get submitted and only process them when processed.

Could you elaborate a bit more? I'm not sure I fully get the suggestion. Is that referring to your comment from #9? The reason IDS are converted to UUIDS is for exportability, and that could be quite handy in a scenario were default content is used and the UUID is preserved for example.

I've been trying to use the "Order contains product variation types" condition which currently doesn't work and I'm wondering about the following code:

$config = [];
      foreach ($commercePlugin->defaultConfiguration() as $key => $value) {
        if (is_array($value) && is_string($this->configuration[$key])) {
          $config[$key] = array_map('trim', explode(',', $this->configuration[$key]));
        }
      }
      $commercePlugin->setConfiguration($config);

$this->configuration already looks ok, and the result of this code will be ECA Commerce passing an empty $config array. instead of using the correctly set configuration values? Am I missing anything?

Shouldn't we initialize $config with $this->configuration? And convert the string to an array only if necessary like it is today but preserving the config is set?

@jurgenhaas: I tried returning early in the ProductTrait like the following:

// Convert selected IDs into UUIDs, and store them.
    $values = $form_state->getValue($form['#parents']);
    if (!is_array($values['products'])) {
      return;
    }

That does fix the error reported here, but then the configuration isn't set... So I'm not sure what else we could/should do... The ECA Commerce module probably has to translate the string into an array when submitting the configuration form.

Even if Commerce doesn't cause a crash by assuming the "products" key is an array, you then end up with an empty configuration?

@jsacksick great observation in #15, this was introduced in 🐛 "Field xxx is required" error even when a value is provided Active and unfortunately, although the bug was pressing to some users, none ever tested it and I then merged it as it also solved some obvious exceptions.

But you're right, line 70 with $config = []; needs to be $config = $this->configuration;.

As for #16 I honestly don't know the answer. I'm not deep enough into Commerce and I don't have the bandwidth to dive deep into it as well. Would be really great if you could help the users out of these issues.

On your question in #14 of what I meant, this wasn't specific on one condition from commerce, this is a more general statement. I've seen the pattern that e.g. commerce conditions and also tamper plugins convert submitted config values into something entirely different. They then have to convert back during form build. Changing that pattern to move the transformation to the beginning of the processing, or at least into their own methods, would be a cleaner architecture. I know, this pattern comes from the assumption that processing follows right after submission. But that's not true for ECA, and I can already see that AI Agents will run into similar issues as well.

But I take your point, that for config sharing between sites, entity IDs are not appropriate.