fix: Balance View category/grand totals still counting deferred transactions #234

Merged
Copilot merged 1 commit from copilot/fix-balance-view-issues into main 2026-04-26 08:40:54 +00:00
Copilot commented 2026-04-26 08:11:20 +00:00 (Migrated from github.com)

The Balance View's category totals and grand total continued to inflate expected amounts with deferred transactions despite the previous frontend fix, because the frontend checks (is_deferred, is_virtual) were always undefined.

Root Cause

BalanceService::getDetailedMonthlyBalance returned DTO objects directly to Inertia. PHP's json_encode only serializes public properties — it never calls toArray(). The fields is_virtual and is_deferred are computed in toArray(), not public properties, so they were absent from the serialized JSON.

On the frontend, transaction.is_deferred === undefined, and !undefined === true, so every deferred transaction bypassed the exclusion guard added in the previous fix.

Fix

Map both collections through ->toArray() before returning from getDetailedMonthlyBalance:

$incomeDetails = $transactions
    ->where('type', TransactionType::INCOME)
    ->values()
    ->map(fn ($t) => $t->toArray())  // ensures is_virtual, is_deferred et al. are in JSON
    ->values();

The manually-pushed stdClass items (mortgages, credit payments, purchases) already had explicit is_deferred/is_virtual properties and are unaffected.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (code change that neither fixes a bug nor adds a feature)
  • Performance improvement
  • Test coverage improvement

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this manually (if applicable)

Code Quality

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors

CI/CD Checks

  • All automated tests pass
  • Code formatting (Pint) passes
  • JavaScript/TypeScript linting passes
  • Build process completes successfully

Additional Notes

The real fix lives entirely in BalanceService.php. The frontend logic from the previous commit (CategorySummaryList.vue, MonthView.vue) was correct — it just never had valid data to act on.

The Balance View's category totals and grand total continued to inflate expected amounts with deferred transactions despite the previous frontend fix, because the frontend checks (`is_deferred`, `is_virtual`) were always `undefined`. ## Root Cause `BalanceService::getDetailedMonthlyBalance` returned DTO objects directly to Inertia. PHP's `json_encode` only serializes **public properties** — it never calls `toArray()`. The fields `is_virtual` and `is_deferred` are computed in `toArray()`, not public properties, so they were absent from the serialized JSON. On the frontend, `transaction.is_deferred === undefined`, and `!undefined === true`, so every deferred transaction bypassed the exclusion guard added in the previous fix. ## Fix Map both collections through `->toArray()` before returning from `getDetailedMonthlyBalance`: ```php $incomeDetails = $transactions ->where('type', TransactionType::INCOME) ->values() ->map(fn ($t) => $t->toArray()) // ensures is_virtual, is_deferred et al. are in JSON ->values(); ``` The manually-pushed stdClass items (mortgages, credit payments, purchases) already had explicit `is_deferred`/`is_virtual` properties and are unaffected. ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Refactoring (code change that neither fixes a bug nor adds a feature) - [ ] Performance improvement - [ ] Test coverage improvement ## Testing - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [x] I have tested this manually (if applicable) ## Code Quality - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] My changes generate no new warnings or errors ## CI/CD Checks - [ ] All automated tests pass - [ ] Code formatting (Pint) passes - [ ] JavaScript/TypeScript linting passes - [ ] Build process completes successfully ## Additional Notes The real fix lives entirely in `BalanceService.php`. The frontend logic from the previous commit (`CategorySummaryList.vue`, `MonthView.vue`) was correct — it just never had valid data to act on.
coderabbitai[bot] commented 2026-04-26 08:11:27 +00:00 (Migrated from github.com)

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 97930623-7251-4815-9ad0-c6eb19f97242

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands and usage tips.

<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- This is an auto-generated comment: skip review by coderabbit.ai --> > [!IMPORTANT] > ## Review skipped > > Bot user detected. > > To trigger a single review, invoke the `@coderabbitai review` command. > > <details> > <summary>⚙️ Run configuration</summary> > > **Configuration used**: defaults > > **Review profile**: CHILL > > **Plan**: Free > > **Run ID**: `97930623-7251-4815-9ad0-c6eb19f97242` > > </details> > > You can disable this status message by setting the `reviews.review_status` to `false` in the CodeRabbit configuration file. > > Use the checkbox below for a quick retry: > - [ ] <!-- {"checkboxId": "e9bb8d72-00e8-4f67-9cb2-caf3b22574fe"} --> 🔍 Trigger review <!-- end of auto-generated comment: skip review by coderabbit.ai --> <!-- tips_start --> --- <sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub> <!-- tips_end -->
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-04-26 08:18:52 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes Balance View category/grand totals incorrectly including deferred transactions by ensuring computed DTO fields (is_deferred, is_virtual) are serialized to the frontend.

Changes:

  • Convert monthly income/expense DTO collections to arrays before returning them to Inertia.
  • Add an entry documenting the root cause and fix in DEVLOG.md.
  • Clarify the partial/complete nature of the previous fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
app/Services/Finance/BalanceService.php Maps DTOs through toArray() so computed flags are present in serialized JSON
DEVLOG.md Documents root cause (DTO serialization) and the applied fix
CHANGELOG.md Notes that the Balance View exclusion needed a backend data-shape fix
## Pull request overview > [!NOTE] > Copilot was unable to run its full agentic suite in this review. Fixes Balance View category/grand totals incorrectly including deferred transactions by ensuring computed DTO fields (`is_deferred`, `is_virtual`) are serialized to the frontend. **Changes:** - Convert monthly income/expense DTO collections to arrays before returning them to Inertia. - Add an entry documenting the root cause and fix in `DEVLOG.md`. - Clarify the partial/complete nature of the previous fix in `CHANGELOG.md`. ### Reviewed changes Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments. | File | Description | | ---- | ----------- | | app/Services/Finance/BalanceService.php | Maps DTOs through `toArray()` so computed flags are present in serialized JSON | | DEVLOG.md | Documents root cause (DTO serialization) and the applied fix | | CHANGELOG.md | Notes that the Balance View exclusion needed a backend data-shape fix |
@ -344,0 +343,4 @@
// Convert DTOs to arrays so that computed fields like is_virtual and is_deferred
// are included in the JSON serialized to the frontend (plain PHP objects only
// expose public properties, not toArray() results, when json_encode'd).
$incomeDetails = $transactions
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-04-26 08:18:51 +00:00

This comment is slightly over-broad: json_encode() also respects JsonSerializable (and in Laravel, some response paths may also use Arrayable/Jsonable). Consider rewording to specifically reflect the observed behavior here (DTOs being encoded as plain objects without computed toArray() fields), so readers don’t infer that json_encode() never serializes anything other than public properties.

This comment is slightly over-broad: `json_encode()` also respects `JsonSerializable` (and in Laravel, some response paths may also use `Arrayable`/`Jsonable`). Consider rewording to specifically reflect the observed behavior here (DTOs being encoded as plain objects without computed `toArray()` fields), so readers don’t infer that `json_encode()` *never* serializes anything other than public properties.
@ -349,2 +356,4 @@
->values()
->map(fn ($t) => $t->toArray())
->values();
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-04-26 08:18:51 +00:00

The intermediate ->values() calls before ->map(...) are redundant since you call ->values() again at the end. You can simplify these chains (and do slightly less work) by removing the earlier ->values() and keeping a single ->values() after map() (or at the very end of the pipeline).

The intermediate `->values()` calls before `->map(...)` are redundant since you call `->values()` again at the end. You can simplify these chains (and do slightly less work) by removing the earlier `->values()` and keeping a single `->values()` after `map()` (or at the very end of the pipeline).
Sign in to join this conversation.
No description provided.