Custom Reports: Include current

[carkom]

Testing an option to add/remove current interval to "live" ranges.

WARNING: Do not use live budget files with this PR as it contains a migration. Please use a copy or a test budget ONLY.

I still need to add a migration so that it can be saved as part of the report. Holding off on that right now as I don't want it to mess up anyone's budget file.

[netlify]

✅ Deploy Preview for actualbudget ready!

Name	Link
🔨 Latest commit	efb21b8
🔍 Latest deploy log	https://app.netlify.com/sites/actualbudget/deploys/664e09469cf0f80008f454f7
😎 Deploy Preview	https://deploy-preview-2684.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]

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.71 MB → 4.72 MB (+2.61 kB)	+0.05%

Changeset

File	Δ	Size
src/components/reports/disabledList.ts	📈 +731 B (+21.28%)	3.35 kB → 4.07 kB
src/components/reports/getLiveRange.ts	📈 +94 B (+9.54%)	985 B → 1.05 kB
src/components/reports/ReportSidebar.tsx	📈 +1.27 kB (+8.06%)	15.71 kB → 16.97 kB
home/runner/work/actual/actual/packages/loot-core/src/client/data-hooks/reports.ts	📈 +57 B (+4.82%)	1.16 kB → 1.21 kB
src/components/reports/reports/CustomReport.tsx	📈 +298 B (+1.49%)	19.49 kB → 19.79 kB
src/components/reports/ReportOptions.ts	📈 +83 B (+1.26%)	6.41 kB → 6.49 kB
home/runner/work/actual/actual/packages/loot-core/src/shared/months.ts	📈 +85 B (+1.10%)	7.53 kB → 7.61 kB
src/components/reports/reports/GetCardData.tsx	📈 +31 B (+0.72%)	4.2 kB → 4.23 kB
src/components/reports/reportRanges.ts	📉 -1 B (-0.03%)	2.8 kB → 2.8 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
static/js/ReportRouter.js	1.23 MB → 1.23 MB (+2.47 kB)	+0.20%
static/js/index.js	3 MB → 3 MB (+142 B)	+0.00%

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.41 kB	0%
static/js/narrow.js	59.97 kB	0%
static/js/wide.js	262.29 kB	0%

[github-actions]

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 (+160 B)	+0.01%

Changeset

File	Δ	Size
packages/loot-core/src/server/reports/app.ts	📈 +131 B (+2.99%)	4.27 kB → 4.4 kB
packages/loot-core/src/shared/months.ts	📈 +89 B (+0.77%)	11.36 kB → 11.44 kB
packages/loot-core/src/server/aql/schema/index.ts	📈 +75 B (+0.53%)	13.9 kB → 13.97 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 (+160 B)	+0.01%

Smaller

No assets were smaller

Unchanged

No assets were unchanged

[Teprifer]

The 'current interval' controlled by the option is 'month' rather than what is defined by the interval option? if so I'd suggest renaming it to Include current month for clarity. e.g. interval is weekly, with a live range of last 3 months, this option toggles the 4 weeks starting this month.
How it is now vs making it go by the interval option both makes sense to me (as the budgeting is monthly).

Noticed option is disabled when using a static range(makes obvious sense). Were you thinking to disable it for the interval + live ranges that it doesn't make sense for, or not worry about that?

[carkom]

The 'current interval' controlled by the option is 'month' rather than what is defined by the interval option? if so I'd suggest renaming it to Include current month for clarity. e.g. interval is weekly, with a live range of last 3 months, this option toggles the 4 weeks starting this month. How it is now vs making it go by the interval option both makes sense to me (as the budgeting is monthly).

Noticed option is disabled when using a static range(makes obvious sense). Were you thinking to disable it for the interval + live ranges that it doesn't make sense for, or not worry about that?

It's not always monthly. It's based on the range interval. Example: choosing "Last week" and Interval = daily. The toggle would give you all the days from this week as well.

If we changed it to match the interval instead and you chose "last month" and interval = weekly. You'd see all 4 weeks of March. Select the new toggle and you'd get the 4th week of April - the first three weeks would be missing. Not sure that makes sense to ever have a view like that. Also, it makes the code needlessly complicated.

I see your point with labeling, because it's not actually "current interval". I've changed the wording to follow the "range" selection. So it says "Include current month" when a monthly range is chosen and "Include current week" when a weekly range is chosen.

[Teprifer]

Ah! I see where I was getting confused, thanks. :) (there was an extra layer of misunderstanding on my part too)

[carkom]

@Teprifer, can you check this meets your expectations?

Make sure to only use a copied budget file or a test file as the migration in this PR will cause the budget file to not work with edge.

[Teprifer]

Awww <3

Yeah I always test in a browser data cleared chrome window with a freshly exported budget.

• Year to Date, includes up to current date regardless of the include current option setting.
  Not sure this is incorrect, since by definition 'to date' means, well, to the current day. But also there's the option to include/exclude the current X (see next point). Eh, thoughts? For what it's worth I can see it both ways reasonably equally.
  Mayyyybe it could be a thing that if 'year to date' or 'all time' is selected this option is forced to On automatically(to keep the current behaviour), and then users could optionally turn it off?

• The option 'include current <week/month>' has no time unit when using a range of 'year to date' 'last year' or 'all time', and so just reads 'include current', wonder if for these it should be 'month' for year to date, and 'year' for all time. For 'last year' eh, disable/grey out?? Have a think on this and the above and what you think makes sense - happy to discuss further :)

Besides those minor points it looks pretty good.

[carkom]

@Teprifer So there's a regression from a previous PR that broke filters. This PR is not focused on fixing those at all. You can check #2785 for the fix for filters corrections.

Can you edit your review (above) and take out anything that relates to filters or show activity clicks? It would be much easier if we could keep the issues separate. Cheers!

[Teprifer]

@carkom Sorry didn't realise the filter correction stuff included the dates bit (only aware of amounts) so thought maybe related as this has a date affecting option. And the first part was linked to the new option in this PR, anyway, removed.

[carkom]

Awww <3

Yeah I always test in a browser data cleared chrome window with a freshly exported budget.

* Year to Date, includes up to current date regardless of the include current option setting.
  Not sure this is incorrect, since by definition 'to date' means, well, to the current day. But also there's the option to include/exclude the current X (see next point). Eh, thoughts? For what it's worth I can see it both ways reasonably equally.
  Mayyyybe it could be a thing that if 'year to date' or 'all time' is selected this option is forced to On automatically(to keep the current behaviour), and then users could optionally turn it off?

* The option 'include current <week/month>' has no time unit when using a range of 'year to date' 'last year' or 'all time', and so just reads 'include current', wonder if for these it should be 'month' for year to date, and 'year' for all time. For 'last year' eh, disable/grey out?? Have a think on this and the above and what you think makes sense - happy to discuss further  :)

Besides those minor points it looks pretty good.

I think I've fixed both issues. Have a look.

Making "Year to date" and "All time" function within the "include current" logic is messy because the interval type can change. Example: when interval = Monthly currentInterval would be current month, when interval = weekly currentInterval would be current week. Making this change dynamically adds complexity that I don't see as neccessary.

"Year to date" is meant to be Jan 1st through today. If someone wants Jan of this year through this month minus one then they can utilize the static filters. Same with "All time" range. It's one extra click to use these static fields.

[Teprifer]

I think I've fixed both issues. Have a look.

Making "Year to date" and "All time" function within the "include current" logic is messy because the interval type can change. Example: when interval = Monthly currentInterval would be current month, when interval = weekly currentInterval would be current week. Making this change dynamically adds complexity that I don't see as neccessary.

"Year to date" is meant to be Jan 1st through today. If someone wants Jan of this year through this month minus one then they can utilize the static filters. Same with "All time" range. It's one extra click to use these static fields.

Ah yep, I was thinking it'd be static as month for YTD and year for all time(regardless of interval setting), but as I mentioned I don't think there's an incorrect approach here, so the simplest makes sense. Besides which I imagine it's fairly niche anyhoo.

Had a look, seems fine, my brain is a bit fried, but if you're happy with it, onwards! :)