Add support for commerce conditions

Created on 2 July 2022, over 2 years ago
Updated 27 July 2023, over 1 year ago

Problem/Motivation

When adding a ECA condition with commerce_condition element type in the build form, it does not displays the table conditions instead we get an empty field.

$form['commerce_conditions'] = [
        '#type' => 'commerce_conditions',
        '#title' => $this->t('Applies to'),
        '#parent_entity_type' => 'eca_condition',
        '#entity_types' => ['commerce_order'],
        '#default_value' => $this->configuration['get_conditions'],
      ];

What I see:

I should get something similar to this:

Steps to reproduce

Install commerce demo

composer create-project -s dev centarro/commerce-kickstart-project commerce_kickstart
composer require drupal/commerce_demo:^3.0
composer require drupal/bpmn_io
composer require drupal/eca_state_machine

Add and install attached sandbox eca_commerce β†’ module.

Add ECA Commerce Condition on a State Machine Event.

✨ Feature request
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡¬πŸ‡§United Kingdom lexsoft London

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

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

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

    Moving this to the new project which integrates ECA with Commerce.

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

    Tagging for Drupal Dev Days next week.

  • Assigned to czigor
  • @czigor opened merge request.
  • Status changed to Needs review over 1 year ago
  • πŸ‡­πŸ‡ΊHungary czigor

    Conditions where the config form features a commerce_entity_select form element (e.g.

    Drupal\commerce_product\Plugin\Commerce\Condition\OrderProductType
    

    , not sure if there's more) don't yet work with bpmn. Once this gets merged we need a followup issue for these.

  • Status changed to Needs work over 1 year ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    This is great work, thanks a lot @czigor for this contribution.

    I've reviewed the code and left some suggestions; only minor stuff. But I haven't tested it.

  • Status changed to Needs review over 1 year ago
  • πŸ‡­πŸ‡ΊHungary czigor

    Thanks for the review, fixed everything.

    Little new to the merge request workflow... I tried to squash the commits, but they keep getting separated. Is this going to be fixed on merge?

  • Status changed to Needs work over 1 year ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Nice, thanks. Only one minor comment left. I wonder if anyone could also test this in a commerce installation? I don't have one around right now, but that would be important before we merge this.

    @czigor no worries about the commits. While working on an MR it's actually quite handy to have access to each individual commit to follow the taken steps. When the MR gets merged, then we can optionally sqaush them.

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

    I'm planning on testing it today, thanks for the code review @jurgenhaas

  • Status changed to Fixed over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024