[PR #3679] [MERGED] Fix incorrect cumulative totals for Days 28+ on the Spending Report #5000

New Issue

GiteaMirror · 2026-02-28T21:04:29-06:00

GiteaMirror commented

2026-02-28 21:04:29 -06:00

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/3679
Author: @joel-rich
Created: 10/16/2024
Status: ✅ Merged
Merged: 10/21/2024
Merged by: @carkom

Base: master ← Head: spending-report

📝 Commits (2)

d8dbd82 Fix bug with spending report cumulative totals
db28f82 release notes

📊 Changes

2 files changed (+7 additions, -1 deletions)

View changed files

📝 packages/desktop-client/src/components/reports/spreadsheets/spending-spreadsheet.ts (+1 -1)
➕ upcoming-release-notes/3679.md (+6 -0)

📄 Description

Hey, this is my first pull request and I'm not very familiar with contributing to projects in general, so let me know if this is the correct process or if I should do anything in a different way.

This change fixes incorrect cumulative totals on days 28+ in the Spending Report that can occur when there are deposit transactions.

Brief summary of code's purpose:
Loops through an array with objects for days 28 to 30/31. Objects have a "cumulative total" already calculated. We want to use the latest "real" value (ie: if it's the 29th of the current month, the 30th's value doesn't exist, so use the 29th's cumulative total).

Explanation of issue:
The old code seemed to assume each "real" day's cumulative total would always decrease (larger negative value). It checked each day's value against the next day's and kept the smaller number. Presumably, it assumed the larger number would be 0 or null due to the day not having happened yet, but this isn't the always the case when there are deposit transactions. This caused the largest negative and [possibly] incorrect cumulative total to show up on the graph.

eg:
If we had these totals in a 31-day month

Day, Daily Total, Cumulative Total
...
28, -500, -2000
29, 500, -1500
30, -300, -1800
31, -150, -1950

2000 would show up on the graph rather than 1950 since 2000 is the largest negative value and would be checked against the rest.

If the 31st day had a Daily Total of 250 instead, the correct Cumulative Total of 2050 would show up on the graph since 2050 is now the largest negative cumulative total.

Fix:
I noticed the Cumulative total for days that haven't happened yet is null, so I just changed it to use the last non-null cumulative total. If applicable, days that have passed will have a cumulative total of 0 rather than null, so checking against null should only impact future days.

I wrote this comment about it, but I hadn't looked at the code so was off on some details at the time. https://github.com/actualbudget/actual/issues/2820#issuecomment-2408100582

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/3679 **Author:** [@joel-rich](https://github.com/joel-rich) **Created:** 10/16/2024 **Status:** ✅ Merged **Merged:** 10/21/2024 **Merged by:** [@carkom](https://github.com/carkom) **Base:** `master` ← **Head:** `spending-report` --- ### 📝 Commits (2) - [`d8dbd82`](https://github.com/actualbudget/actual/commit/d8dbd828461325a26aca2ed13cd95040fb0bd247) Fix bug with spending report cumulative totals - [`db28f82`](https://github.com/actualbudget/actual/commit/db28f8286562aac8ff80971ab7c1c37b246085c2) release notes ### 📊 Changes **2 files changed** (+7 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `packages/desktop-client/src/components/reports/spreadsheets/spending-spreadsheet.ts` (+1 -1) ➕ `upcoming-release-notes/3679.md` (+6 -0) </details> ### 📄 Description Hey, this is my first pull request and I'm not very familiar with contributing to projects in general, so let me know if this is the correct process or if I should do anything in a different way. This change fixes incorrect cumulative totals on days 28+ in the Spending Report that can occur when there are deposit transactions. Brief summary of code's purpose: Loops through an array with objects for days 28 to 30/31. Objects have a "cumulative total" already calculated. We want to use the latest "real" value (ie: if it's the 29th of the current month, the 30th's value doesn't exist, so use the 29th's cumulative total). Explanation of issue: The old code seemed to assume each "real" day's cumulative total would always decrease (larger negative value). It checked each day's value against the next day's and kept the smaller number. Presumably, it assumed the larger number would be 0 or null due to the day not having happened yet, but this isn't the always the case when there are deposit transactions. This caused the largest negative and [possibly] incorrect cumulative total to show up on the graph. eg: If we had these totals in a 31-day month ``` Day, Daily Total, Cumulative Total ... 28, -500, -2000 29, 500, -1500 30, -300, -1800 31, -150, -1950 ``` 2000 would show up on the graph rather than 1950 since 2000 is the largest negative value and would be checked against the rest. If the 31st day had a Daily Total of 250 instead, the correct Cumulative Total of 2050 would show up on the graph since 2050 is now the largest negative cumulative total. Fix: I noticed the Cumulative total for days that haven't happened yet is null, so I just changed it to use the last non-null cumulative total. If applicable, days that have passed will have a cumulative total of 0 rather than null, so checking against null should only impact future days. I wrote this comment about it, but I hadn't looked at the code so was off on some details at the time. https://github.com/actualbudget/actual/issues/2820#issuecomment-2408100582 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-02-28 21:04:29 -06:00

GiteaMirror closed this issue