feat: invoice show redesign, design-system kit, and analyst role access#157
Merged
Conversation
Constrain counterparty and title columns to fixed widths with titles for full values on hover, so long names don't push the row layout around. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the stacked sub-sections on the invoice detail page with a tab bar (Payments / Activity / Comments / Access), refreshes the Details and Classification cards to the small-caps label style, and rebuilds the Access tab around a stateful share-link banner and a columned grants table. Highlights - New <.tabs> core component with count badges and URL-backed state via ?tab=<id>; Access count hidden in team-default mode. - Access tab: "Team default" / "Invited only" mode toggle, share-link banner with Create/Copy/Revoke actions (stateful — reuses existing token, displays the URL, and revoke deletes it), and a grants table with User / Role / Granted by / On columns. - Activity timeline redrawn with a left-anchored vertical line and action-colored circular icons (approve→emerald, reject→red, classification/extraction→blue, duplicate→amber, payment_request→ emerald). - Approve/Reject buttons restyled (primary + check / outline + circle-x). - Inline onclick="this.select()" replaced with a SelectOnClick phx-hook. Backend / refactor - Invoices.get_public_token_for/2 and revoke_public_token/2 added on top of existing PublicTokens module; list_access_grants/1 now preloads :granted_by. - create_public_link and revoke_public_link events gated by the existing can_mutate guard. - AccessCard and ActivityTimeline extracted from show.ex into their own modules under KsefHubWeb.InvoiceLive.*. - Distinct role palettes (owner/admin/accountant/approver/editor/ viewer). Tests (+50) - PublicTokens: get_public_token_for hit/miss/expired, revoke_ public_token hit/miss/isolation. - Show LiveView: create→copy→revoke flow, no-link copy error flash, accountant cannot create or revoke share links, team-default and invited-only empty states, grants table columns. - New component tests: core_components_tabs_test, activity_timeline_ test (icon palette branches + describe_action), access_card_test (summary label, granter label, role palette distinctness). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ne kit Expands the admin UI kit with the new empty-state system (Pattern A filter-empty string vs Pattern B <EmptyState> component with default/locked/warning tones), a 3-pane InvoiceDetail layout, and a dev-only Tweaks panel that toggles demo zero-data views across Dashboard, Companies, and Settings. Restores a11y and dark-mode regressions introduced during the Claude Designs import: Button focus-visible ring, MobileNav for sub-md viewports, label/ARIA wiring on input previews, dark-mode CSS var overrides for purple/brand-foreground/brand-strong, localStorage theme persistence, timezone-safe invoice-row date parsing, and the error icon glyph. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a §Empty states section to docs/frontend.md codifying the two- pattern rule (filter-empty terse string vs zero-data <.empty_state> component) with a decision rule, tone table, and six authoring rules. Updates the CoreComponents row for <.empty_state> to reflect the target tone/action API and flags that the Phoenix component in core_components.ex still needs extension before porting. Adds a visible pointer from CLAUDE.md and the top of docs/frontend.md to the prototype kit (DESIGN_SYSTEM.md + docs/design_system/) so UI work reaches for kit tokens and copy instead of inventing its own, without auto-loading the ~260KB kit into every session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ayments polish - Extract NotesCard and CommentsCard modules; show.ex renders the tabs through these sibling components instead of inline markup. - Add <.avatar>, <.empty_state> rich shape (title/description/<:action>/tone) to CoreComponents; AccessCard picks up the shared avatar. - Move has_note?/1 onto the Invoice schema. - Reorder tabs to Activity / Payments / Notes / Comments / Access; tab pill refreshes in place after comment submit/delete and note save via refresh_tabs/1 on the socket. - Soften the activity-icon palette from saturated bg-*-600 to tinted bg-*-100 matching the avatar/role-badge system; add palette branches for comment_*, access_changed, public_link_generated, excluded, included. - Payments tab: Pattern B empty state with "Add payment request" CTA; rows are fully clickable with a hover chevron; header row shows "N payment request(s)" + Add payment button. - Drop two stale purchase_order display tests (the details-table row was removed in the earlier tab redesign; only the edit-form field remains). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oard Upgrades Companies, Tokens, Sync, Certificates, and Dashboard chart empty states to the shared <.empty_state> primitive with kit-canonical copy. Hides table chrome on list pages when empty so a bare header doesn't float above the empty state. Companies copy mentions the invitation path alongside NIP registration, and DESIGN_SYSTEM.md §7 is updated to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…wn company Analyst previously only held :view_invoices, which hid Settings (blocked by :view_dashboard) and the Companies nav (blocked by :manage_company). That left analysts with no way to change theme or register their own company. - Move /settings (General) out of :require_view_dashboard into :authenticated so every company member can reach theme controls. - Drop :manage_company gate on the Companies nav entry — any member can create a new company, where Companies.create_company_with_owner makes them :owner of the new one. - Gate Dashboard nav entry with :view_dashboard to hide it for roles that the route would redirect anyway. - Add :view_syncs and :manage_tokens to analyst permissions so the settings sidebar shows General / Syncs / API Tokens (matching the approver set). :trigger_sync stays owner/admin/approver-only; hide the "Sync Now" button client-side to match the existing server check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds tabbed invoice detail UI (Activity, Payments, Notes, Comments, Access) with public-share create/copy/revoke flows; new LiveView components (AccessCard, ActivityTimeline, CommentsCard, NotesCard); expands core components (tabs, avatar, EmptyState); converts many views to EmptyState patterns; adds public-token APIs/tests and a SelectOnClick LiveView hook. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InvoiceShow as Invoice Show<br/>(LiveView)
participant PublicTokens as PublicTokens<br/>Module
participant Repo as Database
User->>InvoiceShow: Click "Create public link"
InvoiceShow->>PublicTokens: ensure/create token (invoice_id, user_id)
PublicTokens->>Repo: insert or upsert token
Repo-->>PublicTokens: token record
PublicTokens-->>InvoiceShow: token (URL)
InvoiceShow->>User: copy URL + flash "created and copied"
User->>InvoiceShow: Click "Revoke public link"
InvoiceShow->>PublicTokens: revoke_public_token(invoice_id, user_id)
PublicTokens->>Repo: delete matching tokens
Repo-->>PublicTokens: deletion count
PublicTokens-->>InvoiceShow: {:ok, :revoked} or {:ok, :no_op}
InvoiceShow->>User: flash "revoked" / update UI
sequenceDiagram
participant User
participant Tabs as Tabs Component
participant InvoiceShow as Invoice Show<br/>(LiveView)
participant AccessCard as AccessCard<br/>Component
User->>InvoiceShow: Load invoice page
InvoiceShow->>Tabs: render(tab list with counts)
Tabs-->>User: tab bar
User->>Tabs: Click "Access"
Tabs->>InvoiceShow: phx-click select_tab (id=access)
InvoiceShow->>AccessCard: render Access pane
AccessCard-->>User: Access UI (share banner, grants table, revoke actions)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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. 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/design_system/ui_kits/admin/data.jsx (1)
121-127:⚠️ Potential issue | 🟡 MinorFix
CERTfixture inconsistency (expiresvsdaysLeft).
expires: "2026-04-20"conflicts withdaysLeft: 96, so demo UI can show contradictory certificate state.🛠️ Suggested fixture correction
const CERT = { subject: "CN=Appunite sp. z o.o., O=Appunite, C=PL", serial: "03:9A:2B:F1:88:C4:0E:11", issued: "2025-01-14", - expires: "2026-04-20", + expires: "2026-07-25", daysLeft: 96, status: "ok", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design_system/ui_kits/admin/data.jsx` around lines 121 - 127, The CERT fixture has inconsistent expiry data: the expires field ("2026-04-20") does not match daysLeft (96). Update the CERT object (symbol CERT) so the two fields agree—either set expires to a date that is 96 days from the reference date used by the demo or recompute daysLeft based on the expires value; ensure you update the expires and/or daysLeft fields together so the demo UI shows a consistent certificate state.docs/design_system/ui_kits/admin/Dashboard.jsx (1)
96-150:⚠️ Potential issue | 🟡 MinorEmpty-demo mode still shows populated dashboard data.
When
window.KSH_EMPTY_DEMOis true, the stat cards switch to zero-data copy, but the sync-health chart and Queue card still render active job/queue numbers. The demo ends up showing contradictory states on the same screen.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design_system/ui_kits/admin/Dashboard.jsx` around lines 96 - 150, Dashboard currently uses window.KSH_EMPTY_DEMO to alter StatCard content but still renders active data in SyncHealthChart, the Queue Card and RecentSyncJobs, causing mixed states; update the Dashboard component to conditionally render those components/blocks based on the same empty flag (window.KSH_EMPTY_DEMO) — e.g., render a placeholder/empty state or hide SyncHealthChart, the entire Queue Card block (the dl + Open queue Button) and RecentSyncJobs when empty is true, otherwise render the existing components so the page consistently reflects the empty-demo mode.lib/ksef_hub_web/live/invoice_live/show.ex (2)
730-741:⚠️ Potential issue | 🟡 MinorRefresh the tab model after access mutations.
visible_tabs/1derives the Access pill from@invoice.access_restrictedand@access_grants, but these handlers only update the assigns. After toggle/grant/revoke, the tab badge keeps the old count until a full reload.Suggested fix
{:ok, updated} -> {:noreply, socket |> assign( invoice: reload_details(updated, socket), access_grants: Invoices.list_access_grants(updated.id) ) + |> refresh_tabs() |> put_flash( :info, if(new_value, do: "Access restricted.", else: "Access opened to all approvers.") )} @@ {:ok, _grant} -> - {:noreply, assign(socket, access_grants: Invoices.list_access_grants(invoice.id))} + {:noreply, + socket + |> assign(access_grants: Invoices.list_access_grants(invoice.id)) + |> refresh_tabs()} @@ {:ok, _} -> - {:noreply, assign(socket, access_grants: Invoices.list_access_grants(invoice.id))} + {:noreply, + socket + |> assign(access_grants: Invoices.list_access_grants(invoice.id)) + |> refresh_tabs()}Also applies to: 760-763, 777-779
🤖 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 730 - 741, The tab badge isn't refreshed because visible_tabs/1 depends on `@invoice.access_restricted` and `@access_grants` but handlers (e.g., the case handling Invoices.set_access_restricted in show.ex and the similar handlers at the other locations) only update assigns invoice and access_grants; after updating those assigns also recompute and assign the tabs so the UI updates immediately. Concretely, in the block handling Invoices.set_access_restricted (and the grant/revoke handlers referenced) after assigning invoice: reload_details(updated, socket) and access_grants: Invoices.list_access_grants(updated.id), also call assign with tabs: visible_tabs(%{socket.assigns | invoice: reload_details(updated, socket), access_grants: Invoices.list_access_grants(updated.id)}) (or recompute visible_tabs using the new assigns) so visible_tabs/1 sees the new values and the pill count updates.
838-843:⚠️ Potential issue | 🟡 MinorRefresh
@visible_tabswhen streaming new activity.The stream insert increments
:activity_log_count, but the Activity tab count is read from the cached@visible_tabsassign. New events update the timeline immediately while the pill stays stale.Suggested fix
def handle_info({:new_activity, audit_log}, socket) do {:noreply, socket |> assign(:activity_log_empty, false) |> update(:activity_log_count, &(&1 + 1)) + |> refresh_tabs() |> stream_insert(:activity_log, audit_log, at: 0)} end🤖 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 838 - 843, The Activity tab pill stays stale because handle_info/2 (the clause handling {:new_activity, audit_log}) updates :activity_log_count and streams the new entry but does not refresh the cached :visible_tabs assign; update handle_info/2 in show.ex so that after updating :activity_log_count and streaming the log you also update or recompute the :visible_tabs assign (e.g. call the existing function that builds visible tabs or adjust the Activity tab's count inside :visible_tabs) so the UI pill reflects the new :activity_log_count immediately.
🧹 Nitpick comments (6)
test/ksef_hub_web/live/dashboard_live_test.exs (1)
54-57: Prefer selector-based assertions instead of copy checks.Line 56–57 and Line 66–67 assert raw text, which is brittle for UI copy changes. Consider asserting stable empty-state selectors (
id/data-testid) like you already do for chart containers.Test-side refactor example
- assert html =~ "No expenses yet" - assert html =~ "No categorised expenses" + assert has_element?(view, "#expense-empty-state") + assert has_element?(view, "#category-empty-state") - refute html =~ "No expenses yet" - refute html =~ "No categorised expenses" + refute has_element?(view, "#expense-empty-state") + refute has_element?(view, "#category-empty-state")As per coding guidelines: “Instead of relying on testing text content which can change, favor testing for the presence of key elements in LiveView tests”.
Also applies to: 66-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/live/dashboard_live_test.exs` around lines 54 - 57, The test "renders Pattern B empty states when no expenses exist" uses brittle copy-based assertions; update it to assert presence of the empty-state DOM elements instead: replace the html =~ "No expenses yet" and html =~ "No categorised expenses" checks with selector-based assertions against the stable empty-state identifiers used in the UI (e.g., the data-testid or id attributes for the pattern B empty state or chart containers) using the LiveView test helper (the view returned by live/2 and helpers like has_element/2 or render_hook/assert_select equivalents) so the test looks for those elements (e.g., "#pattern-b-empty" or "[data-testid=\"empty-state\"]") rather than exact text; apply the same change to the similar assertions later in this file (the assertions at lines 66–67).lib/ksef_hub_web/live/invoice_live/notes_card.ex (1)
21-24: Consider more specific type annotations for attrs.The
:maptype works but could be more precise:attr :invoice, Invoice.t(), required: true attr :note_form, Phoenix.HTML.Form.t(), required: trueThis is optional since the current types work correctly at runtime.
🤖 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/notes_card.ex` around lines 21 - 24, Replace the generic :map annotations with more specific types for clarity: change the attr declaration for :invoice to use the domain struct type (e.g., Invoice.t()) and change :note_form to use the Phoenix form type (Phoenix.HTML.Form.t()); keep the boolean attrs (:editing_note and :can_mutate) as-is. Locate the attr lines for :invoice, :note_form, :editing_note, and :can_mutate in the NotesCard LiveView component and update the type specs to Invoice.t() and Phoenix.HTML.Form.t() respectively so tooling and dialyzer can better understand the shapes.test/ksef_hub_web/components/core_components_avatar_test.exs (1)
1-6: Add a@moduledocfor the new test module.This is a new module, but it doesn't document its purpose yet.
As per coding guidelines, "
**/*.{ex,exs}: Every module must have a@moduledocdescribing the module's purpose".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/components/core_components_avatar_test.exs` around lines 1 - 6, Add a descriptive `@moduledoc` to the test module KsefHubWeb.CoreComponents.AvatarTest: insert a module-level `@moduledoc` attribute right after the defmodule line that briefly explains the purpose of this test module (e.g., testing avatar-related core component rendering/behavior) so the module complies with the rule that every .ex/.exs module must have documentation.docs/design_system/ui_kits/admin/Primitives.jsx (1)
137-152: Preventlockedempty states from rendering CTAs.
actionis currently rendered for every tone, so one bad call site can violate the newlockedcontract. It’s safer to enforce that rule inside the primitive.Proposed fix
const EmptyState = ({ icon = "info", title, sub, action, tone = "default", className = "" }) => { const tones = { default: "bg-[var(--muted)] text-[var(--muted-foreground)] border-[var(--border)]", locked: "bg-[var(--muted)] text-[var(--muted-foreground)] border-[var(--border)] opacity-80", warning: "bg-[color-mix(in_oklch,var(--warning)_12%,transparent)] text-[var(--warning)] border-[color-mix(in_oklch,var(--warning)_30%,transparent)]", }; return ( <div className={`flex flex-col items-center text-center gap-3 py-14 px-6 ${className}`}> <span className={`inline-flex items-center justify-center w-10 h-10 rounded-full border ${tones[tone]}`}> <Icon name={icon} size={18} /> </span> <div className="space-y-1 max-w-sm"> <div className="text-sm font-medium text-[var(--foreground)]">{title}</div> {sub && <div className="text-xs text-[var(--muted-foreground)] leading-relaxed">{sub}</div>} </div> - {action && <div className="mt-1">{action}</div>} + {tone !== "locked" && action && <div className="mt-1">{action}</div>} </div> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design_system/ui_kits/admin/Primitives.jsx` around lines 137 - 152, The EmptyState component currently renders the action prop regardless of tone; enforce the locked contract by only rendering the CTA when action is present and tone !== "locked". Update the EmptyState return logic (component name: EmptyState, props: action and tone) so the action div is conditionally shown only if action && tone !== "locked" to prevent any call site from displaying CTAs for locked empty states.test/ksef_hub_web/live/invoice_live/activity_timeline_test.exs (1)
1-4: Add a@moduledocfor the new test module.This new module is missing the repository-required top-level docs.
As per coding guidelines, "
**/*.{ex,exs}: Every module must have a@moduledocdescribing the module's purpose".🤖 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/activity_timeline_test.exs` around lines 1 - 4, Add a top-level `@moduledoc` to the test module KsefHubWeb.InvoiceLive.ActivityTimelineTest that briefly describes the purpose of the tests (e.g., "Tests for ActivityTimeline live view behavior and rendering"); place the `@moduledoc` annotation directly under the defmodule line for KsefHubWeb.InvoiceLive.ActivityTimelineTest so the module has the required documentation per repository guidelines.lib/ksef_hub_web/live/invoice_live/comments_card.ex (1)
35-36: Documentcomments_card/1.The module has a
@moduledoc, but this public component entrypoint is still missing its@doc.As per coding guidelines, "
**/*.{ex,exs}: Every public function must have a@docannotation".🤖 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/comments_card.ex` around lines 35 - 36, Add a `@doc` for the public component comments_card/1: describe its purpose (renders the comments card UI for an invoice), document the single argument (assigns map expected keys like :invoice or :comments if applicable) and the return type (Phoenix.LiveView.Rendered.t()), and include any important usage notes or options; place the `@doc` immediately above the `@spec/def` for comments_card to satisfy the project rule that every public function has documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DESIGN_SYSTEM.md`:
- Around line 257-265: Update the "Companies" row in the "App-wide examples"
table in DESIGN_SYSTEM.md so its empty-state copy matches the prototype: replace
the old message with "Add your first company — Register a NIP to start syncing
invoices, or wait for an invitation from an existing company." Ensure the
"Companies" row under the "App-wide examples" heading exactly matches the string
rendered by the UI kit/prototype to keep the spec canonical.
- Around line 269-278: The handoff workflow references the wrong path
`docs/design-system/`; update the instructions in DESIGN_SYSTEM.md (the "Handoff
workflow (recommended)" section and any occurrences of `docs/design-system/`) to
use the actual repo locations used in this PR (`docs/design_system/` and the
repo-root `DESIGN_SYSTEM.md`) so links and steps point to the correct files;
ensure example commands and PR text in steps 1–3 consistently mention
`docs/design_system/` (and clarify when the file is at repo root) and update any
sample paths like `ui_kits/admin/Payments.jsx` only if they too should reference
the underscore-style directory.
In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx`:
- Around line 543-547: The current code injects c.body via
dangerouslySetInnerHTML which allows untrusted HTML/JS execution; replace this
by escaping HTML and rendering mentions as React nodes instead of raw HTML.
Implement a helper like renderCommentBody(body) that splits c.body by the
mention regex /(@[\w\s.]+)(?=[,\s]|$)/g, maps parts to either a <span
className="font-medium text-[var(--info)]"> mention node or plain text fragments
(with keys), and then remove the dangerouslySetInnerHTML usage in the
inline-block div so its children are the output of renderCommentBody(c.body);
ensure you preserve the existing conditional classes (mine branch) and return
React nodes (not HTML strings).
In `@lib/ksef_hub_web/live/invoice_live/activity_timeline.ex`:
- Around line 103-105: The HEEx template contains improper whitespace/formatting
in the span that renders actor_label and the result of describe_action(entry),
which causes mix format failures; update the HEEx expression to follow standard
formatting (e.g., remove extra spaces between elements and ensure proper
interpolation/use of braces) around {entry.actor_label || "System"} and
{describe_action(entry)} in the template within activity_timeline.ex so the file
passes mix format --check-formatted; locate the span rendering these expressions
and reformat it to a single consistent HEEx expression to satisfy formatter
rules.
In `@lib/ksef_hub/invoices.ex`:
- Around line 199-201: The revoke_public_token flow must be changed to accept an
opts map (with user_id and actor_label), use TrackedRepo.delete_all instead of
Repo.delete_all, and emit an activity event (e.g.,
Events.invoice_public_token_revoked) with actor context; update the function in
PublicTokens (revoke_public_token/3 or revoke_public_token(invoice_id, user_id,
opts)) to call TrackedRepo.delete_all and then push the event via Events with
the invoice_id and opts actor info, change the delegate in invoices.ex
(defdelegate revoke_public_token(...)) to pass through the opts parameter, and
update the caller in lib/ksef_hub_web/live/invoice_live/show.ex to pass
actor_opts(socket) as the opts argument so the actor_label/user_id travel with
the deletion and event emission.
In `@lib/ksef_hub/invoices/public_tokens.ex`:
- Around line 77-89: Revoke currently deletes the InvoicePublicToken but doesn't
emit an audit event; add a new helper Events.invoice_public_link_revoked/3 (or
matching signature used by invoice_public_link_generated) in the Events module
and invoke it from the LiveView handler immediately after
revoke_public_token(invoice_id, user_id) returns :ok. Keep revoke_public_token
as-is (it must use Repo.delete_all), and call Events.invoice_public_link_revoked
with the invoice_id, user_id and any contextual metadata from the LiveView so
the revocation is recorded (do not attempt to use TrackedRepo since delete_all
has no changeset).
In `@test/ksef_hub_web/live/certificate_live_test.exs`:
- Line 72: The test uses raw HTML matching via render(view) =~ "No certificate
uploaded", which is brittle; replace it with a LiveView selector assertion using
has_element?/3 to check the presence of the message in the rendered DOM (e.g.,
assert has_element?(view, "<appropriate selector>", "No certificate uploaded")).
Update the assertion that references the view variable (view) to call
has_element?/3 with the correct CSS selector used elsewhere in this test file
(e.g., the paragraph, div, or element class that contains the message) so the
test verifies the element content via selector rather than raw HTML string
matching.
In `@test/ksef_hub_web/live/invoice_live/show_test.exs`:
- Around line 1556-1558: The call site for render_click(view, "select_tab",
%{"id" => "definitely_not_a_real_tab_#{System.unique_integer([:positive])}"}) is
not formatted per mix format; reformat this invocation (or run mix format on the
file) so the arguments and interpolation comply with formatter rules, ensuring
the render_click/3 call is properly wrapped/indented and the map argument is
formatted consistently.
---
Outside diff comments:
In `@docs/design_system/ui_kits/admin/Dashboard.jsx`:
- Around line 96-150: Dashboard currently uses window.KSH_EMPTY_DEMO to alter
StatCard content but still renders active data in SyncHealthChart, the Queue
Card and RecentSyncJobs, causing mixed states; update the Dashboard component to
conditionally render those components/blocks based on the same empty flag
(window.KSH_EMPTY_DEMO) — e.g., render a placeholder/empty state or hide
SyncHealthChart, the entire Queue Card block (the dl + Open queue Button) and
RecentSyncJobs when empty is true, otherwise render the existing components so
the page consistently reflects the empty-demo mode.
In `@docs/design_system/ui_kits/admin/data.jsx`:
- Around line 121-127: The CERT fixture has inconsistent expiry data: the
expires field ("2026-04-20") does not match daysLeft (96). Update the CERT
object (symbol CERT) so the two fields agree—either set expires to a date that
is 96 days from the reference date used by the demo or recompute daysLeft based
on the expires value; ensure you update the expires and/or daysLeft fields
together so the demo UI shows a consistent certificate state.
In `@lib/ksef_hub_web/live/invoice_live/show.ex`:
- Around line 730-741: The tab badge isn't refreshed because visible_tabs/1
depends on `@invoice.access_restricted` and `@access_grants` but handlers (e.g., the
case handling Invoices.set_access_restricted in show.ex and the similar handlers
at the other locations) only update assigns invoice and access_grants; after
updating those assigns also recompute and assign the tabs so the UI updates
immediately. Concretely, in the block handling Invoices.set_access_restricted
(and the grant/revoke handlers referenced) after assigning invoice:
reload_details(updated, socket) and access_grants:
Invoices.list_access_grants(updated.id), also call assign with tabs:
visible_tabs(%{socket.assigns | invoice: reload_details(updated, socket),
access_grants: Invoices.list_access_grants(updated.id)}) (or recompute
visible_tabs using the new assigns) so visible_tabs/1 sees the new values and
the pill count updates.
- Around line 838-843: The Activity tab pill stays stale because handle_info/2
(the clause handling {:new_activity, audit_log}) updates :activity_log_count and
streams the new entry but does not refresh the cached :visible_tabs assign;
update handle_info/2 in show.ex so that after updating :activity_log_count and
streaming the log you also update or recompute the :visible_tabs assign (e.g.
call the existing function that builds visible tabs or adjust the Activity tab's
count inside :visible_tabs) so the UI pill reflects the new :activity_log_count
immediately.
---
Nitpick comments:
In `@docs/design_system/ui_kits/admin/Primitives.jsx`:
- Around line 137-152: The EmptyState component currently renders the action
prop regardless of tone; enforce the locked contract by only rendering the CTA
when action is present and tone !== "locked". Update the EmptyState return logic
(component name: EmptyState, props: action and tone) so the action div is
conditionally shown only if action && tone !== "locked" to prevent any call site
from displaying CTAs for locked empty states.
In `@lib/ksef_hub_web/live/invoice_live/comments_card.ex`:
- Around line 35-36: Add a `@doc` for the public component comments_card/1:
describe its purpose (renders the comments card UI for an invoice), document the
single argument (assigns map expected keys like :invoice or :comments if
applicable) and the return type (Phoenix.LiveView.Rendered.t()), and include any
important usage notes or options; place the `@doc` immediately above the `@spec/def`
for comments_card to satisfy the project rule that every public function has
documentation.
In `@lib/ksef_hub_web/live/invoice_live/notes_card.ex`:
- Around line 21-24: Replace the generic :map annotations with more specific
types for clarity: change the attr declaration for :invoice to use the domain
struct type (e.g., Invoice.t()) and change :note_form to use the Phoenix form
type (Phoenix.HTML.Form.t()); keep the boolean attrs (:editing_note and
:can_mutate) as-is. Locate the attr lines for :invoice, :note_form,
:editing_note, and :can_mutate in the NotesCard LiveView component and update
the type specs to Invoice.t() and Phoenix.HTML.Form.t() respectively so tooling
and dialyzer can better understand the shapes.
In `@test/ksef_hub_web/components/core_components_avatar_test.exs`:
- Around line 1-6: Add a descriptive `@moduledoc` to the test module
KsefHubWeb.CoreComponents.AvatarTest: insert a module-level `@moduledoc` attribute
right after the defmodule line that briefly explains the purpose of this test
module (e.g., testing avatar-related core component rendering/behavior) so the
module complies with the rule that every .ex/.exs module must have
documentation.
In `@test/ksef_hub_web/live/dashboard_live_test.exs`:
- Around line 54-57: The test "renders Pattern B empty states when no expenses
exist" uses brittle copy-based assertions; update it to assert presence of the
empty-state DOM elements instead: replace the html =~ "No expenses yet" and html
=~ "No categorised expenses" checks with selector-based assertions against the
stable empty-state identifiers used in the UI (e.g., the data-testid or id
attributes for the pattern B empty state or chart containers) using the LiveView
test helper (the view returned by live/2 and helpers like has_element/2 or
render_hook/assert_select equivalents) so the test looks for those elements
(e.g., "#pattern-b-empty" or "[data-testid=\"empty-state\"]") rather than exact
text; apply the same change to the similar assertions later in this file (the
assertions at lines 66–67).
In `@test/ksef_hub_web/live/invoice_live/activity_timeline_test.exs`:
- Around line 1-4: Add a top-level `@moduledoc` to the test module
KsefHubWeb.InvoiceLive.ActivityTimelineTest that briefly describes the purpose
of the tests (e.g., "Tests for ActivityTimeline live view behavior and
rendering"); place the `@moduledoc` annotation directly under the defmodule line
for KsefHubWeb.InvoiceLive.ActivityTimelineTest so the module has the required
documentation per repository guidelines.
🪄 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: 8a9fb2f7-f5a9-42aa-a31e-275aedc7650f
📒 Files selected for processing (49)
CLAUDE.mdDESIGN_SYSTEM.mdassets/js/app.jsdocs/design_system/README.mddocs/design_system/preview/components-buttons.htmldocs/design_system/ui_kits/admin/AppShell.jsxdocs/design_system/ui_kits/admin/Dashboard.jsxdocs/design_system/ui_kits/admin/InvoiceDetail.jsxdocs/design_system/ui_kits/admin/Payments.jsxdocs/design_system/ui_kits/admin/Primitives.jsxdocs/design_system/ui_kits/admin/Settings.jsxdocs/design_system/ui_kits/admin/data.jsxdocs/design_system/ui_kits/admin/index.htmldocs/frontend.mdlib/ksef_hub/authorization.exlib/ksef_hub/invoices.exlib/ksef_hub/invoices/access_control.exlib/ksef_hub/invoices/invoice.exlib/ksef_hub/invoices/public_tokens.exlib/ksef_hub_web/components/core_components.exlib/ksef_hub_web/components/invoice_components.exlib/ksef_hub_web/components/layouts.exlib/ksef_hub_web/live/certificate_live.exlib/ksef_hub_web/live/company_live/index.exlib/ksef_hub_web/live/dashboard_live.exlib/ksef_hub_web/live/invoice_live/access_card.exlib/ksef_hub_web/live/invoice_live/activity_timeline.exlib/ksef_hub_web/live/invoice_live/comments_card.exlib/ksef_hub_web/live/invoice_live/index.exlib/ksef_hub_web/live/invoice_live/notes_card.exlib/ksef_hub_web/live/invoice_live/show.exlib/ksef_hub_web/live/payment_request_live/index.exlib/ksef_hub_web/live/sync_live.exlib/ksef_hub_web/live/token_live.exlib/ksef_hub_web/router.extest/ksef_hub/authorization_test.exstest/ksef_hub/invoices/invoice_test.exstest/ksef_hub/invoices/public_token_test.exstest/ksef_hub_web/components/core_components_avatar_test.exstest/ksef_hub_web/components/core_components_empty_state_test.exstest/ksef_hub_web/components/core_components_tabs_test.exstest/ksef_hub_web/live/certificate_live_test.exstest/ksef_hub_web/live/company_live_test.exstest/ksef_hub_web/live/dashboard_live_test.exstest/ksef_hub_web/live/invoice_live/access_card_test.exstest/ksef_hub_web/live/invoice_live/activity_timeline_test.exstest/ksef_hub_web/live/invoice_live/show_test.exstest/ksef_hub_web/live/live_auth_test.exstest/ksef_hub_web/live/sync_live_test.exs
Comment on lines
+257
to
+265
| ### App-wide examples | ||
|
|
||
| | Surface | State | Tone | Copy | | ||
| |---|---|---|---| | ||
| | Dashboard tile | New tenant, no invoices yet | `default` | *No invoices this month yet* — Sync runs hourly, or add one manually. | | ||
| | Companies | New tenant | `default` | *Add your first company* — Register a NIP to start syncing invoices, or wait for an invitation from an existing company. | | ||
| | Settings → Certificates | Not uploaded | `warning` | *No certificate uploaded* — Upload a XAdES certificate to enable KSeF sync. | | ||
| | Sync jobs | New tenant | `default` | *No sync jobs yet* — KSeF sync runs hourly, or trigger one manually. | | ||
| | Payments list | New tenant | `default` | *No payment requests yet* — Approve an expense to generate your first payment. | |
There was a problem hiding this comment.
Align the Companies empty-state copy with the prototype.
This table still uses the older Companies message, while docs/design_system/ui_kits/admin/index.html now renders the new “Register a NIP…” copy. If the spec and the kit diverge here, porters won’t know which string is canonical.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@DESIGN_SYSTEM.md` around lines 257 - 265, Update the "Companies" row in the
"App-wide examples" table in DESIGN_SYSTEM.md so its empty-state copy matches
the prototype: replace the old message with "Add your first company — Register a
NIP to start syncing invoices, or wait for an invitation from an existing
company." Ensure the "Companies" row under the "App-wide examples" heading
exactly matches the string rendered by the UI kit/prototype to keep the spec
canonical.
Comment on lines
+199
to
+201
| @doc "Revokes the public sharing token for a given (invoice, user) pair." | ||
| @spec revoke_public_token(Ecto.UUID.t(), Ecto.UUID.t()) :: :ok | ||
| defdelegate revoke_public_token(invoice_id, user_id), to: PublicTokens |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify public token revoke API shape + activity tracking coverage
fd -i "public_tokens.ex"
rg -n --type=elixir -C3 '\bdef\s+(ensure_public_token|get_public_token_for|revoke_public_token)\b'
rg -n --type=elixir -C3 '\brevoke_public_token\('
rg -n --type=elixir -C3 'TrackedRepo|Events\.|actor_label|user_id|actor_type'Repository: appunite/appunite-ksef-ex
Length of output: 50382
🏁 Script executed:
cat -n lib/ksef_hub/invoices/public_tokens.ex | sed -n '78,95p'Repository: appunite/appunite-ksef-ex
Length of output: 850
🏁 Script executed:
cat -n lib/ksef_hub_web/live/invoice_live/show.ex | sed -n '475,490p'Repository: appunite/appunite-ksef-ex
Length of output: 566
🏁 Script executed:
rg -n "revoke.*public.*token" test/ksef_hub_web/live/invoice_live/show_test.exs -A 5 -B 5Repository: appunite/appunite-ksef-ex
Length of output: 368
🏁 Script executed:
rg -n "activity_event|TrackedRepo" lib/ksef_hub/invoices/ -lRepository: appunite/appunite-ksef-ex
Length of output: 184
🏁 Script executed:
rg -n "TrackedRepo" lib/ksef_hub/invoices/access_control.ex -B 3 -A 3 | head -50Repository: appunite/appunite-ksef-ex
Length of output: 594
🏁 Script executed:
rg -n "grant_access|revoke_access" lib/ksef_hub_web/live/invoice_live/show_test.exs -A 8 | head -40Repository: appunite/appunite-ksef-ex
Length of output: 157
🏁 Script executed:
find test -name "*show_test*" -type f | head -5Repository: appunite/appunite-ksef-ex
Length of output: 229
🏁 Script executed:
rg -n "grant_access|revoke_access" test/ -l | head -10Repository: appunite/appunite-ksef-ex
Length of output: 307
🏁 Script executed:
rg -n "grant_access|revoke_access" test/ksef_hub/activity_log/integration_test.exs -B 3 -A 8Repository: appunite/appunite-ksef-ex
Length of output: 1549
🏁 Script executed:
rg -n "public.*token" test/ksef_hub/activity_log/integration_test.exs -B 3 -A 8Repository: appunite/appunite-ksef-ex
Length of output: 51
🏁 Script executed:
rg -n "ensure_public_token" lib/ksef_hub/invoices/public_tokens.ex -A 15 | head -40Repository: appunite/appunite-ksef-ex
Length of output: 819
🏁 Script executed:
rg -n "Events\." lib/ksef_hub/invoices/access_control.ex | head -20Repository: appunite/appunite-ksef-ex
Length of output: 269
🏁 Script executed:
rg -n "rotate_public_token" lib/ksef_hub/invoices/public_tokens.ex -A 10 | head -30Repository: appunite/appunite-ksef-ex
Length of output: 1091
🏁 Script executed:
cat -n lib/ksef_hub/invoices.ex | sed -n '190,210p'Repository: appunite/appunite-ksef-ex
Length of output: 1428
Add activity event emission and actor context to public token revocation.
Token revocation is a user-visible state change that should be tracked in the activity log. Currently, revoke_public_token in lib/ksef_hub/invoices/public_tokens.ex uses plain Repo.delete_all() and does not emit any activity events, unlike similar mutations like revoke_access in access_control.ex which use TrackedRepo and emit Events.invoice_access_revoked.
Update the implementation to:
- Accept an
optsparameter (containinguser_idandactor_label) - Use
TrackedRepo.delete_all()instead of plainRepo - Emit an appropriate activity event (e.g.,
Events.invoice_public_token_revoked) - Update the delegate signature in
lib/ksef_hub/invoices.exto pass opts through - Update the call site in
lib/ksef_hub_web/live/invoice_live/show.ex:480to passactor_opts(socket)
See revoke_access in access_control.ex (lines 204–206) for the established pattern.
🤖 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 199 - 201, The revoke_public_token
flow must be changed to accept an opts map (with user_id and actor_label), use
TrackedRepo.delete_all instead of Repo.delete_all, and emit an activity event
(e.g., Events.invoice_public_token_revoked) with actor context; update the
function in PublicTokens (revoke_public_token/3 or
revoke_public_token(invoice_id, user_id, opts)) to call TrackedRepo.delete_all
and then push the event via Events with the invoice_id and opts actor info,
change the delegate in invoices.ex (defdelegate revoke_public_token(...)) to
pass through the opts parameter, and update the caller in
lib/ksef_hub_web/live/invoice_live/show.ex to pass actor_opts(socket) as the
opts argument so the actor_label/user_id travel with the deletion and event
emission.
- Emit invoice.public_link_revoked so revocations appear in the activity log - Refresh visible_tabs after access changes and new activity so pill counts stay live - EmptyState suppresses CTAs when tone=locked; Dashboard demo gates chart + queue on KSH_EMPTY_DEMO - Render comment bodies as React nodes (no dangerouslySetInnerHTML) - Fix mix format loop in activity_timeline.ex around middle-dot separator - Point DESIGN_SYSTEM.md handoff section at the real repo paths Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/design_system/ui_kits/admin/InvoiceDetail.jsx (1)
670-671: Keep the Payments pill count aligned with the rendered fallback.
PaymentsTabfalls back towindow.DETAIL_PAYMENTS.default, but the badge here falls back to[]. Any invoice without an explicit payments entry can therefore show a0count while the tab renders demo rows.Proposed fix
- const payments = window.DETAIL_PAYMENTS[inv.id] || []; + const payments = + window.DETAIL_PAYMENTS[inv.id] || window.DETAIL_PAYMENTS.default || [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx` around lines 670 - 671, The payments badge fallback is inconsistent with PaymentsTab: change the payments lookup so it uses window.DETAIL_PAYMENTS.default when inv.id has no entry (i.e., replace the current fallback of []), ensuring the variable named payments is set via window.DETAIL_PAYMENTS[inv.id] || window.DETAIL_PAYMENTS.default so the pill count matches what PaymentsTab renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DESIGN_SYSTEM.md`:
- Around line 229-245: The spec contradicts the implemented component:
Primitives.jsx exposes an EmptyState with a warning tint but rule 5 in
DESIGN_SYSTEM.md forbids color; resolve by updating the spec to allow an
explicit exception for tone="warning" (used for failures) — state that
EmptyState with tone="warning" may use the warning tint and a muted circle icon
variant, keep other tones colorless, and ensure the guidance for tone="warning"
still requires a retry CTA and succinct sub text; reference EmptyState and
Primitives.jsx when making this clarification.
In `@lib/ksef_hub_web/live/invoice_live/activity_timeline.ex`:
- Around line 19-39: The new events are not explicitly handled: add exact
entries for "invoice.re_extraction_triggered", "invoice.access_granted", and
"invoice.access_revoked" to the `@action_icons` map with appropriate icon names,
add a prefix entry {"invoice.re_extraction_", "<suitable‑icon>"} to
`@action_prefix_icons` if you want prefix matching for re-extraction variants, and
also add "invoice.access_granted" and "invoice.access_revoked" to the file's
split-text/actions-with-two-part-text list (the module attribute in the same
file that controls split text rendering) so access events get the intended split
text treatment rather than falling back to the generic info icon/palette.
In `@lib/ksef_hub_web/live/invoice_live/show.ex`:
- Around line 476-486: The handler for the "revoke_public_link" event
unconditionally calls Events.invoice_public_link_revoked after
Invoices.revoke_public_token/2 returns :ok, but revoke_public_token can be a
no-op (no existing token) so we must avoid emitting the revocation event in that
case; modify the handle_event("revoke_public_link", ...) in show.ex to first
check whether the invoice actually had a public token (e.g. inspect
invoice.public_token or invoice.public_link) or change
Invoices.revoke_public_token/2 to return an indicator (e.g. {:ok, :revoked} vs
{:ok, :no_token}) and only call Events.invoice_public_link_revoked when a real
revocation occurred, leaving the socket flash and assign behavior unchanged for
the no-op path.
- Around line 1312-1328: The empty-state CTA currently renders whenever
`@invoice_payment_requests` == [] and `@can_manage_payment_requests` &&
`@invoice.type` == :expense, which shows the "Add payment request" button for
rejected expenses; update the action guard to also require the invoice is not
rejected (e.g., add && `@invoice.status` != :rejected) so the <:action> only
appears for non-rejected invoices. Locate the empty state block around
`@invoice_payment_requests` and change the <:action :if={...}> condition (the line
referencing `@can_manage_payment_requests` and `@invoice.type`) to include the
additional `@invoice.status` != :rejected check.
---
Nitpick comments:
In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx`:
- Around line 670-671: The payments badge fallback is inconsistent with
PaymentsTab: change the payments lookup so it uses
window.DETAIL_PAYMENTS.default when inv.id has no entry (i.e., replace the
current fallback of []), ensuring the variable named payments is set via
window.DETAIL_PAYMENTS[inv.id] || window.DETAIL_PAYMENTS.default so the pill
count matches what PaymentsTab renders.
🪄 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: abe6049c-b7ae-43d8-9ffd-8905c34efdd1
📒 Files selected for processing (10)
DESIGN_SYSTEM.mddocs/design_system/ui_kits/admin/Dashboard.jsxdocs/design_system/ui_kits/admin/InvoiceDetail.jsxdocs/design_system/ui_kits/admin/Primitives.jsxlib/ksef_hub/activity_log/events.exlib/ksef_hub_web/live/invoice_live/activity_timeline.exlib/ksef_hub_web/live/invoice_live/comments_card.exlib/ksef_hub_web/live/invoice_live/show.extest/ksef_hub_web/live/certificate_live_test.exstest/ksef_hub_web/live/invoice_live/show_test.exs
✅ Files skipped from review due to trivial changes (1)
- test/ksef_hub_web/live/certificate_live_test.exs
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/design_system/ui_kits/admin/Dashboard.jsx
- lib/ksef_hub_web/live/invoice_live/comments_card.ex
- test/ksef_hub_web/live/invoice_live/show_test.exs
…ed payments
- revoke_public_token/2 returns {:ok, :revoked | :no_op} so the LiveView only emits
invoice.public_link_revoked when a token actually existed
- Timeline gets icons, palette, and descriptions for invoice.access_granted,
invoice.access_revoked, and invoice.re_extraction_triggered
- Hide "Add payment request" CTA on rejected expenses
- Payments pill in InvoiceDetail prototype now matches PaymentsTab (fallback to default)
- DESIGN_SYSTEM.md rule 5 carves an explicit exception for tone="warning"
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
lib/ksef_hub_web/live/invoice_live/activity_timeline.ex (1)
249-258: Consider emptychanged_fieldsedge case.The guard
fields != []handles empty list, but ifmetadata["changed_fields"]is a non-list truthy value (e.g., a string from malformed data), the pattern won't match and falls through to the catch-all. This is safe behavior but worth noting.🤖 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/activity_timeline.ex` around lines 249 - 258, The describe_dynamic_action/2 clause for "invoice.updated" should explicitly handle non-list or unexpected types for metadata["changed_fields"] instead of silently falling to the generic branch; update the function to detect when changed_fields is a binary (string) or single value and normalize it to a list (e.g., split a comma-separated string or wrap a single value) before calling humanize_field, and keep the existing empty-list and fallback behavior; refer to describe_dynamic_action and humanize_field when making the change so the normalization happens prior to Enum.map_join.DESIGN_SYSTEM.md (1)
86-91: Clarify thatactioncan usevariant="outline"as well.The spec states
actionis an "optional<Button>(primary,size="sm")", but the prototype (InvoiceDetail.jsxline 539) usesvariant="outline"for some empty-state actions. Consider updating to "optional<Button>(size="sm", typicallyprimaryoroutline)" to match actual usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DESIGN_SYSTEM.md` around lines 86 - 91, Update the EmptyState docs to reflect actual usage: change the `action` description to "optional <Button> (size='sm', typically primary or outline)" so it matches uses like the variant="outline" Button in InvoiceDetail.jsx; specifically update the `<EmptyState icon title sub action tone>` entry to mention both primary and outline as common variants and keep the size guidance.docs/design_system/ui_kits/admin/InvoiceDetail.jsx (1)
232-246: Consider adding ARIA roles for accessibility.The tab strip works well visually but could benefit from semantic ARIA attributes for screen readers.
♿ Suggested accessibility improvement
-const TabStrip = ({ value, onChange, tabs }) => ( - <div className="mb-5 border-b border-[var(--border)] flex items-center gap-0 overflow-x-auto"> +const TabStrip = ({ value, onChange, tabs }) => ( + <div role="tablist" className="mb-5 border-b border-[var(--border)] flex items-center gap-0 overflow-x-auto"> {tabs.map(t => { const active = value === t.id; return ( - <button key={t.id} onClick={() => onChange(t.id)} + <button key={t.id} role="tab" aria-selected={active} onClick={() => onChange(t.id)} className={...}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx` around lines 232 - 246, The TabStrip component should expose proper ARIA semantics: add role="tablist" to the wrapping div and for each tab button (in TabStrip's map) set role="tab", id={`tab-${t.id}`}, aria-controls={`panel-${t.id}`}, aria-selected={active} and tabIndex={active ? 0 : -1}; also add keyboard support (handle Enter/Space in the button's onKeyDown to call onChange(t.id)) so screen readers and keyboard users can navigate; update any corresponding panels to use id={`panel-${t.id}`} and role="tabpanel" to match aria-controls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx`:
- Around line 515-517: The effect invokes lookup() but omits lookup from the
dependency array, causing a stale-closure/lint issue; update the React.useEffect
that calls setComments(lookup()) to either include lookup in the dependency
array (e.g., [inv.id, lookup]) or move the lookup logic inline inside the effect
so dependencies are explicit; ensure the effect still depends on inv.id and any
values used by lookup to prevent stale data.
- Around line 705-708: The JSX uses TypePill, KindBadge, StatusBadge, and
PaymentBadge but they are not imported in this file; add imports for TypePill,
KindBadge, StatusBadge, and PaymentBadge from the module that defines them
(InvoicesList.jsx) at the top of the file so the components referenced in the
render (TypePill, KindBadge, StatusBadge, PaymentBadge) are available to
InvoiceDetail.jsx.
---
Nitpick comments:
In `@DESIGN_SYSTEM.md`:
- Around line 86-91: Update the EmptyState docs to reflect actual usage: change
the `action` description to "optional <Button> (size='sm', typically primary or
outline)" so it matches uses like the variant="outline" Button in
InvoiceDetail.jsx; specifically update the `<EmptyState icon title sub action
tone>` entry to mention both primary and outline as common variants and keep the
size guidance.
In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx`:
- Around line 232-246: The TabStrip component should expose proper ARIA
semantics: add role="tablist" to the wrapping div and for each tab button (in
TabStrip's map) set role="tab", id={`tab-${t.id}`},
aria-controls={`panel-${t.id}`}, aria-selected={active} and tabIndex={active ? 0
: -1}; also add keyboard support (handle Enter/Space in the button's onKeyDown
to call onChange(t.id)) so screen readers and keyboard users can navigate;
update any corresponding panels to use id={`panel-${t.id}`} and role="tabpanel"
to match aria-controls.
In `@lib/ksef_hub_web/live/invoice_live/activity_timeline.ex`:
- Around line 249-258: The describe_dynamic_action/2 clause for
"invoice.updated" should explicitly handle non-list or unexpected types for
metadata["changed_fields"] instead of silently falling to the generic branch;
update the function to detect when changed_fields is a binary (string) or single
value and normalize it to a list (e.g., split a comma-separated string or wrap a
single value) before calling humanize_field, and keep the existing empty-list
and fallback behavior; refer to describe_dynamic_action and humanize_field when
making the change so the normalization happens prior to Enum.map_join.
🪄 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: f2935d90-f74e-4e5a-a92d-d467bdf5131a
📒 Files selected for processing (7)
DESIGN_SYSTEM.mddocs/design_system/ui_kits/admin/InvoiceDetail.jsxlib/ksef_hub/invoices.exlib/ksef_hub/invoices/public_tokens.exlib/ksef_hub_web/live/invoice_live/activity_timeline.exlib/ksef_hub_web/live/invoice_live/show.extest/ksef_hub/invoices/public_token_test.exs
🚧 Files skipped from review as they are similar to previous changes (2)
- test/ksef_hub/invoices/public_token_test.exs
- lib/ksef_hub/invoices/public_tokens.ex
Comment on lines
705
to
+708
| <TypePill type={inv.type} /> | ||
| <KindBadge kind={inv.kind} /> | ||
| <StatusBadge status={inv.status} /> | ||
| <PaymentBadge status={inv.paymentStatus} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for these component definitions in the prototype kit
rg -n "const (TypePill|KindBadge|StatusBadge|PaymentBadge)" docs/design_system/Repository: appunite/appunite-ksef-ex
Length of output: 424
🏁 Script executed:
rg -n "import|from" docs/design_system/ui_kits/admin/InvoiceDetail.jsx | head -30Repository: appunite/appunite-ksef-ex
Length of output: 233
🏁 Script executed:
head -60 docs/design_system/ui_kits/admin/InvoiceDetail.jsxRepository: appunite/appunite-ksef-ex
Length of output: 3212
🏁 Script executed:
grep -n "^import\|^const.*Badge\|^const.*Pill\|^const TypePill\|TypePill\|KindBadge\|StatusBadge\|PaymentBadge" docs/design_system/ui_kits/admin/InvoiceDetail.jsxRepository: appunite/appunite-ksef-ex
Length of output: 317
🏁 Script executed:
wc -l docs/design_system/ui_kits/admin/InvoiceDetail.jsx && rg "^import|^const|^function" docs/design_system/ui_kits/admin/InvoiceDetail.jsx | head -20Repository: appunite/appunite-ksef-ex
Length of output: 813
🏁 Script executed:
sed -n '700,715p' docs/design_system/ui_kits/admin/InvoiceDetail.jsxRepository: appunite/appunite-ksef-ex
Length of output: 871
🏁 Script executed:
sed -n '1,10p' docs/design_system/ui_kits/admin/InvoiceDetail.jsx && echo "---" && sed -n '580,595p' docs/design_system/ui_kits/admin/InvoiceDetail.jsxRepository: appunite/appunite-ksef-ex
Length of output: 1182
Import TypePill, KindBadge, StatusBadge, and PaymentBadge from InvoicesList.jsx.
These components are used at lines 705–708 but are not imported or defined in this file. They are defined in InvoicesList.jsx and must be imported for the code to function correctly. Add import statements at the top of the file to make these components available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design_system/ui_kits/admin/InvoiceDetail.jsx` around lines 705 - 708,
The JSX uses TypePill, KindBadge, StatusBadge, and PaymentBadge but they are not
imported in this file; add imports for TypePill, KindBadge, StatusBadge, and
PaymentBadge from the module that defines them (InvoicesList.jsx) at the top of
the file so the components referenced in the render (TypePill, KindBadge,
StatusBadge, PaymentBadge) are available to InvoiceDetail.jsx.
- TabStrip carries role=tablist/tab with aria-selected, aria-controls, and Enter/Space activation - Panel wrapper gets role=tabpanel + aria-labelledby - Hoist lookupComments so the comments effect has no stale-closure - Export Status/Kind/Type/Payment badges from InvoicesList so InvoiceDetail can render them - Document outline as a valid EmptyState action variant Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
DESIGN_SYSTEM.md (1)
255-255: Minor: Consider rewording to avoid "only" repetition.The static analysis flagged "Only owner" and "Only you" as adverb repetition. Consider:
*You're the sole user with access*or*No one else has access yet*.✏️ Suggested alternative
-| Access | Only owner | `default` | *Only you have access* — Share this invoice with teammates to collaborate. | +| Access | Only owner | `default` | *You're the sole user with access* — Share this invoice with teammates to collaborate. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DESIGN_SYSTEM.md` at line 255, The phrase repeats "Only" in the Access table row: the label "Only owner" and the description "*Only you have access* — Share this invoice..."; update both to avoid adverb repetition by replacing "Only owner" with a clearer label like "Owner only" or "Sole owner" and change the description to a rephrasing such as "*You're the sole user with access* — Share this invoice with teammates to collaborate." Locate and edit the exact table row string "Access | Only owner | `default` | *Only you have access* — Share this invoice with teammates to collaborate." and apply the new label and description consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@DESIGN_SYSTEM.md`:
- Line 255: The phrase repeats "Only" in the Access table row: the label "Only
owner" and the description "*Only you have access* — Share this invoice...";
update both to avoid adverb repetition by replacing "Only owner" with a clearer
label like "Owner only" or "Sole owner" and change the description to a
rephrasing such as "*You're the sole user with access* — Share this invoice with
teammates to collaborate." Locate and edit the exact table row string "Access |
Only owner | `default` | *Only you have access* — Share this invoice with
teammates to collaborate." and apply the new label and description consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 933270db-9bd3-4952-bc82-d6329dd7a7d5
📒 Files selected for processing (3)
DESIGN_SYSTEM.mddocs/design_system/ui_kits/admin/InvoiceDetail.jsxdocs/design_system/ui_kits/admin/InvoicesList.jsx
Avoid repeating "Only" in both label and copy. Co-Authored-By: Claude Opus 4.7 (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.
Summary
Three coordinated threads on this branch:
EmptyStateprimitive, invoice-detail 3-pane kit, Pattern B empty states adopted across Settings, Companies, Dashboard, Certificates, Syncs, Tokens, and Payment Requests. Component tests added for avatars, empty state, and tabs.Role-access changes (new in last commit)
/settings(General):view_dashboardgate):manage_company:view_dashboard[:view_invoices][:view_invoices, :view_syncs, :manage_tokens]:trigger_syncTest plan
mix teston affected suites (auth, live_auth, company_live, sync_live, token_live, settings general) — 67 tests green/settingsshows theme toggle + sidebar (General/Syncs/API Tokens)/settings/syncs— no "Sync Now" button; approver sees it🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UX / Visual
Documentation
Authorization