Skip to content

refactor: service-layer hardening & test coverage #79

Description

@smartlabsAT

TL;DR: Harden the service layer — split the DirectusApiClient god-class, make error handling consistent and safe, fix the permission-check race + PII logging, and add the missing unit tests — so the data/permission/preset layer is testable and trustworthy.

Context

The audit of 2026-06-22 found the service layer is the least-tested, highest-risk area: DirectusApiClient (services/api-client.ts, 865 lines, 18 public methods spanning 4 domains) has no unit tests, nor do usePermissionChecks or useUserPresets. Error handling is inconsistent (the utils/error-helpers.ts helpers are used by only 2 of ~110 catch blocks) and in places unsafe (silent permission-denying catches). This epic groups the service-layer concerns that are out of scope for #31 (block handling) and the item-selector epic.

Success Criteria

  • DirectusApiClient split into focused domain clients over a shared transport
  • Error handling is consistent: one documented pattern, no silent permission/destructive defaults
  • usePermissionChecks returns correct results (no race) and logs no PII
  • Unit-test coverage exists for api-client, usePermissionChecks, useUserPresets

Scope

In Scope:

  • Splitting DirectusApiClient into domain clients
  • Unifying error handling (adopt or remove error-helpers.ts); fixing silent catches
  • Fixing the usePermissionChecks race condition + removing PII logging
  • Adding unit tests for the untested service/permission/preset code

Out of Scope:

Architecture Decisions

Binding for ALL sub-issues. Deviations require an update to this section.

  • Domain clients over one god-class: split DirectusApiClient into ItemsClient / MetadataClient / PermissionsClient / PresetsClient sharing one transport (retry + error + availability). No raw getApi() leak.
  • One error-handling pattern: either route API catches through utils/error-helpers.ts (handleApiError) everywhere, or remove it — no half-adopted helper. Destructive/permission operations must never default to "allow"/"granted" on error without an explicit, logged decision.
  • No PII in logs: never log full user objects (email, etc.).
  • Tests are part of the refactor, not after it: add tests before/alongside each split.

Sub-Issues

Order Issue Description Status Depends on
1 #80 Add unit tests for api-client, usePermissionChecks, useUserPresets open
2 #81 Fix usePermissionChecks race condition + remove PII logging open
3 #82 Unify error handling (adopt or remove error-helpers.ts), fix silent catches open
4 #83 Split DirectusApiClient into domain clients open #80

Parallelization: #80, #81, #82 are independent. #83 should follow #80 (tests as a safety net before the split).

References

  • God-class: src/services/api-client.ts (865 lines)
  • Error helpers (under-used): src/utils/error-helpers.ts
  • Permissions: src/composables/usePermissionChecks.ts (210 lines)
  • Presets: src/composables/useUserPresets.ts (371 lines)
  • Tested sibling for reference: src/services/RelationChecker.ts (has a spec)

Risks

  1. Permission regressions — changing permission resolution must preserve correct gating. → Add tests first (test: add unit tests for api-client, usePermissionChecks and useUserPresets #80).
  2. Breaking the public client APIDirectusApiClient is used across composables; the split must keep call sites working. → Keep a thin facade if needed.

Dependencies

Metadata

Metadata

Assignees

No one assigned

    Labels

    apiAPI related issues and improvementsarchitectureArchitectural improvements and refactoringrefactorCode refactoring without changing functionality

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions