"Field xxx is required" error even when a value is provided

Created on 9 February 2024, 10 months ago
Updated 7 June 2024, 6 months ago

Problem/Motivation

Creating a BPMN.io model, I want to use the "Commerce: Order contains product variation types" or the "Commerce: Order contains product types" conditions.
Although I fill in the appropriate field ("Product variation types" or "Product types" respectively), I get an error message : condition "Commerce: Order contains product types" (Flow_1a93t7f): Product types field is required and the model cannot be saved.

Steps to reproduce

Using latest version of core and modules (Commerce core 2.38, Drupal core 10.1.8, ECA 1.1.4, BPMN.IO 1.1.3)

1/ Create a new BPMN.IO model
2/ Use the "Order Paid" start event
3/ Append a task ("Display a message to the user" for instance)
4/ Edit the sequence flow, and select the "Commerce: Order contains product types"
5/ Enter a value in the "Product types" field
6/ Click the save button

πŸ› Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

πŸ‡«πŸ‡·France quimic Paris

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @quimic
  • Status changed to Postponed: needs info 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Can you try providing the token name of the entity that you are working with?

    If that is it maybe we can work on the error messaging.

  • πŸ‡«πŸ‡·France quimic Paris

    @nicxvan Whatever value I enter in the entity field, I get the same error message when saving. No entry in the log.

  • πŸ‡«πŸ‡·France quimic Paris

    I decided to see if I would get a similar error using the Classic Modeler, and indeed I do, but I get a little more details on what might cause it.
    When I use the "Order Paid" event and try to add the "Commerce: Order contains product variation types (ECA Commerce)" condiftion, I get a white page with the "The website encountered an unexpected error. Please try again later." message.
    Looking in the log, I see "TypeError: Cannot access offset of type string on string in Drupal\eca_commerce\Plugin\ECA\Condition\Commerce->filterFormFields() (line 145 of [...]/modules/contrib/eca_commerce/src/Plugin/ECA/Condition/Commerce.php)."

    Using the "Commerce: Order contains product types (ECA Commerce)" brings the exact same error.

    Hope this helps.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    As soon as I find some time, I'm going to debug the form validation error.

    Using the classic modeller is a bad idea, since it doesn't validate the configuration at all. The other error you're seeing is something that is something different, but it could also be a follow-up on this one.

  • Status changed to Active 10 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Found some time to debug the issue, and it turns out that the "Type" field is not a plain textfield as it appears in the UI. The original condition plugin from commerce uses a field type called commerce_entity_select. That's of course not supported by the bpmn_io UI and we therefore replaced that with a textfield, see ✨ Add support for commerce conditions Fixed .

    That replacement now seems to cause the issue with the form validation, which still expects the complex field widget instead of the simple textfield.

    I'll have a look if I can make that replacement more solid.

  • Status changed to Needs review 10 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    It seems that the commerce conditions did never receive any configuration values from the bpmn UI so far, not only those commerce_entity_select were broken. Turns out, there have been 2 issues:

    The first issue was the handling of the commerce_entity_select fields. The commerce conditions left it to the bpmn_io modeller to replace that unsupported field type with the fallback of a text field. That works for the UI, but not for their validation. To resolve this, I've now replaced that complex field type with a text field in the already existing \Drupal\eca_commerce\Plugin\ECA\Condition\Commerce::filterFormFields method. That resolves the validation issue.

    However, the second problem is that commerce condition forms are somehow nested in a way, that I never come across before. That leads to a problem where their config submit handler reads values from the form state with $values = $form_state->getValue($form['#parents']);, but that returns NULL in our context. The submitted values are available, but not at the expected place, so I've fixed that by setting the values with $form_state->setValue($form['#parents'], $form_state->getValues()); before calling the submit (and also the validate) handler.

    It now seems to work. Not sure if this is just a terrible workaround or a valid fix!?!

  • πŸ‡«πŸ‡·France quimic Paris

    Thank you Jurgen. I would not be able to assess if this is the right fix.

    I can confirm that after applying the patch, the form validates and I can now save the model I described (display a message if a paid order contains a specific product variation type).

    But when I trigger the model (by paying an order), I get an error from the commerce module:
    TypeError: in_array(): Argument #2 ($haystack) must be of type array, string given in in_array() (line 36 of [...]/modules/contrib/commerce/modules/product/src/Plugin/Commerce/Condition/OrderVariationType.php)

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thanks @quimic for the quick feedback. I can see the reason for that: commerce_entity_select expects an array of values, not only a single text value. So, we may have to extend that field in a way that it allows multiple values (i.e. a single string with comma separated values) and then explode the given value into an array of strings. I'll see now, if I can make that happen.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    I've applied a method that has also been used for the eca_tamper module: we verify the data type of each configuration value and adjust that if necessary before calling the commerce condition plugin to evaluate the conditions. This should do the job. Please note, that the config field now also has a description that should explain that you can provide multiple values separated by commas.

  • πŸ‡«πŸ‡·France quimic Paris

    Thank you Jurgen.
    I now get an "opposite" error, from ECA Commerce. It complains about receiving an array instead of a string.
    TypeError: explode(): Argument #2 ($string) must be of type string, array given in explode() (line 71 of [...]/modules/contrib/eca_commerce/src/Plugin/ECA/Condition/Commerce.php)

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Hmm, I need to fly pretty blind here since I'm not a commerce user and I don't have any commerce installation where I could debug this. I've now added another check before converting a config value from string to array. Hope that catches it and doesn't introduce any unwanted side effects.

  • πŸ‡«πŸ‡·France quimic Paris

    Okay, so now there is no error any more.

    BUT, the condition of the sequence flow should have matched and it did not.

    More specifically:
    I payed an order that contains a specific product variation type
    I check for that variation type in my model, and display (or log) a message if it matches

    I did not get the message. "Negating" the condition in the sequence flow does give me the message.
    So it means that the values comparison fails.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Then you may have to go through debugging as described in https://ecaguide.org/eca/debugging/ which works completely in the UI, i.e. in your browser.

    Even better would be if you could use XDebug, to step through the code to see where it goes wrong. If that is an option, the ideal starting point would be \Drupal\eca_commerce\Plugin\ECA\Condition\Commerce::evaluate which is what's called when a condition gets evaluated. This then calls $commercePlugin->evaluate() from the respective commerce plugin where the real evaluation is happening.

    Hope that either of those 2 options provides us with more detail what's going on.

  • πŸ‡«πŸ‡·France quimic Paris

    It looks like the Commerce tokens give me inconsistent values, so I need to investigate further to understand what is happening here.

  • Status changed to Postponed: needs info 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Please let us know if you find where you're getting the inconsistencies.

  • Hi, jurgenhaas, the solution has not been released to production or any release, right?

    Regards,

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    @sdsc the fix exists in the MR/patch and will be merged, when it's been tested and confirmed that this really fixes the big.

  • I'll wait for your next version release -- sorry, I don't know how to apply MR/patch πŸ˜…, I am not technical and don't want to break code in case I did anything wrong.

    When would you expect next version update(with this fix merged)?

    Thank you again for making ECA available while Rules is really useless at this point...

  • Status changed to Needs review 9 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    @sdsc are you running a Drupal site without a single patch? I've never seen such a thing, TBH. Our Drupal sites have a few dozen if not a hundred patches applied.

    Assuming you're building your Drupal site with composer, it should be very easy. Detailed instructions can be found at https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa... β†’

    The link to the patch you want to apply is below the original post above, left to MR !8 mergeable behind the text "plain diff".

    When would you expect next version update(with this fix merged)?

    As soon, but not before, someone who hasn't developed the fix, will have tested it and can confirm, that it really works and doesn't break anything else.

  • jurgenhaas, oh, yeah, embarrassing, right? My D7 site is actually very complex, with shopping, membership, lots of content type (LOTs of content) and internal request/orders...yet no patches (sorry πŸ˜‚)

    Now I am in the process of building D10. Really headaches about: Rules, Ubercart to Commerce, and many good modules and features gone...Luckily ECA is coming to replace Rules, with many advantages, yet some room to fill up. The great simplicity and flexibility in Ubercart are much not available in Commerce, ECA is probably my last resort to get solution.

    I'll definitely try the patch method per your advice. But commerce and its related modules already broke part of code. Now I am starting a new site just to test all required commerce features.

    Thank yo again!

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    If you're building a commerce site you'll want to get familiar with patching.

    For commerce 2 many companion modules will require patches to fix minor things.

    As Jurgen said once someone confirms the patch fixes the issue we can merge it in, it won't likely get merged in otherwise because we cannot be sure it fixes the issue and or introduces other issues.

  • I am unable to apply the patch. I must have done something wrong:

    1. go to https://git.drupalcode.org/project/eca_commerce/-/merge_requests/8.diff and copied content to a txt file, name it as eca.patch
    2. created a directory "patches" at the same level as core
    3. moved the eca.patch to the folder
    4. add the bolded section to composer.json:
    "extra": {
    "patches":{
    "drupal/core":{
    "fix issues of check commence order as condition": "patches/eca.patch"
    }
    },
    "drupal-scaffold": {
    "locations": {
    "web-root": "./"
    }
    5. run "composer install --prefer-source". Error:

    - Applying patches for drupal/core
    patches/eca.patch (fix issues of check commence order as condition)
    Could not apply patch! Skipping. The error was: Cannot apply patch patches/eca.patch

    Double checked the folder name and file name. Please advice.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    You're really close i think you're missing two things:

    1. You're patching eca_commerce not core so the party that says drupal/core should be drupal/eca_commerce
    2 make sure you have required cweagans/composer-patches

  • Yes, I did.

    1. Before I updated the patch, I did "composer require cweagans/composer-patches:~1.0 --update-with-dependencies". It asked me whether to trust"...", I typed "y". Looks like this action was performed successfully.
    2. then "composer update drupal/eca_commerce", with the same error,
    3. then I tried "composer install --prefer-source"

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Yes, the part in composer json where you have drupal/core needs to be drupal/eca_commerce

    That cost key needs to be the project you are patching.

  • Hi, nicxvan,

    I changed to "drupal/eca_commerce" in composer.json file, but still with the same error:

    - Applying patches for drupal/eca_commerce
    patches/eca.patch (fix issues of check commence order as condition)
    Could not apply patch! Skipping. The error was: Cannot apply patch patches/eca.patch

    Is it possible the patch file I generated (copy from the site to txt file) is not good? I loaded it here,, just in case.

    Thank you for your patience!

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Yes, that patch is not correct, it should be equal to the content on this page: https://git.drupalcode.org/project/eca_commerce/-/merge_requests/8.diff

    If you notice in your patch there are a ton of extra / and a whole section at the top:

    {\rtf1\ansi\ansicpg1252\cocoartf2758
    \cocoatextscaling0\cocoaplatform0{\fonttbl\f0\fmodern\fcharset0 Courier;}
    {\colortbl;\red255\green255\blue255;}
    {\*\expandedcolortbl;;}
    \margl1440\margr1440\vieww11520\viewh8400\viewkind0
    \deftab720
    \pard\pardeftab720\partightenfactor0
    
    \f0\fs26 \cf0 \expnd0\expndtw0\kerning0
    

    I noticed two other things.

    One, you probably need ./patches as the relative path and two, you may need the dev version of eca_commerce since patches are generated against the dev version and not the current version.

    If this does not work please open a separate ticket asking for support to apply this patch. I'm willing to help there, but I don't want to clutter this issue up with support comments.

  • Thank you Nicvan! I was able to apply the patch and did some testing. Here is where I got:

    1. Order contains product types
    -->saved successfully
    2. Order contains product variation types
    -->saved successfully
    3. Order contains specific products
    -->put product id in field "Products": failed to save
    -->put product title in field "Products": saved successfully
    -->put a wrong product title in field "Products": failed to save

    May I suggest:
    a. use product id for field "Products" so editing product title will not break the code? It would be nice to add a description there.
    b. If entered data in products field doesn't exist, the code should still be saved and working, because it is possible that a product was modified or removed later. I think the code should be able to handle "not found" situation.

    4. Order contains specific commerce purchasable entities
    -->seems not developed yet.
    5. Order contains product categories
    -->not sure what it is, but errors pointing to term matching not found

    Let me know if you need more testing regarding this patch.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    This is great news, thanks @sdsc for testing all this.

    Note, the configuration forms are implemented in commerce. ECA Commerce is just pulling the plugin that exist in commerce and makes them available in ECA. So, the labels, descriptions and validations are out of scope for eca_commerce. Any changes would have to be made in the commerce module, that provides those plugins.

    3. Order contains specific products
    -->put product id in field "Products": failed to save
    -->put product title in field "Products": saved successfully
    -->put a wrong product title in field "Products": failed to save

    Can you provide the exact error messages for the 2 failing attempts, please?

  • the error for the two scenario is the same:

    (Flow_olkv32k): There are no products matching "3" -- 3 is what I put in the field. The code is looking for a product with title "3'.

    ECA Commerce is just pulling the plugin that exist in commerce and makes them available in ECA. So, the labels, descriptions and validations are out of scope for eca_commerce.

    --is it possible commerce has another filed for product ID that ECA Commerce can pull to check against? I don't know the coding, just from experience perspective, it is usually compare to machine name that is relatively fixed rather than an editable field.

    Thank you for taking effort to make your module better and better!

  • Assigned to jurgenhaas
  • Status changed to Needs work 8 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thank you @sdsc for that error message, that helped me to find out this:

    MR !8 fixes the issue with config field of type commerce_entity_select and with commerce plugins in general.

    The conditions "Order contains specific products" and "Specific products" use another config field type entity_autocomplete, which isn't available in BPMN. We need to see, if we find a way to support that config field type as well.

    Would be great if you could test this further, when I'm ready with new code, since I'm not a commerce user, i.e. I don't have real sites where I could run those conditions. My coding is just based on theory, so if you can test something every now and then, that would be amazing. I can't make any promises by when I could look into this in more detail, though.

  • Glad to help!

    entity_autocomplete should refer to the title. It's surprising that they don't have a field for product_id for eca to check against. But if it is what you can get, would be nice to put a description there, because the normal thought would be product_id.

  • Issue was unassigned.
  • Status changed to Needs review 8 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    The fix is probably as simple as treating the entity_autocomplete similar to the commerce_entity_select, i.e. that would allow comma separated list of IDs.

    I've added that to the existing MR, please give that another try.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    @sdsc any chance for a review?

  • @jurgenhaas, didn't know you were waiting for my response.

    one or more commerce related module broke my code, and couldn't fix. So I had to rebuild my site and import all data.

    I am now very cautious when it comes to commerce. ECA has helped me A LOT to make up some features missed from Commerce. I haven't got a chance to re-apply the patch yet. I'll let you know as soon as possible.

    Thank you again for making ECA available!

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    This is now also merged into the 2.0.x release. For 1.0.x this is still living in the MR waiting for the review.

  • Today I had a chance to test 1.0.x dev in my testing environment. I successfully applied the patch, but for "containing specific product" condition, It appears the same as before, not presented as "entity_autocomplete".

    I am unable to update to 2.0.x alpha release somehow. My testing environment may have something broken from other commerce related modules. The condition of "containing specific product" works as long as the product is a full name of product title. This should be fine.

    Thank you very much for working hard to make ECA better and better!

  • Pipeline finished with Skipped
    7 months ago
    #181138
  • Status changed to Fixed 7 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    I've now merged this to make it part of the 2.0 beta1 release. Also marking the issue as fixed. If any more findings need to be reported, please open new issues for those.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024