fix(bug 12): P&L report honours from_date/to_date across multiple periods#9
Merged
fix(bug 12): P&L report honours from_date/to_date across multiple periods#9
Conversation
…iods
getProfitAndLoss() was applying a hard single-period filter:
.where('transaction_lines.period_id', opts.period_id)
which silently neutralised the from_date/to_date filters below it. Any
caller passing a multi-period date range got back only the single period
named in period_id.
Fix: make the period filter conditional — apply it only when no date
range is supplied. When from_date or to_date is present, the date
filters select across periods as the caller expects.
This mirrors the pattern already used by getBalanceSheet() in the same
file.
New tests cover:
- default single-period behaviour still works (regression guard)
- date range spanning two periods aggregates both
- date range confined to a different period than period_id returns that
period's data (confirms period_id is ignored once dates are set)
- aggregate across range equals sum of individual monthly reports
Verified: 205 unit tests pass, 363 integration tests pass (was 359 — 4
new).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getProfitAndLoss()in `src/engine/reports.ts` was applying a hard single-period filter:```typescript
.where('transaction_lines.period_id', opts.period_id)
```
which silently neutralised the `from_date`/`to_date` filters further down. Any caller passing a multi-period date range got back only the single period named in `period_id` — the date parameters were accepted but had no effect.
Fix
Make the `period_id` filter conditional — apply it only when no date range is supplied. When `from_date` or `to_date` is present, the existing date filters select across periods as the caller expects. This mirrors the pattern already used by `getBalanceSheet()` in the same file.
Tests
Added 4 new integration tests covering:
Test plan
🤖 Generated with Claude Code