feat: replace billing_date with billing_date_from/to for multi-month …#99
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces single Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Controller
participant Service as Invoices Service
participant Repo as Database
participant Alloc as Allocator
Client->>API: GET expense_monthly_totals(params)
API->>Service: build query with overlap filters
Service->>Repo: fetch invoices (billing_date_from, billing_date_to, net_amount, metadata)
Repo-->>Service: invoice rows
Service->>Alloc: expand rows into monthly allocations (allocate_across_months)
Alloc-->>Service: per-month allocations (month, amount, metadata)
Service->>Service: trim to window, group, sum amounts
Service-->>API: aggregated monthly results
API-->>Client: return response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ksef_hub_web/live/invoice_live/show.ex (1)
396-406:⚠️ Potential issue | 🟠 MajorRefresh
edit_formafter saving the billing period.The full details editor still includes
billing_date_from/billing_date_to, but this branch only refreshesinvoiceandbilling_date_form. If both editors are open, the main form keeps the old range and a latersave_editcan overwrite the just-saved billing period with stale values.Suggested fix
{:noreply, socket |> put_flash(:info, "Billing period updated.") |> assign( invoice: reloaded, + edit_form: build_edit_form(reloaded), editing_billing_date: false, billing_date_form: billing_date_form(reloaded) )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/invoice_live/show.ex` around lines 396 - 406, The billing period save branch updates invoice and billing_date_form but fails to refresh the main edit form, causing stale billing_date_from/to to persist and potentially overwrite the saved period; modify the {:ok, updated} branch (in the save billing period handler where reload_details(updated, socket) is called and billing_date_form(reloaded) is assigned) to also regenerate and assign the edit form (e.g., assign edit_form: edit_form(reloaded) or the equivalent function used to build the main editor) and clear any editing flags so both editors reflect the fresh billing period values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ksef_hub/invoices.ex`:
- Around line 1102-1117: The expanded allocations returned by
expand_to_monthly_allocations() are not being trimmed to the requested billing
window, so expense_monthly_totals/2 (and the analogous income_monthly_totals
function) still include months outside filters.billing_date_from /
filters.billing_date_to; after Repo.all() |> expand_to_monthly_allocations()
insert a filter step that drops any allocation whose billing_date is before
filters.billing_date_from or after filters.billing_date_to (guarding when those
keys are present), then continue grouping and summing—i.e., filter the list of
allocations by comparing each allocation.billing_date to
filters.billing_date_from and filters.billing_date_to before Enum.group_by to
prevent leakage outside the requested period.
In `@lib/ksef_hub/invoices/invoice.ex`:
- Around line 372-390: validate_billing_date_range/1 currently only enforces
ordering when both :billing_date_from and :billing_date_to are present, allowing
one-sided ranges; update validate_billing_date_range to require either both nil
or both set: if both present, keep the Date.compare check and add_error on
:billing_date_to when from > to; if exactly one is present (from xor to), add an
error on the missing field (e.g. on :billing_date_from when :billing_date_to is
set, and on :billing_date_to when :billing_date_from is set) so the changeset
enforces that both endpoints must be provided. Ensure you keep the function name
validate_billing_date_range and the field names :billing_date_from and
:billing_date_to so callers like validate_billing_dates continue to work.
In `@priv/repo/migrations/20260323221841_replace_billing_date_with_range.exs`:
- Around line 21-35: The down/0 currently overwrites multi-month ranges by
setting billing_date = billing_date_from; instead, add a guard at the top of
down/0 to detect any rows where billing_date_to IS NOT NULL AND billing_date_to
> billing_date_from and abort the rollback (raise an Ecto.MigrationError or
similar) so the migration fails rather than losing data; specifically, in the
down/0 function (where execute "UPDATE invoices SET billing_date =
billing_date_from" is used) run a SQL check via execute or Repo.query to count
offending rows (using billing_date_from and billing_date_to) and raise if count
> 0, or alternatively mark the migration irreversible by raising immediately
when such rows exist, before performing the alter/drop/create steps.
---
Outside diff comments:
In `@lib/ksef_hub_web/live/invoice_live/show.ex`:
- Around line 396-406: The billing period save branch updates invoice and
billing_date_form but fails to refresh the main edit form, causing stale
billing_date_from/to to persist and potentially overwrite the saved period;
modify the {:ok, updated} branch (in the save billing period handler where
reload_details(updated, socket) is called and billing_date_form(reloaded) is
assigned) to also regenerate and assign the edit form (e.g., assign edit_form:
edit_form(reloaded) or the equivalent function used to build the main editor)
and clear any editing flags so both editors reflect the fresh billing period
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e2a06af-50ba-4af1-a1ae-d6de6791bf75
📒 Files selected for processing (14)
docs/adr/0033-billing-date-field.mddocs/adr/0034-billing-date-range.mdlib/ksef_hub/invoices.exlib/ksef_hub/invoices/invoice.exlib/ksef_hub_web/components/invoice_components.exlib/ksef_hub_web/controllers/api/invoice_controller.exlib/ksef_hub_web/live/invoice_live/show.exlib/ksef_hub_web/schemas/create_invoice_request.exlib/ksef_hub_web/schemas/invoice.exlib/ksef_hub_web/schemas/update_invoice_request.expriv/repo/migrations/20260323221841_replace_billing_date_with_range.exstest/ksef_hub/invoices_test.exstest/ksef_hub_web/controllers/api/invoice_controller_test.exstest/support/factory.ex
…cost allocation Invoices can now span multiple billing months (e.g. quarterly contracts). The net_amount is proportionally allocated across each month in the range, with rounding remainder assigned to the last month. - Migration: add billing_date_from/to, backfill from billing_date, drop old column - Schema: validate both fields as first-of-month, enforce from <= to - Context: expand multi-month invoices in aggregation queries (expense_monthly_totals, expense_by_category, income_monthly_summary), overlap-based filter semantics - API: replace billing_date with billing_date_from/to in request/response schemas - LiveView: dual month picker for billing period editing, format_billing_period display - ADR 0034 created, ADR 0033 superseded Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Trim expanded monthly allocations to the requested billing window so expense_monthly_totals and expense_by_category don't leak months outside the filter range - Require both billing_date_from and billing_date_to or neither (reject one-sided ranges in validate_billing_date_range) - Guard migration rollback against data loss when multi-month ranges exist (raise instead of silently truncating to billing_date_from) - Refresh edit_form after billing period save in LiveView to prevent stale values from overwriting the saved period Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e874327 to
d28573f
Compare
…d billing_date The CsvBuilder and its tests still referenced the old billing_date field which was replaced by billing_date_from/billing_date_to in the schema. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cost allocation
Invoices can now span multiple billing months (e.g. quarterly contracts). The net_amount is proportionally allocated across each month in the range, with rounding remainder assigned to the last month.
Summary by CodeRabbit
New Features
Documentation
User Interface
Tests
Chores