DEV: pluck_first

DEV: pluck_first

Doing .pluck(:column).first is a very common pattern in Discourse and in most cases, a limit cause isn’t being added. Instead of adding a limit clause to all these callsites, this commit adds two new methods to ActiveRecord::Relation:

pluck_first, equivalent to limit(1).pluck(*columns).first

and pluck_first! which, like other finder methods, raises an exception when no record is found

diff --git a/app/controllers/directory_items_controller.rb b/app/controllers/directory_items_controller.rb
index 8a0ce35fd2..e6d6ea4b3c 100644
--- a/app/controllers/directory_items_controller.rb
+++ b/app/controllers/directory_items_controller.rb
@@ -47,7 +47,7 @@ class DirectoryItemsController < ApplicationController
     end
 
     if params[:username]
-      user_id = User.where(username_lower: params[:username].to_s.downcase).pluck(:id).first
+      user_id = User.where(username_lower: params[:username].to_s.downcase).pluck_first(:id)
       if user_id
         result = result.where(user_id: user_id)
       else
diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb
index 312b721fb8..7a3caaabdf 100644
--- a/app/controllers/list_controller.rb
+++ b/app/controllers/list_controller.rb
@@ -410,7 +410,7 @@ class ListController < ApplicationController
   end
 
   def self.best_period_for(previous_visit_at, category_id = nil)
-    default_period = ((category_id && Category.where(id: category_id).pluck(:default_top_period).first) ||
+    default_period = ((category_id && Category.where(id: category_id).pluck_first(:default_top_period)) ||
           SiteSetting.top_page_default_timeframe).to_sym
 
     best_period_with_topics_for(previous_visit_at, category_id, default_period) || default_period
diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb
index e5474da2dd..b986de1e3c 100644
--- a/app/controllers/stylesheets_controller.rb
+++ b/app/controllers/stylesheets_controller.rb
@@ -61,7 +61,7 @@ class StylesheetsController < ApplicationController
     cache_path = "#{Rails.root}/#{Stylesheet::Manager::CACHE_PATH}"
     location = "#{cache_path}/#{target}#{underscore_digest}#{extension}"
 
-    stylesheet_time = query.pluck(:created_at).first
+    stylesheet_time = query.pluck_first(:created_at)
 
     if !stylesheet_time
       handle_missing_cache(location, target, digest)
@@ -72,7 +72,7 @@ class StylesheetsController < ApplicationController
     end
 
     unless File.exist?(location)
-      if current = query.limit(1).pluck(source_map ? :source_map : :content).first
+      if current = query.pluck_first(source_map ? :source_map : :content)
         FileUtils.mkdir_p(cache_path)
         File.write(location, current)
       else
diff --git a/app/controllers/theme_javascripts_controller.rb b/app/controllers/theme_javascripts_controller.rb
index cfa48dca0c..0fb5f526a5 100644
--- a/app/controllers/theme_javascripts_controller.rb
+++ b/app/controllers/theme_javascripts_controller.rb
@@ -21,7 +21,7 @@ class ThemeJavascriptsController < ApplicationController
     cache_file = "#{DISK_CACHE_PATH}/#{params[:digest]}.js"
 
     unless File.exist?(cache_file)
-      content = query.pluck(:content).first
+      content = query.pluck_first(:content)
       raise Discourse::NotFound if content.nil?
 
       FileUtils.mkdir_p(DISK_CACHE_PATH)
@@ -41,7 +41,7 @@ class ThemeJavascriptsController < ApplicationController
   end
 
   def last_modified
-    @last_modified ||= query.pluck(:updated_at).first
+    @last_modified ||= query.pluck_first(:updated_at)
   end
 
   def not_modified?
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 1b2bc12702..7e50ec7400 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -711,7 +711,7 @@ class TopicsController < ApplicationController
     guardian.ensure_can_move_posts!(topic)
 
     # when creating a new topic, ensure the 1st post is a regular post
-    if params[:title].present? && Post.where(topic: topic, id: post_ids).order(:post_number).pluck(:post_type).first != Post.types[:regular]
+    if params[:title].present? && Post.where(topic: topic, id: post_ids).order(:post_number).pluck_first(:post_type) != Post.types[:regular]
       return render_json_error("When moving posts to a new topic, the first post must be a regular post.")
     end
 
diff --git a/app/controllers/user_badges_controller.rb b/app/controllers/user_badges_controller.rb
index fe306f65ff..ddb5a3e8ae 100644
--- a/app/controllers/user_badges_controller.rb
+++ b/app/controllers/user_badges_controller.rb
@@ -13,7 +13,7 @@ class UserBadgesController < ApplicationController
     grant_count = nil
 
     if params[:username]
-      user_id = User.where(username_lower: params[:username].downcase).pluck(:id).first
+      user_id = User.where(username_lower: params[:username].downcase).pluck_first(:id)
       user_badges = user_badges.where(user_id: user_id) if user_id
       grant_count = badge.user_badges.where(user_id: user_id).count
     end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 84d8e383b5..96531a9a18 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -127,7 +127,7 @@ module ApplicationHelper
 
     if current_user.present? &&
         current_user.primary_group_id &&
-        primary_group_name = Group.where(id: current_user.primary_group_id).pluck(:name).first
+        primary_group_name = Group.where(id: current_user.primary_group_id).pluck_first(:name)
       result << "primary-group-#{primary_group_name.downcase}"
     end
 
diff --git a/app/jobs/regular/update_username.rb b/app/jobs/regular/update_username.rb
index 308dc75c99..3295fd0800 100644
--- a/app/jobs/regular/update_username.rb
+++ b/app/jobs/regular/update_username.rb
@@ -184,7 +184,7 @@ module Jobs
       Post.where(
         topic_id: aside["data-topic"],
         post_number: aside["data-post"]
-      ).pluck(:user_id).first == @user_id
+      ).pluck_first(:user_id) == @user_id
     end
   end
 end
diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index bdf9c85698..a3450b00d6 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -395,7 +395,7 @@ class UserNotifications < ActionMailer::Base
     user_name = notification_data[:original_username]
 
     if post && SiteSetting.enable_names && SiteSetting.display_name_on_email_from
-      name = User.where(id: post.user_id).pluck(:name).first
+      name = User.where(id: post.user_id).pluck_first(:name)
       user_name = name unless name.blank?
     end
 
@@ -476,7 +476,7 @@ class UserNotifications < ActionMailer::Base
 
       # subcategory case
       if !category.parent_category_id.nil?
-        show_category_in_subject = "#{Category.where(id: category.parent_category_id).pluck(:name).first}/#{show_category_in_subject}"
+        show_category_in_subject = "#{Category.where(id: category.parent_category_id).pluck_first(:name)}/#{show_category_in_subject}"
       end
     else
       show_category_in_subject = nil
diff --git a/app/models/application_request.rb b/app/models/application_request.rb
index 1e4459f681..2447501445 100644
--- a/app/models/application_request.rb
+++ b/app/models/application_request.rb
@@ -63,7 +63,7 @@ class ApplicationRequest < ActiveRecord::Base
     req_type_id = req_types[req_type]
 
     # a poor man's upsert
-    id = where(date: date, req_type: req_type_id).pluck(:id).first
+    id = where(date: date, req_type: req_type_id).pluck_first(:id)
     id ||= create!(date: date, req_type: req_type_id, count: 0).id
 
   rescue # primary key violation
diff --git a/app/models/backup_metadata.rb b/app/models/backup_metadata.rb
index 6cddb82576..db4bf7e9ad 100644
--- a/app/models/backup_metadata.rb
+++ b/app/models/backup_metadata.rb
@@ -2,7 +2,7 @@
 
 class BackupMetadata < ActiveRecord::Base
   def self.value_for(name)
-    where(name: name).pluck(:value).first.presence
+    where(name: name).pluck_first(:value).presence
   end
 end
 
diff --git a/app/models/category.rb b/app/models/category.rb
index d3273ea63d..40f5791090 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -608,8 +608,8 @@ class Category < ActiveRecord::Base
 
   def self.query_parent_category(parent_slug)

[... diff too long, it was truncated ...]

GitHub sha: 55a13943

1 Like