feat(account): query handlers for account list/get/traffic (task 22)#126
Merged
feat(account): query handlers for account list/get/traffic (task 22)#126
Conversation
Adds three CQRS query handlers wired through `QueryBus.with_account_repo`: `list_accounts`, `get_account`, `get_account_traffic`. New read models `AccountViewDto` and `AccountTrafficDto` (camelCase serde) carry every persisted column except passwords — no credential field exists on the type, so JSON output cannot leak one even by accident. `AccountFilter` AND-combines `service_name`, `account_type`, `enabled`. Service name is delegated to the repo's `list_by_service` for SQL-level pruning; type and enabled filter in memory afterwards. `get_account_traffic` returns persisted counters only — the upstream refresh path is the existing `account_validate` command (task 21), so queries stay side-effect free per the project CQRS rule. 21 new unit tests against an `InMemoryAccountRepoForQueries` fixture cover filter combinations, missing-id 404s, missing-repo validation, camelCase shape, and explicit "no password field" assertions on `serde_json::to_value` output. Coverage: account_view 100%, list_accounts 99.23%, get_account 98.29%, get_account_traffic 98.43%. Unblocks task 23 (Vue Accounts).
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughAdds account-focused CQRS query handlers, new account read-model DTOs, QueryBus wiring for an optional account repository, in-memory test repository support, and CHANGELOG entry describing the account features. Changes
Sequence DiagramsequenceDiagram
actor Client
participant QueryBus
participant Handler as Query Handler
participant Repo as AccountRepository
participant Domain as Account (domain)
participant DTO as AccountViewDto
Client->>QueryBus: handle_list_accounts(query)
QueryBus->>Handler: dispatch query
Handler->>QueryBus: account_repo()?
alt repository present
Handler->>Repo: list() / list_by_service(filter.service_name)
Repo-->>Handler: Vec<Account>
Handler->>Handler: apply remaining filters (type, enabled)
loop per matching Account
Handler->>Domain: select Account
Domain->>DTO: From<Account> -> AccountViewDto
DTO-->>Handler: AccountViewDto
end
Handler-->>QueryBus: Ok(Vec<AccountViewDto>)
else repository missing
Handler-->>QueryBus: Err(AppError::Validation)
end
QueryBus-->>Client: Result<Vec<AccountViewDto>, AppError>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src-tauri/src/application/query_bus.rs (1)
62-72: Add a focused unit test for the new account-repo builder/accessor pair.This new wiring is central to the account query handlers; a small constructor-chain test here would guard regressions in
with_account_repo(...).account_repo().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/query_bus.rs` around lines 62 - 72, Add a focused unit test that constructs the builder, wraps a simple test implementation of AccountRepository in an Arc, calls with_account_repo(...) and then asserts that account_repo() returns Some(...) and refers to the same underlying instance; implement a tiny test-double struct implementing AccountRepository (or use a mock helper if available), pass Arc::new(test_double) into with_account_repo on the builder, and assert the returned Option<&dyn AccountRepository> is Some and behaves or compares equal to the original Arc-held instance to guard the with_account_repo/account_repo wiring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/application/queries/list_accounts.rs`:
- Around line 4-6: The module-level doc comment in list_accounts.rs should be
reworded to clarify that “no credential can leak” means there is no password or
raw secret material in the DTO, not that it contains no credential-related
identifiers; update the comment that currently reads “The DTO has no password
field, so no credential can leak through this path by construction.” to
explicitly state something like “The DTO contains no password or raw secret
material (no plaintext secrets), though non-secret credential references (e.g.
usernames or credential IDs) may still be present.” Ensure you edit the
top-of-file doc comment where that sentence appears.
In `@src-tauri/src/application/read_models/account_view.rs`:
- Around line 15-18: The doc comment currently says the DTO excludes credential
references but AccountViewDto contains the field credential_ref; update the
comment to state that the DTO mirrors persisted accounts and does include a
non-secret credential_ref (used only by the frontend to look up a keyring entry
for "test connection"), and clarify that the actual password/secret is fetched
server-side and not carried in credential_ref; refer to AccountViewDto and its
credential_ref field when making this doc change.
---
Nitpick comments:
In `@src-tauri/src/application/query_bus.rs`:
- Around line 62-72: Add a focused unit test that constructs the builder, wraps
a simple test implementation of AccountRepository in an Arc, calls
with_account_repo(...) and then asserts that account_repo() returns Some(...)
and refers to the same underlying instance; implement a tiny test-double struct
implementing AccountRepository (or use a mock helper if available), pass
Arc::new(test_double) into with_account_repo on the builder, and assert the
returned Option<&dyn AccountRepository> is Some and behaves or compares equal to
the original Arc-held instance to guard the with_account_repo/account_repo
wiring.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c86d702-04c5-4949-9cd3-057370d3db09
📒 Files selected for processing (9)
CHANGELOG.mdsrc-tauri/src/application/queries/get_account.rssrc-tauri/src/application/queries/get_account_traffic.rssrc-tauri/src/application/queries/list_accounts.rssrc-tauri/src/application/queries/mod.rssrc-tauri/src/application/query_bus.rssrc-tauri/src/application/read_models/account_view.rssrc-tauri/src/application/read_models/mod.rssrc-tauri/src/application/test_support.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
- Reword list_accounts.rs doc to make explicit the secret-leak guarantee is about password/raw secret material, not non-secret credential references like usernames or opaque keyring URIs. - Fix AccountViewDto doc that contradicted the struct: it does carry a non-secret credential_ref (opaque keyring URI), while passwords remain server-side via the keyring. - Add unit test wiring with_account_repo() through account_repo() to guard the builder/accessor pair against silent regressions.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src-tauri/src/application/queries/list_accounts.rs (1)
29-40: Optional: avoid intermediate allocation in filter→map pipeline.You can map directly to
AccountViewDtoafter filtering to skip building a temporaryVec<Account>.♻️ Suggested micro-refactor
- let filtered: Vec<Account> = accounts - .into_iter() - .filter(|a| match &query.filter { - None => true, - Some(f) => { - f.account_type.is_none_or(|t| a.account_type() == t) - && f.enabled.is_none_or(|e| a.is_enabled() == e) - } - }) - .collect(); - - Ok(filtered.into_iter().map(AccountViewDto::from).collect()) + Ok(accounts + .into_iter() + .filter(|a| match &query.filter { + None => true, + Some(f) => { + f.account_type.is_none_or(|t| a.account_type() == t) + && f.enabled.is_none_or(|e| a.is_enabled() == e) + } + }) + .map(AccountViewDto::from) + .collect())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/queries/list_accounts.rs` around lines 29 - 40, Current code creates an intermediate Vec<Account> named filtered before mapping to AccountViewDto; change the pipeline to avoid that allocation by chaining the iterator: call accounts.into_iter().filter(...) and then .map(AccountViewDto::from).collect() directly, keeping the same filter closure that uses query.filter, f.account_type/is_none_or and f.enabled/is_none_or with a.account_type()/a.is_enabled() so the behavior is unchanged but the temporary Vec<Account> is eliminated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src-tauri/src/application/queries/list_accounts.rs`:
- Around line 29-40: Current code creates an intermediate Vec<Account> named
filtered before mapping to AccountViewDto; change the pipeline to avoid that
allocation by chaining the iterator: call accounts.into_iter().filter(...) and
then .map(AccountViewDto::from).collect() directly, keeping the same filter
closure that uses query.filter, f.account_type/is_none_or and
f.enabled/is_none_or with a.account_type()/a.is_enabled() so the behavior is
unchanged but the temporary Vec<Account> is eliminated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 956e2f44-8615-4ead-af41-374d5f388a9e
📒 Files selected for processing (3)
src-tauri/src/application/queries/list_accounts.rssrc-tauri/src/application/query_bus.rssrc-tauri/src/application/read_models/account_view.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src-tauri/src/application/query_bus.rs
This was referenced Apr 29, 2026
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
• Add three async query handlers for accounts:
list_accounts,get_account,get_account_traffic• Implement
AccountViewDtoread model without password field (type-level security guarantee)• Support AND-combining optional filters on service_name, account_type, enabled
• Deliberately keep
get_account_trafficside-effect-free (upstream refresh lives in command handler per CQRS rule)Details
Acceptance Criteria
✅ All 3 handlers implemented and async-tested
✅ AccountViewDto never serializes password (compiler enforced)
✅ Filters combine correctly (service + type + enabled = AND)
✅ Tests: 16 new unit tests with InMemoryAccountRepoForQueries mock
✅ Coverage: 98-100% on query handlers
✅ All acceptance criteria from task spec verified
Summary by cubic
Adds account query handlers for list, get, and traffic, wired into
QueryBus, with safe account DTOs that include an opaquecredential_refbut never a password. Implements Linear task 22 and keeps traffic queries side-effect free.New Features
list_accounts,get_account,get_account_traffichandlers added toQueryBus.AccountViewDtoandAccountTrafficDtouse camelCase, includecredential_ref, and never include a password or raw credential.service_name,account_type,enabled) AND-combine;service_nameis pruned via repolist_by_service, others filter in memory.validate_accountcommand.Migration
QueryBus, pass the account repo via.with_account_repo(...).Written for commit 991d18f. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Documentation