venmo/business-rules

Use both ANY and ALL conditions together in one rule

Opened this issue · 0 comments

Based on the rules example in ReadMe, we are able to have both ANY and ALL conditions in one rule and pass it to Venmo/business-rules.

# current_inventory < 5 OR (current_month = "December" AND current_inventory < 20)
{ "conditions": { "any": [
      { "name": "current_inventory",
        "operator": "less_than",
        "value": 5,
      },
    ]},
      { "all": [
        {  "name": "current_month",
          "operator": "equal_to",
          "value": "December",
        },
        { "name": "current_inventory",
          "operator": "less_than",
          "value": 20,
        }
      ]},
  },
  "actions": [
    { "name": "order_more",
      "params":{"number_to_order": 40},
    },
  ],
}]

However, in the actual code, it doesn't look like working as it described in the example. "==" seems to limit that only when one kind of condition is in the condition list, it will work. For mixed situation it doesn't seem work.

Current Code in business_rules/engine.py :

def check_conditions_recursively(conditions, defined_variables):
    keys = list(conditions.keys())
    if keys == ['all']:
        assert len(conditions['all']) >= 1
        for condition in conditions['all']:
            if not check_conditions_recursively(condition, defined_variables):
                return False
        return True

    elif keys == ['any']:
        assert len(conditions['any']) >= 1
        for condition in conditions['any']:
            if check_conditions_recursively(condition, defined_variables):
                return True
        return False

    else:
        # help prevent errors - any and all can only be in the condition dict
        # if they're the only item
        assert not ('any' in keys or 'all' in keys)
        return check_condition(conditions, defined_variables)

Here, me and my team want to have a mixed rule with both ANY and ALL conditions, and also wish it won't make any final decision when just validating one of it. So first to check if ANY or ALL exists, then start checking one of it, if doesn't apply, then go to validate the other one if exists.

Proposal code (mainly update "keys == ..." to "... in keys" and update the return condition):

def check_conditions_recursively(conditions, defined_variables):
    keys = list(conditions.keys())
    if 'any' in keys:
        assert len(conditions['any']) >= 1
        for condition in conditions['any']:
            if check_conditions_recursively(condition, defined_variables):
                return True
        if 'all' not in keys:
            return False

    elif 'all' in keys:
        assert len(conditions['all']) >= 1
        for condition in conditions['all']:
            if not check_conditions_recursively(condition, defined_variables):
                return False
        return True

    else:
        # help prevent errors - any and all can only be in the condition dict
        # if they're the only item
        assert not ('any' in keys or 'all' in keys)
        return check_condition(conditions, defined_variables)

Please feel free to leave any comments to help me have a better understanding of this part of code. Thank you!