Merge pull request #4578 from kshlyk/remove_soft_delete_from_product_and_variant_api

Removing duplicate API method soft_delete for both products and variants

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

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

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
 

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]

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,

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

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 [

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.