Skip to content

refactor: simplify handlers and user service#200

Merged
appleboy merged 1 commit into
mainfrom
refactor/simplify-cleanup
May 29, 2026
Merged

refactor: simplify handlers and user service#200
appleboy merged 1 commit into
mainfrom
refactor/simplify-cleanup

Conversation

@appleboy
Copy link
Copy Markdown
Member

Summary

Pure code-quality cleanup from a /simplify pass over the handlers and user service. No behavior changes — eliminates redundant work, dead code, and nested conditionals flagged against the project's "no if/else chains" convention.

  • Compute the device verification user code once instead of formatting it twice in the response
  • Drop the unused revoked-token count variable in the client revoke-all handler
  • Hoist the static OIDC discovery subject-types and claims-supported lists to package scope so the Discovery endpoint stops reallocating two slices per request
  • Flatten nested email/username uniqueness checks in the user service into sequential guard clauses

AI Authorship

  • AI was used. Details:
    • Tool / model: Claude Opus 4.8 (1M context) via Claude Code (/simplify skill)
    • AI-authored files: internal/handlers/client.go, internal/handlers/device.go, internal/handlers/oidc.go, internal/services/user.go
    • Human line-by-line reviewed: ⚠️ Not yet — line-by-line human review still pending at time of opening

Change classification

  • Leaf node (local impact) — refactors only, no behavior change; verified by the existing test suite
  • Note: internal/services/user.go touches user uniqueness logic (core-adjacent). See reviewer guide.

Verification

  • make generatego build ./... clean
  • go vet ./... clean
  • Existing tests pass: internal/handlers, internal/services, internal/util, internal/token
  • No new tests added — changes are behavior-preserving refactors covered by existing tests.
  • Manual verification: N/A (no behavioral change)

Verifiability check

  • Refactors preserve existing public signatures and response shapes
  • Reviewer can judge correctness from the diff alone (small, mechanical)

Risk & rollback

  • Risk: Low. The only logic-shaped change is the flattening of the user uniqueness guards — a transcription error there could alter conflict detection. Behavior is intended to be identical.
  • Rollback: Revert the single commit on this branch.

Reviewer guide

  • Read carefully: internal/services/user.go — confirm the flattened email/username guard clauses are logically equivalent to the prior if/else if (no inverted conditions, err reuse is correct).
  • Spot-check OK: oidc.go (verbatim slice hoist), device.go (dedup of one call), client.go (dead-var removal).

🤖 Generated with Claude Code

- Compute the device verification user code once instead of formatting it twice
- Drop the unused revoked-token count variable in the client revoke handler
- Hoist the static OIDC discovery subject and claims lists to package scope to avoid per-request allocation
- Flatten nested email and uniqueness checks into guard clauses

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 29, 2026 13:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs a small refactor pass across handlers and the user service to reduce redundant work, remove dead code, and simplify conditional logic, while keeping behavior and API responses unchanged.

Changes:

  • Deduplicates device-flow user_code formatting and removes an unused revoke-all return value.
  • Hoists static OIDC discovery lists to package scope to avoid per-request slice allocations.
  • Flattens user uniqueness checks into guard clauses in UserService.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
internal/services/user.go Simplifies email/username uniqueness checks and removes unnecessary local shadowing.
internal/handlers/oidc.go Moves static discovery subject_types_supported / claims_supported slices to package scope and reuses them in responses.
internal/handlers/device.go Computes formatted device verification user_code once and reuses it in the response payload.
internal/handlers/client.go Drops unused revokedCount from revoke-all handler call site.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/services/user.go 55.55% 1 Missing and 3 partials ⚠️
internal/handlers/client.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@appleboy appleboy merged commit aa5c455 into main May 29, 2026
18 checks passed
@appleboy appleboy deleted the refactor/simplify-cleanup branch May 29, 2026 13:46
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