CV2-3177: replace with_options validation with inline conditions (#1670)

* CV2-3177: replace with_option with inline conditions * CV2-3177: fix tests
meedan · Sep 27, 2023 · 1ae10ed · 1ae10ed
1 parent 6693968
commit 1ae10ed
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 9 deletions.
diff --git a/app/models/tipline_newsletter.rb b/app/models/tipline_newsletter.rb
@@ -36,15 +36,13 @@ class TiplineNewsletter < ApplicationRecord
   validates_inclusion_of :content_type, in: ['static', 'rss']
   # Should be executed only when `enabled: 1`
   validates_presence_of :send_on, if: ->(newsletter) { newsletter.content_type == 'static' && newsletter.enabled }
-  with_options if: :enabled do |obj|
-    obj.validates_presence_of :time, :timezone, :introduction
-    obj.validates_inclusion_of :number_of_articles, in: 0..3, allow_blank: true, allow_nil: true
-    obj.validates :first_article, length: { maximum: proc { |newsletter| MAXIMUM_ARTICLE_LENGTH[newsletter.number_of_articles].to_i } }, allow_blank: true, allow_nil: true, if: proc { |newsletter| newsletter.number_of_articles >= 1 }
-    obj.validates :second_article, length: { maximum: proc { |newsletter| MAXIMUM_ARTICLE_LENGTH[newsletter.number_of_articles].to_i } }, allow_blank: true, allow_nil: true, if: proc { |newsletter| newsletter.number_of_articles >= 2 }
-    obj.validates :third_article, length: { maximum: proc { |newsletter| MAXIMUM_ARTICLE_LENGTH[newsletter.number_of_articles].to_i } }, allow_blank: true, allow_nil: true, if: proc { |newsletter| newsletter.number_of_articles == 3 }
-    obj.validate :send_every_is_a_list_of_days_of_the_week
-    obj.validate :not_scheduled_for_the_past, unless: proc { |newsletter| newsletter.time.blank? || newsletter.timezone.blank? }
-  end
+  validates_presence_of :time, :timezone, :introduction, if: ->(newsletter) { newsletter.enabled }
+  validates_inclusion_of :number_of_articles, in: 0..3, allow_blank: true, allow_nil: true, if: ->(newsletter) { newsletter.enabled }
+  validates :first_article, length: { maximum: proc { |newsletter| MAXIMUM_ARTICLE_LENGTH[newsletter.number_of_articles].to_i } }, allow_blank: true, allow_nil: true, if: proc { |newsletter| newsletter.number_of_articles >= 1 && newsletter.enabled }
+  validates :second_article, length: { maximum: proc { |newsletter| MAXIMUM_ARTICLE_LENGTH[newsletter.number_of_articles].to_i } }, allow_blank: true, allow_nil: true, if: proc { |newsletter| newsletter.number_of_articles >= 2 && newsletter.enabled }
+  validates :third_article, length: { maximum: proc { |newsletter| MAXIMUM_ARTICLE_LENGTH[newsletter.number_of_articles].to_i } }, allow_blank: true, allow_nil: true, if: proc { |newsletter| newsletter.number_of_articles == 3 && newsletter.enabled }
+  validate :send_every_is_a_list_of_days_of_the_week, if: ->(newsletter) { newsletter.enabled }
+  validate :not_scheduled_for_the_past, unless: proc { |newsletter| newsletter.time.blank? || newsletter.timezone.blank? || !newsletter.enabled }
 
   after_save :reschedule_delivery, unless: proc { |newsletter| newsletter.time.blank? || newsletter.timezone.blank? }
 

diff --git a/test/models/tipline_newsletter_test.rb b/test/models/tipline_newsletter_test.rb
@@ -87,6 +87,7 @@ def teardown
   test 'should validate maximum length of articles' do
     @newsletter.content_type = 'static'
     @newsletter.introduction = 'Foo'
+    @newsletter.enabled = true
 
     # 1 article
     @newsletter.number_of_articles = 1