From 49e55b4dc416d22929adc80d23857558a835a41a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 23 Oct 2019 20:05:40 +0200
Subject: [PATCH] Apply Rails/DynamicFindBy rubocop rule

We were already using `find_by` most of the time.

Since there are false positives related to our `find_by_slug_or_id!` and
`find_by_manger_login` methods, which cannot be replaced with `find_by`,
I'm adding it indicating the "refactor" severity.
---
 .rubocop_basic.yml                                   | 4 ++++
 app/models/budget/phase.rb                           | 2 +-
 app/models/concerns/sluggable.rb                     | 4 ++--
 config/initializers/i18n_translation.rb              | 2 +-
 db/pages/accessibility.rb                            | 2 +-
 db/pages/conditions.rb                               | 2 +-
 db/pages/faq.rb                                      | 2 +-
 db/pages/privacy.rb                                  | 2 +-
 db/pages/welcome_level_three_verified.rb             | 2 +-
 db/pages/welcome_level_two_verified.rb               | 2 +-
 db/pages/welcome_not_verified.rb                     | 2 +-
 spec/features/admin/site_customization/pages_spec.rb | 2 +-
 12 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml
index 08fb42b47..94d7c4916 100644
--- a/.rubocop_basic.yml
+++ b/.rubocop_basic.yml
@@ -156,6 +156,10 @@ Rails/ApplicationRecord:
 Rails/Date:
   Enabled: true
 
+Rails/DynamicFindBy:
+  Enabled: true
+  Severity: refactor
+
 Rails/FindBy:
   Enabled: true
 
diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb
index 859beb51f..c13a7825c 100644
--- a/app/models/budget/phase.rb
+++ b/app/models/budget/phase.rb
@@ -30,7 +30,7 @@ class Budget
     scope :published,         -> { enabled.where.not(kind: "drafting") }
 
     PHASE_KINDS.each do |phase|
-      define_singleton_method(phase) { find_by_kind(phase) }
+      define_singleton_method(phase) { find_by(kind: phase) }
     end
 
     def next_enabled_phase
diff --git a/app/models/concerns/sluggable.rb b/app/models/concerns/sluggable.rb
index bdcdff856..7f9bff56b 100644
--- a/app/models/concerns/sluggable.rb
+++ b/app/models/concerns/sluggable.rb
@@ -5,11 +5,11 @@ module Sluggable
     before_validation :generate_slug, if: :generate_slug?
 
     def self.find_by_slug_or_id(slug_or_id)
-      find_by_slug(slug_or_id) || find_by_id(slug_or_id)
+      find_by(slug: slug_or_id) || find_by(id: slug_or_id)
     end
 
     def self.find_by_slug_or_id!(slug_or_id)
-      find_by_slug(slug_or_id) || find(slug_or_id)
+      find_by(slug: slug_or_id) || find(slug_or_id)
     end
   end
 
diff --git a/config/initializers/i18n_translation.rb b/config/initializers/i18n_translation.rb
index aa500468c..aae984e1f 100644
--- a/config/initializers/i18n_translation.rb
+++ b/config/initializers/i18n_translation.rb
@@ -9,7 +9,7 @@ module ActionView
       def t(key, options = {})
         current_locale = options[:locale].presence || I18n.locale
 
-        i18_content = I18nContent.find_by_key(key)
+        i18_content = I18nContent.find_by(key: key)
         translation = I18nContentTranslation.where(i18n_content_id: i18_content&.id,
                                                    locale: current_locale).first&.value
         translation.presence || translate(key, options)
diff --git a/db/pages/accessibility.rb b/db/pages/accessibility.rb
index 5741a57cc..006368164 100644
--- a/db/pages/accessibility.rb
+++ b/db/pages/accessibility.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("accessibility").nil?
+if SiteCustomization::Page.find_by(slug: "accessibility").nil?
   page = SiteCustomization::Page.new(slug: "accessibility", status: "published")
   page.title = I18n.t("pages.accessibility.title")
 
diff --git a/db/pages/conditions.rb b/db/pages/conditions.rb
index 8a57b63cc..c47216b51 100644
--- a/db/pages/conditions.rb
+++ b/db/pages/conditions.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("conditions").nil?
+if SiteCustomization::Page.find_by(slug: "conditions").nil?
   page = SiteCustomization::Page.new(slug: "conditions", status: "published")
   page.print_content_flag = true
   page.title = I18n.t("pages.conditions.title")
diff --git a/db/pages/faq.rb b/db/pages/faq.rb
index 6479e1b57..3e960ae14 100644
--- a/db/pages/faq.rb
+++ b/db/pages/faq.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("faq").nil?
+if SiteCustomization::Page.find_by(slug: "faq").nil?
   page = SiteCustomization::Page.new(slug: "faq", status: "published")
   page.title = I18n.t("pages.help.faq.page.title")
   page.content = "<p>#{I18n.t("pages.help.faq.page.description")}</p>"
diff --git a/db/pages/privacy.rb b/db/pages/privacy.rb
index f06b1687c..71e79880a 100644
--- a/db/pages/privacy.rb
+++ b/db/pages/privacy.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("privacy").nil?
+if SiteCustomization::Page.find_by(slug: "privacy").nil?
   page = SiteCustomization::Page.new(slug: "privacy", status: "published")
   page.print_content_flag = true
   page.title = I18n.t("pages.privacy.title")
diff --git a/db/pages/welcome_level_three_verified.rb b/db/pages/welcome_level_three_verified.rb
index 2c00e4fb0..8c2caaaac 100644
--- a/db/pages/welcome_level_three_verified.rb
+++ b/db/pages/welcome_level_three_verified.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("welcome_level_three_verified").nil?
+if SiteCustomization::Page.find_by(slug: "welcome_level_three_verified").nil?
   page = SiteCustomization::Page.new(slug: "welcome_level_three_verified", status: "published")
   page.title = I18n.t("welcome.welcome.title")
 
diff --git a/db/pages/welcome_level_two_verified.rb b/db/pages/welcome_level_two_verified.rb
index 911a0c850..a459a6fa6 100644
--- a/db/pages/welcome_level_two_verified.rb
+++ b/db/pages/welcome_level_two_verified.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("welcome_level_two_verified").nil?
+if SiteCustomization::Page.find_by(slug: "welcome_level_two_verified").nil?
   page = SiteCustomization::Page.new(slug: "welcome_level_two_verified", status: "published")
   page.title = I18n.t("welcome.welcome.title")
 
diff --git a/db/pages/welcome_not_verified.rb b/db/pages/welcome_not_verified.rb
index 51df2ac39..3ff99142d 100644
--- a/db/pages/welcome_not_verified.rb
+++ b/db/pages/welcome_not_verified.rb
@@ -1,4 +1,4 @@
-if SiteCustomization::Page.find_by_slug("welcome_not_verified").nil?
+if SiteCustomization::Page.find_by(slug: "welcome_not_verified").nil?
   page = SiteCustomization::Page.new(slug: "welcome_not_verified", status: "published")
   page.title = I18n.t("welcome.welcome.title")
 
diff --git a/spec/features/admin/site_customization/pages_spec.rb b/spec/features/admin/site_customization/pages_spec.rb
index 4be5819c4..17b619659 100644
--- a/spec/features/admin/site_customization/pages_spec.rb
+++ b/spec/features/admin/site_customization/pages_spec.rb
@@ -25,7 +25,7 @@ describe "Admin custom pages" do
 
       expect(SiteCustomization::Page.count).to be 7
       slugs.each do |slug|
-        expect(SiteCustomization::Page.find_by_slug(slug).status).to eq "published"
+        expect(SiteCustomization::Page.find_by(slug: slug).status).to eq "published"
       end
 
       expect(all("[id^='site_customization_page_']").count).to be 7