Skip to content

Commit

Permalink
Deprecate duplicated variant routes
Browse files Browse the repository at this point in the history
As in solidusio#4385:

- We keep shallow member routes. We deprecate nested member routes.
- We keep creation nested route. We deprecate creation shallow route.
- We keep both shallow and nested collection routes (more flexibility
for storefronts).
  • Loading branch information
waiting-for-dev authored and cpfergus1 committed Aug 25, 2022
1 parent 500a1a9 commit 35fb2a6
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 9 deletions.
20 changes: 20 additions & 0 deletions api/app/controllers/spree/api/variants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ class VariantsController < Spree::Api::BaseController
before_action :product

def create
Spree::Deprecation.warn <<~MSG unless request.path.include?('/products/')
This route is deprecated. Use the route nested within the product resource:
POST api/products/{product_id}/variants
MSG

authorize! :create, Variant
@variant = scope.new(variant_params)
if @variant.save
Expand All @@ -16,6 +22,8 @@ def create
end

def destroy
warn_if_nested_member_route

@variant = scope.accessible_by(current_ability, :destroy).find(params[:id])
@variant.discard
respond_with(@variant, status: 204)
Expand All @@ -42,12 +50,16 @@ def new
end

def show
warn_if_nested_member_route

@variant = scope.includes(include_list)
.find(params[:id])
respond_with(@variant)
end

def update
warn_if_nested_member_route

@variant = scope.accessible_by(current_ability, :update).find(params[:id])
if @variant.update(variant_params)
respond_with(@variant, status: 200, default_template: :show)
Expand All @@ -58,6 +70,14 @@ def update

private

def warn_if_nested_member_route
Spree::Deprecation.warn <<~MSG if request.path.include?('/products/')
This route is deprecated. Use shallow version instead:
#{request.method.upcase} api/variants/:id
MSG
end

def product
@product ||= Spree::Product.accessible_by(current_ability, :show).friendly.find(params[:product_id]) if params[:product_id]
end
Expand Down
10 changes: 6 additions & 4 deletions api/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@

resources :products do
resources :images
# TODO: Use shallow option on Solidus v4.0
resources :variants
resources :product_properties
end

# TODO: Use only: :index on Solidus v4.0
resources :variants do
resources :images
end

concern :order_routes do
resources :line_items
resources :payments do
Expand Down Expand Up @@ -47,10 +53,6 @@
end
end

resources :variants do
resources :images
end

resources :option_types do
# TODO: Use shallow option on Solidus v4.0
resources :option_values
Expand Down
20 changes: 16 additions & 4 deletions api/openapi/solidus-api.oas.yml
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,10 @@ paths:
'404':
$ref: '#/components/responses/not-found'
summary: Get product variant
description: Retrieves a product's variant.
description: |-
**DEPRECATED**: Use *GET /variants/{id}* instead
Retrieves a product's variant.
operationId: get-product-variant
tags:
- Variants
Expand Down Expand Up @@ -714,7 +717,10 @@ paths:
'422':
$ref: '#/components/responses/delete-restriction'
summary: Delete product variant
description: Deletes a product's variant.
description: |-
**DEPRECATED**: Use *DELETE /variants/{id}* instead
Deletes a product's variant.
operationId: delete-product-variant
tags:
- Variants
Expand All @@ -735,7 +741,10 @@ paths:
'422':
$ref: '#/components/responses/unprocessable-entity'
summary: Update product variant
description: Updates a product's variant.
description: |-
**DEPRECATED**: Use *PUT /variants/{id}* instead
Updates a product's variant.
operationId: update-product-variant
tags:
- Variants
Expand Down Expand Up @@ -857,7 +866,10 @@ paths:
'422':
$ref: '#/components/responses/unprocessable-entity'
summary: Create variant
description: 'Creates a variant. Only users with `can :create, Variant` permissions can perform this action.'
description: |-
**DEPRECATED**: Use *POST /products/{product_id}/variants* instead
Creates a variant. Only users with `can :create, Variant` permissions can perform this action.
operationId: create-variant
tags:
- Variants
Expand Down
33 changes: 32 additions & 1 deletion api/spec/requests/spree/api/variants_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,14 @@ module Spree::Api
:option_type_id])
end

it "deprecates showing a variant through a nested route" do
expect(Spree::Deprecation).to receive(:warn).with(%r[deprecated.*GET api/variants]m)

get spree.api_product_variant_path(product, variant)

expect(json_response).to have_attributes(show_attributes)
end

it "can see a single variant with images" do
variant.images.create!(attachment: image("blank.jpg"))

Expand Down Expand Up @@ -276,7 +284,7 @@ module Spree::Api
end

it "cannot create a new variant if not an admin" do
post spree.api_variants_path, params: { variant: { sku: "12345" } }
post spree.api_product_variants_path(product), params: { variant: { sku: "12345" } }
assert_unauthorized!
end

Expand Down Expand Up @@ -315,6 +323,13 @@ module Spree::Api
expect(variant.product.variants.count).to eq(1)
end

it "deprecates creating a variant through a shallow route" do
expect(Spree::Deprecation).to receive(:warn).with(%r[deprecated.*api/products/{product_id}/variants]m)

post spree.api_variants_path, params: { variant: { sku: "12345", "product_id": product.id } }
expect(json_response).to have_attributes(new_attributes)
end

it "creates new variants with nested option values" do
option_values = create_list(:option_value, 2)
expect do
Expand Down Expand Up @@ -346,12 +361,28 @@ module Spree::Api
expect(response.status).to eq(200)
end

it "deprecates updating a variant through a nested route" do
expect(Spree::Deprecation).to receive(:warn).with(%r[deprecated.*PUT api/variants]m)

put spree.api_product_variant_path(product, variant), params: { variant: { sku: "12345" } }

expect(json_response).to have_attributes(show_attributes)
end

it "can delete a variant" do
delete spree.api_variant_path(variant)
expect(response.status).to eq(204)
expect { Spree::Variant.find(variant.id) }.to raise_error(ActiveRecord::RecordNotFound)
end

it "deprecates updating a variant through a nested route" do
expect(Spree::Deprecation).to receive(:warn).with(%r[deprecated.*DELETE api/variants]m)

delete spree.api_product_variant_path(product, variant)

expect { Spree::Variant.find(variant.id) }.to raise_error(ActiveRecord::RecordNotFound)
end

it 'variants returned contain cost price data' do
get spree.api_variants_path
expect(json_response["variants"].first.key?(:cost_price)).to eq true
Expand Down

0 comments on commit 35fb2a6

Please sign in to comment.