Merge pull request #13654 from pacodelaluna/check-enterprise-image-logic

Improve enterprise images-related logic

app/controllers/api/v0/enterprises_controller.rb

@@ -43,7 +43,7 @@ module Api
         @enterprise = Enterprise.find_by(permalink: params[:id]) || Enterprise.find(params[:id])
         authorize! :update, @enterprise
 
-        if params[:logo] && @enterprise.update( logo: params[:logo] )
+        if params[:logo] && @enterprise.update!(logo: params[:logo])
           render(html: @enterprise.logo_url(:medium), status: :ok)
         elsif params[:promo] && @enterprise.update!( promo_image: params[:promo] )
           render(html: @enterprise.promo_image_url(:medium), status: :ok)

app/models/enterprise.rb

@@ -115,6 +115,9 @@ class Enterprise < ApplicationRecord
   validates :promo_image,
             processable_image: true,
             content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+  validates :white_label_logo,
+            processable_image: true,
+            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
   validates :terms_and_conditions, content_type: {
     in: "application/pdf",
     message: I18n.t(:enterprise_terms_and_conditions_type_error),

app/serializers/api/admin/enterprise_serializer.rb

@@ -62,7 +62,7 @@ module Api
       #     medium: LOGO_MEDIUM_URL
       #   }
       def attachment_urls(attachment, styles)
-        return unless attachment.variable?
+        return unless attachment.persisted? && attachment.variable?
 
         styles.index_with do |style|
           Rails.application.routes.url_helpers.

spec/models/enterprise_spec.rb

@@ -386,33 +386,65 @@ RSpec.describe Enterprise do
       end
     end
 
-    describe "white label logo link" do
+    describe "white label logo" do
+      let(:enterprise) { build(:enterprise) }
+      let(:content_type) { 'image/png' }
+
       before do
-        # validate white_label_logo_link only if white_label_logo is present
-        allow_any_instance_of(Enterprise).to receive(:white_label_logo).and_return(true)
+        blob = instance_double(
+          "ActiveStorage::Blob",
+          filename: ActiveStorage::Filename.new('white-label-logo.png'),
+          content_type:,
+          byte_size: 1024
+        )
+
+        # InstanceDouble is not working for attachment case as the blob method is not yet defined
+        # on instantiation.
+        attachment = double(
+          "ActiveStorage::Attached::One",
+          blank?: false,
+          attached?: true,
+          blob:
+        )
+
+        allow(enterprise)
+          .to receive(:white_label_logo).and_return(attachment)
+      end
+
+      context 'when the file attached is a PNG image' do
+        it 'is valid' do
+          expect(enterprise).to be_valid
+        end
+      end
+
+      context 'when the file attached is a BMP image' do
+        let(:content_type) { 'image/bmp' }
+        it 'is not valid' do
+          expect(enterprise).not_to be_valid
+        end
       end
 
       it "validates the white_label_logo_link attribute" do
-        e = build(:enterprise, white_label_logo_link: 'http://www.example.com')
-        expect(e).to be_valid
-        expect(e.white_label_logo_link).to eq "http://www.example.com"
+        enterprise.white_label_logo_link = 'http://www.example.com'
+        expect(enterprise).to be_valid
+        expect(enterprise.white_label_logo_link).to eq "http://www.example.com"
       end
 
       it "adds http:// to the white_label_logo_link attribute if it is missing" do
-        e = build(:enterprise, white_label_logo_link: 'www.example.com')
-        expect(e).to be_valid
-        expect(e.white_label_logo_link).to eq "http://www.example.com"
+        enterprise.white_label_logo_link = 'www.example.com'
+        expect(enterprise).to be_valid
+        expect(enterprise.white_label_logo_link).to eq "http://www.example.com"
       end
 
       it "ignores whitespace around the URL form copying and pasting" do
-        e = build(:enterprise, white_label_logo_link: ' www.example.com ')
-        expect(e).to be_valid
-        expect(e.white_label_logo_link).to eq "http://www.example.com"
+        enterprise.white_label_logo_link = ' www.example.com '
+        expect(enterprise).to be_valid
+        expect(enterprise.white_label_logo_link).to eq "http://www.example.com"
       end
 
       it "does not validate if URL is invalid and can't be infered" do
-        e = build(:enterprise, white_label_logo_link: 'with spaces')
-        expect(e).not_to be_valid
+        enterprise.white_label_logo_link = 'with spaces'
+        expect(enterprise).not_to be_valid
       end
     end