fix(users): fetch_user raises NameError after EVO-1947 removed the users helper - #79
Open
eniatec wants to merge 1 commit into
Open
Conversation
…EVO-1947 The EVO-1947 refactor moved the index listing into Users::FilterService and deleted the private `users` helper, but fetch_user — the before_action for show, update and check_permission — still called it. Every /users/:id route then raised NameError (500). Callers that gate on check_permission treat the 500 as a denial, so any consumer relying on it loses authorization entirely. Resolve the record directly (keeping the user_roles preload that role_data uses) and add a request spec that fails with NameError on the previous code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer's GuideFixes the users controller’s before_action to resolve the user record directly via ActiveRecord, preserving role preloading, and adds a focused request spec to prevent regressions on GET /api/v1/users/:id. Sequence diagram for updated GET /api/v1/users/:id request handlingsequenceDiagram
actor Client
participant UsersController as ApiV1UsersController
participant UserModel as User
participant DB
Client->>UsersController: GET /api/v1/users/:id
activate UsersController
UsersController->>UserModel: includes(user_roles)
UserModel->>DB: SELECT users WITH user_roles WHERE id = params[:id]
DB-->>UserModel: user + preloaded user_roles
UserModel-->>UsersController: @user
UsersController-->>Client: 200 OK with user response
deactivate UsersController
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
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.
Problem
The EVO-1947 refactor (#50) moved the users index into
Users::FilterServiceand deleted the privateusershelper fromApi::V1::UsersController— butfetch_user, thebefore_actionforshow,updateandcheck_permission, still calls it:Every
/api/v1/users/:idroute now returns 500. The practical impact is severe: services that gate requests onPOST /users/:id/check_permissionreceive the 500 and treat it as a denial, so every consumer relying on the permission check loses authorization entirely (we hit this in production-like validation: the CRM turned every request into 403).The EVO-1947 request specs only cover the index action, which is the one route that does not run
fetch_user— that is why CI stayed green.Fix
Resolve the record directly in
fetch_user, keeping theuser_rolespreload thatrole_datauses, and add a request spec forGET /users/:idthat fails with the NameError on current develop.🤖 Generated with Claude Code
Summary by Sourcery
Fix user fetch in the users API controller and add a regression test for the users show endpoint.
Bug Fixes:
Tests: