This serves as a tracking and patch testing issue for porting D7 Ubercart Rules Conditions to D8. I am also using this to document the process, because currently Rules has very little documentation for D8 and no information about porting between the two very different major versions.

These are the steps in the process:

Tasks

  1. Make a list of conditions that Ubercart defines. Publish that here!
  2. Ubercart conditions are defined by hook_rules_condition_info(), which may be found in <modulename>/<modulename>.rules.inc. Refer to the D7 version of these files to ensure that you port ALL the conditions that were in D7 Ubercart.
  3. Write a <modulename>/src/Plugin/Condition/<conditionname>.php class for each condition that the module defines. Use the classes provided by the Rules module in rules/src/Plugin/Condition/ as examples.
  4. Manually test, in the Rules UI, that each condition shows up and has the same arguments as it did in D7.
  5. Manually review the conditions plugins to make sure all comments and text strings are correct for each condition.
  6. Manually test, by creating test reaction rules in the Rules UI, that each condition works as expected.
  7. Write unit test cases for each condition. Use the code in rules/tests/src/Unit/Integration/Condition/* as examples.

Remaining tasks

Tasks 1-4 have been performed. Task 3 REQUIRES fixes to core Rules - there is a patch in #2664280-24: Select lists in action & condition configuration forms that works for this. Help with tasks 5-7 is still needed.

  1. Manually review the conditions plugins to make sure all comments and text strings are correct for each condition.
  2. Manually test, by creating test reaction rules in the Rules UI, that each condition works as expected.
  3. Write unit test cases for each condition. Use the code in rules/tests/src/Unit/Integration/Condition/* as examples.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR created an issue. See original summary.

TR’s picture

Here is a list of Rules Conditions provided by Ubercart in Drupal 7:

Node

  • Content is a product (node_is_product)

Order

  • Check an order's state (uc_order_condition_order_state)
  • Check an order's shipping country (uc_order_condition_delivery_country)
  • Check an order's billing country (uc_order_condition_billing_country)
  • Check if an order can be shipped (uc_order_condition_is_shippable)
  • Check an order's total (uc_order_condition_total)
  • Check an order's subtotal (uc_order_condition_subtotal)

Order: Product

  • Order has a product with a particular attribute option (uc_attribute_ordered_product_option)
  • Check an order's products (uc_order_condition_has_products)
  • Check an order's number of products (uc_order_condition_count_products)
  • Check an order's total weight (uc_order_condition_products_weight)
  • Check an order's product classes (uc_order_condition_has_product_class)

Payment

  • Check the order balance (uc_payment_condition_order_balance)

Order: Shipping Quote

  • Order has a shipping quote from a particular method (uc_quote_condition_order_shipping_method)
TR’s picture

Issue summary: View changes
TR’s picture

TR’s picture

Here is my first pass at porting all the Rules conditions. I think they work, but it's going to take a second set of eyes to be sure since we don't have any tests yet. I just counted and apparently I didn't port one of the conditions yet (uc_quote_condition_order_shipping_method). I will add that to the patch later.

Remember, to try this you will need to enable the latest -dev of Rules and also apply the patch from #2664280-24: Select lists in action & condition configuration forms.

I expect one failure from the testbot, since we still have that core 8.5.x regression. Should pass on 8.4.x.

Status: Needs review » Needs work

The last submitted patch, 5: 2924151-rules-conditions.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

TR’s picture

Fixed the coding standards problems and added the missing condition that I mentioned in #5.

Status: Needs review » Needs work

The last submitted patch, 7: 2924151-7-rules-conditions.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

TR’s picture

Hmm, looks like random test failures in the 8.5.x branch. 8.4.x passed. Let's correct some coding standards and run it again ... Want to get it down to just the 1 uc_tax failure in 8.5.x.

Status: Needs review » Needs work

The last submitted patch, 9: 2924151-9-rules-conditions.patch, failed testing. View results

TR’s picture

Status: Needs review » Needs work

The last submitted patch, 11: 2924151-9-rules-conditions.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

TR’s picture

Status: Needs work » Needs review

1 fail on 8.5.x, 0 fails on 8.4.x, as expected.

TR’s picture

TR’s picture

Issue summary: View changes

Committed #14.

Remember, these still require the patch to Rules from #2664280-24: Select lists in action & condition configuration forms.

The following conditions still cause problems with task #6:

  • Content is a product (node_is_product)
  • Check an order's number of products (uc_order_condition_count_products)
  • Check an order's total weight (uc_order_condition_products_weight)
  • Check an order's product classes (uc_order_condition_has_product_class)

  • TR committed 15f65ca on 8.x-4.x
    Issue #2924151 by TR: Port Rules Conditions to D8
    
TR’s picture

Issue summary: View changes
TR’s picture

Adding the missing uc_payment_condition_order_balance condition and cleaning up some others.

TR’s picture

And one more small change to node_is_product.

  • TR committed 0ec6263 on 8.x-4.x
    Issue #2924151 by TR: Port Rules Conditions to D8. Added the missing...

  • TR committed 9532923 on 8.x-4.x
    Issue #2924151 by TR: $this->compareComparisonOptions() in uc_order...
TR’s picture

TR’s picture

I think this should take care of the remaining problems mentioned in #15.

  • TR committed fbafcf6 on 8.x-4.x
    Issue #2924151 by TR: Port Rules Conditions to D8 - fix problems...
TR’s picture

Committed.

TR’s picture

TR’s picture

Issue summary: View changes