Fix amount filter to include both incoming and outgoing amounts

[mirdaki]

Hi folks, this address #1935, which had some confusing UX. I also added some simple tests to verify the behavior.

I want to acknowledge that changing it in the transaction rules may not be the best spot, I don't have a ton of context on this repo yet. I also considered updating the filter to allow using oneOf behind the scenes for the number type, but that seemed to have more knock on effects that weren't as obvious.

Happy to update the logic if there is a better approach. Thank you!

[netlify[bot]]

✅ Deploy Preview for actualbudget ready!

Name	Link
🔨 Latest commit	0be747785ee818cad90300164ddd75f2cbef5fb7
🔍 Latest deploy log	https://app.netlify.com/sites/actualbudget/deploys/6642d2323e07060008288a30
😎 Deploy Preview	https://deploy-preview-2643.demo.actualbudget.org
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[github-actions[bot]]

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
9	4.72 MB	0%

Changeset

No files were changed

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset	File Size	% Changed
static/js/indexeddb-main-thread-worker-e59fee74.js	13.5 kB	0%
static/js/resize-observer.js	18.37 kB	0%
static/js/BackgroundImage.js	122.29 kB	0%
static/js/usePreviewTransactions.js	790 B	0%
static/js/AppliedFilters.js	20.54 kB	0%
static/js/narrow.js	59.81 kB	0%
static/js/wide.js	262.4 kB	0%
static/js/ReportRouter.js	1.23 MB	0%
static/js/index.js	3 MB	0%

[github-actions[bot]]

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
1	1.2 MB → 1.2 MB (+17 B)	+0.00%

Changeset

File	Δ	Size
packages/loot-core/src/server/accounts/transaction-rules.ts	📉 -81 B (-0.29%)	27.29 kB → 27.21 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
kcab.worker.js	1.2 MB → 1.2 MB (+17 B)	+0.00%

Smaller

No assets were smaller

Unchanged

No assets were unchanged

[mirdaki]

Apologies, I'll need some time to address the E2E test failing. I would appreciate some comments on the approach if there are issues with this or concerns it might have negative effects on others

[carkom]

I think as long as it only affects "amount" and the "inflow/outflow" are unchanged then you are fine. Also, it would need to be applied to all ops and not just "eq".

[youngcw]

I think that it probably would be good to maybe prevent negatives when looking at "amount" since the sign is ignored.

Also, you cant save the different amount types as unique filters. They all show up as the same between "amount", inflow, or outflow.

[mirdaki]

Thanks for the comments folks! I've updated the logic to apply to all operations. Note: This change does mean filtering with a negative number will show no results. If that's undesirable, I can add some logic to change that

I still haven't had time to figure out the E2E tests yet, so I'll move this to draft for the time being

[mirdaki]

Pulling in changes seemed to resolve the E2E test problem! So ready for review!

[psybers]

@youngcw It looks like this merge might have broken the filters:

(the video is on another PR, but I get the same behavior on edge)

[psybers]

Ok I see the behavior was intentionally changed. But this is super confusing for users now. Many will probably do as I did, select "Amount" and see there is a less than and try to use it.

[psybers]

Also the terminology "inflow" and "outflow" does not match the use of "payment" and "deposit" elsewhere.

[mirdaki]

I think it's a fair point that this may be a breaking change for existing users workflows. From the perspective of a new user, the old behavior was confusing and not intuitive. Is there a way we can clearly signal to users that this behavior has changed other than the release notes?

[carkom]

Ok I see the behavior was intentionally changed. But this is super confusing for users now. Many will probably do as I did, select "Amount" and see there is a less than and try to use it.

I'm not sure it's all that confusing for users this way. There were complaints that it was more confusing the other way because there's no negatives in the app then we have to assume that payments should be treated as negative when using filters.

It's a bit confusing for existing users because it's opposite of what it used to be but from the perspective of a new user it's much more intuitive this way.

Less than can still be used. It doesn't have to mean negative. If you want transactions less than 5 you'd use it.

[carkom]

Also the terminology "inflow" and "outflow" does not match the use of "payment" and "deposit" elsewhere.

I agree here, I'd be happy to see this changed.

[psybers]

Correct me if I am wrong, but with the new change you can never match a transaction if you set the filter "Amount" to be "less than 0"? Maybe a simple warning message there would help educate users about the change?

[youngcw]

I reverted this change for the time being since it affected schedules working. If we are going to go with this path for filters, then there will be some fixes in how rules and schedules use the filters. Another option would be look into changing how the filter tooltip presents the value instead of changing how the filter works. That would also avoid the need to have a db migration to fix all the schedules that break with this change