feat: add Expense/Income tabs to invoices page#92
Merged
Conversation
Replace the type dropdown filter with dedicated tabs for switching between expense and income invoices. Defaults to the Expense tab. Reviewers (without view_all_invoice_types permission) see a static Expense label instead of clickable tabs. - Remove Type column from table (redundant with tabs) - Use badge component for tag list (blue with outline) - Add border to filter chips for visual consistency - Preserve active tab when filtering or clearing filters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <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 (1)
📝 WalkthroughWalkthroughAdds Expense/Income tabbed filtering to the invoices LiveView (Expense default), sanitizes/preserves type across params/events based on role, removes Type select/column from UI, updates components styling and invoice badge text, and updates tests to reflect new tabbed behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant LiveView as InvoiceLiveView
participant Sanitizer as TypeSanitizer
participant Store as FilterStore
participant Renderer as RenderEngine
User->>LiveView: GET /invoices (no type param)
LiveView->>Sanitizer: sanitize_type(params.type, user.role)
Sanitizer-->>LiveView: :expense (default or restricted)
LiveView->>Store: build_filters(params + type)
Store-->>LiveView: filters
LiveView->>Renderer: render(filters, tabs)
Renderer-->>User: show Expense tab active + invoices
User->>LiveView: Click "Income" tab
LiveView->>Sanitizer: sanitize_type(:income, user.role)
Sanitizer-->>LiveView: :income or :expense (if unauthorized)
LiveView->>Store: build_filters(type=:income, other_params)
LiveView->>Renderer: render(updated filters)
Renderer-->>User: show Income tab active (if allowed)
User->>LiveView: Apply other filter (category/tag)
LiveView->>Store: merge_filters(keep current_type)
Store-->>LiveView: merged filters
LiveView->>Renderer: render(filtered invoices with preserved type)
Renderer-->>User: updated list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 unit tests (beta)
📝 Coding Plan
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 (1)
test/ksef_hub_web/live/invoice_live/index_test.exs (1)
118-138: Please add aclear_filtersregression test.The source now has dedicated type-preservation logic for
"clear_filters", but this file only exercises"filter","remove_filter", and direct tab clicks. One interaction test from the Income tab—and ideally the reviewer-on-type=incomeedge—would close the main gap.🤖 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/index_test.exs` around lines 118 - 138, Add a new test (e.g., test "clear_filters preserves type param", %{conn: conn, company: company}) that opens the LiveView with a URL containing type=income plus other filters, uses live(...) to mount, verifies a chip is rendered, triggers the clear_filters event by calling element(view, "button[phx-click=clear_filters]") |> render_click(), and then asserts the patched URL still contains type=income (using assert_patched). Ensure the test mirrors the existing pattern in invoice_live/index_test.exs and uses the same helpers/variables (live, render, element, render_click, assert_patched) so the dedicated clear_filters type-preservation logic is exercised.
🤖 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_web/live/invoice_live/index.ex`:
- Around line 32-35: The incoming params' :type must be normalized before saving
into assigns so unauthorized values like "income" cannot persist; in
handle_params/3, after calling parse_filters() but before Map.put_new(:type,
:expense) or assigning to `@filters`, coerce/validate the :type field (e.g., via a
small sanitizer function or pattern match) to only allow :expense (or known
values) and replace any other value with :expense, then assign the sanitized map
to the socket (`@filters`) and ensure remove_filter/3 and pagination use that
sanitized assign; update references to filters, parse_filters, handle_params,
and remove_filter/3 accordingly so the raw params never become the socket
`@filters`.
---
Nitpick comments:
In `@test/ksef_hub_web/live/invoice_live/index_test.exs`:
- Around line 118-138: Add a new test (e.g., test "clear_filters preserves type
param", %{conn: conn, company: company}) that opens the LiveView with a URL
containing type=income plus other filters, uses live(...) to mount, verifies a
chip is rendered, triggers the clear_filters event by calling element(view,
"button[phx-click=clear_filters]") |> render_click(), and then asserts the
patched URL still contains type=income (using assert_patched). Ensure the test
mirrors the existing pattern in invoice_live/index_test.exs and uses the same
helpers/variables (live, render, element, render_click, assert_patched) so the
dedicated clear_filters type-preservation logic is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc9541f8-aeee-4dec-8358-fff0d262196f
📒 Files selected for processing (4)
lib/ksef_hub_web/components/core_components.exlib/ksef_hub_web/components/invoice_components.exlib/ksef_hub_web/live/invoice_live/index.extest/ksef_hub_web/live/invoice_live/index_test.exs
…roles Raw params could persist an unauthorized :type (e.g. :income for reviewers) in @filters, leaking into URLs built by remove_filter, clear_filters, and pagination. Now sanitize_type/2 forces :expense before storing in assigns. Also adds a test verifying clear_filters preserves the type param. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/ksef_hub_web/live/invoice_live/index.ex (1)
314-333: Optional accessibility polish for tab UI.Consider adding active-state semantics (e.g.,
aria-current) to the selected tab link to improve screen-reader feedback.🤖 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 314 - 333, Add active-state accessibility by setting aria-current="page" on the selected tab elements: when rendering the .link for Expense and Income (the calls to tab_url/3 and tab_class/1) include aria-current="page" conditionally when `@filters`[:type] matches the tab symbol, and for the fallback <span> (used when !@can_view_all_types) add aria-current="page" since it represents the active tab; use the existing tab_class(`@filters`[:type] == ...) logic to determine the condition and ensure both link and non-link states expose the same active semantics.
🤖 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 314-333: Add active-state accessibility by setting
aria-current="page" on the selected tab elements: when rendering the .link for
Expense and Income (the calls to tab_url/3 and tab_class/1) include
aria-current="page" conditionally when `@filters`[:type] matches the tab symbol,
and for the fallback <span> (used when !@can_view_all_types) add
aria-current="page" since it represents the active tab; use the existing
tab_class(`@filters`[:type] == ...) logic to determine the condition and ensure
both link and non-link states expose the same active semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 431392c4-3f3f-4ca9-8589-5a0ac543f5c6
📒 Files selected for processing (2)
lib/ksef_hub_web/live/invoice_live/index.extest/ksef_hub_web/live/invoice_live/index_test.exs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 31, 2026
6 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
view_all_invoice_typespermission) see a static Expense label instead of clickable tabs<.badge variant="info">for tag list (blue with outline, consistent with other badges)borderto filter chips for visual consistency with badgesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI/UX Improvements