Implement mandatory fields validation in request approval process

Author: Olexii Kasianenko

app/models/partner.rb

@@ -48,8 +48,6 @@ class Partner < ApplicationRecord
 
   has_many_attached :documents
 
-  # validates :name, presence: true, uniqueness: { scope: :organization }
-
   validates :email, presence: true, uniqueness: { case_sensitive: false }, format: { with: URI::MailTo::EMAIL_REGEXP }
 
   validates :quota, numericality: { greater_than_or_equal_to: 0 }, allow_blank: true

app/models/partners/profile.rb

@@ -191,40 +191,6 @@ def self.agency_types_for_selection
       agency_types.keys.map(&:to_sym).sort_by { |sym| I18n.t(sym, scope: :partners_profile) }.partition { |v| v != :other }.flatten
     end
 
-    def check_social_media
-      return if organization.one_step_partner_invite
-      return if website.present? || twitter.present? || facebook.present? || instagram.present?
-      return if partner.partials_to_show.exclude?("media_information")
-
-      unless no_social_media_presence
-        errors.add(:no_social_media_presence, "No social media presence must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.")
-      end
-      errors
-    end
-
-    def check_mandatory_fields
-      return if organization.one_step_partner_invite
-
-      mandatory_fields = [
-        :agency_type,
-        :address1,
-        :city,
-        :state,
-        :zip_code,
-        :program_name,
-        :program_description
-      ]
-      messages = []
-      messages << "Name can't be blank" if partner.name.blank?
-      mandatory_fields.each do |field|
-        if send(field).blank?
-          messages << "#{field.to_s.humanize.capitalize} can't be blank"
-        end
-      end
-      errors.add(:base, messages.join(", ")) if messages.any?
-      errors
-    end
-
     private
 
     def client_share_is_0_or_100

app/services/partner_profile_update_service.rb

@@ -7,13 +7,14 @@ def initialize(old_partner, new_partner_params, new_profile_params)
     @profile = @partner.profile
     @partner_params = new_partner_params
     @profile_params = new_profile_params
+    @error_messages = []
   end
 
   def call
     return self unless validation
 
     perform_profile_service do
-      @partner.update(@partner_params)
+      @partner.update!(@partner_params)
       @profile.served_areas.destroy_all
       @profile.attributes = @profile_params
       @profile.save!(context: :edit)
@@ -52,6 +53,7 @@ def validation
 
     check_social_media
     check_mandatory_fields
+    @error = @error_messages.join(". ") if @error_messages.any?
     @error.nil?
   end
 
@@ -60,14 +62,14 @@ def check_mandatory_fields
     missing_fields = mandatory_fields.select { |field| @profile_params[field].blank? }
     missing_fields.prepend :agency_name if @partner_params[:name].blank?
     if missing_fields.any?
-      @error = "Missing mandatory fields: #{missing_fields.join(', ')}"
+      @error_messages << "Missing mandatory fields: #{missing_fields.join(", ")}"
     end
   end
 
   def check_social_media
     social_media_fields = %i[website facebook twitter instagram]
-    if social_media_fields.all? { |field| @profile_params[field].blank? } && @profile_params[:no_social_media_presence] == '0'
-      @error = "At least one social media field must be filled out or 'No social media presence' must be checked."
+    if social_media_fields.all? { |field| @profile_params[field].blank? } && !@profile_params[:no_social_media_presence]
+      @error_messages << "At least one social media field must be filled out or 'No social media presence' must be checked."
     end
   end
 end

app/services/partner_update_service.rb

@@ -25,7 +25,7 @@ def validation
     mandatory_fields = %i[name]
     missing_fields = mandatory_fields.select { |field| params[field].blank? }
     if missing_fields.any?
-      @error = "Missing mandatory fields: #{missing_fields.join(', ')}"
+      @error = "Missing mandatory fields: #{missing_fields.join(", ")}"
       return false
     end
     true

app/services/partners/request_approval_service.rb

@@ -33,10 +33,44 @@ def valid?
         errors.add :base, partner.profile.errors.full_messages.join('. ')
       end
 
-      errors.merge! partner.profile.errors if partner.profile.check_social_media
-      errors.merge! partner.profile.errors if partner.profile.check_mandatory_fields
+      check_social_media
+      check_mandatory_fields
 
       errors.none?
     end
+
+    def check_mandatory_fields
+      return if partner.organization.one_step_partner_invite
+
+      mandatory_fields = [
+        :agency_type,
+        :address1,
+        :city,
+        :state,
+        :zip_code,
+        :program_name,
+        :program_description
+      ]
+      messages = []
+      messages << "Name can't be blank" if partner.name.blank?
+      mandatory_fields.each do |field|
+        if partner.profile.send(field).blank?
+          messages << "#{field.to_s.humanize.capitalize} can't be blank"
+        end
+      end
+      errors.add(:base, messages.join(", ")) if messages.any?
+      errors
+    end
+
+    def check_social_media
+      return if partner.organization.one_step_partner_invite
+      return if partner.profile.website.present? || partner.profile.twitter.present? || partner.profile.facebook.present? || partner.profile.instagram.present?
+      return if partner.partials_to_show.exclude?("media_information")
+
+      unless partner.profile.no_social_media_presence
+        errors.add(:base, "No social media presence must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.")
+      end
+      errors
+    end
   end
 end

clock.rb

@@ -6,7 +6,7 @@
 
 module Clockwork
   handler do |job|
-    puts "Running #{job}"
+    Rails.logger.info("Running #{job}")
   end
 
   DATA_TYPES = %w[Distribution Purchase Donation]

spec/requests/partners/profiles_requests_spec.rb

@@ -107,18 +107,18 @@
         {
           partner: {
             name: "Partnerdude", profile: {
-              no_social_media_presence: '0',
+              no_social_media_presence: "0",
               website: "",
               twitter: "",
               facebook: "",
               instagram: "",
-              agency_type: 'OTHER',
-              address1: '123 Main St',
-              city: 'Anytown',
-              state: 'CA',
-              zip_code: '12345',
-              program_name: 'Test Program',
-              program_description: 'Test Description'
+              agency_type: "OTHER",
+              address1: "123 Main St",
+              city: "Anytown",
+              state: "CA",
+              zip_code: "12345",
+              program_name: "Test Program",
+              program_description: "Test Description"
             }
           }
         }

spec/services/partner_profile_update_service_spec.rb

@@ -21,11 +21,14 @@
         end
 
         context "and there are other errors in the profile" do
+          before do
+            profile.partner.awaiting_review!
+          end
           it "does not store the new values and it returns a failure" do
             expect(profile.served_areas.size).to eq(0)
             result = PartnerProfileUpdateService.new(profile.partner, partner_params, other_incorrect_attributes).call
             expect(result.success?).to eq(false)
-            expect(result.error.to_s).to include("No social media presence must be checked")
+            expect(result.error.to_s).to include("'No social media presence' must be checked")
             profile.reload
             expect(profile.served_areas.size).to eq(0)
           end
@@ -59,13 +62,16 @@
           end
         end
         context "and there are errors on the profile" do
+          before do
+            profile.partner.awaiting_review!
+          end
           it "maintains the old values and returns failure" do
             profile.reload
             expect(profile.served_areas.size).to eq(2)
-            PartnerProfileUpdateService.new(profile.partner, partner_params, other_incorrect_attributes).call
+            result = PartnerProfileUpdateService.new(profile.partner, partner_params, other_incorrect_attributes).call
             profile.reload
             expect(profile.served_areas.size).to eq(2)
-            expect(profile.errors).to_not be_empty
+            expect(result.success?).to eq(false)
           end
         end
       end
@@ -108,7 +114,7 @@
       it "returns failure" do
         result = PartnerProfileUpdateService.new(partner, partner_params, basic_correct_attributes).call
         expect(result.success?).to eq(false)
-        expect(result.error.to_s).to include("Partner '#{partner.name}' had error(s) preventing the profile from being updated: Email is invalid")
+        expect(result.error.to_s).to include("Validation failed: Email is invalid")
       end
 
       it "doesn't update the partner profile" do

spec/services/partners/request_approval_service_spec.rb

@@ -31,9 +31,28 @@
       end
     end
 
+    context 'when the partner is not yet awaiting approval and mandatory fields are missed' do
+      it 'should return an error re mandatory fields' do
+        partner.update(name: '')
+        partner.profile.update(
+          agency_type: '',
+          address1: '',
+          city: '',
+          state: '',
+          zip_code: '',
+          program_name: '',
+          program_description: ''
+        )
+
+        expect(subject.errors.full_messages)
+          .to include("Name can't be blank, Agency type can't be blank, Address1 can't be blank, City can't be blank, State can't be blank, Zip code can't be blank, Program name can't be blank, Program description can't be blank")
+      end
+    end
+
     context 'when the partner is not yet waiting for approval and there is no profile error' do
       it 'does not have an error' do
         partner.profile.update(website: 'website URL', facebook: '', twitter: '', instagram: '', no_social_media_presence: false)
+        partner.organization.update(one_step_partner_invite: true)
         expect(subject.errors.full_messages).to be_empty
       end
     end
@@ -44,6 +63,7 @@
         allow(OrganizationMailer).to receive(:partner_approval_request).with(partner: partner, organization: partner.organization).and_return(fake_mailer)
         expect(partner.status).not_to eq(:awaiting_review)
         partner.profile.update(website: 'website URL')
+        partner.organization.update(one_step_partner_invite: true)
       end
 
       it 'should set the status on the partner record to awaiting_review' do

spec/system/partners/approval_process_spec.rb

@@ -109,89 +109,4 @@
       end
     end
   end
-
-  describe "request approval with invalid details" do
-    let(:partner_user) { partner.primary_user }
-    let(:partner) { FactoryBot.create(:partner) }
-
-    before do
-      login_as(partner_user)
-      visit partner_user_root_path
-      click_on 'My Profile'
-    end
-
-    subject { all('button', text: 'Submit for Approval').last.click }
-
-    context "Social media information is absent" do
-      before do
-        partner.profile.update(website: '', facebook: '', twitter: '', instagram: '', no_social_media_presence: false)
-      end
-
-      context "partner status is invited" do
-        before do
-          partner.update(status: :invited)
-        end
-
-        it "should render an error message", :aggregate_failures do
-          subject
-          assert page.has_content? 'No social media presence must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.'
-        end
-
-        context "partner's organization one_step_partner_invite is true" do
-          before do
-            partner.organization.update(one_step_partner_invite: true)
-          end
-
-          it "should render an error message about social media presence only", :aggregate_failures do
-            subject
-            assert page.has_content? 'You have submitted your details for approval.'
-          end
-        end
-      end
-
-      context "partner status is awaiting_review" do
-        before do
-          partner.update(status: :awaiting_review)
-        end
-
-        it "should render an error message", :aggregate_failures do
-          subject
-          assert page.has_content? 'This partner has already requested approval.'
-        end
-      end
-    end
-
-    context "Mandatory fields are empty" do
-      before do
-        partner.update(name: '')
-        partner.profile.update(
-          agency_type: '',
-          address1: '',
-          city: '',
-          state: '',
-          zip_code: '',
-          program_name: '',
-          program_description: ''
-        )
-      end
-
-      context "partner status is invited" do
-        before do
-          partner.update(status: :invited)
-        end
-
-        it "should render error messages for each missing field", :aggregate_failures do
-          subject
-          assert page.has_content? "Name can't be blank"
-          assert page.has_content? "Agency type can't be blank"
-          assert page.has_content? "Address1 can't be blank"
-          assert page.has_content? "City can't be blank"
-          assert page.has_content? "State can't be blank"
-          assert page.has_content? "Zip code can't be blank"
-          assert page.has_content? "Program name can't be blank"
-          assert page.has_content? "Program description can't be blank"
-        end
-      end
-    end
-  end
 end