Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix rules for isbetween that was not validated. #47

Merged
merged 4 commits into from
Jul 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion actual/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ def get_ruleset(s: Session) -> RuleSet:
for rule in get_rules(s):
conditions = TypeAdapter(list[Condition]).validate_json(rule.conditions)
actions = TypeAdapter(list[Action]).validate_json(rule.actions)
rs = Rule(conditions=conditions, operation=rule.conditions_op, actions=actions, stage=rule.stage)
rs = Rule(conditions=conditions, operation=rule.conditions_op, actions=actions, stage=rule.stage) # noqa
rule_set.append(rs)
return RuleSet(rules=rule_set)

Expand Down
50 changes: 40 additions & 10 deletions actual/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,26 @@ class ActionType(enum.Enum):
LINK_SCHEDULE = "link-schedule"


class BetweenValue(pydantic.BaseModel):
"""Used for `isbetween` rules."""

num_1: typing.Union[int, float] = pydantic.Field(alias="num1")
num_2: typing.Union[int, float] = pydantic.Field(alias="num2")

def __str__(self):
return f"({self.num_1}, {self.num_2})"

@pydantic.model_validator(mode="after")
def convert_value(self):
if isinstance(self.num_1, float):
self.num_1 = int(self.num_1 * 100)
if isinstance(self.num_2, float):
self.num_2 = int(self.num_2 * 100)
# sort the values
self.num_1, self.num_2 = sorted((self.num_1, self.num_2))
return self


class ValueType(enum.Enum):
DATE = "date"
ID = "id"
Expand All @@ -56,9 +76,9 @@ def is_valid(self, operation: ConditionType) -> bool:
# must be BOOLEAN
return operation.value in ("is",)

def validate(self, value: typing.Union[int, list[str], str, None], as_list: bool = False) -> bool:
if isinstance(value, list) and as_list:
return all(self.validate(v) for v in value)
def validate(self, value: typing.Union[int, list[str], str, None], operation: ConditionType = None) -> bool:
if isinstance(value, list) and operation in (ConditionType.ONE_OF, ConditionType.NOT_ONE_OF):
return all(self.validate(v, None) for v in value)
if value is None:
return True
if self == ValueType.ID:
Expand All @@ -73,7 +93,10 @@ def validate(self, value: typing.Union[int, list[str], str, None], as_list: bool
res = False
return res
elif self == ValueType.NUMBER:
return isinstance(value, int)
if operation == ConditionType.IS_BETWEEN:
return isinstance(value, BetweenValue)
else:
return isinstance(value, int)
else:
# must be BOOLEAN
return isinstance(value, bool)
Expand Down Expand Up @@ -113,7 +136,7 @@ def get_value(
def condition_evaluation(
op: ConditionType,
true_value: typing.Union[int, list[str], str, datetime.date, None],
self_value: typing.Union[int, list[str], str, datetime.date, None],
self_value: typing.Union[int, list[str], str, datetime.date, BetweenValue, None],
options: dict = None,
) -> bool:
"""Helper function to evaluate the condition based on the true_value, value found on the transaction, and the
Expand Down Expand Up @@ -147,10 +170,14 @@ def condition_evaluation(
# https://github.com/actualbudget/actual/blob/243703b2f70532ec1acbd3088dda879b5d07a5b3/packages/loot-core/src/shared/rules.ts#L261-L263
interval = round(abs(self_value) * 0.075, 2)
return self_value - interval <= true_value <= self_value + interval
elif op in (ConditionType.ONE_OF, ConditionType.CONTAINS):
elif op == ConditionType.ONE_OF:
return true_value in self_value
elif op in (ConditionType.NOT_ONE_OF, ConditionType.DOES_NOT_CONTAIN):
elif op == ConditionType.CONTAINS:
return self_value in true_value
elif op == ConditionType.NOT_ONE_OF:
return true_value not in self_value
elif op == ConditionType.DOES_NOT_CONTAIN:
return self_value not in true_value
elif op == ConditionType.GT:
return true_value > self_value
elif op == ConditionType.GTE:
Expand All @@ -159,6 +186,8 @@ def condition_evaluation(
return self_value > true_value
elif op == ConditionType.LTE:
return self_value >= true_value
elif op == ConditionType.IS_BETWEEN:
return self_value.num_1 <= true_value <= self_value.num_2
else:
raise ActualError(f"Operation {op} not supported")

Expand Down Expand Up @@ -198,7 +227,9 @@ class Condition(pydantic.BaseModel):
"amount_outflow",
]
op: ConditionType
value: typing.Union[int, float, str, list[str], Schedule, list[BaseModel], BaseModel, datetime.date, None]
value: typing.Union[
int, float, str, list[str], Schedule, list[BaseModel], BetweenValue, BaseModel, datetime.date, None
]
type: typing.Optional[ValueType] = None
options: typing.Optional[dict] = None

Expand Down Expand Up @@ -240,8 +271,7 @@ def check_operation_type(self):
elif isinstance(self.value, list) and len(self.value) and isinstance(self.value[0], pydantic.BaseModel):
self.value = [v.id if hasattr(v, "id") else v for v in self.value]
# make sure the data matches the value type
as_list = self.op in (ConditionType.IS_BETWEEN, ConditionType.ONE_OF, ConditionType.NOT_ONE_OF)
if not self.type.validate(self.value, as_list=as_list):
if not self.type.validate(self.value, self.op):
raise ValueError(f"Value {self.value} is not valid for type {self.type.name} and operation {self.op.name}")
return self

Expand Down
20 changes: 19 additions & 1 deletion tests/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ def test_datetime_rule():
assert Condition(field="date", op="lt", value=target_date + datetime.timedelta(days=1)).run(t) is True


def test_string_condition():
mock = MagicMock()
acct = create_account(mock, "Bank")
t = create_transaction(mock, datetime.date(2024, 1, 1), acct, "", "foo")
assert Condition(field="notes", op="oneOf", value=["foo", "bar"]).run(t) is True
assert Condition(field="notes", op="notOneOf", value=["foo", "bar"]).run(t) is False
assert Condition(field="notes", op="contains", value="fo").run(t) is True
assert Condition(field="notes", op="contains", value="foobar").run(t) is False
assert Condition(field="notes", op="doesNotContain", value="foo").run(t) is False
assert Condition(field="notes", op="doesNotContain", value="foobar").run(t) is True


def test_numeric_condition():
t = create_transaction(MagicMock(), datetime.date(2024, 1, 1), "Bank", "", amount=5)
c1 = Condition(field="amount_inflow", op="gt", value=10.0)
Expand All @@ -86,6 +98,10 @@ def test_numeric_condition():
assert c2.run(t) is True
c3 = Condition(field="amount", op="isapprox", value=5.5)
assert c3.run(t) is False
# isbetween condition
c4 = Condition(field="amount", op="isbetween", value={"num1": 5.0, "num2": 10.0})
assert c4.run(t) is True
assert str(c4) == "'amount' isbetween (500, 1000)" # value gets converted when input as float


def test_complex_rule():
Expand Down Expand Up @@ -133,6 +149,8 @@ def test_invalid_inputs():
Action(field="date", value="foo")
with pytest.raises(ValueError):
Condition(field="description", op="is", value="foo") # not an uuid
with pytest.raises(ValueError):
Condition(field="amount", op="isbetween", value=5)
with pytest.raises(ActualError):
Action(field="notes", op="set-split-amount", value="foo").run(None) # noqa: use None instead of transaction
with pytest.raises(ActualError):
Expand Down Expand Up @@ -166,7 +184,7 @@ def test_value_type_value_validation():
assert ValueType.BOOLEAN.validate("") is False
# list and NoneType
assert ValueType.DATE.validate(None)
assert ValueType.DATE.validate(["2024-10-04"], as_list=True) is True
assert ValueType.DATE.validate(["2024-10-04"], ConditionType.ONE_OF) is True


def test_value_type_from_field():
Expand Down
Loading