From 25a226279a54600b9332a2dd0a71bd119e3804af Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Mon, 13 Feb 2023 12:39:45 +0800 Subject: [PATCH] DEV: Replace #pluck_first freedom patch with AR #pick in core (#19893) The #pluck_first freedom patch, first introduced by @danielwaterworth has served us well, and is used widely throughout both core and plugins. It seems to have been a common enough use case that Rails 6 introduced it's own method #pick with the exact same implementation. This allows us to retire the freedom patch and switch over to the built-in ActiveRecord method. There is no replacement for #pluck_first!, but a quick search shows we are using this in a very limited capacity, and in some cases incorrectly (by assuming a nil return rather than an exception), which can quite easily be replaced with #pick plus some extra handling. --- app/controllers/categories_controller.rb | 2 +- app/controllers/directory_items_controller.rb | 2 +- app/controllers/list_controller.rb | 2 +- app/controllers/posts_controller.rb | 2 +- app/controllers/stylesheets_controller.rb | 4 +-- .../theme_javascripts_controller.rb | 6 ++-- app/controllers/topics_controller.rb | 2 +- app/controllers/user_badges_controller.rb | 2 +- app/helpers/application_helper.rb | 4 +-- app/jobs/regular/update_username.rb | 8 +++-- app/mailers/user_notifications.rb | 4 +-- app/models/application_request.rb | 4 +-- app/models/backup_metadata.rb | 2 +- app/models/category.rb | 5 ++-- 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/screened_ip_address.rb | 2 +- app/models/topic.rb | 2 +- app/models/topic_converter.rb | 2 +- app/models/topic_embed.rb | 6 ++-- app/models/topic_user.rb | 3 +- app/models/user.rb | 12 ++++---- app/models/user_action.rb | 2 +- app/models/web_crawler_request.rb | 2 +- app/serializers/suggested_topics_mixin.rb | 2 +- app/serializers/topic_view_serializer.rb | 2 +- app/services/post_alerter.rb | 3 +- db/fixtures/600_themes.rb | 2 +- lib/composer_messages_finder.rb | 2 +- lib/email/sender.rb | 2 +- lib/file_store/s3_store.rb | 2 +- lib/freedom_patches/pluck_first.rb | 10 ++++--- lib/guardian.rb | 3 +- lib/post_creator.rb | 5 ++-- lib/post_revisor.rb | 3 +- lib/search.rb | 18 +++++------ lib/tasks/posts.rake | 2 +- lib/topic_query.rb | 7 ++--- lib/topic_query/private_message_lists.rb | 4 +-- lib/topic_view.rb | 4 +-- lib/trust_level.rb | 2 +- .../max_username_length_validator.rb | 2 +- .../min_username_length_validator.rb | 2 +- .../import_scripts/base/lookup_container.rb | 2 +- script/micro_bench.rb | 2 +- spec/helpers/application_helper_spec.rb | 6 ++-- spec/lib/wizard/step_updater_spec.rb | 6 ++-- spec/models/post_action_spec.rb | 8 ++--- spec/models/top_topic_spec.rb | 30 +++++++++---------- spec/models/topic_link_spec.rb | 4 +-- spec/services/post_alerter_spec.rb | 4 +-- spec/services/user_merger_spec.rb | 2 +- 56 files changed, 112 insertions(+), 123 deletions(-) diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index a7293b65828..2212a9752d4 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -365,7 +365,7 @@ class CategoriesController < ApplicationController if SiteSetting.enable_category_group_moderation? params[:reviewable_by_group_id] = Group.where( name: params[:reviewable_by_group_name], - ).pluck_first(:id) if params[:reviewable_by_group_name] + ).pick(:id) if params[:reviewable_by_group_name] end result = diff --git a/app/controllers/directory_items_controller.rb b/app/controllers/directory_items_controller.rb index 15f1451d5a8..33bac7ec275 100644 --- a/app/controllers/directory_items_controller.rb +++ b/app/controllers/directory_items_controller.rb @@ -70,7 +70,7 @@ class DirectoryItemsController < ApplicationController end if params[:username] - user_id = User.where(username_lower: params[:username].to_s.downcase).pluck_first(:id) + user_id = User.where(username_lower: params[:username].to_s.downcase).pick(: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 6e34b868676..b9e827190f8 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -456,7 +456,7 @@ class ListController < ApplicationController def self.best_period_for(previous_visit_at, category_id = nil) default_period = ( - (category_id && Category.where(id: category_id).pluck_first(:default_top_period)) || + (category_id && Category.where(id: category_id).pick(:default_top_period)) || SiteSetting.top_page_default_timeframe ).to_sym diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 2b2a329e2b5..e311f7ac5ba 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -615,7 +615,7 @@ class PostsController < ApplicationController bookmarkable_id: params[:post_id], bookmarkable_type: "Post", user_id: current_user.id, - ).pluck_first(:id) + ).pick(:id) destroyed_bookmark = BookmarkManager.new(current_user).destroy(bookmark_id) render json: diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb index 97edb80b576..1d6c77f0a65 100644 --- a/app/controllers/stylesheets_controller.rb +++ b/app/controllers/stylesheets_controller.rb @@ -59,7 +59,7 @@ class StylesheetsController < ApplicationController cache_path = Stylesheet::Manager.cache_fullpath location = "#{cache_path}/#{target}#{underscore_digest}#{extension}" - stylesheet_time = query.pluck_first(:created_at) + stylesheet_time = query.pick(:created_at) handle_missing_cache(location, target, digest) if !stylesheet_time @@ -68,7 +68,7 @@ class StylesheetsController < ApplicationController end unless File.exist?(location) - if current = query.pluck_first(source_map ? :source_map : :content) + if current = query.pick(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 159402889fe..f311ebfe391 100644 --- a/app/controllers/theme_javascripts_controller.rb +++ b/app/controllers/theme_javascripts_controller.rb @@ -22,7 +22,7 @@ class ThemeJavascriptsController < ApplicationController cache_file = "#{DISK_CACHE_PATH}/#{params[:digest]}.js" write_if_not_cached(cache_file) do - content, has_source_map = query.pluck_first(:content, "source_map IS NOT NULL") + content, has_source_map = query.pick(:content, "source_map IS NOT NULL") if has_source_map content += "\n//# sourceMappingURL=#{params[:digest]}.map?__ws=#{Discourse.current_hostname}\n" @@ -40,7 +40,7 @@ class ThemeJavascriptsController < ApplicationController # Security: safe due to route constraint cache_file = "#{DISK_CACHE_PATH}/#{params[:digest]}.map" - write_if_not_cached(cache_file) { query.pluck_first(:source_map) } + write_if_not_cached(cache_file) { query.pick(:source_map) } serve_file(cache_file) end @@ -75,7 +75,7 @@ class ThemeJavascriptsController < ApplicationController if params[:action].to_s == "show_tests" File.exist?(@cache_file) ? File.ctime(@cache_file) : nil else - query.pluck_first(:updated_at) + query.pick(:updated_at) end end end diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 27ac78fc52e..612827147e4 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -843,7 +843,7 @@ class TopicsController < ApplicationController if params[:title].present? # when creating a new topic, ensure the 1st post is a regular post - if Post.where(topic: topic, id: post_ids).order(:post_number).pluck_first(:post_type) != + if Post.where(topic: topic, id: post_ids).order(:post_number).pick(:post_type) != Post.types[:regular] return( render_json_error( diff --git a/app/controllers/user_badges_controller.rb b/app/controllers/user_badges_controller.rb index 1617f55516d..ac0e78be29a 100644 --- a/app/controllers/user_badges_controller.rb +++ b/app/controllers/user_badges_controller.rb @@ -22,7 +22,7 @@ class UserBadgesController < ApplicationController grant_count = nil if params[:username] - user_id = User.where(username_lower: params[:username].downcase).pluck_first(:id) + user_id = User.where(username_lower: params[:username].downcase).pick(: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 3f7cafff491..cc7cc1bd928 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -190,7 +190,7 @@ module ApplicationHelper result << "category-#{@category.slug_path.join("-")}" if @category && @category.url.present? if current_user.present? && current_user.primary_group_id && - primary_group_name = Group.where(id: current_user.primary_group_id).pluck_first(:name) + primary_group_name = Group.where(id: current_user.primary_group_id).pick(:name) result << "primary-group-#{primary_group_name.downcase}" end @@ -544,7 +544,7 @@ module ApplicationHelper return if theme_id.blank? - @scheme_id = Theme.where(id: theme_id).pluck_first(:color_scheme_id) + @scheme_id = Theme.where(id: theme_id).pick(:color_scheme_id) end def dark_scheme_id diff --git a/app/jobs/regular/update_username.rb b/app/jobs/regular/update_username.rb index 41114ec140c..26c3f64099d 100644 --- a/app/jobs/regular/update_username.rb +++ b/app/jobs/regular/update_username.rb @@ -197,9 +197,11 @@ module Jobs end def quotes_correct_user?(aside) - Post.where(topic_id: aside["data-topic"], post_number: aside["data-post"]).pluck_first( - :user_id, - ) == @user_id + Post.exists?( + topic_id: aside["data-topic"], + post_number: aside["data-post"], + user_id: @user_id, + ) end end end diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index d5580858274..a5545b05bd5 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -504,7 +504,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: notification_data[:original_user_id] || post.user_id).pluck_first(:name) + name = User.where(id: notification_data[:original_user_id] || post.user_id).pick(:name) user_name = name unless name.blank? end @@ -589,7 +589,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_first(:name)}/#{show_category_in_subject}" + "#{Category.where(id: category.parent_category_id).pick(: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 d422a08c960..bb308a74356 100644 --- a/app/models/application_request.rb +++ b/app/models/application_request.rb @@ -39,9 +39,7 @@ class ApplicationRequest < ActiveRecord::Base def self.req_id(date, req_type, retries = 0) req_type_id = req_types[req_type] - # a poor man's upsert - id = where(date: date, req_type: req_type_id).pluck_first(:id) - id ||= create!(date: date, req_type: req_type_id, count: 0).id + create_or_find_by!(date: date, req_type: req_type_id).id rescue StandardError # primary key violation if retries == 0 req_id(date, req_type, 1) diff --git a/app/models/backup_metadata.rb b/app/models/backup_metadata.rb index 3fdcd071d31..92ea233ba50 100644 --- a/app/models/backup_metadata.rb +++ b/app/models/backup_metadata.rb @@ -4,7 +4,7 @@ class BackupMetadata < ActiveRecord::Base LAST_RESTORE_DATE = "last_restore_date" def self.value_for(name) - where(name: name).pluck_first(:value).presence + where(name: name).pick(:value).presence end def self.last_restore_date diff --git a/app/models/category.rb b/app/models/category.rb index 39a2c70bf4f..06be2853336 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -790,9 +790,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_first( - :id, - ) || self.where(id: parent_slug.to_i).pluck_first(:id) + self.where(slug: (encoded_parent_slug || parent_slug), parent_category_id: nil).pick(:id) || + self.where(id: parent_slug.to_i).pick(:id) end def self.query_category(slug_or_id, parent_category_id) diff --git a/app/models/draft.rb b/app/models/draft.rb index a9dff7f2774..942ce774826 100644 --- a/app/models/draft.rb +++ b/app/models/draft.rb @@ -252,7 +252,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_first(:post_id) + post_id = BackupDraftPost.where(user_id: user.id, key: key).pick(:post_id) post = Post.where(id: post_id).first if post_id BackupDraftPost.where(user_id: user.id, key: key).delete_all if post_id && !post @@ -305,7 +305,7 @@ class Draft < ActiveRecord::Base end def self.ensure_draft_topic!(user) - topic_id = BackupDraftTopic.where(user_id: user.id).pluck_first(:topic_id) + topic_id = BackupDraftTopic.where(user_id: user.id).pick(:topic_id) topic = Topic.find_by(id: topic_id) if topic_id BackupDraftTopic.where(user_id: user.id).delete_all if topic_id && !topic diff --git a/app/models/incoming_link.rb b/app/models/incoming_link.rb index acb7d0b3955..694ce06e503 100644 --- a/app/models/incoming_link.rb +++ b/app/models/incoming_link.rb @@ -37,7 +37,7 @@ class IncomingLink < ActiveRecord::Base if host != opts[:host] && (user_id || referer) post_id = opts[:post_id] post_id ||= - Post.where(topic_id: opts[:topic_id], post_number: opts[:post_number] || 1).pluck_first(:id) + Post.where(topic_id: opts[:topic_id], post_number: opts[:post_number] || 1).pick(: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 86ec8057ac5..825a566faf4 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -348,7 +348,7 @@ class Notification < ActiveRecord::Base end def post_id - Post.where(topic: topic_id, post_number: post_number).pluck_first(:id) + Post.where(topic: topic_id, post_number: post_number).pick(:id) end protected diff --git a/app/models/post.rb b/app/models/post.rb index 736295205cb..5095f463882 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1149,7 +1149,7 @@ class Post < ActiveRecord::Base end upload_id = nil - upload_id = Upload.where(sha1: sha1).pluck_first(:id) if sha1.present? + upload_id = Upload.where(sha1: sha1).pick(: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 a02c8908cf6..d339316367e 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -242,7 +242,7 @@ class PostAction < ActiveRecord::Base end end - topic_id = Post.with_deleted.where(id: post_id).pluck_first(:topic_id) + topic_id = Post.with_deleted.where(id: post_id).pick(:topic_id) # topic_user if post_action_type_key == :like diff --git a/app/models/quoted_post.rb b/app/models/quoted_post.rb index 54d68c9a926..131a9d98aae 100644 --- a/app/models/quoted_post.rb +++ b/app/models/quoted_post.rb @@ -68,7 +68,7 @@ class QuotedPost < ActiveRecord::Base if post.reply_to_post_number reply_post_id = - Post.where(topic_id: post.topic_id, post_number: post.reply_to_post_number).pluck_first(:id) + Post.where(topic_id: post.topic_id, post_number: post.reply_to_post_number).pick(:id) reply_quoted = reply_post_id.present? && QuotedPost.where(post_id: post.id, quoted_post_id: reply_post_id).count > 0 diff --git a/app/models/screened_ip_address.rb b/app/models/screened_ip_address.rb index ed6cc3d800b..136164ecb0a 100644 --- a/app/models/screened_ip_address.rb +++ b/app/models/screened_ip_address.rb @@ -147,7 +147,7 @@ class ScreenedIpAddress < ActiveRecord::Base .where("masklen(ip_address) IN (?)", from_masklen) sum_match_count, max_last_match_at, min_created_at = - old_ips.pluck_first("SUM(match_count), MAX(last_match_at), MIN(created_at)") + old_ips.pick("SUM(match_count), MAX(last_match_at), MIN(created_at)") ScreenedIpAddress.create!( ip_address: subnet, diff --git a/app/models/topic.rb b/app/models/topic.rb index b1e764b7a97..0be942c4815 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -893,7 +893,7 @@ class Topic < ActiveRecord::Base # If a post is deleted we have to update our highest post counters and last post information def self.reset_highest(topic_id) - archetype = Topic.where(id: topic_id).pluck_first(:archetype) + archetype = Topic.where(id: topic_id).pick(:archetype) # ignore small_action replies for private messages post_type = diff --git a/app/models/topic_converter.rb b/app/models/topic_converter.rb index 8eab3a31d0c..3c0b9652a95 100644 --- a/app/models/topic_converter.rb +++ b/app/models/topic_converter.rb @@ -20,7 +20,7 @@ class TopicConverter .where(read_restricted: false) .where.not(id: SiteSetting.uncategorized_category_id) .order("id asc") - .pluck_first(:id) + .pick(:id) end PostRevisor.new(@topic.first_post, @topic).revise!( diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index 8f9bb37c22a..98c171e070f 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -254,9 +254,7 @@ class TopicEmbed < ActiveRecord::Base def self.topic_id_for_embed(embed_url) embed_url = normalize_url(embed_url).sub(%r{\Ahttps?\://}, "") - TopicEmbed.where("embed_url ~* ?", "^https?://#{Regexp.escape(embed_url)}$").pluck_first( - :topic_id, - ) + TopicEmbed.where("embed_url ~* ?", "^https?://#{Regexp.escape(embed_url)}$").pick(:topic_id) end def self.first_paragraph_from(html) @@ -281,7 +279,7 @@ class TopicEmbed < ActiveRecord::Base Discourse .cache .fetch("embed-topic:#{post.topic_id}", expires_in: 10.minutes) do - url = TopicEmbed.where(topic_id: post.topic_id).pluck_first(:embed_url) + url = TopicEmbed.where(topic_id: post.topic_id).pick(: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 0e00f68f2de..6b58cfbe464 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -275,8 +275,7 @@ class TopicUser < ActiveRecord::Base attrs[:notification_level] = notification_levels[:watching] end else - auto_track_after = - UserOption.where(user_id: user_id).pluck_first(:auto_track_topics_after_msecs) + auto_track_after = UserOption.where(user_id: user_id).pick(: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 f188ec78f51..92b2c72be26 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -294,7 +294,7 @@ class User < ActiveRecord::Base ->(filter) { if filter.is_a?(String) && filter =~ /.+@.+/ # probably an email so try the bypass - if user_id = UserEmail.where("lower(email) = ?", filter.downcase).pluck_first(:user_id) + if user_id = UserEmail.where("lower(email) = ?", filter.downcase).pick(:user_id) return where("users.id = ?", user_id) end end @@ -1683,11 +1683,11 @@ class User < ActiveRecord::Base 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_first(:title) + if next_best_group_title = group_titles_query.pick(:title) return next_best_group_title end - next_best_badge_title = badges.where(allow_title: true).pluck_first(:name) + next_best_badge_title = badges.where(allow_title: true).pick(:name) next_best_badge_title ? Badge.display_name(next_best_badge_title) : nil end @@ -2030,9 +2030,7 @@ class User < ActiveRecord::Base def match_primary_group_changes return unless primary_group_id_changed? - if title == Group.where(id: primary_group_id_was).pluck_first(:title) - self.title = primary_group&.title - end + self.title = primary_group&.title if Group.exists?(id: primary_group_id_was, title: title) self.flair_group_id = primary_group&.id if flair_group_id == primary_group_id_was end @@ -2043,7 +2041,7 @@ class User < ActiveRecord::Base .human_users .joins(:user_auth_tokens) .order("user_auth_tokens.created_at") - .pluck_first(:id) + .pick(:id) end private diff --git a/app/models/user_action.rb b/app/models/user_action.rb index d605324de47..5aa7529fa2e 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -69,7 +69,7 @@ class UserAction < ActiveRecord::Base UserAction .where(user_id: user_id, target_topic_id: topic_id, action_type: [RESPONSE, MENTION, QUOTE]) .order("created_at DESC") - .pluck_first(:target_post_id) + .pick(:target_post_id) end def self.stats(user_id, guardian) diff --git a/app/models/web_crawler_request.rb b/app/models/web_crawler_request.rb index c1660f65555..8be7be49e01 100644 --- a/app/models/web_crawler_request.rb +++ b/app/models/web_crawler_request.rb @@ -24,7 +24,7 @@ class WebCrawlerRequest < ActiveRecord::Base protected def self.request_id(date:, user_agent:, retries: 0) - id = where(date: date, user_agent: user_agent).pluck_first(:id) + id = where(date: date, user_agent: user_agent).pick(:id) id ||= create!({ date: date, user_agent: user_agent }.merge(count: 0)).id rescue StandardError # primary key violation if retries == 0 diff --git a/app/serializers/suggested_topics_mixin.rb b/app/serializers/suggested_topics_mixin.rb index a899d73f7d1..e257a9436d6 100644 --- a/app/serializers/suggested_topics_mixin.rb +++ b/app/serializers/suggested_topics_mixin.rb @@ -33,7 +33,7 @@ module SuggestedTopicsMixin object.topic_allowed_group_ids, scope.user.id, ) - .pluck_first(:name) + .pick(:name) end end diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 91eb2a6f2f1..d1f66fe93e5 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -283,7 +283,7 @@ class TopicViewSerializer < ApplicationSerializer owner: true, }, ) - .pluck_first(:name) + .pick(:name) end def include_requested_group_name? diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index b792a6e701d..db6906157d2 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -385,8 +385,7 @@ class PostAlerter stats = (@group_stats[topic.id] ||= group_stats(topic)) return unless stats - group_id = - topic.topic_allowed_groups.where(group_id: user.groups.pluck(:id)).pluck_first(:group_id) + group_id = topic.topic_allowed_groups.where(group_id: user.groups.pluck(:id)).pick(:group_id) stat = stats.find { |s| s[:group_id] == group_id } return unless stat diff --git a/db/fixtures/600_themes.rb b/db/fixtures/600_themes.rb index ce01f8eb1ce..f58ddd69cbf 100644 --- a/db/fixtures/600_themes.rb +++ b/db/fixtures/600_themes.rb @@ -30,7 +30,7 @@ if !Theme.exists? if SiteSetting.default_dark_mode_color_scheme_id == SiteSetting.defaults[:default_dark_mode_color_scheme_id] - dark_scheme_id = ColorScheme.where(base_scheme_id: "Dark").pluck_first(:id) + dark_scheme_id = ColorScheme.where(base_scheme_id: "Dark").pick(:id) SiteSetting.default_dark_mode_color_scheme_id = dark_scheme_id if dark_scheme_id.present? end diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index e394bb39840..b82f2bedde7 100644 --- a/lib/composer_messages_finder.rb +++ b/lib/composer_messages_finder.rb @@ -206,7 +206,7 @@ class ComposerMessagesFinder topic_id: @details[:topic_id], ) - reply_username = User.where(id: last_x_replies[0]).pluck_first(:username) + reply_username = User.where(id: last_x_replies[0]).pick(:username) { id: "get_a_room", diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 1f3c59efd04..bdbc312b9b8 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -122,7 +122,7 @@ module Email if from_address.blank? nil else - Group.where(email_username: from_address, smtp_enabled: true).pluck_first(:id) + Group.where(email_username: from_address, smtp_enabled: true).pick(:id) end ) diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 03d181467bb..0536938c915 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -409,7 +409,7 @@ module FileStore verified_ids = [] files.each do |f| - id = model.where("url LIKE '%#{f.key}' AND etag = '#{f.etag}'").pluck_first(:id) + id = model.where("url LIKE '%#{f.key}' AND etag = '#{f.etag}'").pick(: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 index 6f3d520187f..616cbad93d5 100644 --- a/lib/freedom_patches/pluck_first.rb +++ b/lib/freedom_patches/pluck_first.rb @@ -2,15 +2,17 @@ class ActiveRecord::Relation def pluck_first(*attributes) - limit(1).pluck(*attributes).first + Discourse.deprecate("`#pluck_first` is deprecated, use `#pick` instead.") + pick(*attributes) end def pluck_first!(*attributes) - items = limit(1).pluck(*attributes) + Discourse.deprecate("`#pluck_first!` is deprecated without replacement.") + items = pick(*attributes) - raise_record_not_found_exception! if items.empty? + raise_record_not_found_exception! if items.nil? - items.first + items end end diff --git a/lib/guardian.rb b/lib/guardian.rb index 89ae923b18b..a34dadd7102 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -368,8 +368,7 @@ class Guardian def can_use_flair_group?(user, group_id = nil) return false if !user || !group_id || !user.group_ids.include?(group_id.to_i) - flair_icon, flair_upload_id = - Group.where(id: group_id.to_i).pluck_first(:flair_icon, :flair_upload_id) + flair_icon, flair_upload_id = Group.where(id: group_id.to_i).pick(:flair_icon, :flair_upload_id) flair_icon.present? || flair_upload_id.present? end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index b233de35403..2ce5a51307d 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -328,9 +328,8 @@ class PostCreator if PostCreator.track_post_stats sequence = DraftSequence.current(@user, draft_key) revisions = - Draft.where(sequence: sequence, user_id: @user.id, draft_key: draft_key).pluck_first( - :revisions, - ) || 0 + Draft.where(sequence: sequence, user_id: @user.id, draft_key: draft_key).pick(:revisions) || + 0 @post.build_post_stat( drafts_saved: revisions, diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 1ce1ef284be..e57fb42e542 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -552,8 +552,7 @@ class PostRevisor if revision.modifications.empty? revision.destroy @post.last_editor_id = - PostRevision.where(post_id: @post.id).order(number: :desc).pluck_first(:user_id) || - @post.user_id + PostRevision.where(post_id: @post.id).order(number: :desc).pick(:user_id) || @post.user_id @post.version -= 1 @post.public_version -= 1 @post.save(validate: @validate_post) diff --git a/lib/search.rb b/lib/search.rb index af244565081..8248975e978 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -430,7 +430,7 @@ class Search advanced_filter(/\Ain:wiki\z/i) { |posts, match| posts.where(wiki: true) } advanced_filter(/\Abadge:(.*)\z/i) do |posts, match| - badge_id = Badge.where("name ilike ? OR id = ?", match, match.to_i).pluck_first(:id) + badge_id = Badge.where("name ilike ? OR id = ?", match, match.to_i).pick(:id) if badge_id posts.where( "posts.user_id IN (SELECT ub.user_id FROM user_badges ub WHERE ub.badge_id = ?)", @@ -480,7 +480,7 @@ class Search end advanced_filter(/\Acreated:@(.*)\z/i) do |posts, match| - user_id = User.where(username: match.downcase).pluck_first(:id) + user_id = User.where(username: match.downcase).pick(:id) posts.where(user_id: user_id, post_number: 1) end @@ -563,12 +563,12 @@ class Search parent_category_id: Category.where("lower(slug) = ?", category_slug.downcase).select(:id), ) - .pluck_first(:id) + .pick(:id) else Category .where("lower(slug) = ?", category_slug.downcase) .order("case when parent_category_id is null then 0 else 1 end") - .pluck_first(:id) + .pick(:id) end if category_id @@ -579,7 +579,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_first(:id) + tag_id = Tag.where_name(category_slug).pick(:id) if (tag_id) posts.where(<<~SQL, tag_id) topics.id IN ( @@ -625,7 +625,7 @@ class Search group_query = cb.call(group_query, @term, @guardian) end - group_id = group_query.pluck_first(:id) + group_id = group_query.pick(:id) if group_id posts.where( @@ -644,7 +644,7 @@ class Search .members_visible_groups(@guardian.user) .where(has_messages: true) .where("name ilike ? OR (id = ? AND id > 0)", match, match.to_i) - .pluck_first(:id) + .pick(:id) if group_id posts.where( @@ -661,7 +661,7 @@ class Search User .where(staged: false) .where("username_lower = ? OR id = ?", match.downcase, match.to_i) - .pluck_first(:id) + .pick(:id) if user_id posts.where("posts.user_id = ?", user_id) else @@ -672,7 +672,7 @@ class Search advanced_filter(/\A\@(\S+)\z/i) do |posts, match| username = User.normalize_username(match) - user_id = User.not_staged.where(username_lower: username).pluck_first(:id) + user_id = User.not_staged.where(username_lower: username).pick(:id) user_id = @guardian.user&.id if !user_id && username == "me" diff --git a/lib/tasks/posts.rake b/lib/tasks/posts.rake index 9fd1f480f3c..16f2e84f16a 100644 --- a/lib/tasks/posts.rake +++ b/lib/tasks/posts.rake @@ -543,7 +543,7 @@ def recover_uploads_from_index(path) .pluck(:post_id, :value) .each do |post_id, uploads| uploads = JSON.parse(uploads) - raw = Post.where(id: post_id).pluck_first(:raw) + raw = Post.where(id: post_id).pick(: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 a7049b008e5..cbfbab113f2 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -630,8 +630,7 @@ class TopicQuery category_id = category_id_or_slug.to_i if category_id == 0 - category_id = - Category.where(slug: category_id_or_slug, parent_category_id: nil).pluck_first(:id) + category_id = Category.where(slug: category_id_or_slug, parent_category_id: nil).pick(:id) end category_id @@ -678,7 +677,7 @@ class TopicQuery filter = (options[:filter] || options[:f]) # category default sort order sort_order, sort_ascending = - Category.where(id: category_id).pluck_first(:sort_order, :sort_ascending) + Category.where(id: category_id).pick(:sort_order, :sort_ascending) if sort_order && (filter.blank? || %i[latest unseen].include?(filter)) options[:order] = sort_order options[:ascending] = !!sort_ascending ? "true" : "false" @@ -1023,7 +1022,7 @@ class TopicQuery :first_unread_pm_at, ) else - UserStat.where(user_id: @user.id).pluck_first(:first_unread_pm_at) + UserStat.where(user_id: @user.id).pick(:first_unread_pm_at) end query = query.where("topics.updated_at >= ?", first_unread_pm_at) if first_unread_pm_at diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index 0c4874ba2a8..932a76c6de5 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -188,7 +188,7 @@ class TopicQuery when :user user_first_unread_pm_at(user) when :group - GroupUser.where(user: user, group: group).pluck_first(:first_unread_pm_at) + GroupUser.where(user: user, group: group).pick(:first_unread_pm_at) else user_first_unread_pm_at = user_first_unread_pm_at(user) @@ -284,7 +284,7 @@ class TopicQuery end def user_first_unread_pm_at(user) - UserStat.where(user: user).pluck_first(:first_unread_pm_at) + UserStat.where(user: user).pick(:first_unread_pm_at) end def group_with_messages_ids(user) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 576b6d3c5c3..b73e93a2588 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -671,7 +671,7 @@ class TopicView end def filtered_post_id(post_number) - @filtered_posts.where(post_number: post_number).pluck_first(:id) + @filtered_posts.where(post_number: post_number).pick(:id) end def is_mega_topic? @@ -679,7 +679,7 @@ class TopicView end def last_post_id - @filtered_posts.reverse_order.pluck_first(:id) + @filtered_posts.reverse_order.pick(:id) end def current_post_number diff --git a/lib/trust_level.rb b/lib/trust_level.rb index 184b44ced6e..2589df081b9 100644 --- a/lib/trust_level.rb +++ b/lib/trust_level.rb @@ -48,7 +48,7 @@ class TrustLevel .where(action: UserHistory.actions[:change_trust_level]) .where(target_user_id: user.id) .order(created_at: :desc) - .pluck_first(:new_value) + .pick(:new_value) .to_i end end diff --git a/lib/validators/max_username_length_validator.rb b/lib/validators/max_username_length_validator.rb index fe29824f834..49bcc1b400c 100644 --- a/lib/validators/max_username_length_validator.rb +++ b/lib/validators/max_username_length_validator.rb @@ -13,7 +13,7 @@ class MaxUsernameLengthValidator return false end return false if value < SiteSetting.min_username_length - @username = User.where("length(username) > ?", value).pluck_first(:username) + @username = User.where("length(username) > ?", value).pick(:username) @username.blank? end diff --git a/lib/validators/min_username_length_validator.rb b/lib/validators/min_username_length_validator.rb index 1ae9342fef1..ee0af94b698 100644 --- a/lib/validators/min_username_length_validator.rb +++ b/lib/validators/min_username_length_validator.rb @@ -13,7 +13,7 @@ class MinUsernameLengthValidator return false end return false if value > SiteSetting.max_username_length - @username = User.where("length(username) < ?", value).pluck_first(:username) + @username = User.where("length(username) < ?", value).pick(:username) @username.blank? end diff --git a/script/import_scripts/base/lookup_container.rb b/script/import_scripts/base/lookup_container.rb index 4147daf43f4..ab0fb3c6123 100644 --- a/script/import_scripts/base/lookup_container.rb +++ b/script/import_scripts/base/lookup_container.rb @@ -58,7 +58,7 @@ module ImportScripts def find_username_by_import_id(import_id) user_id = user_id_from_imported_user_id(import_id) - User.where(id: user_id).pluck_first(:username) if user_id.present? + User.where(id: user_id).pick(:username) if user_id.present? end # Get the Discourse Category id based on the id of the source category diff --git a/script/micro_bench.rb b/script/micro_bench.rb index 7ba5dceffe8..18aa85b019b 100644 --- a/script/micro_bench.rb +++ b/script/micro_bench.rb @@ -14,7 +14,7 @@ Benchmark.ips do |b| b.report("pluck with limit") { User.limit(1).pluck(:name).first } - b.report("pluck with pluck_first") { User.pluck_first(:name) } + b.report("pluck with pick") { User.pick(:name) } b.report("raw") { conn.exec("SELECT name FROM users LIMIT 1").getvalue(0, 0) } end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index c25cd5efbb9..50d6f419947 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -683,8 +683,7 @@ RSpec.describe ApplicationHelper do end it "returns two color scheme link tags when dark mode is enabled" do - SiteSetting.default_dark_mode_color_scheme_id = - ColorScheme.where(name: "Dark").pluck_first(:id) + SiteSetting.default_dark_mode_color_scheme_id = ColorScheme.where(name: "Dark").pick(:id) cs_stylesheets = helper.discourse_color_scheme_stylesheets expect(cs_stylesheets).to include("(prefers-color-scheme: dark)") @@ -740,8 +739,7 @@ RSpec.describe ApplicationHelper do helper.request.env[Auth::DefaultCurrentUserProvider::CURRENT_USER_KEY] = user @new_cs = Fabricate(:color_scheme, name: "Custom Color Scheme") - SiteSetting.default_dark_mode_color_scheme_id = - ColorScheme.where(name: "Dark").pluck_first(:id) + SiteSetting.default_dark_mode_color_scheme_id = ColorScheme.where(name: "Dark").pick(:id) end it "returns no dark scheme stylesheet when user has disabled that option" do diff --git a/spec/lib/wizard/step_updater_spec.rb b/spec/lib/wizard/step_updater_spec.rb index 4c725b5051a..69648f8e2db 100644 --- a/spec/lib/wizard/step_updater_spec.rb +++ b/spec/lib/wizard/step_updater_spec.rb @@ -370,7 +370,7 @@ RSpec.describe Wizard::StepUpdater do expect(SiteSetting.contact_email).to eq("eviltrout@example.com") # Should update the TOS topic - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pick(:raw) expect(raw).to eq("ACME, Inc. - New Jersey law - Fairfield, New Jersey template") # Can update the TOS topic again @@ -382,13 +382,13 @@ RSpec.describe Wizard::StepUpdater do city_for_disputes: "San Francisco, California", ) updater.update - raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pluck_first(:raw) + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pick(: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_first(:raw) + raw = Post.where(topic_id: SiteSetting.tos_topic_id, post_number: 1).pick(: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 74dcfe23f5f..25d160afb26 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -49,13 +49,13 @@ RSpec.describe PostAction do expect(topic_user_ids).to include(codinghorror.id) expect(topic_user_ids).to include(mod.id) - expect(topic.topic_users.where(user_id: mod.id).pluck_first(:notification_level)).to eq( + expect(topic.topic_users.where(user_id: mod.id).pick(:notification_level)).to eq( TopicUser.notification_levels[:tracking], ) - expect( - topic.topic_users.where(user_id: codinghorror.id).pluck_first(:notification_level), - ).to eq(TopicUser.notification_levels[:watching]) + expect(topic.topic_users.where(user_id: codinghorror.id).pick(:notification_level)).to eq( + TopicUser.notification_levels[:watching], + ) # reply to PM should not clear flag PostCreator.new( diff --git a/spec/models/top_topic_spec.rb b/spec/models/top_topic_spec.rb index f0fa30b4c94..0d057396fa5 100644 --- a/spec/models/top_topic_spec.rb +++ b/spec/models/top_topic_spec.rb @@ -62,11 +62,11 @@ RSpec.describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - 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( + expect(top_topics.where(topic_id: topic_1.id).pick(:yearly_score)).to eq(27) + expect(top_topics.where(topic_id: topic_2.id).pick(: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( + expect(top_topics.where(topic_id: topic_3.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(10.602059991328) @@ -84,11 +84,11 @@ RSpec.describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - 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( + expect(top_topics.where(topic_id: topic_1.id).pick(:yearly_score)).to eq(27) + expect(top_topics.where(topic_id: topic_2.id).pick(: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( + expect(top_topics.where(topic_id: topic_3.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(11.2041199826559) @@ -106,11 +106,11 @@ RSpec.describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - 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 be_within( + expect(top_topics.where(topic_id: topic_1.id).pick(:yearly_score)).to eq(69) + expect(top_topics.where(topic_id: topic_2.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(33.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to be_within( + expect(top_topics.where(topic_id: topic_3.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(10.602059991328) @@ -128,11 +128,11 @@ RSpec.describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - 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 be_within( + expect(top_topics.where(topic_id: topic_1.id).pick(:yearly_score)).to eq(30) + expect(top_topics.where(topic_id: topic_2.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(21.301029995664) - expect(top_topics.where(topic_id: topic_3.id).pluck_first(:yearly_score)).to be_within( + expect(top_topics.where(topic_id: topic_3.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(10.602059991328) @@ -144,11 +144,11 @@ RSpec.describe TopTopic do TopTopic.refresh! top_topics = TopTopic.all - 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( + expect(top_topics.where(topic_id: topic_1.id).pick(:yearly_score)).to eq(27) + expect(top_topics.where(topic_id: topic_2.id).pick(: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( + expect(top_topics.where(topic_id: topic_3.id).pick(:yearly_score)).to be_within( 0.0000000001, ).of(10.602059991328) end diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index cf837e44bc5..9e5177311f1 100644 --- a/spec/models/topic_link_spec.rb +++ b/spec/models/topic_link_spec.rb @@ -53,8 +53,8 @@ RSpec.describe TopicLink do TopicLink.extract_from(post) # we have a special rule for images title where we pull them out of the filename - expect(topic.topic_links.where(url: png).pluck_first(:title)).to eq(png_title) - expect(topic.topic_links.where(url: non_png).pluck_first(:title)).to eq("amazing") + expect(topic.topic_links.where(url: png).pick(:title)).to eq(png_title) + expect(topic.topic_links.where(url: non_png).pick(:title)).to eq("amazing") expect(topic.topic_links.pluck(:url)).to contain_exactly( png, diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 793239bd8ab..fc4bf047952 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -64,7 +64,7 @@ RSpec.describe PostAlerter do PostAlerter.post_created(reply2) # we get a green notification for a reply - expect(Notification.where(user_id: pm.user_id).pluck_first(:notification_type)).to eq( + expect(Notification.where(user_id: pm.user_id).pick(:notification_type)).to eq( Notification.types[:private_message], ) @@ -104,7 +104,7 @@ RSpec.describe PostAlerter do ) PostAlerter.post_created(op) - expect(Notification.where(user_id: user.id).pluck_first(:notification_type)).to eq( + expect(Notification.where(user_id: user.id).pick(:notification_type)).to eq( Notification.types[:private_message], ) end diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index a38059c728f..9414a984288 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -233,7 +233,7 @@ RSpec.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_first(:user_count)).to eq(2) + expect(Group.where(id: g.id).pick(:user_count)).to eq(2) end expect(GroupUser.where(user_id: source_user.id).count).to eq(0) end