Skip to content

Commit 8292818

Browse files
committed
Use .for_api_v2 scope for GET /api/v2/plans/:id
This change updates `Api::V2::PlansController#show` to use the `.for_api_v2` scope, and applies eager loading of answers (`.includes(answers: { question: :section })`) based on the `complete` param--making the `show` action consistent with `index`. A new `plans_scope` helper DRYs up the controller by centralizing the shared scope and eager loading logic. Since `.for_api_v2` already filters by `.where(roles: { user_id: user_id, active: true })`, only a presence check is now required in the policy.
1 parent c4c75df commit 8292818

File tree

2 files changed

+13
-6
lines changed

2 files changed

+13
-6
lines changed

app/controllers/api/v2/plans_controller.rb

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ class PlansController < BaseApiController # rubocop:todo Style/Documentation
88

99
# GET /api/v2/plans/:id
1010
def show
11-
@plan = Plan.includes(roles: :user).find_by(id: params[:id])
12-
13-
raise Pundit::NotAuthorizedError unless @plan.present?
11+
@plan = plans_scope.find_by(id: params[:id])
1412

1513
plans_policy = PlansPolicy.new(@resource_owner, @plan)
1614
raise Pundit::NotAuthorizedError unless plans_policy.show?
@@ -21,8 +19,7 @@ def show
2119

2220
# GET /api/v2/plans
2321
def index
24-
@plans = PlansPolicy::Scope.new(@resource_owner).resolve
25-
@plans = @plans.includes(answers: { question: :section }) if @complete
22+
@plans = plans_scope
2623
@items = paginate_response(results: @plans)
2724
render '/api/v2/plans/index', status: :ok
2825
end
@@ -33,6 +30,11 @@ def index
3330
def set_complete_param
3431
@complete = params[:complete].to_s.downcase == 'true'
3532
end
33+
34+
def plans_scope
35+
scope = PlansPolicy::Scope.new(@resource_owner).resolve
36+
@complete ? scope.includes(answers: { question: :section }) : scope
37+
end
3638
end
3739
end
3840
end

app/policies/api/v2/plans_policy.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,19 @@ def initialize(resource_owner, plan = nil) # rubocop:todo Lint/MissingSuper
1212
end
1313

1414
def show?
15-
@plan.roles.where(user_id: @resource_owner.id, active: true).exists?
15+
# The show action uses the resolve method, so only a presence check
16+
# is needed here (see the resolve method comment for more).
17+
@plan.present?
1618
end
1719

1820
class Scope < Scope # rubocop:todo Style/Documentation
1921
def initialize(resource_owner) # rubocop:todo Lint/MissingSuper
2022
@resource_owner = resource_owner
2123
end
2224

25+
# Eager loads all associations needed for API v2 serialization,
26+
# and restricts to plans where the user_id has an active role.
27+
# - (i.e. .where(roles: { user_id: @resource_owner.id, active: true }))
2328
def resolve
2429
Plan.for_api_v2(@resource_owner.id)
2530
end

0 commit comments

Comments
 (0)