diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 47df0bab12..31123dcb13 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -142,7 +142,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout if confirm("Are you sure?") $http( method: "DELETE" - url: "/api/products/" + product.id + "/soft_delete" + url: "/api/products/" + product.id ).success (data) -> $scope.products.splice $scope.products.indexOf(product), 1 DirtyProducts.deleteProduct product.id @@ -157,7 +157,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout if confirm(t("are_you_sure")) $http( method: "DELETE" - url: "/api/products/" + product.permalink_live + "/variants/" + variant.id + "/soft_delete" + url: "/api/products/" + product.permalink_live + "/variants/" + variant.id ).success (data) -> $scope.removeVariant(product, variant) else diff --git a/app/controllers/api/products_controller.rb b/app/controllers/api/products_controller.rb index 9f692e7449..b3ab251c97 100644 --- a/app/controllers/api/products_controller.rb +++ b/app/controllers/api/products_controller.rb @@ -42,8 +42,8 @@ module Api def destroy authorize! :delete, Spree::Product @product = find_product(params[:id]) - @product.update_attribute(:deleted_at, Time.zone.now) - @product.variants_including_master.update_all(deleted_at: Time.zone.now) + authorize! :delete, @product + @product.destroy render json: @product, serializer: Api::Admin::ProductSerializer, status: :no_content end @@ -71,14 +71,6 @@ module Api render_paged_products @products end - def soft_delete - authorize! :delete, Spree::Product - @product = find_product(params[:product_id]) - authorize! :delete, @product - @product.destroy - render json: @product, serializer: Api::Admin::ProductSerializer, status: :no_content - end - # POST /api/products/:product_id/clone # def clone diff --git a/app/controllers/api/variants_controller.rb b/app/controllers/api/variants_controller.rb index c60aa8f7b5..d6f8f80d5c 100644 --- a/app/controllers/api/variants_controller.rb +++ b/app/controllers/api/variants_controller.rb @@ -35,18 +35,12 @@ module Api end end - def soft_delete - @variant = scope.find(params[:variant_id]) - authorize! :delete, @variant - - VariantDeleter.new.delete(@variant) - render json: @variant, serializer: Api::VariantSerializer, status: :no_content - end - def destroy authorize! :delete, Spree::Variant @variant = scope.find(params[:id]) - @variant.destroy + authorize! :delete, @variant + + VariantDeleter.new.delete(@variant) render json: @variant, serializer: Api::VariantSerializer, status: :no_content end diff --git a/config/routes/api.rb b/config/routes/api.rb index 59e256acac..fbd7b23378 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -5,12 +5,9 @@ Openfoodnetwork::Application.routes.draw do get :bulk_products get :overridable end - delete :soft_delete post :clone - resources :variants do - delete :soft_delete - end + resources :variants end resources :variants, :only => [:index] diff --git a/spec/controllers/api/products_controller_spec.rb b/spec/controllers/api/products_controller_spec.rb index 861c12eaea..8fb917afbf 100644 --- a/spec/controllers/api/products_controller_spec.rb +++ b/spec/controllers/api/products_controller_spec.rb @@ -74,16 +74,18 @@ describe Api::ProductsController, type: :controller do context "as an enterprise user" do let(:current_api_user) { supplier_enterprise_user(supplier) } - it "soft deletes my products" do - spree_delete :soft_delete, product_id: product.to_param, format: :json + it "can delete my product" do + expect(product.deleted_at).to be_nil + api_delete :destroy, id: product.to_param expect(response.status).to eq(204) expect { product.reload }.not_to raise_error expect(product.deleted_at).not_to be_nil end - it "is denied access to soft deleting another enterprises' product" do - spree_delete :soft_delete, product_id: product_other_supplier.to_param, format: :json + it "is denied access to deleting another enterprises' product" do + expect(product_other_supplier.deleted_at).to be_nil + api_delete :destroy, id: product_other_supplier.to_param assert_unauthorized! expect { product_other_supplier.reload }.not_to raise_error @@ -97,14 +99,6 @@ describe Api::ProductsController, type: :controller do .to receive(:has_spree_role?).with("admin").and_return(true) end - it "soft deletes a product" do - spree_delete :soft_delete, product_id: product.to_param, format: :json - - expect(response.status).to eq(204) - expect { product.reload }.not_to raise_error - expect(product.deleted_at).not_to be_nil - end - it "can create a new product" do api_post :create, product: { name: "The Other Product", price: 19.99, diff --git a/spec/controllers/api/variants_controller_spec.rb b/spec/controllers/api/variants_controller_spec.rb index f0aa0d8da4..a5c81329dd 100644 --- a/spec/controllers/api/variants_controller_spec.rb +++ b/spec/controllers/api/variants_controller_spec.rb @@ -30,16 +30,6 @@ describe Api::VariantsController, type: :controller do expect(attributes.all?{ |attr| keys.include? attr }).to eq(true) end - it "is denied access when trying to delete a variant" do - product = create(:product) - variant = product.master - spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json - - assert_unauthorized! - expect { variant.reload }.not_to raise_error - expect(variant.deleted_at).to be_nil - end - it 'can query the results through a parameter' do expected_result = create(:variant, sku: 'FOOBAR') api_get :index, q: { sku_cont: 'FOO' } @@ -89,6 +79,7 @@ describe Api::VariantsController, type: :controller do assert_unauthorized! expect { variant.reload }.not_to raise_error + expect(variant.deleted_at).to be_nil end end @@ -100,8 +91,8 @@ describe Api::VariantsController, type: :controller do let(:product_other) { create(:product, supplier: supplier_other) } let(:variant_other) { product_other.master } - it "soft deletes a variant" do - spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json + it "deletes a variant" do + api_delete :destroy, id: variant.to_param expect(response.status).to eq(204) expect { variant.reload }.not_to raise_error @@ -109,11 +100,11 @@ describe Api::VariantsController, type: :controller do end it "is denied access to soft deleting another enterprises' variant" do - spree_delete :soft_delete, variant_id: variant_other.to_param, product_id: product_other.to_param, format: :json + api_delete :destroy, id: variant_other.to_param assert_unauthorized! - expect { variant.reload }.not_to raise_error - expect(variant.deleted_at).to be_nil + expect { variant_other.reload }.not_to raise_error + expect(variant_other.deleted_at).to be_nil end end @@ -124,23 +115,6 @@ describe Api::VariantsController, type: :controller do let(:variant) { product.master } let(:resource_scoping) { { product_id: variant.product.to_param } } - it "soft deletes a variant" do - spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json - - expect(response.status).to eq(204) - expect { variant.reload }.not_to raise_error - expect(variant.deleted_at).not_to be_nil - end - - it "doesn't delete the only variant of the product" do - product = create(:product) - variant = product.variants.first - spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json - - expect(variant.reload).to_not be_deleted - expect(assigns(:variant).errors[:product]).to include "must have at least one variant" - end - context "deleted variants" do before do variant.update_column(:deleted_at, Time.zone.now) @@ -173,7 +147,17 @@ describe Api::VariantsController, type: :controller do api_delete :destroy, id: variant.to_param expect(response.status).to eq(204) - expect { Spree::Variant.find(variant.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { variant.reload }.not_to raise_error + expect(variant.deleted_at).not_to be_nil + end + + it "doesn't delete the only variant of the product" do + product = create(:product) + variant = product.variants.first + spree_delete :destroy, id: variant.to_param + + expect(variant.reload).to_not be_deleted + expect(assigns(:variant).errors[:product]).to include "must have at least one variant" end end end diff --git a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee index c2db1cc403..40233684ea 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -741,7 +741,7 @@ describe "AdminProductEditCtrl", -> describe "deleting products", -> - it "deletes products with a http delete request to /api/products/id/soft_delete", -> + it "deletes products with a http delete request to /api/products/id", -> spyOn(window, "confirm").and.returnValue true $scope.products = [ { @@ -754,7 +754,7 @@ describe "AdminProductEditCtrl", -> } ] $scope.dirtyProducts = {} - $httpBackend.expectDELETE("/api/products/13/soft_delete").respond 200, "data" + $httpBackend.expectDELETE("/api/products/13").respond 200, "data" $scope.deleteProduct $scope.products[1] $httpBackend.flush() @@ -773,7 +773,7 @@ describe "AdminProductEditCtrl", -> DirtyProducts.addProductProperty 9, "someProperty", "something" DirtyProducts.addProductProperty 13, "name", "P1" - $httpBackend.expectDELETE("/api/products/13/soft_delete").respond 200, "data" + $httpBackend.expectDELETE("/api/products/13").respond 200, "data" $scope.deleteProduct $scope.products[1] $httpBackend.flush() expect($scope.products).toEqual [ @@ -813,7 +813,7 @@ describe "AdminProductEditCtrl", -> describe "when the variant has been saved", -> - it "deletes variants with a http delete request to /api/products/product_permalink/variants/(variant_id)/soft_delete", -> + it "deletes variants with a http delete request to /api/products/product_permalink/variants/(variant_id)", -> spyOn(window, "confirm").and.returnValue true $scope.products = [ { @@ -835,7 +835,7 @@ describe "AdminProductEditCtrl", -> } ] $scope.dirtyProducts = {} - $httpBackend.expectDELETE("/api/products/apples/variants/3/soft_delete").respond 200, "data" + $httpBackend.expectDELETE("/api/products/apples/variants/3").respond 200, "data" $scope.deleteVariant $scope.products[0], $scope.products[0].variants[0] $httpBackend.flush() @@ -865,7 +865,7 @@ describe "AdminProductEditCtrl", -> DirtyProducts.addVariantProperty 9, 4, "price", 6.0 DirtyProducts.addProductProperty 13, "name", "P1" - $httpBackend.expectDELETE("/api/products/apples/variants/3/soft_delete").respond 200, "data" + $httpBackend.expectDELETE("/api/products/apples/variants/3").respond 200, "data" $scope.deleteVariant $scope.products[0], $scope.products[0].variants[0] $httpBackend.flush() expect($scope.products[0].variants).toEqual [ diff --git a/swagger.yaml b/swagger.yaml index ba79384739..838f09f2d4 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -175,22 +175,6 @@ paths: schema: $ref: '#/components/schemas/Product' - /products/{product_id}/soft_delete: - delete: - description: Soft deletes the Product with the given ID. - tags: - - products - parameters: - - in: path - name: product_id - schema: - type: integer - required: true - description: Numeric ID of the Product. - responses: - '204': - description: successful deletion - /products/{product_id}/variants: get: description: Gets all Variants of the given Product. @@ -330,28 +314,6 @@ paths: '204': description: successful deletion - /products/{product_id}/variants/{variant_id}/soft_delete: - delete: - description: Soft-deletes the Variant with the given ID. - tags: - - product variants - parameters: - - in: path - name: product_id - schema: - type: integer - required: true - description: Numeric ID of the Product. - - in: path - name: variant_id - schema: - type: integer - required: true - description: Numeric ID of the Variant. - responses: - '204': - description: successful deletion - /product_images/{product_id}: post: description: Creates or updates product image.