From 55a139434214a8decc856b192899e53b335873db Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Mon, 21 Oct 2019 11:32:27 +0100 Subject: [PATCH] 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 --- app/controllers/directory_items_controller.rb | 2 +- app/controllers/list_controller.rb | 2 +- app/controllers/stylesheets_controller.rb | 4 +-- .../theme_javascripts_controller.rb | 4 +-- app/controllers/topics_controller.rb | 2 +- app/controllers/user_badges_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- app/jobs/regular/update_username.rb | 2 +- app/mailers/user_notifications.rb | 4 +-- app/models/application_request.rb | 2 +- app/models/backup_metadata.rb | 2 +- app/models/category.rb | 4 +-- app/models/concerns/cached_counting.rb | 2 +- app/models/draft.rb | 4 +-- app/models/incoming_link.rb | 2 +- app/models/notification.rb | 2 +- app/models/post.rb | 2 +- app/models/post_action.rb | 2 +- app/models/quoted_post.rb | 2 +- app/models/topic.rb | 2 +- app/models/topic_converter.rb | 2 +- app/models/topic_embed.rb | 4 +-- app/models/topic_user.rb | 2 +- app/models/user.rb | 8 ++--- app/models/user_action.rb | 2 +- app/services/post_alerter.rb | 2 +- lib/composer_messages_finder.rb | 2 +- lib/file_store/s3_store.rb | 2 +- lib/freedom_patches/pluck_first.rb | 19 ++++++++++++ lib/post_creator.rb | 2 +- lib/search.rb | 10 +++---- lib/tasks/posts.rake | 2 +- lib/topic_query.rb | 6 ++-- lib/topic_view.rb | 6 ++-- .../max_username_length_validator.rb | 2 +- .../min_username_length_validator.rb | 2 +- lib/wizard.rb | 2 +- script/micro_bench.rb | 4 +-- spec/components/wizard/step_updater_spec.rb | 12 ++++---- spec/models/post_action_spec.rb | 4 +-- spec/models/top_topic_spec.rb | 30 +++++++++---------- spec/services/post_alerter_spec.rb | 2 +- spec/services/user_merger_spec.rb | 2 +- 43 files changed, 99 insertions(+), 80 deletions(-) create mode 100644 lib/freedom_patches/pluck_first.rb diff --git a/app/controllers/directory_items_controller.rb b/app/controllers/directory_items_controller.rb index 8a0ce35fd27..e6d6ea4b3c9 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 312b721fb89..7a3caaabdf4 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 e5474da2ddb..b986de1e3c0 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 cfa48dca0c9..0fb5f526a5b 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 1b2bc12702e..7e50ec7400d 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 fe306f65ff4..ddb5a3e8ae7 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 84d8e383b5d..96531a9a187 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 308dc75c999..3295fd08004 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 bdf9c856987..a3450b00d6a 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 1e4459f681d..24475014451 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 6cddb825762..db4bf7e9ad4 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 d3273ea63d0..40f5791090b 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) encoded_parent_slug = CGI.escape(parent_slug) if SiteSetting.slug_generation_method == 'encoded' - self.where(slug: (encoded_parent_slug || parent_slug), parent_category_id: nil).pluck(:id).first || - self.where(id: parent_slug.to_i).pluck(:id).first + self.where(slug: (encoded_parent_slug || parent_slug), parent_category_id: nil).pluck_first(:id) || + self.where(id: parent_slug.to_i).pluck_first(:id) end def self.query_category(slug_or_id, parent_category_id) diff --git a/app/models/concerns/cached_counting.rb b/app/models/concerns/cached_counting.rb index 3c3b09cd407..4430126ced8 100644 --- a/app/models/concerns/cached_counting.rb +++ b/app/models/concerns/cached_counting.rb @@ -58,7 +58,7 @@ module CachedCounting end def request_id(query_params, retries = 0) - id = where(query_params).pluck(:id).first + id = where(query_params).pluck_first(:id) id ||= create!(query_params.merge(count: 0)).id rescue # primary key violation if retries == 0 diff --git a/app/models/draft.rb b/app/models/draft.rb index f05cb8b7fe8..c31186cf576 100644 --- a/app/models/draft.rb +++ b/app/models/draft.rb @@ -104,7 +104,7 @@ class Draft < ActiveRecord::Base reply = JSON.parse(data)["reply"] || "" return if reply.length < SiteSetting.backup_drafts_to_pm_length - post_id = BackupDraftPost.where(user_id: user.id, key: key).pluck(:post_id).first + post_id = BackupDraftPost.where(user_id: user.id, key: key).pluck_first(:post_id) post = Post.where(id: post_id).first if post_id if post_id && !post @@ -159,7 +159,7 @@ class Draft < ActiveRecord::Base end def self.ensure_draft_topic!(user) - topic_id = BackupDraftTopic.where(user_id: user.id).pluck(:topic_id).first + topic_id = BackupDraftTopic.where(user_id: user.id).pluck_first(:topic_id) topic = Topic.find_by(id: topic_id) if topic_id if topic_id && !topic diff --git a/app/models/incoming_link.rb b/app/models/incoming_link.rb index d595eea04d0..4e51a668231 100644 --- a/app/models/incoming_link.rb +++ b/app/models/incoming_link.rb @@ -39,7 +39,7 @@ class IncomingLink < ActiveRecord::Base post_id = opts[:post_id] post_id ||= Post.where(topic_id: opts[:topic_id], post_number: opts[:post_number] || 1) - .pluck(:id).first + .pluck_first(:id) cid = current_user ? (current_user.id) : (nil) ip_address = nil if cid diff --git a/app/models/notification.rb b/app/models/notification.rb index 9b32cae7c0d..6a4413e7f5b 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -211,7 +211,7 @@ class Notification < ActiveRecord::Base end def post_id - Post.where(topic: topic_id, post_number: post_number).pluck(:id).first + Post.where(topic: topic_id, post_number: post_number).pluck_first(:id) end protected diff --git a/app/models/post.rb b/app/models/post.rb index 34d8d24e451..2462b5abb61 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -983,7 +983,7 @@ class Post < ActiveRecord::Base end upload_id = nil - upload_id = Upload.where(sha1: sha1).pluck(:id).first if sha1.present? + upload_id = Upload.where(sha1: sha1).pluck_first(:id) if sha1.present? upload_id ||= yield(post, src, path, sha1) if upload_id.blank? diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 5affd3b8097..077ef253c18 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -215,7 +215,7 @@ class PostAction < ActiveRecord::Base end end - topic_id = Post.with_deleted.where(id: post_id).pluck(:topic_id).first + topic_id = Post.with_deleted.where(id: post_id).pluck_first(:topic_id) # topic_user if [:like, :bookmark].include? post_action_type_key diff --git a/app/models/quoted_post.rb b/app/models/quoted_post.rb index ad07877ac89..03b981e1fb3 100644 --- a/app/models/quoted_post.rb +++ b/app/models/quoted_post.rb @@ -67,7 +67,7 @@ class QuotedPost < ActiveRecord::Base reply_quoted = false if post.reply_to_post_number - reply_post_id = Post.where(topic_id: post.topic_id, post_number: post.reply_to_post_number).pluck(:id).first + reply_post_id = Post.where(topic_id: post.topic_id, post_number: post.reply_to_post_number).pluck_first(:id) reply_quoted = reply_post_id.present? && QuotedPost.where(post_id: post.id, quoted_post_id: reply_post_id).count > 0 end diff --git a/app/models/topic.rb b/app/models/topic.rb index 9061198bab4..85cff4f7e3b 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -607,7 +607,7 @@ class Topic < ActiveRecord::Base # If a post is deleted we have to update our highest post counters def self.reset_highest(topic_id) - archetype = Topic.where(id: topic_id).pluck(:archetype).first + archetype = Topic.where(id: topic_id).pluck_first(:archetype) # ignore small_action replies for private messages post_type = archetype == Archetype.private_message ? " AND post_type <> #{Post.types[:small_action]}" : '' diff --git a/app/models/topic_converter.rb b/app/models/topic_converter.rb index aa23da10ad7..650333749d7 100644 --- a/app/models/topic_converter.rb +++ b/app/models/topic_converter.rb @@ -20,7 +20,7 @@ class TopicConverter Category.where(read_restricted: false) .where.not(id: SiteSetting.uncategorized_category_id) .order('id asc') - .pluck(:id).first + .pluck_first(:id) end PostRevisor.new(@topic.first_post, @topic).revise!( diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index 895f0d59863..a18e34c3ad2 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -208,7 +208,7 @@ class TopicEmbed < ActiveRecord::Base def self.topic_id_for_embed(embed_url) embed_url = normalize_url(embed_url).sub(/^https?\:\/\//, '') - TopicEmbed.where("embed_url ~* ?", "^https?://#{Regexp.escape(embed_url)}$").pluck(:topic_id).first + TopicEmbed.where("embed_url ~* ?", "^https?://#{Regexp.escape(embed_url)}$").pluck_first(:topic_id) end def self.first_paragraph_from(html) @@ -229,7 +229,7 @@ class TopicEmbed < ActiveRecord::Base def self.expanded_for(post) Rails.cache.fetch("embed-topic:#{post.topic_id}", expires_in: 10.minutes) do - url = TopicEmbed.where(topic_id: post.topic_id).pluck(:embed_url).first + url = TopicEmbed.where(topic_id: post.topic_id).pluck_first(:embed_url) response = TopicEmbed.find_remote(url) body = response.body diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index 65df38666fd..5c43a6e728d 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -215,7 +215,7 @@ class TopicUser < ActiveRecord::Base attrs[:notification_level] = notification_levels[:watching] end else - auto_track_after = UserOption.where(user_id: user_id).pluck(:auto_track_topics_after_msecs).first + auto_track_after = UserOption.where(user_id: user_id).pluck_first(:auto_track_topics_after_msecs) auto_track_after ||= SiteSetting.default_other_auto_track_topics_after_msecs if auto_track_after >= 0 && auto_track_after <= (attrs[:total_msecs_viewed].to_i || 0) diff --git a/app/models/user.rb b/app/models/user.rb index 337748b75a3..2a8a55f595e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -199,7 +199,7 @@ class User < ActiveRecord::Base scope :filter_by_username_or_email, ->(filter) do if filter =~ /.+@.+/ # probably an email so try the bypass - if user_id = UserEmail.where("lower(email) = ?", filter.downcase).pluck(:user_id).first + if user_id = UserEmail.where("lower(email) = ?", filter.downcase).pluck_first(:user_id) return where('users.id = ?', user_id) end end @@ -1219,11 +1219,11 @@ class User < ActiveRecord::Base group_titles_query = group_titles_query.order("groups.id = #{primary_group_id} DESC") if primary_group_id group_titles_query = group_titles_query.order("groups.primary_group DESC").limit(1) - if next_best_group_title = group_titles_query.pluck(:title).first + if next_best_group_title = group_titles_query.pluck_first(:title) return next_best_group_title end - next_best_badge_title = badges.where(allow_title: true).limit(1).pluck(:name).first + next_best_badge_title = badges.where(allow_title: true).pluck_first(:name) next_best_badge_title ? Badge.display_name(next_best_badge_title) : nil end @@ -1444,7 +1444,7 @@ class User < ActiveRecord::Base def match_title_to_primary_group_changes return unless primary_group_id_changed? - if title == Group.where(id: primary_group_id_was).pluck(:title).first + if title == Group.where(id: primary_group_id_was).pluck_first(:title) self.title = primary_group&.title end end diff --git a/app/models/user_action.rb b/app/models/user_action.rb index df79c89be4f..fef91feb9e4 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -46,7 +46,7 @@ class UserAction < ActiveRecord::Base def self.last_action_in_topic(user_id, topic_id) UserAction.where(user_id: user_id, target_topic_id: topic_id, - action_type: [RESPONSE, MENTION, QUOTE]).order('created_at DESC').pluck(:target_post_id).first + action_type: [RESPONSE, MENTION, QUOTE]).order('created_at DESC').pluck_first(:target_post_id) end def self.stats(user_id, guardian) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index bcf1f053851..d71e4b4914d 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -221,7 +221,7 @@ class PostAlerter group_id = post.topic .topic_allowed_groups .where(group_id: user.groups.pluck(:id)) - .pluck(:group_id).first + .pluck_first(:group_id) stat = stats.find { |s| s[:group_id] == group_id } return unless stat && stat[:inbox_count] > 0 diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index b91fceffc33..f1f9aa33d82 100644 --- a/lib/composer_messages_finder.rb +++ b/lib/composer_messages_finder.rb @@ -170,7 +170,7 @@ class ComposerMessagesFinder target_user_id: @user.id, topic_id: @details[:topic_id]) - reply_username = User.where(id: last_x_replies[0]).pluck(:username).first + reply_username = User.where(id: last_x_replies[0]).pluck_first(:username) { id: 'get_a_room', diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 0c983607a49..2b1cc0a83cf 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -197,7 +197,7 @@ module FileStore verified_ids = [] files.each do |f| - id = model.where("url LIKE '%#{f.key}' AND etag = '#{f.etag}'").pluck(:id).first + id = model.where("url LIKE '%#{f.key}' AND etag = '#{f.etag}'").pluck_first(:id) verified_ids << id if id.present? marker = f.key end diff --git a/lib/freedom_patches/pluck_first.rb b/lib/freedom_patches/pluck_first.rb new file mode 100644 index 00000000000..6f3d520187f --- /dev/null +++ b/lib/freedom_patches/pluck_first.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class ActiveRecord::Relation + def pluck_first(*attributes) + limit(1).pluck(*attributes).first + end + + def pluck_first!(*attributes) + items = limit(1).pluck(*attributes) + + raise_record_not_found_exception! if items.empty? + + items.first + end +end + +module ActiveRecord::Querying + delegate(:pluck_first, :pluck_first!, to: :all) +end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 561720dfb00..bb00ba335fb 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -298,7 +298,7 @@ class PostCreator sequence = DraftSequence.current(@user, draft_key) revisions = Draft.where(sequence: sequence, user_id: @user.id, - draft_key: draft_key).pluck(:revisions).first || 0 + draft_key: draft_key).pluck_first(:revisions) || 0 @post.build_post_stat( drafts_saved: revisions, diff --git a/lib/search.rb b/lib/search.rb index 7eba399d149..1f618cd56f1 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -344,7 +344,7 @@ class Search end advanced_filter(/^badge:(.*)$/) do |posts, match| - badge_id = Badge.where('name ilike ? OR id = ?', match, match.to_i).pluck(:id).first + badge_id = Badge.where('name ilike ? OR id = ?', match, match.to_i).pluck_first(:id) if badge_id posts.where('posts.user_id IN (SELECT ub.user_id FROM user_badges ub WHERE ub.badge_id = ?)', badge_id) else @@ -481,7 +481,7 @@ class Search posts.where("topics.category_id IN (?)", category_ids) else # try a possible tag match - tag_id = Tag.where_name(category_slug).pluck(:id).first + tag_id = Tag.where_name(category_slug).pluck_first(:id) if (tag_id) posts.where(<<~SQL, tag_id) topics.id IN ( @@ -517,7 +517,7 @@ class Search end advanced_filter(/^group:(.+)$/) do |posts, match| - group_id = Group.where('name ilike ? OR (id = ? AND id > 0)', match, match.to_i).pluck(:id).first + group_id = Group.where('name ilike ? OR (id = ? AND id > 0)', match, match.to_i).pluck_first(:id) if group_id posts.where("posts.user_id IN (select gu.user_id from group_users gu where gu.group_id = ?)", group_id) else @@ -526,7 +526,7 @@ class Search end advanced_filter(/^user:(.+)$/) do |posts, match| - user_id = User.where(staged: false).where('username_lower = ? OR id = ?', match.downcase, match.to_i).pluck(:id).first + user_id = User.where(staged: false).where('username_lower = ? OR id = ?', match.downcase, match.to_i).pluck_first(:id) if user_id posts.where("posts.user_id = #{user_id}") else @@ -535,7 +535,7 @@ class Search end advanced_filter(/^\@([a-zA-Z0-9_\-.]+)$/) do |posts, match| - user_id = User.where(staged: false).where(username_lower: match.downcase).pluck(:id).first + user_id = User.where(staged: false).where(username_lower: match.downcase).pluck_first(:id) if user_id posts.where("posts.user_id = #{user_id}") else diff --git a/lib/tasks/posts.rake b/lib/tasks/posts.rake index fec5a4f0077..0515f4301bf 100644 --- a/lib/tasks/posts.rake +++ b/lib/tasks/posts.rake @@ -533,7 +533,7 @@ def recover_uploads_from_index(path) PostCustomField.where(name: Post::MISSING_UPLOADS).pluck(:post_id, :value).each do |post_id, uploads| uploads = JSON.parse(uploads) - raw = Post.where(id: post_id).pluck(:raw).first + raw = Post.where(id: post_id).pluck_first(:raw) uploads.map! do |upload| orig = upload if raw.scan(upload).length == 0 diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 991ed94dfbb..1982ed133e8 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -350,7 +350,7 @@ class TopicQuery def list_private_messages_group_archive(user) list = private_messages_for(user, :group) - group_id = Group.where('name ilike ?', @options[:group_name]).pluck(:id).first + group_id = Group.where('name ilike ?', @options[:group_name]).pluck_first(:id) list = list.joins("JOIN group_archived_messages gm ON gm.topic_id = topics.id AND gm.group_id = #{group_id.to_i}") create_list(:private_messages, {}, list) @@ -642,7 +642,7 @@ class TopicQuery def get_category_id(category_id_or_slug) return nil unless category_id_or_slug category_id = category_id_or_slug.to_i - category_id = Category.where(slug: category_id_or_slug).pluck(:id).first if category_id == 0 + category_id = Category.where(slug: category_id_or_slug).pluck_first(:id) if category_id == 0 category_id end @@ -685,7 +685,7 @@ class TopicQuery if !@options[:order] # category default sort order - sort_order, sort_ascending = Category.where(id: category_id).pluck(:sort_order, :sort_ascending).first + sort_order, sort_ascending = Category.where(id: category_id).pluck_first(:sort_order, :sort_ascending) if sort_order options[:order] = sort_order options[:ascending] = !!sort_ascending ? 'true' : 'false' diff --git a/lib/topic_view.rb b/lib/topic_view.rb index c93461e035b..aaa4ccc0868 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -566,7 +566,7 @@ class TopicView end def filtered_post_id(post_number) - @filtered_posts.where(post_number: post_number).pluck(:id).first + @filtered_posts.where(post_number: post_number).pluck_first(:id) end def is_mega_topic? @@ -574,11 +574,11 @@ class TopicView end def first_post_id - @filtered_posts.order(sort_order: :asc).limit(1).pluck(:id).first + @filtered_posts.order(sort_order: :asc).pluck_first(:id) end def last_post_id - @filtered_posts.order(sort_order: :desc).limit(1).pluck(:id).first + @filtered_posts.order(sort_order: :desc).pluck_first(:id) end def current_post_number diff --git a/lib/validators/max_username_length_validator.rb b/lib/validators/max_username_length_validator.rb index c871cc32425..bce5a648d13 100644 --- a/lib/validators/max_username_length_validator.rb +++ b/lib/validators/max_username_length_validator.rb @@ -7,7 +7,7 @@ class MaxUsernameLengthValidator def valid_value?(value) return false if value < SiteSetting.min_username_length - @username = User.where('length(username) > ?', value).pluck(:username).first + @username = User.where('length(username) > ?', value).pluck_first(:username) @username.blank? end diff --git a/lib/validators/min_username_length_validator.rb b/lib/validators/min_username_length_validator.rb index 1c4dca7e7d8..0e1fe936a85 100644 --- a/lib/validators/min_username_length_validator.rb +++ b/lib/validators/min_username_length_validator.rb @@ -7,7 +7,7 @@ class MinUsernameLengthValidator def valid_value?(value) return false if value > SiteSetting.max_username_length - @username = User.where('length(username) < ?', value).pluck(:username).first + @username = User.where('length(username) < ?', value).pluck_first(:username) @username.blank? end diff --git a/lib/wizard.rb b/lib/wizard.rb index dbf4d8967e3..a254d8935d8 100644 --- a/lib/wizard.rb +++ b/lib/wizard.rb @@ -87,7 +87,7 @@ class Wizard .human_users .joins(:user_auth_tokens) .order('user_auth_tokens.created_at') - .pluck(:id).first + .pluck_first(:id) if @user&.id && first_admin_id == @user.id !Wizard::Builder.new(@user).build.completed? diff --git a/script/micro_bench.rb b/script/micro_bench.rb index d78d057b001..741d936428e 100644 --- a/script/micro_bench.rb +++ b/script/micro_bench.rb @@ -15,11 +15,11 @@ Benchmark.ips do |b| end b.report("pluck with first") do - User.pluck(:name).first + User.pluck_first(:name) end b.report("pluck with limit") do - User.limit(1).pluck(:name).first + User.pluck_first(:name) end b.report("raw") do diff --git a/spec/components/wizard/step_updater_spec.rb b/spec/components/wizard/step_updater_spec.rb index 5c2fee85556..13d816b606d 100644 --- a/spec/components/wizard/step_updater_spec.rb +++ b/spec/components/wizard/step_updater_spec.rb @@ -98,19 +98,19 @@ describe Wizard::StepUpdater do expect(SiteSetting.site_contact_username).to eq(user.username) # Should update the TOS topic - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck(:raw).first + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) expect(raw).to eq(" template") # Can update the TOS topic again updater = wizard.create_updater('contact', contact_email: 'alice@example.com') updater.update - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck(:raw).first + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) expect(raw).to eq(" template") # Can update the TOS to nothing updater = wizard.create_updater('contact', {}) updater.update - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck(:raw).first + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) expect(raw).to eq(" template") expect(wizard.completed_steps?('contact')).to eq(true) @@ -145,7 +145,7 @@ describe Wizard::StepUpdater do expect(SiteSetting.city_for_disputes).to eq("Fairfield, New Jersey") # Should update the TOS topic - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck(:raw).first + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) expect(raw).to eq("ACME, Inc. - New Jersey law - Fairfield, New Jersey template") # Can update the TOS topic again @@ -154,13 +154,13 @@ describe Wizard::StepUpdater do governing_law: 'California law', city_for_disputes: 'San Francisco, California') updater.update - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck(:raw).first + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) expect(raw).to eq("Pied Piper Inc - California law - San Francisco, California template") # Can update the TOS to nothing updater = wizard.create_updater('corporate', {}) updater.update - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck(:raw).first + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) expect(raw).to eq("company_name - governing_law - city_for_disputes template") expect(wizard.completed_steps?('corporate')).to eq(true) diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 4f477e02c0b..3413cb273a9 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -54,10 +54,10 @@ describe PostAction do expect(topic_user_ids).to include(mod.id) expect(topic.topic_users.where(user_id: mod.id) - .pluck(:notification_level).first).to eq(TopicUser.notification_levels[:tracking]) + .pluck_first(:notification_level)).to eq(TopicUser.notification_levels[:tracking]) expect(topic.topic_users.where(user_id: codinghorror.id) - .pluck(:notification_level).first).to eq(TopicUser.notification_levels[:watching]) + .pluck_first(:notification_level)).to eq(TopicUser.notification_levels[:watching]) # reply to PM should not clear flag PostCreator.new(mod, topic_id: posts[0].topic_id, raw: "This is my test reply to the user, it should clear flags").create diff --git a/spec/models/top_topic_spec.rb b/spec/models/top_topic_spec.rb index 720b3929000..33fc873a1a7 100644 --- a/spec/models/top_topic_spec.rb +++ b/spec/models/top_topic_spec.rb @@ -72,9 +72,9 @@ describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - expect(top_topics.where(topic_id: topic_1.id).pluck(:yearly_score).first).to eq(27) - expect(top_topics.where(topic_id: topic_2.id).pluck(:yearly_score).first).to be_within(0.0000000001).of(18.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck(:yearly_score).first).to be_within(0.0000000001).of(10.602059991328) + expect(top_topics.where(topic_id: topic_1.id).pluck_first(:yearly_score)).to eq(27) + expect(top_topics.where(topic_id: topic_2.id).pluck_first(:yearly_score)).to be_within(0.0000000001).of(18.301029995664) + expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to be_within(0.0000000001).of(10.602059991328) # when 'top_topics_formula_log_views_multiplier' setting is changed SiteSetting.top_topics_formula_log_views_multiplier = 4 @@ -90,9 +90,9 @@ describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - expect(top_topics.where(topic_id: topic_1.id).pluck(:yearly_score).first).to eq(27) - expect(top_topics.where(topic_id: topic_2.id).pluck(:yearly_score).first).to eq(18.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck(:yearly_score).first).to eq(11.2041199826559) + expect(top_topics.where(topic_id: topic_1.id).pluck_first(:yearly_score)).to eq(27) + expect(top_topics.where(topic_id: topic_2.id).pluck_first(:yearly_score)).to eq(18.301029995664) + expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to eq(11.2041199826559) # when 'top_topics_formula_first_post_likes_multiplier' setting is changed SiteSetting.top_topics_formula_log_views_multiplier = 2 # unchanged @@ -108,9 +108,9 @@ describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - expect(top_topics.where(topic_id: topic_1.id).pluck(:yearly_score).first).to eq(69) - expect(top_topics.where(topic_id: topic_2.id).pluck(:yearly_score).first).to eq(33.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck(:yearly_score).first).to eq(10.602059991328) + expect(top_topics.where(topic_id: topic_1.id).pluck_first(:yearly_score)).to eq(69) + expect(top_topics.where(topic_id: topic_2.id).pluck_first(:yearly_score)).to eq(33.301029995664) + expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to eq(10.602059991328) # when 'top_topics_formula_least_likes_per_post_multiplier' setting is changed SiteSetting.top_topics_formula_log_views_multiplier = 2 # unchanged @@ -126,9 +126,9 @@ describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - expect(top_topics.where(topic_id: topic_1.id).pluck(:yearly_score).first).to eq(30) - expect(top_topics.where(topic_id: topic_2.id).pluck(:yearly_score).first).to eq(21.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck(:yearly_score).first).to eq(10.602059991328) + expect(top_topics.where(topic_id: topic_1.id).pluck_first(:yearly_score)).to eq(30) + expect(top_topics.where(topic_id: topic_2.id).pluck_first(:yearly_score)).to eq(21.301029995664) + expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to eq(10.602059991328) # handles invalid string value SiteSetting.top_topics_formula_log_views_multiplier = "not good" @@ -138,9 +138,9 @@ describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - expect(top_topics.where(topic_id: topic_1.id).pluck(:yearly_score).first).to eq(27) - expect(top_topics.where(topic_id: topic_2.id).pluck(:yearly_score).first).to eq(18.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck(:yearly_score).first).to eq(10.602059991328) + expect(top_topics.where(topic_id: topic_1.id).pluck_first(:yearly_score)).to eq(27) + expect(top_topics.where(topic_id: topic_2.id).pluck_first(:yearly_score)).to eq(18.301029995664) + expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to eq(10.602059991328) end end diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index d8e38e7773c..dd7cc234158 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -49,7 +49,7 @@ describe PostAlerter do PostAlerter.post_created(reply2) # we get a green notification for a reply - expect(Notification.where(user_id: pm.user_id).pluck(:notification_type).first).to eq(Notification.types[:private_message]) + expect(Notification.where(user_id: pm.user_id).pluck_first(:notification_type)).to eq(Notification.types[:private_message]) TopicUser.change(pm.user_id, pm.id, notification_level: TopicUser.notification_levels[:tracking]) diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index fcce2e72510..d95e8ca3101 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -228,7 +228,7 @@ describe UserMerger do [group1, group2, group3].each do |g| owner = [group1, group3].include?(g) expect(GroupUser.where(group_id: g.id, user_id: target_user.id, owner: owner).count).to eq(1) - expect(Group.where(id: g.id).pluck(:user_count).first).to eq(2) + expect(Group.where(id: g.id).pluck_first(:user_count)).to eq(2) end expect(GroupUser.where(user_id: source_user.id).count).to eq(0) end