fix: auto-create credentials for all user's companies on cert upload#110
Merged
emilwojtaszek merged 4 commits intoApr 1, 2026
Merged
Conversation
When a user uploads a KSeF certificate, credentials were only created for the currently selected company. If the user owned multiple companies, the others would never sync until re-uploading the same certificate. Now certificate upload creates credentials and triggers auth for all the user's companies. Additionally, creating a new company auto-creates a credential if the user already has a certificate. Extracts ensure_credential_for_company/1 into the Credentials context to avoid duplicating business logic across LiveViews. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Credentials.ensure_credential_for_company/1 and updates certificate upload and company-creation flows to ensure credentials for all companies a user owns, enqueueing auth jobs per company; includes new Companies.list_owned_companies_for_user/1, AuthWorker.enqueue/1, and related tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CertificateLive as Certificate LiveView
participant Companies as Companies Module
participant Credentials as Credentials Module
participant AuthWorker as AuthWorker (Oban)
User->>CertificateLive: Upload certificate
CertificateLive->>Companies: list_owned_companies_for_user(user.id)
Companies-->>CertificateLive: [company1, company2, ...]
loop per company
CertificateLive->>Credentials: ensure_credential_for_company(company.id)
alt credential exists
Credentials-->>CertificateLive: {:ok, credential}
else create credential
Credentials->>Credentials: replace_active_credential(company.id, %{})
Credentials-->>CertificateLive: {:ok, new_credential}
end
CertificateLive->>AuthWorker: enqueue(company.id)
AuthWorker-->>CertificateLive: :ok
end
CertificateLive-->>User: confirm upload and enqueued auth jobs
sequenceDiagram
participant User
participant CompanyForm as Company Form LiveView
participant Companies as Companies Module
participant Credentials as Credentials Module
participant AuthWorker as AuthWorker (Oban)
User->>CompanyForm: Submit create company form
CompanyForm->>Companies: create_company_with_owner(user, params)
Companies-->>CompanyForm: {:ok, %{company: company}}
CompanyForm->>Credentials: get_active_user_certificate(user.id)
alt user has certificate
Credentials-->>CompanyForm: certificate
CompanyForm->>Credentials: ensure_credential_for_company(company.id)
Credentials-->>CompanyForm: {:ok, credential}
CompanyForm->>AuthWorker: enqueue(company.id)
AuthWorker-->>CompanyForm: :ok
else no certificate
Credentials-->>CompanyForm: nil
end
CompanyForm-->>User: company created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
183-185:⚠️ Potential issue | 🔴 CriticalFix unused variable
company— build is failing.The pipeline failure indicates
companyis assigned but never used. Since the credential/auth logic now iterates over all user's companies viaensure_credentials_and_auth_for_user(user), this variable is dead code.🐛 Proposed fix
defp save_certificate(socket, cert_data, password) do user = socket.assigns.current_user - company = socket.assigns.current_company cert_meta = extract_certificate_info(cert_data, password)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/certificate_live.ex` around lines 183 - 185, The local variable company in save_certificate (assigned from socket.assigns.current_company) is unused now and causing the build failure; remove the dead assignment or rename it to _company to silence the warning, leaving the rest of save_certificate intact, since credential/auth flow now uses ensure_credentials_and_auth_for_user(user) and iterates companies elsewhere.
🧹 Nitpick comments (1)
lib/ksef_hub_web/live/company_live/form.ex (1)
228-244: Consider extracting sharedenqueue_auth/1to reduce duplication.The
enqueue_auth/1function is duplicated inlib/ksef_hub_web/live/certificate_live.ex(lines 276-286). Consider extracting to a shared module (e.g.,KsefHub.KsefClient.AuthWorkeritself or a helper module) to maintain DRY.♻️ Example extraction
Add to
lib/ksef_hub/ksef_client/auth_worker.ex:`@spec` enqueue(Ecto.UUID.t()) :: :ok def enqueue(company_id) do case %{company_id: company_id} |> new() |> Oban.insert() do {:ok, _job} -> :ok {:error, reason} -> Logger.error("Failed to enqueue auth job for company #{company_id}: #{inspect(reason)}") :ok end endThen both LiveViews can call
AuthWorker.enqueue(company_id).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/company_live/form.ex` around lines 228 - 244, The duplicate enqueue_auth/1 logic should be moved into a shared function on the AuthWorker module and both LiveViews should call that instead; create KsefHub.KsefClient.AuthWorker.enqueue/1 which accepts company_id, builds the job with AuthWorker.new(company_id) (or new()), inserts via Oban.insert(), and logs failures with the inspected reason, returning :ok; then replace the local enqueue_auth/1 definitions in CompanyLive.Form and CertificateLive with calls to AuthWorker.enqueue(company_id).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/ksef_hub_web/live/certificate_live.ex`:
- Around line 183-185: The local variable company in save_certificate (assigned
from socket.assigns.current_company) is unused now and causing the build
failure; remove the dead assignment or rename it to _company to silence the
warning, leaving the rest of save_certificate intact, since credential/auth flow
now uses ensure_credentials_and_auth_for_user(user) and iterates companies
elsewhere.
---
Nitpick comments:
In `@lib/ksef_hub_web/live/company_live/form.ex`:
- Around line 228-244: The duplicate enqueue_auth/1 logic should be moved into a
shared function on the AuthWorker module and both LiveViews should call that
instead; create KsefHub.KsefClient.AuthWorker.enqueue/1 which accepts
company_id, builds the job with AuthWorker.new(company_id) (or new()), inserts
via Oban.insert(), and logs failures with the inspected reason, returning :ok;
then replace the local enqueue_auth/1 definitions in CompanyLive.Form and
CertificateLive with calls to AuthWorker.enqueue(company_id).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b05d96eb-62be-4479-a8da-1e24049dc4e6
📒 Files selected for processing (6)
lib/ksef_hub/credentials.exlib/ksef_hub_web/live/certificate_live.exlib/ksef_hub_web/live/company_live/form.extest/ksef_hub/credentials_test.exstest/ksef_hub_web/live/certificate_live_test.exstest/ksef_hub_web/live/company_live_test.exs
Move duplicate enqueue_auth/1 from CertificateLive and CompanyLive.Form into AuthWorker.enqueue/1 so both callers share one implementation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ksef_hub_web/live/certificate_live.ex`:
- Around line 216-229: The current ensure_credentials_and_auth_for_user/1 calls
Companies.list_companies_for_user/1 which returns all memberships; if you only
want to create credentials for companies the user owns, change the call to use a
owners-only query (e.g. add a membership role filter like where(... and m.role
== :owner) inside Companies.list_companies_for_user/1 or create and call a new
Companies.list_owned_companies_for_user/1), then keep the existing
Credentials.ensure_credential_for_company/1 handling (enqueue_auth on {:ok, _}
and Logger.warning on {:error, _}) so only owner companies are processed; if
instead the current behavior is intended, add an explicit comment in
ensure_credentials_and_auth_for_user/1 to document that it runs for all active
memberships.
🪄 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: 747918c6-edd1-4f0e-9f5b-24f1105509ad
📒 Files selected for processing (1)
lib/ksef_hub_web/live/certificate_live.ex
list_companies_for_user returns all companies including where user is accountant. Only owners should have credentials created on cert upload. Adds Companies.list_owned_companies_for_user/1 and uses it in ensure_credentials_and_auth_for_user/1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ksef_hub/ksef_client/auth_worker.ex`:
- Around line 16-25: The enqueue/1 function currently logs Oban.insert failures
but still returns :ok, masking errors; change enqueue/1 (and its `@spec`) to
propagate the insertion result instead of always returning :ok: return {:ok,
job} on {:ok, job} and propagate or return {:error, reason} on {:error, reason}
from Oban.insert (keeping the Logger.error call if desired), so callers can
detect and handle enqueue failures; locate the logic in enqueue/1 that calls
new() |> Oban.insert() and update the pattern matches and spec accordingly.
🪄 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: 536888ec-ffed-491b-b397-63cc0cec31c0
📒 Files selected for processing (4)
lib/ksef_hub/companies.exlib/ksef_hub/ksef_client/auth_worker.exlib/ksef_hub_web/live/certificate_live.exlib/ksef_hub_web/live/company_live/form.ex
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ksef_hub_web/live/company_live/form.ex
- lib/ksef_hub_web/live/certificate_live.ex
Comment on lines
+16
to
+25
| @spec enqueue(Ecto.UUID.t()) :: :ok | ||
| def enqueue(company_id) do | ||
| case %{company_id: company_id} |> new() |> Oban.insert() do | ||
| {:ok, _job} -> | ||
| :ok | ||
|
|
||
| {:error, reason} -> | ||
| Logger.error("Failed to enqueue auth job for company #{company_id}: #{inspect(reason)}") | ||
| :ok | ||
| end |
There was a problem hiding this comment.
Do not mask enqueue failures as success
enqueue/1 always returns :ok; on Line 22-Line 24, insertion failures are logged but still reported as success. This makes auth bootstrap failures non-actionable for callers and can silently leave a company without an auth job.
Suggested fix
- `@doc` "Enqueues an auth job for the given company. Returns `:ok` always."
- `@spec` enqueue(Ecto.UUID.t()) :: :ok
+ `@doc` "Enqueues an auth job for the given company."
+ `@spec` enqueue(Ecto.UUID.t()) :: :ok | {:error, term()}
def enqueue(company_id) do
case %{company_id: company_id} |> new() |> Oban.insert() do
{:ok, _job} ->
:ok
{:error, reason} ->
Logger.error("Failed to enqueue auth job for company #{company_id}: #{inspect(reason)}")
- :ok
+ {:error, reason}
end
end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ksef_hub/ksef_client/auth_worker.ex` around lines 16 - 25, The enqueue/1
function currently logs Oban.insert failures but still returns :ok, masking
errors; change enqueue/1 (and its `@spec`) to propagate the insertion result
instead of always returning :ok: return {:ok, job} on {:ok, job} and propagate
or return {:error, reason} on {:error, reason} from Oban.insert (keeping the
Logger.error call if desired), so callers can detect and handle enqueue
failures; locate the logic in enqueue/1 that calls new() |> Oban.insert() and
update the pattern matches and spec accordingly.
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.ensure_credential_for_company/1into the context to avoid duplicating business logic across LiveViewsTest plan
ensure_credential_for_company/1(creates when missing, returns existing)mix credo --strictclean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests