From 7b33712f7ac51bcf2021b82c7df8182d7ef6da87 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:27:25 +0100 Subject: [PATCH 1/4] Add an API endpoint for EnterpriseFeesController#destroy --- .../directives/delete_resource.js.coffee | 2 +- .../api/enterprise_fees_controller.rb | 21 +++++++++++++++++++ config/routes.rb | 2 ++ config/routes/admin.rb | 2 +- 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 app/controllers/api/enterprise_fees_controller.rb diff --git a/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee b/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee index c5936393db..ba37f0518f 100644 --- a/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee +++ b/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee @@ -1,7 +1,7 @@ angular.module('admin.enterpriseFees').directive 'spreeDeleteResource', -> (scope, element, attrs) -> if scope.enterprise_fee.id - url = '/admin/enterprise_fees/' + scope.enterprise_fee.id + url = '/api/enterprise_fees/' + scope.enterprise_fee.id html = '' #var html = 'Delete Delete'; element.append html diff --git a/app/controllers/api/enterprise_fees_controller.rb b/app/controllers/api/enterprise_fees_controller.rb new file mode 100644 index 0000000000..3d03b28515 --- /dev/null +++ b/app/controllers/api/enterprise_fees_controller.rb @@ -0,0 +1,21 @@ +module Api + class EnterpriseFeesController < BaseController + respond_to :json + + def destroy + authorize! :destroy, enterprise_fee + + if enterprise_fee.destroy + render text: I18n.t(:successfully_removed), status: 204 + else + render text: enterprise_fee.errors.full_messages.first, status: 403 + end + end + + private + + def enterprise_fee + @enterprise_fee ||= EnterpriseFee.find_by_id params[:id] + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 1196e950ee..4804e4cae2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -111,6 +111,8 @@ Openfoodnetwork::Application.routes.draw do resources :customers, only: [:index, :update] + resources :enterprise_fees, only: [:destroy] + post '/product_images/:product_id', to: 'product_images#update_product_image' end diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c191322d74..1f55f8d154 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -35,7 +35,7 @@ Openfoodnetwork::Application.routes.draw do resources :enterprise_relationships resources :enterprise_roles - resources :enterprise_fees do + resources :enterprise_fees, except: :destroy do collection do get :for_order_cycle post :bulk_update, :as => :bulk_update From a162a2c50b14ce13521f62293fdcad690051d7bc Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:31:22 +0100 Subject: [PATCH 2/4] Move product distributions check as a before_destroy in EnterpriseFee model --- app/models/enterprise_fee.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 05276cf3d7..60c8bb9726 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -26,6 +26,7 @@ class EnterpriseFee < ActiveRecord::Base validates_presence_of :name before_save :ensure_valid_tax_category_settings + before_destroy :ensure_no_product_distributions scope :for_enterprise, lambda { |enterprise| where(enterprise_id: enterprise) } scope :for_enterprises, lambda { |enterprises| where(enterprise_id: enterprises) } @@ -68,6 +69,15 @@ class EnterpriseFee < ActiveRecord::Base return true end + def ensure_no_product_distributions + dependent_distribution = ProductDistribution.where(enterprise_fee_id: self).first + return unless dependent_distribution + product = dependent_distribution.product + error = I18n.t(:enterprise_fees_destroy_error, id: product.id, name: product.name) + errors.add(:base, error) + false + end + def refresh_products_cache OpenFoodNetwork::ProductsCache.enterprise_fee_changed self end From 0868404e986b5c3e859b38c87718ee642397d172 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:32:03 +0100 Subject: [PATCH 3/4] Add specs for new Api::EnterpriseFeesController --- .../api/enterprise_fees_controller_spec.rb | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 spec/controllers/api/enterprise_fees_controller_spec.rb diff --git a/spec/controllers/api/enterprise_fees_controller_spec.rb b/spec/controllers/api/enterprise_fees_controller_spec.rb new file mode 100644 index 0000000000..fb938d43c5 --- /dev/null +++ b/spec/controllers/api/enterprise_fees_controller_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +module Api + describe EnterpriseFeesController, type: :controller do + include AuthenticationWorkflow + + let!(:unreferenced_fee) { create(:enterprise_fee) } + let!(:referenced_fee) { create(:enterprise_fee) } + let(:product) { create(:product) } + let(:distributor) { create(:distributor_enterprise) } + let!(:product_distribution) { create(:product_distribution, product: product, distributor: distributor, enterprise_fee: referenced_fee) } + let(:current_user) { create(:admin_user) } + + before do + allow(controller).to receive(:spree_current_user) { current_user } + end + + describe "destroy" do + it "removes the fee" do + expect { spree_delete :destroy, id: unreferenced_fee.id, format: :json } + .to change { EnterpriseFee.count }.by -1 + end + + context "when the fee is referenced by a product distribution" do + it "does not remove the fee" do + spree_delete :destroy, id: referenced_fee.id, format: :json + expect(response.status).to eq 403 + expect(response.body).to match(/That enterprise fee cannot be deleted/) + expect(referenced_fee.reload).to eq(referenced_fee) + end + end + end + end +end From a50786be3470faa272ba216796e0492f288ca97d Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:34:22 +0100 Subject: [PATCH 4/4] Remove old do_not_remove_referenced_fees method --- .../admin/enterprise_fees_controller.rb | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/app/controllers/admin/enterprise_fees_controller.rb b/app/controllers/admin/enterprise_fees_controller.rb index 528284c4e4..46e2b535fb 100644 --- a/app/controllers/admin/enterprise_fees_controller.rb +++ b/app/controllers/admin/enterprise_fees_controller.rb @@ -2,7 +2,6 @@ module Admin class EnterpriseFeesController < ResourceController before_filter :load_enterprise_fee_set, :only => :index before_filter :load_data - before_filter :do_not_destroy_referenced_fees, :only => :destroy def index @@ -45,22 +44,6 @@ module Admin private - def do_not_destroy_referenced_fees - product_distribution = ProductDistribution.where(:enterprise_fee_id => @object).first - if product_distribution - p = product_distribution.product - error = I18n.t(:enterprise_fees_destroy_error, id: p.id, name: p.name) - - respond_with(@object) do |format| - format.html do - flash[:error] = error - redirect_to collection_url - end - format.js { render text: error, status: 403 } - end - end - end - def load_enterprise_fee_set @enterprise_fee_set = EnterpriseFeeSet.new :collection => collection end