feat: add cost line classification for expense invoices#107
Merged
Conversation
Cost line maps expense invoices to business cost centers (Growth, Heads, Service, Service delivery, Client success). Auto-suggests from the category's default_cost_line and can be manually overridden. - Migration: add cost_line to invoices, default_cost_line to categories - CostLine module: static enum with values, labels, cast/1 validation - Category schema/form: default_cost_line field with select dropdown - Invoice schema: cost_line field, cast in category_changeset - Context: set_invoice_category auto-populates from default, set_invoice_cost_line for independent updates (expense-only) - Classify LiveView: dropdown between category and tags, auto-updates on category select, persists on save - Show LiveView: displays cost line in classification card (expense only) - API: cost_line in invoice JSON, optional override in set_category, default_cost_line in category JSON and CRUD endpoints - OpenAPI schemas updated for all affected endpoints Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a CostLine enum and fields to Invoice and Category, migration for new columns, business logic to auto-fill or override invoice cost_line (expense-only), API and LiveView support for passing/choosing cost_line, and tests covering the feature across layers. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant DB
Client->>Controller: PUT /api/invoices/:id/category {category_id, cost_line?}
Controller->>Controller: cast & validate cost_line
Controller->>Service: set_invoice_category(invoice, category_id)
Service->>DB: SELECT category WHERE id = ...
DB-->>Service: category (includes default_cost_line)
Service->>Service: if category && invoice.type == :expense and category.default_cost_line -> set default_cost_line
Service->>DB: UPDATE invoices SET category_id[, cost_line?]
DB-->>Service: updated invoice
alt cost_line provided in request (valid)
Controller->>Service: set_invoice_cost_line(invoice, cost_line)
Service->>DB: UPDATE invoices SET cost_line
DB-->>Service: updated invoice
end
Service-->>Controller: {:ok, invoice}
Controller-->>Client: 200 OK {invoice with category_id, cost_line}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/ksef_hub_web/schemas/category.ex (1)
44-50: Avoid duplicating cost-line enum literals in schema definitions.This list can drift from
KsefHub.Invoices.CostLine.values/0. Prefer deriving OpenAPI enum values from that module to keep one source of truth.♻️ Suggested refactor
defmodule KsefHubWeb.Schemas.Category do @@ - alias OpenApiSpex.Schema + alias KsefHub.Invoices.CostLine + alias OpenApiSpex.Schema + + `@cost_line_values` Enum.map(CostLine.values(), &Atom.to_string/1) @@ default_cost_line: %Schema{ type: :string, - enum: ["growth", "heads", "service", "service_delivery", "client_success"], + enum: `@cost_line_values`, nullable: true, description: "Default cost line for invoices assigned to this category. Auto-populated when setting category." },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/schemas/category.ex` around lines 44 - 50, The schema's hardcoded enum for default_cost_line should be replaced with a single source of truth by deriving values from KsefHub.Invoices.CostLine.values/0; update the default_cost_line Schema definition in category.ex to call that function (or map its return) instead of duplicating the string list so OpenAPI enum values come from KsefHub.Invoices.CostLine.values/0.test/ksef_hub_web/live/invoice_live/show_test.exs (1)
1149-1168: Consider adding a test for expense invoices withcost_line: nil.You already test visibility by invoice type; adding the nil case would lock in the
"—"fallback behavior from the LiveView template.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/live/invoice_live/show_test.exs` around lines 1149 - 1168, Add a test case under the "cost line display" describe block that inserts an expense invoice with cost_line: nil (e.g., using insert(:invoice, type: :expense, cost_line: nil, company: company)), mounts the LiveView via live(conn, ~p"/c/#{company.id}/invoices/#{invoice.id}"), asserts the cost-line element exists (has_element?(view, ~s([data-testid="cost-line-display"]))) and asserts the rendered view shows the fallback "—" text to lock in the template's nil fallback behavior.lib/ksef_hub_web/live/invoice_live/classify.ex (1)
63-77: Refactor to reduce nesting depth (Credo warning).The static analysis correctly flags that line 64 is nested 3 levels deep. Extract the invoice processing into a helper function.
♻️ Proposed refactor to reduce nesting
+ `@spec` do_mount_with_invoice(Phoenix.LiveView.Socket.t(), Invoice.t(), Membership.role()) :: + Phoenix.LiveView.Socket.t() + defp do_mount_with_invoice(socket, invoice, role) do + company = socket.assigns[:current_company] + + can_set_category = + invoice.type == :expense && Authorization.can?(role, :set_invoice_category) + + can_set_tags = Authorization.can?(role, :set_invoice_tags) + can_manage_tags = Authorization.can?(role, :manage_tags) + + categories = Invoices.list_categories(company.id) + grouped = group_categories(categories) + all_tags = Invoices.list_tags(company.id, invoice.type) + current_tag_ids = MapSet.new(invoice.tags, & &1.id) + + category_cost_line_map = + Map.new(categories, fn c -> {c.id, c.default_cost_line} end) + + socket + |> assign( + page_title: "Classify #{invoice.invoice_number}", + invoice: invoice, + categories: categories, + grouped_categories: grouped, + all_tags: all_tags, + selected_category_id: invoice.category_id, + selected_tag_ids: current_tag_ids, + selected_cost_line: invoice.cost_line, + category_cost_line_map: category_cost_line_map, + can_set_category: can_set_category, + can_set_tags: can_set_tags, + can_manage_tags: can_manage_tags, + new_tag_form: to_form(%{"name" => ""}), + tag_form_key: 0, + show_all_tags: false, + category_confidence_threshold: InvoiceClassifier.category_confidence_threshold(), + tag_confidence_threshold: InvoiceClassifier.tag_confidence_threshold(), + expanded_group: expanded_group_for(invoice.category_id, categories) + ) + endThen in mount:
invoice -> - can_set_category = ... - ... - {:ok, socket |> assign(...)} + {:ok, do_mount_with_invoice(socket, invoice, role)}🤖 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/classify.ex` around lines 63 - 77, The block in mount that builds category_cost_line_map and the long |> assign(...) is causing deep nesting; extract the invoice-related processing into a helper (e.g., prepare_classify_assigns/4 or build_invoice_assigns/1) that returns the map of assigns or a partially assigned socket, move creation of category_cost_line_map and selected_* values (category_cost_line_map, selected_category_id, selected_tag_ids, selected_cost_line) into that helper, and then call the helper from mount to perform a single, flat assign call (reference the existing category_cost_line_map variable and the |> assign(...) pipeline to identify where to replace with the helper).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@priv/repo/migrations/20260331123732_add_cost_line.exs`:
- Around line 5-11: The new free-form string columns (:cost_line on invoices and
:default_cost_line on categories) need DB check constraints preventing invalid
enum values; update the migration that alters table(:invoices) and alter
table(:categories) to add CHECK constraints (e.g. create a constraint named
invoices_cost_line_check and categories_default_cost_line_check) that restrict
the column to the allowed set of values used by the app’s cost-line enum,
implement them via Ecto’s create constraint/execute SQL in the up block and drop
the constraints in the down/rollback so migrations are reversible.
---
Nitpick comments:
In `@lib/ksef_hub_web/live/invoice_live/classify.ex`:
- Around line 63-77: The block in mount that builds category_cost_line_map and
the long |> assign(...) is causing deep nesting; extract the invoice-related
processing into a helper (e.g., prepare_classify_assigns/4 or
build_invoice_assigns/1) that returns the map of assigns or a partially assigned
socket, move creation of category_cost_line_map and selected_* values
(category_cost_line_map, selected_category_id, selected_tag_ids,
selected_cost_line) into that helper, and then call the helper from mount to
perform a single, flat assign call (reference the existing
category_cost_line_map variable and the |> assign(...) pipeline to identify
where to replace with the helper).
In `@lib/ksef_hub_web/schemas/category.ex`:
- Around line 44-50: The schema's hardcoded enum for default_cost_line should be
replaced with a single source of truth by deriving values from
KsefHub.Invoices.CostLine.values/0; update the default_cost_line Schema
definition in category.ex to call that function (or map its return) instead of
duplicating the string list so OpenAPI enum values come from
KsefHub.Invoices.CostLine.values/0.
In `@test/ksef_hub_web/live/invoice_live/show_test.exs`:
- Around line 1149-1168: Add a test case under the "cost line display" describe
block that inserts an expense invoice with cost_line: nil (e.g., using
insert(:invoice, type: :expense, cost_line: nil, company: company)), mounts the
LiveView via live(conn, ~p"/c/#{company.id}/invoices/#{invoice.id}"), asserts
the cost-line element exists (has_element?(view,
~s([data-testid="cost-line-display"]))) and asserts the rendered view shows the
fallback "—" text to lock in the template's nil fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9efe999a-cea9-4ca2-8e8a-87dd22415e17
📒 Files selected for processing (20)
lib/ksef_hub/invoices.exlib/ksef_hub/invoices/category.exlib/ksef_hub/invoices/cost_line.exlib/ksef_hub/invoices/invoice.exlib/ksef_hub_web/controllers/api/category_controller.exlib/ksef_hub_web/controllers/api/invoice_controller.exlib/ksef_hub_web/json_helpers.exlib/ksef_hub_web/live/category_live/form.exlib/ksef_hub_web/live/invoice_live/classify.exlib/ksef_hub_web/live/invoice_live/show.exlib/ksef_hub_web/schemas/category.exlib/ksef_hub_web/schemas/create_category_request.exlib/ksef_hub_web/schemas/invoice.exlib/ksef_hub_web/schemas/set_category_request.exlib/ksef_hub_web/schemas/update_category_request.expriv/repo/migrations/20260331123732_add_cost_line.exstest/ksef_hub/invoices_test.exstest/ksef_hub_web/controllers/api/invoice_controller_test.exstest/ksef_hub_web/live/invoice_live/classify_test.exstest/ksef_hub_web/live/invoice_live/show_test.exs
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…urce of truth for enum - Add CHECK constraints in migration to restrict cost_line values at DB level - Derive OpenAPI enum from CostLine.values/0 instead of hardcoding - Add test for nil cost_line fallback display Co-Authored-By: Claude Opus 4.6 (1M context) <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
1 participant
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.
Cost line maps expense invoices to business cost centers (Growth, Heads, Service, Service delivery, Client success). Auto-suggests from the category's default_cost_line and can be manually overridden.
Summary by CodeRabbit
New Features
API
UI
Database
Tests