Skip to content

Commit

Permalink
Deprecate nested option_type->option_value routes for member actions
Browse files Browse the repository at this point in the history
There's no need to fetch the parent id for those routes.
  • Loading branch information
waiting-for-dev committed May 27, 2022
1 parent 3dcc3fa commit 7c49645
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 3 deletions.
14 changes: 14 additions & 0 deletions api/app/controllers/spree/api/option_values_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def index
end

def show
warn_if_nested_member_route

@option_value = scope.find(params[:id])
respond_with(@option_value)
end
Expand All @@ -35,6 +37,8 @@ def create
end

def update
warn_if_nested_member_route

@option_value = scope.accessible_by(current_ability, :update).find(params[:id])
if @option_value.update(option_value_params)
render :show
Expand All @@ -44,6 +48,8 @@ def update
end

def destroy
warn_if_nested_member_route

@option_value = scope.accessible_by(current_ability, :destroy).find(params[:id])
@option_value.destroy
render plain: nil, status: 204
Expand All @@ -62,6 +68,14 @@ def scope
def option_value_params
params.require(:option_value).permit(permitted_option_value_attributes)
end

def warn_if_nested_member_route
Spree::Deprecation.warn <<~MSG if request.path.include?('option_types')
This route is deprecated. Use shallow version instead:
#{request.method.upcase} api/option_values/:id
MSG
end
end
end
end
4 changes: 3 additions & 1 deletion api/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@
end

resources :option_types do
# TODO: Use shallow option on Solidus v4.0
resources :option_values
end
# TODO: Remove :create once option_type is required on Solidus v4.0
# TODO: Use only: :index on Solidus v4.0 and use shallow option on the nested routes
# within option_types
resources :option_values

get '/orders/mine', to: 'orders#mine', as: 'my_orders'
Expand Down
12 changes: 10 additions & 2 deletions api/openapi/solidus-api.oas.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2717,7 +2717,10 @@ paths:
'404':
$ref: '#/components/responses/not-found'
summary: Get option type value
description: Retrieves an option type's value.
description: |-
**DEPRECATED**: Use *GET /option_values/{id}* instead
Retrieves an option type's value.
operationId: get-option-type-value
tags:
- Option values
Expand Down Expand Up @@ -2751,7 +2754,10 @@ paths:
'422':
$ref: '#/components/responses/delete-restriction'
summary: Delete option type value
description: Deletes an option type's value.
description: |-
**DEPRECATED**: Use *DELETE /option_values/{id}* instead
Deletes an option type's value.
operationId: delete-option-type-value
tags:
- Option values
Expand All @@ -2773,6 +2779,8 @@ paths:
$ref: '#/components/responses/unprocessable-entity'
summary: Update option type value
description: |-
**DEPRECATED**: Use *PATCH /option_values/{id}* instead
Updates an option type's value.
Only users with the `update` permission on the option value can perform this action.
Expand Down
20 changes: 20 additions & 0 deletions api/spec/requests/spree/api/option_values_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ module Spree::Api
expect(json_response).to have_attributes(attributes)
end

it "deprecates listing a single option value through a nested route" do
expect(Spree::Deprecation).to receive(:warn).with(/deprecated/)

get spree.api_option_type_option_value_path(option_type, option_value)
end

it "cannot create a new option value" do
post spree.api_option_type_option_values_path(option_type), params: {
option_value: {
Expand Down Expand Up @@ -110,6 +116,14 @@ module Spree::Api
expect(option_value.name).to eq("Option Value")
end

it "deprecates updating an option value through a nested route" do
expect(Spree::Deprecation).to receive(:warn).with(/deprecated/)

put spree.api_option_type_option_value_path(option_type, option_value), params: { option_value: {
name: "Option Value"
} }
end

it "can create but deprecates creating an option value without option type" do
expect(Spree::Deprecation).to receive(:warn).with(/deprecated/).at_least(:once)

Expand Down Expand Up @@ -138,6 +152,12 @@ module Spree::Api
delete spree.api_option_value_path(option_value)
expect(response.status).to eq(204)
end

it "deprecates deleting an option value through a nested route" do
expect(Spree::Deprecation).to receive(:warn).with(/deprecated/)

delete spree.api_option_type_option_value_path(option_type, option_value)
end
end
end
end
Expand Down

0 comments on commit 7c49645

Please sign in to comment.