Skip to content

[FFS-4303] Make RegisterRoutes only wire HTTP paths to handlers.#67

Merged
iannorriswork merged 10 commits intomainfrom
refactor/make-registerRoutes-only-wire-routes-to-handlers
May 6, 2026
Merged

[FFS-4303] Make RegisterRoutes only wire HTTP paths to handlers.#67
iannorriswork merged 10 commits intomainfrom
refactor/make-registerRoutes-only-wire-routes-to-handlers

Conversation

@imhben
Copy link
Copy Markdown
Contributor

@imhben imhben commented May 4, 2026

Jira Ticket

Description

Moved all dependency creation out of router.go and into app.go. RegisterRoutes should not be a factory for dependencies and services but instead just for route composition.

Submitter Checklist

  • Jira Ticket: The relevant Jira ticket is linked in the header.
  • Evidence of Testing: I have included evidence of bench testing (e.g., terminal output, screenshots, logs) below.
  • Security Boundary: I have checked if any security boundaries are changing.
  • Backwards Compatibility: I have verified that these changes do not introduce any backwards compatibility issues.
  • I've updated the Confluence documentation, content in the /docs folder, and any API spec docs

Reviewer Checklist

  • Verification: I have verified that the changes work as intended.
  • Backwards Compatibility: I have confirmed that the code does not introduce any backwards compatibility issues.
  • Security & Privacy: I have reviewed the changes for potential security or privacy risks (e.g., PII handling).
  • Contract Compliance: If the public API contract changed, I have verified that it is reflected in api-spec/ and follows the organization's standards.
  • Code Quality: The code is clean, well-tested, and follows the established patterns in the codebase.
  • Code Quality: The code has unit and integration tests that cover relevant code changes.

Evidence of Testing

Paste test results here

imhben added 2 commits May 4, 2026 15:51
Create education and veteran service instances in app.New and provide a WithCB circuit-breaker wrapper using a Redis breaker. Pass EDU, VA and WithCB through RouterParams and update route registration to use the injected services and WithCB instead of constructing services/middleware in the router. Clean up related imports and centralize service/circuit-breaker setup in the app initializer.
Introduce stubEducationService and stubVeteranService in api/routes/router_test.go and add context import. Update RegisterRoutes calls in tests to provide EDU, VA and a no-op WithCB instead of the previous encryption mock. Also update imports (remove encryption, add education and veteran) and wire stubs to decouple route tests from encryption/circuit-breaker behavior.
@imhben imhben changed the title Add EDU/VA services and circuit breaker wiring [FFS-4303] Make RegisterRoutes only wire HTTP paths to handlers. May 4, 2026
@imhben imhben requested a review from iannorriswork May 4, 2026 20:03
iannorriswork
iannorriswork previously approved these changes May 4, 2026
Remove unused fields (DB, RDB, Encryption) from RouterParams and drop corresponding imports (database/sql, redis, encryption). Update api/app.go to construct RouterParams with only the needed fields (CFG, Reporter, Logger, EDU, VA, WithCB). This simplifies the router interface and removes unnecessary dependencies.
imhben added 7 commits May 4, 2026 16:12
Add runtime nil checks in RegisterRoutes to panic if required dependencies are missing (app, CFG, Reporter, EDU, VA, WithCB) while preserving a default slog logger when Logger is nil. Simplify tests by removing the Redis client, getRedisAddr helper, and the redis import; tests no longer pass RDB to RouterParams, avoiding an external Redis dependency.
Replace named receiver variables with the blank identifier in stubEducationService and stubVeteranService methods in api/routes/router_test.go. This removes unused receiver names and avoids linter/warning noise for methods: LookupEnrollmentStatus, RegisterBatch, GetBatchStatus, GetBatchDetails, and LookupDisabilityRating.
@iannorriswork iannorriswork merged commit 4f9b6ed into main May 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants