feat: certificate expiry warning and error alerts#121
Merged
Conversation
Confirmed duplicates are treated as discarded, so showing extraction warnings is noise. Suppress the "incomplete" badge and "missing data" alert when duplicate_status is :confirmed — on both list and detail views. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Show warning alert 7 days before certificate expiration on the invoices list and certificate settings pages. Show error alert when certificate has already expired. Extracted shared CertificateComponents module to keep alert markup DRY across both pages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds certificate expiry status computation and UI alerts: a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
Client->>LiveView: HTTP/WebSocket request (page render)
LiveView->>Credentials: certificate_expiry_status(company_id)
Credentials->>Repo/DB: get_certificate_for_company(company_id)
Repo/DB-->>Credentials: certificate (with not_after) or nil
Credentials-->>LiveView: expiry status (:ok/:no_certificate/:expiring_soon/:expired)
LiveView->>CertificateComponents: render cert_expiry_alert(status, link_target)
CertificateComponents-->>Client: HTML banner (or empty)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/ksef_hub_web/live/certificate_live_test.exs (2)
469-482: Add element presence assertion before text check.This test only verifies text content without confirming the banner element exists. If "1 day" appears elsewhere on the page, this could pass incorrectly. Consider adding an explicit element check for consistency with the other expiry tests.
♻️ Suggested improvement
{:ok, view, _html} = live(conn, ~p"/c/#{company.id}/settings/certificates") - assert render(view) =~ "1 day" + assert has_element?(view, "[data-testid='certificate-expiring-banner']") + assert has_element?(view, "[data-testid='certificate-expiring-banner']", "1 day")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/live/certificate_live_test.exs` around lines 469 - 482, The test "shows singular 'day' when 1 day left" only asserts the string via render(view) but doesn't confirm the expiry banner element exists; update the test to first assert the specific banner element is present (use the same selector used by the other expiry tests, e.g. has_element?(view, "<banner_selector>") or equivalent) against the view returned by live(conn, ~p"/c/#{company.id}/settings/certificates") and then assert render(view) =~ "1 day" to ensure the text is coming from the expected UI element.
446-449: Consider usinghas_element?/3for text assertions.Per coding guidelines, prefer
has_element?over raw HTML matching withrender(view) =~. The third argument tohas_element?/3accepts a text filter:♻️ Suggested refactor using has_element?/3
{:ok, view, _html} = live(conn, ~p"/c/#{company.id}/settings/certificates") assert has_element?(view, "[data-testid='certificate-expired-banner']") - assert render(view) =~ "Certificate expired" - assert render(view) =~ "no longer working" + assert has_element?(view, "[data-testid='certificate-expired-banner']", "Certificate expired") + assert has_element?(view, "[data-testid='certificate-expired-banner']", "no longer working")This applies similarly to lines 465-466 for the expiring soon test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/live/certificate_live_test.exs` around lines 446 - 449, Replace raw HTML text assertions that use render(view) =~ with has_element?/3 checks: for the expired banner use assertions like has_element?(view, "[data-testid='certificate-expired-banner']", "Certificate expired") and has_element?(view, "[data-testid='certificate-expired-banner']", "no longer working") instead of render(view) =~ "..."; do the same for the expiring-soon test (lines around 465-466) by checking the appropriate data-testid and expected text via has_element?/3 using the same view variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/ksef_hub_web/live/certificate_live_test.exs`:
- Around line 469-482: The test "shows singular 'day' when 1 day left" only
asserts the string via render(view) but doesn't confirm the expiry banner
element exists; update the test to first assert the specific banner element is
present (use the same selector used by the other expiry tests, e.g.
has_element?(view, "<banner_selector>") or equivalent) against the view returned
by live(conn, ~p"/c/#{company.id}/settings/certificates") and then assert
render(view) =~ "1 day" to ensure the text is coming from the expected UI
element.
- Around line 446-449: Replace raw HTML text assertions that use render(view) =~
with has_element?/3 checks: for the expired banner use assertions like
has_element?(view, "[data-testid='certificate-expired-banner']", "Certificate
expired") and has_element?(view, "[data-testid='certificate-expired-banner']",
"no longer working") instead of render(view) =~ "..."; do the same for the
expiring-soon test (lines around 465-466) by checking the appropriate
data-testid and expected text via has_element?/3 using the same view variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54cc1295-0caf-4591-ba40-b0e2dd7c59b3
📒 Files selected for processing (9)
lib/ksef_hub/credentials.exlib/ksef_hub_web/components/certificate_components.exlib/ksef_hub_web/components/invoice_components.exlib/ksef_hub_web/live/certificate_live.exlib/ksef_hub_web/live/invoice_live/index.exlib/ksef_hub_web/live/invoice_live/show.extest/ksef_hub/credentials_test.exstest/ksef_hub_web/live/certificate_live_test.exstest/ksef_hub_web/live/invoice_live/index_test.exs
Replace loose render(view) =~ checks with has_element?/3 scoped to the specific data-testid banner selectors, ensuring text assertions target the correct UI element. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Credentials.certificate_expiry_status/1context function with typed return valuesCertificateComponentsmodule to keep alert markup DRY across pagesTest plan
mix format— cleanmix credo --strict— zero issues🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests