Add endpoints for removing enterprise images

app/controllers/api/base_controller.rb

@@ -9,5 +9,9 @@ module Api
     include ActionController::UrlFor
     include Rails.application.routes.url_helpers
     use_renderers :json
+
+    def respond_with_conflict(json_hash)
+      render json: json_hash, status: :conflict
+    end
   end
 end

app/controllers/api/enterprise_attachment_controller.rb

@@ -0,0 +1,37 @@
+module Api
+  class EnterpriseAttachmentController < BaseController
+    class MissingImplementationError < StandardError; end
+    class UnknownEnterpriseAuthorizationActionError < StandardError; end
+
+    before_filter :load_enterprise
+
+    respond_to :json
+
+    def destroy
+      return respond_with_conflict(error: destroy_attachment_does_not_exist_error_message) unless @enterprise.public_send("#{attachment_name}?")
+
+      @enterprise.update_attributes!(attachment_name => nil)
+      render json: @enterprise, serializer: Admin::EnterpriseSerializer, spree_current_user: spree_current_user
+    end
+
+    protected
+
+    def attachment_name
+      raise MissingImplementationError, "Method attachment_name should be defined"
+    end
+
+    def enterprise_authorize_action
+      raise MissingImplementationError, "Method enterprise_authorize_action should be defined"
+    end
+
+    def load_enterprise
+      @enterprise = Enterprise.find_by_permalink(params[:enterprise_id].to_s)
+      raise UnknownEnterpriseAuthorizationActionError if enterprise_authorize_action.blank?
+      authorize!(enterprise_authorize_action, @enterprise)
+    end
+
+    def destroy_attachment_does_not_exist_error_message
+      I18n.t("api.enterprise_#{attachment_name}.destroy_attachment_does_not_exist")
+    end
+  end
+end

app/controllers/api/logos_controller.rb

@@ -0,0 +1,16 @@
+module Api
+  class LogosController < EnterpriseAttachmentController
+    private
+
+    def attachment_name
+      :logo
+    end
+
+    def enterprise_authorize_action
+      case action_name.to_sym
+      when :destroy
+        :remove_logo
+      end
+    end
+  end
+end

app/controllers/api/promo_images_controller.rb

@@ -0,0 +1,16 @@
+module Api
+  class PromoImagesController < EnterpriseAttachmentController
+    private
+
+    def attachment_name
+      :promo_image
+    end
+
+    def enterprise_authorize_action
+      case action_name.to_sym
+      when :destroy
+        :remove_promo_image
+      end
+    end
+  end
+end

app/models/spree/ability_decorator.rb

@@ -99,7 +99,7 @@ class AbilityDecorator
     end
 
     can [:admin, :index, :create], Enterprise
-    can [:read, :edit, :update, :bulk_update, :resend_confirmation], Enterprise do |enterprise|
+    can [:read, :edit, :update, :remove_logo, :remove_promo_image, :bulk_update, :resend_confirmation], Enterprise do |enterprise|
       OpenFoodNetwork::Permissions.new(user).editable_enterprises.include? enterprise
     end
     can [:welcome, :register], Enterprise do |enterprise|

config/locales/en.yml

@@ -1077,6 +1077,14 @@ en:
       stripe_connect_settings:
         resource: Stripe Connect configuration
 
+# API
+#
+  api:
+    enterprise_logo:
+      destroy_attachment_does_not_exist: "Logo does not exist"
+    enterprise_promo_image:
+      destroy_attachment_does_not_exist: "Promo image does not exist"
+
 # Frontend views
 #
 # These keys are referenced relatively like `t('.message')` in

config/routes.rb

@@ -95,7 +95,11 @@ Openfoodnetwork::Application.routes.draw do
       post :update_image, on: :member
       get :managed, on: :collection
       get :accessible, on: :collection
+
+      resource :logo, only: [:destroy]
+      resource :promo_image, only: [:destroy]
     end
+
     resources :order_cycles do
       get :managed, on: :collection
       get :accessible, on: :collection

spec/controllers/api/logos_controller_spec.rb

@@ -0,0 +1,90 @@
+require "spec_helper"
+
+module Api
+  describe LogosController, type: :controller do
+    include AuthenticationWorkflow
+
+    let(:admin_user) { create(:admin_user) }
+    let(:enterprise_owner) { create(:user) }
+    let(:enterprise) { create(:enterprise, owner: enterprise_owner ) }
+    let(:enterprise_manager) { create(:user, enterprise_limit: 10, enterprises: [enterprise]) }
+    let(:other_enterprise_owner) { create(:user) }
+    let(:other_enterprise) { create(:enterprise, owner: other_enterprise_owner ) }
+    let(:other_enterprise_manager) { create(:user, enterprise_limit: 10, enterprises: [other_enterprise]) }
+
+    describe "removing logo" do
+      image_path = File.open(Rails.root.join("app", "assets", "images", "logo-black.png"))
+      let(:image) { Rack::Test::UploadedFile.new(image_path, "image/png") }
+
+      let(:enterprise) { create(:enterprise, owner: enterprise_owner, logo: image) }
+
+      before do
+        allow(controller).to receive(:spree_current_user) { current_user }
+      end
+
+      context "as manager" do
+        let(:current_user) { enterprise_manager }
+
+        it "removes logo" do
+          spree_delete :destroy, enterprise_id: enterprise
+
+          expect(response).to be_success
+          expect(json_response["id"]).to eq enterprise.id
+          enterprise.reload
+          expect(enterprise.logo?).to be false
+        end
+
+        context "when logo does not exist" do
+          let(:enterprise) { create(:enterprise, owner: enterprise_owner, logo: nil) }
+
+          it "responds with error" do
+            spree_delete :destroy, enterprise_id: enterprise
+
+            expect(response.status).to eq(409)
+            expect(json_response["error"]).to eq I18n.t("api.enterprise_logo.destroy_attachment_does_not_exist")
+          end
+        end
+      end
+
+      context "as owner" do
+        let(:current_user) { enterprise_owner }
+
+        it "allows removal of logo" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response).to be_success
+        end
+      end
+
+      context "as super admin" do
+        let(:current_user) { admin_user }
+
+        it "allows removal of logo" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response).to be_success
+        end
+      end
+
+      context "as manager of other enterprise" do
+        let(:current_user) { other_enterprise_manager }
+
+        it "does not allow removal of logo" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response.status).to eq(401)
+          enterprise.reload
+          expect(enterprise.logo?).to be true
+        end
+      end
+
+      context "as owner of other enterprise" do
+        let(:current_user) { other_enterprise_owner }
+
+        it "does not allow removal of logo" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response.status).to eq(401)
+          enterprise.reload
+          expect(enterprise.logo?).to be true
+        end
+      end
+    end
+  end
+end

spec/controllers/api/promo_images_controller_spec.rb

@@ -0,0 +1,90 @@
+require "spec_helper"
+
+module Api
+  describe PromoImagesController, type: :controller do
+    include AuthenticationWorkflow
+
+    let(:admin_user) { create(:admin_user) }
+    let(:enterprise_owner) { create(:user) }
+    let(:enterprise) { create(:enterprise, owner: enterprise_owner ) }
+    let(:enterprise_manager) { create(:user, enterprise_limit: 10, enterprises: [enterprise]) }
+    let(:other_enterprise_owner) { create(:user) }
+    let(:other_enterprise) { create(:enterprise, owner: other_enterprise_owner ) }
+    let(:other_enterprise_manager) { create(:user, enterprise_limit: 10, enterprises: [other_enterprise]) }
+
+    describe "removing promo image" do
+      image_path = File.open(Rails.root.join("app", "assets", "images", "logo-black.png"))
+      let(:image) { Rack::Test::UploadedFile.new(image_path, "image/png") }
+
+      let(:enterprise) { create(:enterprise, owner: enterprise_owner, promo_image: image) }
+
+      before do
+        allow(controller).to receive(:spree_current_user) { current_user }
+      end
+
+      context "as manager" do
+        let(:current_user) { enterprise_manager }
+
+        it "removes promo image" do
+          spree_delete :destroy, enterprise_id: enterprise
+
+          expect(response).to be_success
+          expect(json_response["id"]).to eq enterprise.id
+          enterprise.reload
+          expect(enterprise.promo_image?).to be false
+        end
+
+        context "when promo image does not exist" do
+          let(:enterprise) { create(:enterprise, owner: enterprise_owner, promo_image: nil) }
+
+          it "responds with error" do
+            spree_delete :destroy, enterprise_id: enterprise
+
+            expect(response.status).to eq(409)
+            expect(json_response["error"]).to eq I18n.t("api.enterprise_promo_image.destroy_attachment_does_not_exist")
+          end
+        end
+      end
+
+      context "as owner" do
+        let(:current_user) { enterprise_owner }
+
+        it "allows removal of promo image" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response).to be_success
+        end
+      end
+
+      context "as super admin" do
+        let(:current_user) { admin_user }
+
+        it "allows removal of promo image" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response).to be_success
+        end
+      end
+
+      context "as manager of other enterprise" do
+        let(:current_user) { other_enterprise_manager }
+
+        it "does not allow removal of promo image" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response.status).to eq(401)
+          enterprise.reload
+          expect(enterprise.promo_image?).to be true
+        end
+      end
+
+      context "as owner of other enterprise" do
+        let(:current_user) { other_enterprise_owner }
+
+        it "does not allow removal of promo image" do
+          spree_delete :destroy, enterprise_id: enterprise
+          expect(response.status).to eq(401)
+          enterprise.reload
+          expect(enterprise.promo_image?).to be true
+        end
+      end
+    end
+  end
+end

spec/models/spree/ability_spec.rb

@@ -297,11 +297,11 @@ module Spree
           let!(:er_pd) { create(:enterprise_relationship, parent: d_related, child: d1, permissions_list: [:edit_profile]) }
 
           it "should be able to edit enterprises it manages" do
-            should have_ability([:read, :edit, :update, :bulk_update, :resend_confirmation], for: d1)
+            should have_ability([:read, :edit, :update, :remove_logo, :remove_promo_image, :bulk_update, :resend_confirmation], for: d1)
           end
 
           it "should be able to edit enterprises it has permission to" do
-            should have_ability([:read, :edit, :update, :bulk_update, :resend_confirmation], for: d_related)
+            should have_ability([:read, :edit, :update, :remove_logo, :remove_promo_image, :bulk_update, :resend_confirmation], for: d_related)
           end
 
           it "should be able to manage shipping methods, payment methods and enterprise fees for enterprises it manages" do