Feat/cloud run deploy#22
Conversation
Replace colorful Phoenix/Elixir themes with neutral zinc-based palette mimicking shadcn/ui defaults. Replace all shadow-sm/shadow-xl with border border-base-300 for a flat, clean look. Replace DaisyUI stat component with plain bordered divs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace colorful DaisyUI component classes (badge-success, badge-warning, alert-info, table-zebra, card-body, etc.) with subtle, bordered pill styles using Tailwind opacity modifiers (bg-success/10, text-success, border-success/20). This creates a consistent, flat, neutral aesthetic across all pages while keeping DaisyUI structural components (drawer, dropdown, menu) intact. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reduce Docker build context and prevent secrets from leaking. docker-compose.yml provides Gotenberg sidecar for local dev. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- KsefHub.Release: migrate/0 and rollback/2 for prod releases - GET /healthz returns 200 JSON for Cloud Run probes - Health route placed outside auth pipelines Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Provisions Artifact Registry, service accounts, Workload Identity Federation for GitHub OIDC, and Secret Manager secrets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Declarative service.yaml with ksef-hub (ingress) + gotenberg (sidecar), health probes, secret references, and 0-3 autoscaling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Runs on PR + push to main: deps, format check, compile with warnings as errors, credo, and tests against PostgreSQL 16. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Authenticates via Workload Identity Federation, builds and pushes Docker image to Artifact Registry, runs DB migrations via Cloud Run Jobs, and deploys multi-container service. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add PORT, PHX_SERVER, POOL_SIZE, and ECTO_IPV6 entries used by the Cloud Run deployment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds CI/CD and deployment workflows, GCP provisioning and Docker tooling, a Cloud Run service (with gotenberg sidecar), a health endpoint and release helpers, extensive Tailwind/DaisyUI theme and component restyling, LiveView token streaming, updated tests, and multiple ignore/config files. Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant WIF as Workload Identity (GCP)
participant AR as Artifact Registry
participant CR as Cloud Run
participant DB as Postgres DB
GHA->>WIF: Request OIDC credentials
activate WIF
WIF-->>GHA: Return short-lived credentials
deactivate WIF
GHA->>AR: Build & push Docker image
activate AR
AR-->>GHA: Return image URL/digest
deactivate AR
GHA->>CR: Trigger migration job using pushed image
activate CR
CR->>DB: Run Ecto migrations
DB-->>CR: Return migration result
deactivate CR
GHA->>CR: Deploy service referencing image
CR-->>GHA: Deployment complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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
🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Around line 40-84: Quote all unquoted variable expansions and the image/tag
arguments in the workflow steps to satisfy shellcheck (e.g.,
"${GCP_REGION}-docker.pkg.dev", "${IMAGE_NAME}:${{ github.sha }}",
"${IMAGE_NAME}:latest", "${SERVICE_NAME}", "${GCP_PROJECT_ID}"); ensure the
docker build/push tags and sed replacement use quoted expansions; change the
gcloud run jobs --args usage to pass a single quoted comma-separated string
(e.g., --args "eval,KsefHub.Release.migrate()") both in the execute and create
branches; and quote the --set-secrets value and service-account expansion when
creating the job and in the gcloud run services add-iam-policy-binding command
to avoid word-splitting and SC2086/SC2140 warnings.
🧹 Nitpick comments (9)
.dockerignore (1)
1-19: Consider adding.github/to reduce Docker build context.The
.gcloudignorefile excludes.github/but this file doesn't. Adding it would reduce the Docker build context size and maintain consistency between ignore files.Suggested addition
docs/ +.github/lib/ksef_hub_web/live/token_live.ex (2)
119-119: Use<.form>component instead of plain<form>tag.Per coding guidelines, always use the imported
Phoenix.Component.form/1function component for forms. This ensures consistent behavior with LiveView form handling.Suggested fix
- <form phx-submit="create" phx-change="validate" class="space-y-4 mt-2"> + <.form for={`@form`} phx-submit="create" phx-change="validate" class="space-y-4 mt-2" id="create-token-form">And close with
</.form>instead of</form>.
172-173: Add@specto private functions.Per coding guidelines, every function (public and private) must have a
@spectypespec.Suggested fix
+ `@spec` format_datetime(DateTime.t() | NaiveDateTime.t() | nil) :: String.t() defp format_datetime(nil), do: "Never" defp format_datetime(dt), do: Calendar.strftime(dt, "%Y-%m-%d %H:%M")test/ksef_hub_web/live/dashboard_live_test.exs (1)
38-40: Consider using more specific element IDs for stat assertions.The current assertions all target
[class*='text-2xl font-bold']which matches multiple elements. While this works, it doesn't verify which stat card displays which count. Addingidattributes to the stat elements in the LiveView template would enable more precise assertions likehas_element?(view, "#income-count", "2").test/ksef_hub_web/live/invoice_live/show_test.exs (1)
76-76: Consider more specific selectors for status badge assertions.The
[class*=rounded-md]selector is quite broad and could match unintended elements. Sincestatus_badgeis a reusable component, consider adding adata-testid="status-badge"attribute to make tests more resilient to styling changes.Also applies to: 90-90, 106-107, 117-117
lib/ksef_hub_web/components/invoice_components.ex (2)
12-16: Consider adding explicit fallback styling for unknown types.If an unexpected type value is passed, only the base classes apply without any color styling. While this may be intentional (graceful degradation), you might want to add an explicit fallback for visual consistency:
"inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border", `@type` == "income" && "bg-success/10 text-success border-success/20", - `@type` == "expense" && "bg-warning/10 text-warning border-warning/20" + `@type` == "expense" && "bg-warning/10 text-warning border-warning/20", + `@type` not in ["income", "expense"] && "bg-base-200 text-base-content/60 border-base-300" ]}>This matches the fallback pattern used in
status_classes/1insync_live.ex.
26-31: Same consideration for status_badge fallback styling.Similar to
type_badge, consider adding explicit fallback styling for unknown status values:Suggested change
"inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border", `@status` == "pending" && "bg-warning/10 text-warning border-warning/20", `@status` == "approved" && "bg-success/10 text-success border-success/20", - `@status` == "rejected" && "bg-error/10 text-error border-error/20" + `@status` == "rejected" && "bg-error/10 text-error border-error/20", + `@status` not in ["pending", "approved", "rejected"] && "bg-base-200 text-base-content/60 border-base-300" ]}>lib/ksef_hub_web/live/dashboard_live.ex (1)
162-176: Missing@specforstat_card/1.Per coding guidelines, every function (public and private) must have a
@spectypespec.Add typespec for stat_card
+ `@spec` stat_card(map()) :: Phoenix.LiveView.Rendered.t() defp stat_card(assigns) do assigns = assign_new(assigns, :color, fn -> "text-base-content" end)As per coding guidelines: "Every function (public and private) must have
@spectypespec".lib/ksef_hub_web/components/core_components.ex (1)
274-281: Missing@specfor privateerror/1helper.Per coding guidelines, every function must have a
@spectypespec.Add typespec for error
# Helper used by inputs to generate form errors + `@spec` error(map()) :: Phoenix.LiveView.Rendered.t() defp error(assigns) doAs per coding guidelines: "Every function (public and private) must have
@spectypespec".
- .dockerignore: add .github/ to reduce build context - deploy.yml: quote all variable expansions, fix --args syntax - token_live: use <.form> component, add @SPEC on format_datetime/1 - invoice_components: add fallback styling for unknown type/status - dashboard_live: add @SPEC on stat_card/1 - core_components: add @SPEC on error/1 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ksef_hub_web/live/token_live.ex (1)
135-167:⚠️ Potential issue | 🟠 MajorSwitch tokens list to LiveView streams to prevent memory bloat.
The LiveView assigns
@tokensas a plain list inmount/3and on create/revoke events by refetching all tokens. Per coding guidelines, collections must use streams to avoid memory ballooning. Updatemount/3to initialize the stream, usestream()orstream(..., reset: true)on create/revoke instead of full list reassignment, tracktokens_countseparately, and update the template to consume stream tuples withphx-update="stream"on the parent container.🔧 Required changes
In
mount/3:tokens = Accounts.list_api_tokens(socket.assigns.current_user.id) {:ok, socket |> stream(:tokens, tokens) |> assign( page_title: "API Tokens", tokens_count: length(tokens), # ... rest of assigns )}In
handle_event("create", ...)andhandle_event("revoke", ...):
Replaceassign(tokens: tokens)with:|> stream(:tokens, tokens, reset: true) |> assign(tokens_count: length(tokens))In template:
<div class="mt-6 overflow-x-auto" phx-update="stream" id="tokens"> <.table id="tokens" rows={`@streams.tokens`} row_id={fn {id, _} -> id end}> <:col :let={{_id, token}} label="Name">{token.name}</:col> <!-- Apply {_id, token} pattern to all columns --> </.table> </div> <p :if={`@tokens_count` == 0} class="text-center text-base-content/60 py-8"> No API tokens yet. Create one to get started. </p>
🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Around line 55-69: The migration step currently executes an existing job which
can use a stale image and hides stderr; change it to first upsert/update the job
with the new image and same flags (for job name ksef-hub-migrate, use --image
"${IMAGE_NAME}:${{ github.sha }}", --set-secrets and --service-account) so the
job definition is updated if it exists (use gcloud run jobs update or create as
appropriate), then run the job separately with gcloud run jobs execute (no
redirection of stderr like 2>/dev/null) to ensure the migration runs with the
latest image and real errors surface.
In `@lib/ksef_hub_web/live/token_live.ex`:
- Around line 172-174: Add `@doc` and `@spec` annotations for the module's public
LiveView callbacks: annotate mount/3, handle_event/3 and render/1. For each
function add a short descriptive `@doc` string and a `@spec`: e.g. mount(params ::
map(), session :: map(), socket :: Phoenix.LiveView.Socket.t()) :: {:ok,
Phoenix.LiveView.Socket.t()} | {:ok, Phoenix.LiveView.Socket.t(), keyword()},
handle_event(event :: String.t() | atom(), params :: map(), socket ::
Phoenix.LiveView.Socket.t()) :: {:noreply, Phoenix.LiveView.Socket.t()} |
{:reply, map(), Phoenix.LiveView.Socket.t()}, and render(assigns :: map()) ::
Phoenix.LiveView.Rendered.t(); place these annotations immediately above the
corresponding function definitions (mount/3, handle_event/3, render/1).
🧹 Nitpick comments (2)
.github/workflows/deploy.yml (1)
78-84: Consider logging when IAM binding fails for non-idempotency reasons.The
2>/dev/null || truepattern silently ignores all errors. While this is appropriate for idempotency (binding already exists), it could mask permission issues or misconfigurations.♻️ Optional: Log errors while still allowing failure
- name: Ensure service allows traffic run: | gcloud run services add-iam-policy-binding "${SERVICE_NAME}" \ --region "${GCP_REGION}" \ --member="allUsers" \ --role="roles/run.invoker" \ - --quiet 2>/dev/null || true + --quiet || echo "::warning::IAM binding may already exist or failed"lib/ksef_hub_web/live/dashboard_live.ex (1)
111-126: Extract the duplicated expense status rows into a reusable function component.Three nearly identical row blocks (Pending, Approved, Rejected) repeat the same layout with only the label, badge color, and value changing. Consolidating these into a stateless function component improves maintainability.
♻️ Suggested refactor
<div class="space-y-2.5"> - <div class="flex items-center justify-between"> - <span class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-warning/10 text-warning border-warning/20">Pending</span> - <span class="font-mono text-sm">{`@pending_expense`}</span> - </div> - <div class="flex items-center justify-between"> - <span class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-success/10 text-success border-success/20">Approved</span> - <span class="font-mono text-sm">{`@approved_expense`}</span> - </div> - <div class="flex items-center justify-between"> - <span class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-error/10 text-error border-error/20">Rejected</span> - <span class="font-mono text-sm">{`@rejected_expense`}</span> - </div> + <.expense_status_row + label="Pending" + value={`@pending_expense`} + badge_class="bg-warning/10 text-warning border-warning/20" + /> + <.expense_status_row + label="Approved" + value={`@approved_expense`} + badge_class="bg-success/10 text-success border-success/20" + /> + <.expense_status_row + label="Rejected" + value={`@rejected_expense`} + badge_class="bg-error/10 text-error border-error/20" + /> </div> </div> </div> + `@spec` expense_status_row(map()) :: Phoenix.LiveView.Rendered.t() + defp expense_status_row(assigns) do + ~H""" + <div class="flex items-center justify-between"> + <span class={["inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border", `@badge_class`]}> + {`@label`} + </span> + <span class="font-mono text-sm">{`@value`}</span> + </div> + """ + end
| - name: Run database migrations | ||
| run: | | ||
| gcloud run jobs execute ksef-hub-migrate \ | ||
| --region "${GCP_REGION}" \ | ||
| --wait \ | ||
| --args "eval,KsefHub.Release.migrate()" \ | ||
| 2>/dev/null || \ | ||
| gcloud run jobs create ksef-hub-migrate \ | ||
| --region "${GCP_REGION}" \ | ||
| --image "${IMAGE_NAME}:${{ github.sha }}" \ | ||
| --set-secrets "DATABASE_URL=database-url:latest,SECRET_KEY_BASE=secret-key-base:latest" \ | ||
| --service-account "ksef-hub-runner@${GCP_PROJECT_ID}.iam.gserviceaccount.com" \ | ||
| --execute-now \ | ||
| --wait \ | ||
| --args "eval,KsefHub.Release.migrate()" |
There was a problem hiding this comment.
Migration job uses stale image when the job already exists.
When gcloud run jobs execute runs on an existing job, it uses the image the job was created with—not the newly pushed image. This means migrations for new code changes won't actually run.
Additionally, 2>/dev/null suppresses all stderr, potentially masking real errors (not just "job not found").
🐛 Proposed fix: Update job image before execution
- name: Run database migrations
run: |
- gcloud run jobs execute ksef-hub-migrate \
- --region "${GCP_REGION}" \
- --wait \
- --args "eval,KsefHub.Release.migrate()" \
- 2>/dev/null || \
- gcloud run jobs create ksef-hub-migrate \
+ gcloud run jobs update ksef-hub-migrate \
--region "${GCP_REGION}" \
--image "${IMAGE_NAME}:${{ github.sha }}" \
--set-secrets "DATABASE_URL=database-url:latest,SECRET_KEY_BASE=secret-key-base:latest" \
--service-account "ksef-hub-runner@${GCP_PROJECT_ID}.iam.gserviceaccount.com" \
- --execute-now \
+ --args "eval,KsefHub.Release.migrate()" \
+ 2>/dev/null || \
+ gcloud run jobs create ksef-hub-migrate \
+ --region "${GCP_REGION}" \
+ --image "${IMAGE_NAME}:${{ github.sha }}" \
+ --set-secrets "DATABASE_URL=database-url:latest,SECRET_KEY_BASE=secret-key-base:latest" \
+ --service-account "ksef-hub-runner@${GCP_PROJECT_ID}.iam.gserviceaccount.com" \
+ --args "eval,KsefHub.Release.migrate()"
+
+ gcloud run jobs execute ksef-hub-migrate \
+ --region "${GCP_REGION}" \
--wait \
- --args "eval,KsefHub.Release.migrate()"
+ --args "eval,KsefHub.Release.migrate()"This approach:
- Updates the existing job with the new image (or creates it if it doesn't exist)
- Then executes the job separately, ensuring it always runs with the latest image
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Run database migrations | |
| run: | | |
| gcloud run jobs execute ksef-hub-migrate \ | |
| --region "${GCP_REGION}" \ | |
| --wait \ | |
| --args "eval,KsefHub.Release.migrate()" \ | |
| 2>/dev/null || \ | |
| gcloud run jobs create ksef-hub-migrate \ | |
| --region "${GCP_REGION}" \ | |
| --image "${IMAGE_NAME}:${{ github.sha }}" \ | |
| --set-secrets "DATABASE_URL=database-url:latest,SECRET_KEY_BASE=secret-key-base:latest" \ | |
| --service-account "ksef-hub-runner@${GCP_PROJECT_ID}.iam.gserviceaccount.com" \ | |
| --execute-now \ | |
| --wait \ | |
| --args "eval,KsefHub.Release.migrate()" | |
| - name: Run database migrations | |
| run: | | |
| gcloud run jobs update ksef-hub-migrate \ | |
| --region "${GCP_REGION}" \ | |
| --image "${IMAGE_NAME}:${{ github.sha }}" \ | |
| --set-secrets "DATABASE_URL=database-url:latest,SECRET_KEY_BASE=secret-key-base:latest" \ | |
| --service-account "ksef-hub-runner@${GCP_PROJECT_ID}.iam.gserviceaccount.com" \ | |
| --args "eval,KsefHub.Release.migrate()" \ | |
| 2>/dev/null || \ | |
| gcloud run jobs create ksef-hub-migrate \ | |
| --region "${GCP_REGION}" \ | |
| --image "${IMAGE_NAME}:${{ github.sha }}" \ | |
| --set-secrets "DATABASE_URL=database-url:latest,SECRET_KEY_BASE=secret-key-base:latest" \ | |
| --service-account "ksef-hub-runner@${GCP_PROJECT_ID}.iam.gserviceaccount.com" \ | |
| --args "eval,KsefHub.Release.migrate()" | |
| gcloud run jobs execute ksef-hub-migrate \ | |
| --region "${GCP_REGION}" \ | |
| --wait \ | |
| --args "eval,KsefHub.Release.migrate()" |
🤖 Prompt for AI Agents
In @.github/workflows/deploy.yml around lines 55 - 69, The migration step
currently executes an existing job which can use a stale image and hides stderr;
change it to first upsert/update the job with the new image and same flags (for
job name ksef-hub-migrate, use --image "${IMAGE_NAME}:${{ github.sha }}",
--set-secrets and --service-account) so the job definition is updated if it
exists (use gcloud run jobs update or create as appropriate), then run the job
separately with gcloud run jobs execute (no redirection of stderr like
2>/dev/null) to ensure the migration runs with the latest image and real errors
surface.
| @spec format_datetime(DateTime.t() | NaiveDateTime.t() | nil) :: String.t() | ||
| defp format_datetime(nil), do: "Never" | ||
| defp format_datetime(dt), do: Calendar.strftime(dt, "%Y-%m-%d %H:%M") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the relevant sections
find . -name "token_live.ex" -type f | head -5Repository: appunite/appunite-ksef-ex
Length of output: 106
🏁 Script executed:
# Get the file size to see if it's reasonable to read
wc -l lib/ksef_hub_web/live/token_live.exRepository: appunite/appunite-ksef-ex
Length of output: 108
🏁 Script executed:
# Check the actual content around the mentioned lines and find the functions
cat -n lib/ksef_hub_web/live/token_live.ex | head -200Repository: appunite/appunite-ksef-ex
Length of output: 7371
Add @doc and @spec annotations to all public functions.
The module has @moduledoc, and the private helper format_datetime/1 has @spec, but all public functions (mount/3, handle_event/3, and render/1) lack both @doc and @spec annotations. These are required per lib/ guidelines.
🧾 Example (apply similarly to other public functions)
+ `@doc` "Initializes the API token management LiveView."
+ `@spec` mount(map(), map(), Phoenix.LiveView.Socket.t()) :: {:ok, Phoenix.LiveView.Socket.t()}
def mount(_params, _session, socket) do🤖 Prompt for AI Agents
In `@lib/ksef_hub_web/live/token_live.ex` around lines 172 - 174, Add `@doc` and
`@spec` annotations for the module's public LiveView callbacks: annotate mount/3,
handle_event/3 and render/1. For each function add a short descriptive `@doc`
string and a `@spec`: e.g. mount(params :: map(), session :: map(), socket ::
Phoenix.LiveView.Socket.t()) :: {:ok, Phoenix.LiveView.Socket.t()} | {:ok,
Phoenix.LiveView.Socket.t(), keyword()}, handle_event(event :: String.t() |
atom(), params :: map(), socket :: Phoenix.LiveView.Socket.t()) :: {:noreply,
Phoenix.LiveView.Socket.t()} | {:reply, map(), Phoenix.LiveView.Socket.t()}, and
render(assigns :: map()) :: Phoenix.LiveView.Rendered.t(); place these
annotations immediately above the corresponding function definitions (mount/3,
handle_event/3, render/1).
Fixes credo cyclomatic complexity warning (was 10, max 9). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/ksef_hub_web/live/token_live.ex`:
- Around line 159-170: The template currently iterates over `@tokens` and checks
`@tokens` == [] which must be changed to use LiveView streams to avoid large
memory use: in the LiveView module where you assign `@tokens` (e.g., mount/3 or
fetch_tokens in TokenLive.*) replace assigning a raw list with stream(:tokens,
tokens_list) using Phoenix.LiveView.stream/3 and also keep a separate assign
like :tokens_count = length(tokens_list) for empty-state checks; update the
template to iterate over `@streams.tokens` (e.g., :for={{token <-
`@streams.tokens`}}) and put phx-update="stream" on the container (tbody or
wrapper) so you can use stream_insert/stream_delete/stream_replace in callbacks
when tokens change instead of reassigning the whole list. Ensure code
references: the assign of `@tokens` (replace usages of `@tokens`), the stream/3
call, and the template iteration over `@streams.tokens` plus tokens_count for the
empty check.
🧹 Nitpick comments (1)
lib/ksef_hub_web/live/certificate_live.ex (1)
219-230: Consider extracting the status badge into a helper component.The status badge styling (lines 221-224 and 227) duplicates the pattern used in the Active Certificate section (lines 151-153). For maintainability, consider extracting this into a small private function component.
♻️ Optional refactor to extract status badge
+ defp status_badge(assigns) do + ~H""" + <span + :if={`@active`} + class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-success/10 text-success border-success/20" + > + Active + </span> + <span + :if={!@active} + class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-base-200 text-base-content/60 border-base-300" + > + Inactive + </span> + """ + endThen use
<.status_badge active={cred.is_active} />in both locations.
- token_live: replace @tokens list with stream(:tokens) to reduce memory usage; use stream_insert for create/revoke updates - certificate_live: extract duplicated status badge into active_badge/1 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ksef_hub_web/live/certificate_live.ex (1)
205-230:⚠️ Potential issue | 🟠 MajorSwitch certificates table to LiveView streams.
@credentialsis still rendered as a plain list in this LiveView, which conflicts with the project's streams requirement. Please convert the collection to a stream and replace the@credentials != []check with a boolean assign since streams aren't enumerable.💡 Suggested update (template + assigns)
- <div :if={`@credentials` != []} class="mt-6"> + <div :if={`@has_credentials`} class="mt-6"> <h2 class="text-lg font-semibold mb-3">All Certificates</h2> <div class="overflow-x-auto"> - <.table id="credentials" rows={`@credentials`} row_id={fn c -> "cred-#{c.id}" end}> - <:col :let={cred} label="NIP">{cred.nip}</:col> - <:col :let={cred} label="Subject">{cred.certificate_subject || "-"}</:col> - <:col :let={cred} label="Expires"> + <.table id="credentials" rows={`@streams.credentials`} row_id={fn {id, _} -> id end}> + <:col :let={{_id, cred}} label="NIP">{cred.nip}</:col> + <:col :let={{_id, cred}} label="Subject">{cred.certificate_subject || "-"}</:col> + <:col :let={{_id, cred}} label="Expires"> {if cred.certificate_expires_at, do: Calendar.strftime(cred.certificate_expires_at, "%Y-%m-%d"), else: "-"} </:col> - <:col :let={cred} label="Status"> + <:col :let={{_id, cred}} label="Status"> <.active_badge active={cred.is_active} /> </:col> - <:action :let={cred}> + <:action :let={{_id, cred}}> <button :if={cred.is_active} phx-click="deactivate"- defp load_credentials(%{assigns: %{current_company: nil}} = socket) do - assign(socket, credentials: [], active_credential: nil) - end + defp load_credentials(%{assigns: %{current_company: nil}} = socket) do + socket + |> assign(has_credentials: false, active_credential: nil) + |> stream(:credentials, [], reset: true) + end @@ - defp load_credentials(%{assigns: %{current_company: company}} = socket) do - credentials = Credentials.list_credentials(company.id) - active = Credentials.get_active_credential(company.id) - - assign(socket, - credentials: credentials, - active_credential: active - ) - end + defp load_credentials(%{assigns: %{current_company: company}} = socket) do + credentials = Credentials.list_credentials(company.id) + active = Credentials.get_active_credential(company.id) + + socket + |> assign(has_credentials: credentials != [], active_credential: active) + |> stream(:credentials, credentials, reset: true) + end
🧹 Nitpick comments (1)
lib/ksef_hub_web/live/certificate_live.ex (1)
236-252: Refactoractive_badge/1to use pattern-matched clauses instead of inline conditionals.Function components should leverage pattern matching in function heads over conditional rendering in templates. The suggested refactor eliminates the
:ifdirectives and makes the component's intent explicit across separate clauses.♻️ Suggested refactor
`@spec` active_badge(map()) :: Phoenix.LiveView.Rendered.t() -defp active_badge(assigns) do - ~H""" - <span - :if={`@active`} - class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-success/10 text-success border-success/20" - > - Active - </span> - <span - :if={!@active} - class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-base-200 text-base-content/60 border-base-300" - > - Inactive - </span> - """ -end +defp active_badge(%{active: true} = assigns) do + ~H""" + <span class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-success/10 text-success border-success/20"> + Active + </span> + """ +end + +defp active_badge(%{active: active} = assigns) when active in [false, nil] do + ~H""" + <span class="inline-flex items-center px-2 py-0.5 rounded-md text-xs font-medium border bg-base-200 text-base-content/60 border-base-300"> + Inactive + </span> + """ +end
…badge - Replace @credentials list with stream(:credentials) and @has_credentials boolean for empty-state check - Refactor active_badge/1 to use pattern matching in function heads Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
UI Updates
Infrastructure
Tests