[Maintenence] Refactor Goals Schedule file #2102

shall0pass · 2023-12-18T20:32:21Z

This is an attempt to simplify the schedules file by breaking out the logic in separate functions and hopefully squashing a few bugs in the process. I think this is a little easier to follow than the currently written method of nested if-else logic and will hopefully be easier to debug in the future. The calculation method remains the same, so hopefully there are no regressions 🤞.

One possible gotcha is the way the schedule templates are now filtered. It is possible that a schedule could end up in both the sinkingFund and full template arrays. I haven't seen it happen in any of the iterations I've tried so far but I may have missed a situation.

Believed to fixed:

When placing a non-repeating schedule into the future, the schedule template would not budget.
Monthly recurring schedules would sometimes have non-expected values.

@jfdoming You added startDate and started variables in #1899. I removed them because it was causing issue 1 above. Is there a schedule condition I need to check that I'm overlooking? It's simply commented for now so it's easy to turn back on.

This is an attempt at a flow chart for all of the logic happening in this file.

netlify · 2023-12-18T20:32:25Z

✅ Deploy Preview for actualbudget ready!

Name	Link
🔨 Latest commit	`462faa7`
🔍 Latest deploy log	https://app.netlify.com/sites/actualbudget/deploys/659a06d566116000089ca6f6
😎 Deploy Preview	https://deploy-preview-2102.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 · 2023-12-18T20:34:41Z

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
15	3.08 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/main.js	1.43 MB	0%
static/js/665.chunk.js	824.28 kB	0%
static/media/Inter-italic.var.woff2	239.29 kB	0%
static/media/Inter-roman.var.woff2	221.86 kB	0%
static/js/231.chunk.js	117.38 kB	0%
static/js/wide-components.chunk.js	112.27 kB	0%
static/js/reports.chunk.js	77.81 kB	0%
static/js/narrow-components.chunk.js	42.42 kB	0%
static/js/553.chunk.js	13.14 kB	0%
static/js/473.chunk.js	13 kB	0%
static/js/resize-observer-polyfill.chunk.js	8.88 kB	0%
static/css/main.css	7.41 kB	0%
asset-manifest.json	1.92 kB	0%
index.html	1.66 kB	0%
static/media/browser-server.js	911 B	0%

github-actions · 2023-12-18T20:34:43Z

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
2	2.23 MB → 2.23 MB (+109 B)	+0.00%

Changeset

File	Δ	Size
`packages/loot-core/src/server/budget/goals/goalsSchedule.ts`	📈 +18 B (+0.26%)	6.75 kB → 6.77 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
kcab.worker.js	1.24 MB → 1.24 MB (+109 B)	+0.01%

Smaller

No assets were smaller

Unchanged

Asset	File Size	% Changed
xfo.xfo.kcab.worker.js	1014.45 kB	0%

youngcw · 2023-12-18T21:38:06Z

I think this is overwriting previous templates budgeted value.

For example, this setup doesn't work right. This only ends up with the schedule value ($15) instead of the full 25.

#template 10
#template schedule Netflix
expect 25

youngcw · 2023-12-18T22:18:30Z

That overwrite issue is fixed now. I still need to go through and make sure the chart and code make sense

jfdoming · 2023-12-22T22:25:19Z

@shall0pass thanks for looking! I think I added those variables to fix a different calculation issue I found, but maybe I misunderstood how things were supposed to work. I'm on vacation right now so I don't have my computer but I'll take a look tomorrow and let you know if I noticed anything weird

shall0pass · 2024-01-03T18:58:45Z

Remerged master and made sure #2125 was applied.

jfdoming

I wasn't able to find/reproduce the original issue I found, so feel free to merge this as is. I'll open an issue in the repo if I can reproduce it.

Kidglove57 · 2024-01-07T09:52:05Z

Sorry I did not respond to your original review request @shall0pass. I don’t think my GitHub notifications are set right! Will look later - however I am seeing the same issues on current release as was reported in GitHub

shall0pass · 2024-01-07T16:54:23Z

Sorry I did not respond to your original review request @shall0pass. I don’t think my GitHub notifications are set right! Will look later - however I am seeing the same issues on current release as was reported in GitHub

You're seeing the issue on 24.1.0 or you're seeing the issue in this PR? Nothing was merged for 24.1.0 around that issue, so it should still be unresolved.

Kidglove57 · 2024-01-07T19:27:05Z

Sorry I did not respond to your original review request @shall0pass. I don’t think my GitHub notifications are set right! Will look later - however I am seeing the same issues on current release as was reported in GitHub

You're seeing the issue on 24.1.0 or you're seeing the issue in this PR? Nothing was merged for 24.1.0 around that issue, so it should still be unresolved.

Understood - I have now been able to test my own file in this PR. It now fills the budget as it should for a single line annual "#template schedule x" but gives me an odd number (too low) where I have a series of annual "#template schedule ..." listed in the same category.

* refactor pass 1 * refactor pass 2 * refactor pass 3 * commented out startDate * remove console logging * release note * non-repeating error * add daily * move else * Fix compounding to_budget * lint * reapply 2125

shall0pass added 5 commits December 17, 2023 20:15

refactor pass 1

14802ae

refactor pass 2

236cc44

Merge branch 'actualbudget:master' into schedules_refactor

11e7cfc

refactor pass 3

4f062cc

commented out startDate

79701aa

shall0pass requested review from youngcw and Kidglove57 December 18, 2023 20:32

trafico-bot bot added the 🔍 Ready for Review label Dec 18, 2023

remove console logging

e763e16

shall0pass added 4 commits December 18, 2023 14:37

release note

9ef9509

non-repeating error

accb7d3

add daily

06c353e

move else

2b1d2ae

Fix compounding to_budget

00bc0c2

lint

85e7173

youngcw previously approved these changes Dec 22, 2023

View reviewed changes

trafico-bot bot added ✅ Approved and removed 🔍 Ready for Review labels Dec 22, 2023

Merge branch 'master' into schedules_refactor

d96f5cc

shall0pass dismissed youngcw’s stale review via d96f5cc January 3, 2024 14:11

trafico-bot bot added 🔍 Ready for Review and removed ✅ Approved labels Jan 3, 2024

reapply 2125

f573871

shall0pass added 2 commits January 6, 2024 11:59

Merge branch 'master' into schedules_refactor

eee8c5c

Merge branch 'actualbudget:master' into schedules_refactor

462faa7

jfdoming approved these changes Jan 7, 2024

View reviewed changes

trafico-bot bot added ✅ Approved and removed 🔍 Ready for Review labels Jan 7, 2024

youngcw approved these changes Jan 7, 2024

View reviewed changes

shall0pass merged commit 33e778f into actualbudget:master Jan 7, 2024
19 checks passed

trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved labels Jan 7, 2024

shall0pass mentioned this pull request Jan 7, 2024

[Enhancement] Goals: Schedules with exactly one month repeat are force to full #2088

Closed

shall0pass deleted the schedules_refactor branch January 9, 2024 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Maintenence] Refactor Goals Schedule file #2102

[Maintenence] Refactor Goals Schedule file #2102

shall0pass commented Dec 18, 2023

netlify bot commented Dec 18, 2023 •

edited

Loading

github-actions bot commented Dec 18, 2023 •

edited

Loading

github-actions bot commented Dec 18, 2023 •

edited

Loading

youngcw commented Dec 18, 2023 •

edited

Loading

youngcw commented Dec 18, 2023

jfdoming commented Dec 22, 2023

shall0pass commented Jan 3, 2024

jfdoming left a comment

Kidglove57 commented Jan 7, 2024

shall0pass commented Jan 7, 2024

Kidglove57 commented Jan 7, 2024 •

edited

Loading

[Maintenence] Refactor Goals Schedule file #2102

[Maintenence] Refactor Goals Schedule file #2102

Conversation

shall0pass commented Dec 18, 2023

netlify bot commented Dec 18, 2023 • edited Loading

✅ Deploy Preview for actualbudget ready!

github-actions bot commented Dec 18, 2023 • edited Loading

Bundle Stats — desktop-client

github-actions bot commented Dec 18, 2023 • edited Loading

Bundle Stats — loot-core

youngcw commented Dec 18, 2023 • edited Loading

youngcw commented Dec 18, 2023

jfdoming commented Dec 22, 2023

shall0pass commented Jan 3, 2024

jfdoming left a comment

Choose a reason for hiding this comment

Kidglove57 commented Jan 7, 2024

shall0pass commented Jan 7, 2024

Kidglove57 commented Jan 7, 2024 • edited Loading

netlify bot commented Dec 18, 2023 •

edited

Loading

github-actions bot commented Dec 18, 2023 •

edited

Loading

github-actions bot commented Dec 18, 2023 •

edited

Loading

youngcw commented Dec 18, 2023 •

edited

Loading

Kidglove57 commented Jan 7, 2024 •

edited

Loading