feat: add duplicate filter to invoice status dropdown#135
Merged
Conversation
Add "Duplicate" option to the status filter so users can explicitly toggle visibility of confirmed-duplicate invoices. Confirmed duplicates are now excluded by default from query results, and the default filter pre-selects Pending + Approved for a cleaner list view. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 (3)
📝 WalkthroughWalkthroughInvoice filtering now treats Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,230,255,0.5)
participant User
end
rect rgba(200,255,200,0.5)
participant LiveView
end
rect rgba(255,230,200,0.5)
participant InvoicesModule
end
rect rgba(255,200,200,0.5)
participant DB
end
User->>LiveView: Change filters (statuses include/exclude "duplicate")
LiveView->>InvoicesModule: build_query_params / list_invoices(filters)
InvoicesModule->>InvoicesModule: apply_status_and_duplicate_filters(filters)
alt only :duplicate requested
InvoicesModule->>DB: WHERE duplicate_status == :confirmed
else duplicate + normal requested
InvoicesModule->>DB: WHERE (status IN real_statuses) OR (duplicate_status == :confirmed)
else only normal requested
InvoicesModule->>DB: WHERE status IN real_statuses AND duplicate_status != :confirmed
end
DB-->>InvoicesModule: rows
InvoicesModule-->>LiveView: invoices list
LiveView-->>User: render results / push_patch with serialized statuses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
🧹 Nitpick comments (3)
test/ksef_hub/invoices_test.exs (1)
356-368: Add an explicit%{statuses: []}test case.Current coverage checks
%{}but not the explicit empty-statuses path produced by LiveView (statuses=""→[]). Adding it would lock the “all deselected = all non-duplicates” behavior end-to-end.✅ Suggested test addition
+ test "excludes confirmed duplicates when statuses is explicitly empty", %{company: company} do + insert(:invoice, type: :expense, company: company) + insert(:invoice, type: :expense, company: company, duplicate_status: :confirmed) + + result = Invoices.list_invoices(company.id, %{statuses: []}) + assert length(result) == 1 + assert is_nil(hd(result).duplicate_status) + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub/invoices_test.exs` around lines 356 - 368, Add an extra test that calls Invoices.list_invoices(company.id, %{statuses: []}) to ensure the explicit empty statuses path (produced by LiveView as statuses="") excludes confirmed duplicates the same as the %{ } case; update the test suite by adding a new test (e.g., "excludes confirmed duplicates when explicit statuses filter is an empty list") that inserts one normal invoice and one with duplicate_status: :confirmed for the same company and asserts the result length is 1 and that hd(result).duplicate_status is nil, using the same setup as the existing test that calls Invoices.list_invoices/2.lib/ksef_hub/invoices.ex (1)
2445-2525: Refactorapply_filters/2to reduce branching complexity.This function is now doing too much (and CI is already flagging it). Extract duplicate/status handling into a dedicated helper to keep filter composition maintainable.
♻️ Proposed refactor outline
defp apply_filters(query, filters) do - # Exclude confirmed duplicates unless "duplicate" is explicitly selected - include_duplicates = :duplicate in (filters[:statuses] || []) - - query = - if include_duplicates do - query - else - where(query, [i], is_nil(i.duplicate_status) or i.duplicate_status != :confirmed) - end + query = + query + |> apply_duplicate_visibility(filters) Enum.reduce(filters, query, fn @@ - {:statuses, statuses}, q when is_list(statuses) and statuses != [] -> - real_statuses = Enum.reject(statuses, &(&1 == :duplicate)) - show_duplicates = :duplicate in statuses - - case {real_statuses, show_duplicates} do - {[], true} -> - where(q, [i], i.duplicate_status == :confirmed) - {_, true} -> - where(q, [i], i.status in ^real_statuses or i.duplicate_status == :confirmed) - {[_ | _], false} -> - where(q, [i], i.status in ^real_statuses) - _ -> - q - end + {:statuses, statuses}, q when is_list(statuses) and statuses != [] -> + apply_statuses_filter(q, statuses) @@ end) end + +defp apply_duplicate_visibility(query, filters) do + if :duplicate in (filters[:statuses] || []) do + query + else + where(query, [i], is_nil(i.duplicate_status) or i.duplicate_status != :confirmed) + end +end + +defp apply_statuses_filter(query, statuses) do + real_statuses = Enum.reject(statuses, &(&1 == :duplicate)) + show_duplicates = :duplicate in statuses + + case {real_statuses, show_duplicates} do + {[], true} -> where(query, [i], i.duplicate_status == :confirmed) + {[_ | _], true} -> where(query, [i], i.status in ^real_statuses or i.duplicate_status == :confirmed) + {[_ | _], false} -> where(query, [i], i.status in ^real_statuses) + _ -> query + end +end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub/invoices.ex` around lines 2445 - 2525, The apply_filters/2 function has grown overly complex around the duplicate/status logic; extract that whole block (the include_duplicates assignment and the {:statuses, statuses} case plus the initial global exclude where(...) that checks i.duplicate_status) into a focused helper (e.g. apply_status_and_duplicate_filters(query, filters) or build_status_filter(query, statuses, include_duplicates)) and call it from apply_filters/2, leaving apply_filters to only enumerate simple per-filter clauses (keeping calls to apply_payment_status_filter/2 and apply_text_search/2 intact); ensure the helper preserves the current behavior: compute include_duplicates = :duplicate in (filters[:statuses] || []) and then implement the four case branches ({[], true}, {_, true}, {[_|_], false}, _) and the initial global exclusion when include_duplicates is false.lib/ksef_hub_web/live/invoice_live/index.ex (1)
84-118: Splitfilter_assigns/4to reduce cyclomatic complexity.The new status-count branching pushed this function over the CI complexity threshold. Extracting
statuses_count/1(and optionallyfilter_count/1) keeps this handler easier to evolve.🤖 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/index.ex` around lines 84 - 118, Extract the status-count calculation out of filter_assigns/4 into a new private helper statuses_count/1 (and optionally extract the whole filter_count computation into filter_count/2 or filter_count/1) to reduce cyclomatic complexity: move the logic that builds default_statuses, inspects filters[:statuses], compares to the default MapSet and returns the integer count into statuses_count/1 and replace the inline case in filter_assigns/4 with a call to statuses_count(filters); if you extract filter_count as well, accept filters (and result if needed) and return the final integer which filter_assigns/4 will assign to filter_count while keeping all existing behavior and references (e.g., filter_assigns/4, statuses_count/1, filter_count/1) identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/ksef_hub_web/live/invoice_live/index.ex`:
- Around line 84-118: Extract the status-count calculation out of
filter_assigns/4 into a new private helper statuses_count/1 (and optionally
extract the whole filter_count computation into filter_count/2 or
filter_count/1) to reduce cyclomatic complexity: move the logic that builds
default_statuses, inspects filters[:statuses], compares to the default MapSet
and returns the integer count into statuses_count/1 and replace the inline case
in filter_assigns/4 with a call to statuses_count(filters); if you extract
filter_count as well, accept filters (and result if needed) and return the final
integer which filter_assigns/4 will assign to filter_count while keeping all
existing behavior and references (e.g., filter_assigns/4, statuses_count/1,
filter_count/1) identical.
In `@lib/ksef_hub/invoices.ex`:
- Around line 2445-2525: The apply_filters/2 function has grown overly complex
around the duplicate/status logic; extract that whole block (the
include_duplicates assignment and the {:statuses, statuses} case plus the
initial global exclude where(...) that checks i.duplicate_status) into a focused
helper (e.g. apply_status_and_duplicate_filters(query, filters) or
build_status_filter(query, statuses, include_duplicates)) and call it from
apply_filters/2, leaving apply_filters to only enumerate simple per-filter
clauses (keeping calls to apply_payment_status_filter/2 and apply_text_search/2
intact); ensure the helper preserves the current behavior: compute
include_duplicates = :duplicate in (filters[:statuses] || []) and then implement
the four case branches ({[], true}, {_, true}, {[_|_], false}, _) and the
initial global exclusion when include_duplicates is false.
In `@test/ksef_hub/invoices_test.exs`:
- Around line 356-368: Add an extra test that calls
Invoices.list_invoices(company.id, %{statuses: []}) to ensure the explicit empty
statuses path (produced by LiveView as statuses="") excludes confirmed
duplicates the same as the %{ } case; update the test suite by adding a new test
(e.g., "excludes confirmed duplicates when explicit statuses filter is an empty
list") that inserts one normal invoice and one with duplicate_status: :confirmed
for the same company and asserts the result length is 1 and that
hd(result).duplicate_status is nil, using the same setup as the existing test
that calls Invoices.list_invoices/2.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd7b0683-bc56-48c7-9704-82ccbd68a700
📒 Files selected for processing (4)
lib/ksef_hub/invoices.exlib/ksef_hub_web/live/invoice_live/index.extest/ksef_hub/invoices_test.exstest/ksef_hub_web/live/invoice_live/index_test.exs
…ses test Extract statuses_count/1 and filter_count/1 from filter_assigns/4 in LiveView, and apply_status_and_duplicate_filters/2 from apply_filters/2 in the Invoices context to reduce cyclomatic complexity. Add test covering explicit empty statuses list path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai, can you also run review even if i've just merged this PR? |
|
✅ Actions performedReview triggered.
|
5 tasks
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.
Summary
Test plan
mix testpasses (4 new tests for duplicate filtering)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests