From 30afb64bac4e3a5164b28b9806cf9da0e3912740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= Date: Fri, 5 Nov 2021 11:53:46 +0100 Subject: [PATCH] Do not consider attributes using the `:unless` option as required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove some of the factories introduced in commit 66334b5 as now we do not need them anymore. Co-Authored-By: Javi Martín <35156+javierm@users.noreply.github.com> --- app/models/concerns/globalizable.rb | 17 +++++++++++------ spec/factories/proposals.rb | 2 -- spec/models/concerns/globalizable.rb | 3 --- spec/models/milestone_spec.rb | 1 + spec/models/progress_bar_spec.rb | 1 + spec/models/proposal_spec.rb | 2 +- spec/system/admin/translatable_spec.rb | 17 ++++++++++++++++- 7 files changed, 30 insertions(+), 13 deletions(-) diff --git a/app/models/concerns/globalizable.rb b/app/models/concerns/globalizable.rb index 98613b5a7..c2e04554f 100644 --- a/app/models/concerns/globalizable.rb +++ b/app/models/concerns/globalizable.rb @@ -32,14 +32,19 @@ module Globalizable private def required_attribute?(attribute) - presence_validators = [ActiveModel::Validations::PresenceValidator, - ActiveRecord::Validations::PresenceValidator] - - attribute_validators(attribute).any? { |validator| presence_validators.include? validator } + self.class.validators_on(attribute).any? do |validator| + validator.kind == :presence && !conditional_validator?(validator) + end end - def attribute_validators(attribute) - self.class.validators_on(attribute).map(&:class) + def conditional_validator?(validator) + return false unless validator.options[:unless] + + if validator.options[:unless].to_proc.arity.zero? + instance_exec(&validator.options[:unless]) + else + validator.options[:unless].to_proc.call(self) + end end def check_translations_number diff --git a/spec/factories/proposals.rb b/spec/factories/proposals.rb index 925849b8d..68953799b 100644 --- a/spec/factories/proposals.rb +++ b/spec/factories/proposals.rb @@ -86,8 +86,6 @@ FactoryBot.define do evaluator.voters.each { |voter| create(:vote, votable: proposal, voter: voter) } evaluator.followers.each { |follower| create(:follow, followable: proposal, user: follower) } end - - factory :retired_proposal, traits: [:retired] end factory :proposal_notification do diff --git a/spec/models/concerns/globalizable.rb b/spec/models/concerns/globalizable.rb index a3e046a3f..8b6555b87 100644 --- a/spec/models/concerns/globalizable.rb +++ b/spec/models/concerns/globalizable.rb @@ -13,9 +13,6 @@ shared_examples_for "globalizable" do |factory_name| let(:attribute) { required_fields.sample || fields.sample } before do - if factory_name == :budget || factory_name == :budget_phase - record.main_link_url = "https://consulproject.org" - end record.update!(attribute => "In English") I18n.with_locale(:es) do diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index e24c43314..3c10ac838 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -1,6 +1,7 @@ require "rails_helper" describe Milestone do + it_behaves_like "globalizable", :milestone it_behaves_like "globalizable", :milestone_with_description describe "Validations" do diff --git a/spec/models/progress_bar_spec.rb b/spec/models/progress_bar_spec.rb index 41e681538..f5ea05c8f 100644 --- a/spec/models/progress_bar_spec.rb +++ b/spec/models/progress_bar_spec.rb @@ -3,6 +3,7 @@ require "rails_helper" describe ProgressBar do let(:progress_bar) { build(:progress_bar) } + it_behaves_like "globalizable", :progress_bar it_behaves_like "globalizable", :secondary_progress_bar it "is valid" do diff --git a/spec/models/proposal_spec.rb b/spec/models/proposal_spec.rb index cc72a1a23..5e14c7688 100644 --- a/spec/models/proposal_spec.rb +++ b/spec/models/proposal_spec.rb @@ -7,7 +7,7 @@ describe Proposal do it_behaves_like "has_public_author" it_behaves_like "notifiable" it_behaves_like "map validations" - it_behaves_like "globalizable", :retired_proposal + it_behaves_like "globalizable", :proposal it_behaves_like "sanitizable" it_behaves_like "acts as paranoid", :proposal end diff --git a/spec/system/admin/translatable_spec.rb b/spec/system/admin/translatable_spec.rb index aa656ac89..fe9d80a8d 100644 --- a/spec/system/admin/translatable_spec.rb +++ b/spec/system/admin/translatable_spec.rb @@ -355,7 +355,9 @@ describe "Admin edit translatable records", :admin do context "Remove all translations" do let(:translatable) { create(:milestone) } - scenario "Shows an error message" do + scenario "Shows an error message when there's a mandatory translatable field" do + translatable.update!(status: nil) + visit admin_polymorphic_path(translatable, action: :edit) click_link "Remove language" @@ -365,6 +367,19 @@ describe "Admin edit translatable records", :admin do expect(page).to have_content "Is mandatory to provide one translation at least" end + + scenario "Is successful when there isn't a mandatory translatable field" do + translatable.update!(status: Milestone::Status.first) + + visit admin_polymorphic_path(translatable, action: :edit) + + click_link "Remove language" + click_link "Remove language" + + click_button "Update milestone" + + expect(page).to have_content "Milestone updated successfully" + end end context "Remove a translation with invalid data" do