diff --git a/api/app/controllers/spree/api/variants_controller.rb b/api/app/controllers/spree/api/variants_controller.rb index d6746245443..d2a6e380e05 100644 --- a/api/app/controllers/spree/api/variants_controller.rb +++ b/api/app/controllers/spree/api/variants_controller.rb @@ -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 @@ -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) @@ -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) @@ -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 diff --git a/api/config/routes.rb b/api/config/routes.rb index 88c22be2ab4..230ec5d724e 100644 --- a/api/config/routes.rb +++ b/api/config/routes.rb @@ -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 @@ -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 diff --git a/api/openapi/solidus-api.oas.yml b/api/openapi/solidus-api.oas.yml index 83f35045a67..9a9facceb26 100644 --- a/api/openapi/solidus-api.oas.yml +++ b/api/openapi/solidus-api.oas.yml @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/api/spec/requests/spree/api/variants_spec.rb b/api/spec/requests/spree/api/variants_spec.rb index 5aee03e9d3d..41c9d0d16b3 100644 --- a/api/spec/requests/spree/api/variants_spec.rb +++ b/api/spec/requests/spree/api/variants_spec.rb @@ -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")) @@ -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 @@ -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 @@ -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