Skip to content

fix(rules): allow Smart Rules to set Transaction Type as Expense#515

Open
saadaltabari wants to merge 1 commit into
sarim2000:mainfrom
saadaltabari:fix/smart-rules-transaction-type-expense
Open

fix(rules): allow Smart Rules to set Transaction Type as Expense#515
saadaltabari wants to merge 1 commit into
sarim2000:mainfrom
saadaltabari:fix/smart-rules-transaction-type-expense

Conversation

@saadaltabari

Copy link
Copy Markdown
Contributor

Summary

Smart Rules could not reliably set or match Transaction Type → Expense. The rule editor showed “Outgoing” instead of “Expense”, kept incompatible operators when switching from Amount to Transaction Type (for example LESS_THAN), and duplicated fragile TYPE pre-filter logic across the repository and engine.

This change centralizes transaction-type rule helpers, uses proper is / is not operators for TYPE conditions, resets operators when the condition field changes, validates TYPE values, and adds unit tests for EXPENSE matching and SET TYPE actions.

Type of change

  • fix: Bug fix
  • test: Add/update tests

How to test

  1. Open Settings → Smart Rules → Create Rule.
  2. Under Then, choose Set Field → Set Transaction Type and select Expense; save the rule.
  3. Add a condition Transaction Type → is → Expense (optionally combined with another condition); confirm Save is enabled and the preview shows “Expense”.
  4. Enable the rule and verify it applies on matching transactions.
  5. Run:
    ./gradlew :app:testStandardDebugUnitTest --tests "com.pennywiseai.tracker.domain.service.RuleEngineTest" --tests "com.pennywiseai.tracker.domain.model.rule.RuleConditionTest"

Breaking changes

No breaking changes.

Checklist

  • Focused PR (single purpose)
  • Tests added/updated
  • Targeted unit tests pass locally
  • Follows existing code style and patterns

Made with Cursor

…pense

Use proper TYPE operators and labels in the rule editor so Expense conditions and actions work reliably instead of failing with stale or invalid operators.

Co-authored-by: Cursor <cursoragent@cursor.com>
@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Smart Rules being unable to set or match Transaction Type → Expense by centralising TYPE helpers into RuleCondition.kt, adopting is/is not operators for TYPE conditions, resetting operators on field change, and using EXPENSE (canonical enum name) instead of the misleading \"Outgoing\" label.

  • Core logic (RuleCondition.kt, RuleEngine.kt, RuleRepositoryImpl.kt): duplicate TYPE pre-filter blocks removed; parseRuleTransactionType and matchesTransactionTypeCondition now serve as the single source of truth, and validate() gains a TYPE branch.
  • UI (CreateRuleScreen.kt): operator chips and labels are now driven by conditionOperatorsForField/RULE_TRANSACTION_TYPE_OPTIONS; field selection resets to the default operator via defaultConditionOperator(); however, the type-value chip onClick always forces operator = EQUALS, which prevents "Transaction Type is not Expense" conditions from being saved through the UI.
  • Tests (RuleConditionTest, RuleEngineTest): new cases cover EXPENSE validation, invalid-value rejection, isRuleApplicableToTransactionType filtering, SET TYPE action, and evaluateRulesForType matching.

Confidence Score: 4/5

The engine and repository changes are safe; the one active defect is that "Transaction Type is not X" conditions cannot be created through the UI because the type-value chip click overwrites the operator.

The centralisation of TYPE helpers and the EXPENSE label fix work correctly end-to-end. The type-value chip onClick in ConditionFieldSelector always writes operator = ConditionOperator.EQUALS, making the "is not" operator chip decorative — any rule saved this way will silently evaluate as "is" rather than "is not". The fix is a one-line removal of the operator assignment.

app/src/main/java/com/pennywiseai/tracker/ui/screens/rules/CreateRuleScreen.kt — specifically the type-value chip onClick in ConditionFieldSelector (around line 874).

Important Files Changed

Filename Overview
app/src/main/java/com/pennywiseai/tracker/domain/model/rule/RuleCondition.kt New file-level helpers centralise TYPE pre-filter logic, operator defaults, and label lookups; the validate() TYPE guard is correct.
app/src/main/java/com/pennywiseai/tracker/domain/service/RuleEngine.kt Duplicate TYPE pre-filter removed; parseRuleTransactionType replaces the bare try/catch; logic is unchanged and correct.
app/src/main/java/com/pennywiseai/tracker/data/repository/RuleRepositoryImpl.kt Duplicate filter logic removed; getActiveRulesByTypes now returns a rule if it matches ANY of the requested types, which is a semantic correction for NOT_EQUALS rules that the old code incorrectly excluded.
app/src/main/java/com/pennywiseai/tracker/ui/screens/rules/CreateRuleScreen.kt Labels, preset value, and operator lists correctly updated; however, the type-chip onClick always resets operator to EQUALS, preventing "is not" TYPE conditions from being saved.
app/src/test/java/com/pennywiseai/tracker/domain/model/rule/RuleConditionTest.kt New tests cover EXPENSE validation, invalid-value rejection, and isRuleApplicableToTransactionType for both matching and non-matching types.
app/src/test/java/com/pennywiseai/tracker/domain/service/RuleEngineTest.kt Two new tests verify SET TYPE to EXPENSE and evaluateRulesForType pre-filter matching; both are well-structured.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User opens Create Rule] --> B{Condition field changed?}
    B -->|Yes| C[condition.copy with defaultConditionOperator]
    B -->|No| D{field == TYPE &&\noperator invalid?}
    D -->|Yes| E[LaunchedEffect resets\noperator to EQUALS]
    D -->|No| F[Operator unchanged]
    C --> G[Show operator chips\nfrom conditionOperatorsForField]
    E --> G
    F --> G
    G --> H{User clicks operator chip\neg NOT_EQUALS}
    H --> I[condition.copy operator=NOT_EQUALS]
    I --> J[Show TYPE value chips\nIncome / Expense / Credit ...]
    J --> K{User clicks type value chip}
    K -->|Current code| L["condition.copy(value=type,\noperator=EQUALS) BUG:\nNOT_EQUALS overwritten"]
    K -->|Fixed code| M["condition.copy(value=type)\noperator preserved"]
    L --> N[Rule saves with IS not IS NOT]
    M --> O[Rule saves with IS NOT correctly]
    O --> P[isRuleApplicableToTransactionType]
    P --> Q[RuleEngine.evaluateRulesForType]
    Q --> R[parseRuleTransactionType SET TYPE action]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User opens Create Rule] --> B{Condition field changed?}
    B -->|Yes| C[condition.copy with defaultConditionOperator]
    B -->|No| D{field == TYPE &&\noperator invalid?}
    D -->|Yes| E[LaunchedEffect resets\noperator to EQUALS]
    D -->|No| F[Operator unchanged]
    C --> G[Show operator chips\nfrom conditionOperatorsForField]
    E --> G
    F --> G
    G --> H{User clicks operator chip\neg NOT_EQUALS}
    H --> I[condition.copy operator=NOT_EQUALS]
    I --> J[Show TYPE value chips\nIncome / Expense / Credit ...]
    J --> K{User clicks type value chip}
    K -->|Current code| L["condition.copy(value=type,\noperator=EQUALS) BUG:\nNOT_EQUALS overwritten"]
    K -->|Fixed code| M["condition.copy(value=type)\noperator preserved"]
    L --> N[Rule saves with IS not IS NOT]
    M --> O[Rule saves with IS NOT correctly]
    O --> P[isRuleApplicableToTransactionType]
    P --> Q[RuleEngine.evaluateRulesForType]
    Q --> R[parseRuleTransactionType SET TYPE action]
Loading

Comments Outside Diff (1)

  1. app/src/main/java/com/pennywiseai/tracker/ui/screens/rules/CreateRuleScreen.kt, line 870-880 (link)

    P1 Type chip click clobbers the user-selected operator

    When a user selects "is not" in the operator chips and then taps a type value chip, the onClick handler hardcodes operator = ConditionOperator.EQUALS, silently overwriting the chosen operator. The resulting condition is always "is", making "Transaction Type is not Expense" (or any NOT_EQUALS TYPE condition) impossible to save through the UI despite the operator chip being present.

Reviews (1): Last reviewed commit: "fix(rules): allow Smart Rules to set and..." | Re-trigger Greptile

Comment on lines +783 to +787
LaunchedEffect(condition.field, condition.operator) {
if (condition.field == TransactionField.TYPE && condition.operator !in supportedOperators) {
onConditionChange(condition.copy(operator = ConditionOperator.EQUALS))
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LaunchedEffect operator-reset scope limited to TYPE field

The LaunchedEffect only resets the operator when condition.field == TransactionField.TYPE. If an existing rule was persisted with a different incompatible (field, operator) pair — for example ACCOUNT with CONTAINS — loading and editing it in the UI will silently render an operator chip that isn't in the allowed list for that field, with no automatic correction. Since field-selection in the dropdown already calls defaultConditionOperator(), dropping the TYPE-only check and applying broadly (reset whenever condition.operator !in supportedOperators) would make this guard generally safe.

@sarim2000

Copy link
Copy Markdown
Owner

Nice fix, and the getActiveRulesByTypes ANY-match correction is right. One active defect to fix first: in CreateRuleScreen, the transaction-type value chip's onClick always forces operator = EQUALS, so a "Transaction Type is not Expense" condition can't be saved through the UI (even though the engine/repo now handle NOT_EQUALS correctly). Please preserve the selected operator there.

@sarim2000

Copy link
Copy Markdown
Owner

Thanks @saadaltabari — the EXPENSE-type fix and helper centralisation look good (Greptile 4/5). One real thing it caught: in ConditionFieldSelector (CreateRuleScreen, ~line 874) the type-value chip onClick always sets operator = EQUALS, so an "is not" condition silently saves as "is". Looks like a one-line removal.

It's fallen behind main and now conflicts — a rebase when you have a moment would clear that and re-trigger Greptile.

Gentle housekeeping heads-up: if there's no update in ~a week we might close it just to keep things tidy — no rejection implied, happy to reopen the moment you're back on it. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants