Merge pull request #3651 from rubyforgood/submit-for-approval-behavio…

…r-change

Submit for approval behavior change

app/controllers/partners/approval_requests_controller.rb

@@ -7,7 +7,7 @@ def create
       if svc.errors.none?
         flash[:success] = "You have submitted your details for approval."
       else
-        flash[:error] = "Sorry we've encountered an issue. Please try requesting this approval again"
+        flash[:error] = svc.errors.to_a.first
       end
 
       redirect_to partners_profile_path

app/models/partners/profile.rb

@@ -93,8 +93,7 @@ class Profile < Base
     accepts_nested_attributes_for :served_areas, allow_destroy: true
 
     has_many_attached :documents
-
-    validates :no_social_media_presence, acceptance: {message: "must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram."}, if: :has_no_social_media?
+    validate :check_social_media, on: :edit
 
     validate :client_share_is_0_or_100
     validate :has_at_least_one_request_setting
@@ -113,14 +112,21 @@ class Profile < Base
       ages_served
     ]
 
-    def has_no_social_media?
-      website.blank? && twitter.blank? && facebook.blank? && instagram.blank?
-    end
-
     def client_share_total
       served_areas.sum(&:client_share)
     end
 
+    private
+
+    def check_social_media
+      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, "must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.")
+      end
+    end
+
     def client_share_is_0_or_100
       # business logic:  the client share has to be 0 or 100 -- although it is an estimate only,  making it 0 (not
       # specified at all) or 100 means we won't have people overallocating (> 100) and that they think about what

app/services/partner_profile_update_service.rb

@@ -17,7 +17,8 @@ def call
       if @return_value
         @profile.served_areas.destroy_all
         @profile.reload
-        @profile.update!(@profile_params)
+        @profile.attributes = @profile_params
+        @profile.save!(context: :edit)
         @profile.reload
       end
     end

app/services/partners/request_approval_service.rb

@@ -26,7 +26,11 @@ def call
 
     def valid?
       if partner.status == 'awaiting_review'
-        errors.add(:base, 'partner has already requested approval')
+        errors.add(:base, 'This partner has already requested approval.')
+      end
+
+      unless partner.profile.valid?(:edit)
+        errors.copy!(partner.profile)
       end
 
       errors.none?

spec/models/partners/profile_spec.rb

@@ -120,15 +120,21 @@
       let(:profile) { FactoryBot.build(:partner_profile, website: "", twitter: "", facebook: "", instagram: "", no_social_media_presence: false) }
 
       it "should not be valid" do
-        expect(profile.valid?).to eq(false)
+        expect(profile.valid?(:edit)).to eq(false)
+      end
+
+      it "should be valid if media_information is not needed" do
+        org = profile.partner.organization
+        org.update!(partner_form_fields: %w[agency_stability])
+        expect(profile.valid?(:edit)).to eq(true)
       end
     end
 
     context "no social media presence and the checkbox is checked" do
       let(:profile) { FactoryBot.build(:partner_profile, website: "", twitter: "", facebook: "", instagram: "", no_social_media_presence: true) }
 
       it "should be valid" do
-        expect(profile.valid?).to eq(true)
+        expect(profile.valid?(:edit)).to eq(true)
       end
     end
 
@@ -137,27 +143,27 @@
 
       it "with just a website it should be valid" do
         profile.update(website: "some website URL", twitter: "", facebook: "", instagram: "")
-        expect(profile.valid?).to eq(true)
+        expect(profile.valid?(:edit)).to eq(true)
       end
 
       it "with just twitter it should be valid" do
         profile.update(website: "", twitter: "some twitter URL", facebook: "", instagram: "")
-        expect(profile.valid?).to eq(true)
+        expect(profile.valid?(:edit)).to eq(true)
       end
 
       it "with just facebook it should be valid" do
         profile.update(website: "", twitter: "", facebook: "some facebook URL", instagram: "")
-        expect(profile.valid?).to eq(true)
+        expect(profile.valid?(:edit)).to eq(true)
       end
 
       it "with just instagram it should be valid" do
         profile.update(website: "", twitter: "", facebook: "", instagram: "some instagram URL")
-        expect(profile.valid?).to eq(true)
+        expect(profile.valid?(:edit)).to eq(true)
       end
 
       it "with every social media option it should be valid" do
         profile.update(website: "some website URL", twitter: "some twitter URL", facebook: "some facebook URL", instagram: "some instagram URL")
-        expect(profile.valid?).to eq(true)
+        expect(profile.valid?(:edit)).to eq(true)
       end
     end
   end

spec/requests/partners/profiles_requests_spec.rb

@@ -33,6 +33,15 @@
       expect(response).to redirect_to(partners_profile_path)
     end
 
+    context "with no social media" do
+      it "shows an error" do
+        put partners_profile_path(partner,
+          partner: {name: "Partnerdude", profile: {website: "", no_social_media: false}})
+        expect(response).not_to redirect_to(anything)
+        expect(response.body).to include("No social media presence must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.")
+      end
+    end
+
     context "when updating an existing value to a blank value" do
       before do
         partner.profile.update!(city: "")

spec/requests/profiles_requests_spec.rb

@@ -40,7 +40,10 @@
     context "when updating an existing value to a blank value" do
       let(:partner_params) do
         { name: "Awesome Partner", profile:
-                               { executive_director_email: "awesomepartner@example.com", facebook: "", website: "" } }
+                               { executive_director_email: "awesomepartner@example.com",
+                                 no_social_media_presence: true,
+                                 facebook: "",
+                                 website: "" } }
       end
 
       it "update partner" do

spec/services/partners/request_approval_service_spec.rb

@@ -5,17 +5,28 @@
     subject { described_class.new(partner: partner).call }
     let(:partner) { create(:partner) }
 
-    it 'should return an instance of itself' do
-      expect(subject).to be_a_kind_of(Partners::RequestApprovalService)
-    end
-
     context 'when the partner is already awaiting approval' do
       before do
         partner.update!(status: 'awaiting_review')
       end
 
       it 'should return an error saying it the partner is already requested approval' do
-        expect(subject.errors[:base]).to eq(["partner has already requested approval"])
+        expect(subject.errors[:base]).to include("This partner has already requested approval.")
+      end
+    end
+
+    context 'when the partner is not yet waiting for approval and there is a profile error' do
+      it 'should return an error' do
+        partner.profile.update(website: '', facebook: '', twitter: '', instagram: '', no_social_media_presence: false)
+        expect(subject.errors.full_messages)
+          .to eq(['no_social_media_presence must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.'])
+      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)
+        expect(subject.errors.full_messages).to be_empty
       end
     end
 
@@ -24,6 +35,7 @@
       before do
         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')
       end
 
       it 'should set the status on the partner record to awaiting_review' do

spec/system/partners/approval_process_spec.rb

@@ -67,4 +67,21 @@
       end
     end
   end
+
+  describe "request approval with invalid details" do
+    let(:partner_user) { partner.primary_user }
+    let(:partner) { FactoryBot.create(:partner) }
+
+    before do
+      partner.profile.update(website: '', facebook: '', twitter: '', instagram: '', no_social_media_presence: false, partner_status: 'pending')
+      login_as(partner_user)
+      visit partner_user_root_path
+      click_on 'My Organization'
+      click_on 'Submit for Approval'
+    end
+
+    it "should render an error message", :aggregate_failures do
+      assert page.has_content? 'no_social_media_presence must be checked if you have not provided any of Website, Twitter, Facebook, or Instagram.'
+    end
+  end
 end