From b55e6da9a5d7ac30d52a730b64f4fb7feaf4a270 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sun, 26 Nov 2023 19:32:50 +0100 Subject: [PATCH] Use subqueries to check reaction/subscription state --- app/controllers/answer_controller.rb | 6 +----- app/controllers/concerns/paginates_answers.rb | 8 +------- app/controllers/discover_controller.rb | 8 ++------ app/controllers/question_controller.rb | 3 +-- app/controllers/timeline_controller.rb | 6 ++---- app/controllers/user_controller.rb | 4 ++-- app/models/answer.rb | 14 ++++++++++++++ app/models/answer/timeline_methods.rb | 3 ++- app/models/list/timeline_methods.rb | 1 + app/models/question/answer_methods.rb | 1 + app/models/user/answer_methods.rb | 3 ++- app/models/user/timeline_methods.rb | 1 + app/views/actions/_answer.html.haml | 2 +- app/views/answer/show.html.haml | 2 +- app/views/answerbox/_actions.html.haml | 4 ++-- app/views/application/_answerbox.html.haml | 4 ++-- app/views/discover/tab/_answers.html.haml | 2 +- app/views/discover/tab/_discussed.html.haml | 2 +- app/views/question/show.html.haml | 2 +- app/views/question/show.turbo_stream.haml | 2 +- app/views/timeline/timeline.html.haml | 2 +- app/views/timeline/timeline.turbo_stream.haml | 2 +- app/views/user/show.html.haml | 4 ++-- app/views/user/show.turbo_stream.haml | 2 +- 24 files changed, 45 insertions(+), 43 deletions(-) diff --git a/app/controllers/answer_controller.rb b/app/controllers/answer_controller.rb index a1b5c516..917501e5 100644 --- a/app/controllers/answer_controller.rb +++ b/app/controllers/answer_controller.rb @@ -8,15 +8,11 @@ class AnswerController < ApplicationController turbo_stream_actions :pin, :unpin def show - @answer = Answer.includes(question: [:user], smiles: [:user]).find(params[:id]) + @answer = Answer.for_user(current_user).includes(question: [:user], smiles: [:user]).find(params[:id]) @display_all = true - @reacted_answer_ids = [] - @subscribed_answer_ids = [] return unless user_signed_in? - @reacted_answer_ids = Reaction.where(user: current_user, parent: @answer).pluck(:parent_id) - @subscribed_answer_ids = Subscription.where(user: current_user, answer: @answer).pluck(:answer_id) mark_notifications_as_read end diff --git a/app/controllers/concerns/paginates_answers.rb b/app/controllers/concerns/paginates_answers.rb index b4c22c64..fe0b0c0b 100644 --- a/app/controllers/concerns/paginates_answers.rb +++ b/app/controllers/concerns/paginates_answers.rb @@ -5,12 +5,6 @@ module PaginatesAnswers @answers = yield(last_id: params[:last_id]) answer_ids = @answers.map(&:id) @answers_last_id = answer_ids.min - answer_ids += @pinned_answers.pluck(:id) if @pinned_answers.present? - @more_data_available = !yield(last_id: @answers_last_id, size: 1).count.zero? - - return unless user_signed_in? - - @reacted_answer_ids = Reaction.where(user: current_user, parent_type: "Answer", parent_id: answer_ids).pluck(:parent_id) - @subscribed_answer_ids = Subscription.where(user: current_user, answer_id: answer_ids).pluck(:answer_id) + @more_data_available = !yield(last_id: @answers_last_id, size: 1).select("answers.id").count.zero? end end diff --git a/app/controllers/discover_controller.rb b/app/controllers/discover_controller.rb index eb01e080..2af00fc1 100644 --- a/app/controllers/discover_controller.rb +++ b/app/controllers/discover_controller.rb @@ -8,15 +8,11 @@ class DiscoverController < ApplicationController top_x = 10 # only display the top X items - @popular_answers = Answer.where("created_at > ?", Time.now.ago(1.week)).order(:smile_count).reverse_order.limit(top_x).includes(:question, :user, :comments) - @most_discussed = Answer.where("created_at > ?", Time.now.ago(1.week)).order(:comment_count).reverse_order.limit(top_x).includes(:question, :user, :comments) + @popular_answers = Answer.for_user(current_user).where("created_at > ?", Time.now.ago(1.week)).order(:smile_count).reverse_order.limit(top_x).includes(:question, :user, :comments) + @most_discussed = Answer.for_user(current_user).where("created_at > ?", Time.now.ago(1.week)).order(:comment_count).reverse_order.limit(top_x).includes(:question, :user, :comments) @popular_questions = Question.where("created_at > ?", Time.now.ago(1.week)).order(:answer_count).reverse_order.limit(top_x).includes(:user) @new_users = User.where("asked_count > 0").order(:id).reverse_order.limit(top_x).includes(:profile) - answer_ids = @popular_answers.map(&:id) + @most_discussed.map(&:id) - @reacted_answer_ids = Reaction.where(user: current_user, parent_type: "Answer", parent_id: answer_ids).pluck(:parent_id) - @subscribed_answer_ids = Subscription.where(user: current_user, answer_id: answer_ids).pluck(:answer_id) - # .user = the user # .question_count = how many questions did the user ask @users_with_most_questions = Question.select('user_id, COUNT(*) AS question_count'). diff --git a/app/controllers/question_controller.rb b/app/controllers/question_controller.rb index 14dbe80e..6d334ea3 100644 --- a/app/controllers/question_controller.rb +++ b/app/controllers/question_controller.rb @@ -8,8 +8,7 @@ class QuestionController < ApplicationController @answers = @question.cursored_answers(last_id: params[:last_id], current_user:) answer_ids = @answers.map(&:id) @answers_last_id = answer_ids.min - @more_data_available = !@question.cursored_answers(last_id: @answers_last_id, size: 1, current_user:).count.zero? - @subscribed = Subscription.where(user: current_user, answer_id: answer_ids).pluck(:answer_id) if user_signed_in? + @more_data_available = !@question.cursored_answers(last_id: @answers_last_id, size: 1, current_user:).select("answers.id").count.zero? respond_to do |format| format.html diff --git a/app/controllers/timeline_controller.rb b/app/controllers/timeline_controller.rb index 747798e7..e5c7fd33 100644 --- a/app/controllers/timeline_controller.rb +++ b/app/controllers/timeline_controller.rb @@ -32,11 +32,9 @@ class TimelineController < ApplicationController def paginate_timeline @timeline = yield(last_id: params[:last_id]) - timeline_ids = @timeline.map(&:id) + timeline_ids = @timeline.select("answers.id").map(&:id) @timeline_last_id = timeline_ids.min - @more_data_available = !yield(last_id: @timeline_last_id, size: 1).count.zero? - @reacted_answer_ids = Reaction.where(user: current_user, parent_type: "Answer", parent_id: timeline_ids).pluck(:parent_id) - @subscribed_answer_ids = Subscription.where(user: current_user, answer_id: timeline_ids).pluck(:answer_id) + @more_data_available = !yield(last_id: @timeline_last_id, size: 1).select("answers.id").count.zero? respond_to do |format| format.html { render "timeline/timeline" } diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 1e9940d4..6f03231b 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -8,8 +8,8 @@ class UserController < ApplicationController after_action :mark_notification_as_read, only: %i[show] def show - @pinned_answers = @user.answers.pinned.includes([{ user: :profile }, :question]).order(pinned_at: :desc).limit(10).load_async - paginate_answers { |args| @user.cursored_answers(**args) } + @pinned_answers = @user.answers.for_user(current_user).pinned.includes([{ user: :profile }, :question]).order(pinned_at: :desc).limit(10).load_async + paginate_answers { |args| @user.cursored_answers(current_user_id: current_user, **args) } respond_to do |format| format.html diff --git a/app/models/answer.rb b/app/models/answer.rb index 3c8736e1..702b7702 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -15,6 +15,20 @@ class Answer < ApplicationRecord # rubocop:enable Rails/UniqueValidationWithoutIndex scope :pinned, -> { where.not(pinned_at: nil) } + scope :for_user, lambda { |current_user| + next select("answers.*", "false as is_subscribed", "false as has_reacted") if current_user.nil? + + select("answers.*", + "EXISTS(SELECT 1 + FROM subscriptions + WHERE answer_id = answers.id + AND user_id = #{current_user.id}) as is_subscribed", + "EXISTS(SELECT 1 + FROM reactions + WHERE parent_id = answers.id + AND parent_type = 'Answer' + AND user_id = #{current_user.id}) as has_reacted") + } SHORT_ANSWER_MAX_LENGTH = 640 diff --git a/app/models/answer/timeline_methods.rb b/app/models/answer/timeline_methods.rb index 3245aa7e..465869da 100644 --- a/app/models/answer/timeline_methods.rb +++ b/app/models/answer/timeline_methods.rb @@ -6,7 +6,8 @@ module Answer::TimelineMethods define_cursor_paginator :cursored_public_timeline, :public_timeline def public_timeline(current_user: nil) - includes([{ user: :profile }, :question]) + for_user(current_user) + .includes([{ user: :profile }, :question]) .then do |query| next query unless current_user diff --git a/app/models/list/timeline_methods.rb b/app/models/list/timeline_methods.rb index b02d87e3..c55c9471 100644 --- a/app/models/list/timeline_methods.rb +++ b/app/models/list/timeline_methods.rb @@ -8,6 +8,7 @@ module List::TimelineMethods # @return [ActiveRecord::Relation] the lists' timeline def timeline(current_user: nil) Answer + .for_user(current_user) .includes([{ user: :profile }, :question]) .then do |query| next query unless current_user diff --git a/app/models/question/answer_methods.rb b/app/models/question/answer_methods.rb index 1a01dc7b..63301904 100644 --- a/app/models/question/answer_methods.rb +++ b/app/models/question/answer_methods.rb @@ -7,6 +7,7 @@ module Question::AnswerMethods def ordered_answers(current_user: nil) answers + .for_user(current_user) .includes([{ user: :profile }, :question]) .then do |query| next query unless current_user diff --git a/app/models/user/answer_methods.rb b/app/models/user/answer_methods.rb index 74e1a44e..211e4d93 100644 --- a/app/models/user/answer_methods.rb +++ b/app/models/user/answer_methods.rb @@ -6,8 +6,9 @@ module User::AnswerMethods define_cursor_paginator :cursored_answers, :ordered_answers # @return [ActiveRecord::Relation] List of a user's answers - def ordered_answers + def ordered_answers(current_user_id:) answers + .for_user(current_user_id) .order(:created_at) .reverse_order .includes(question: { user: [:profile] }) diff --git a/app/models/user/timeline_methods.rb b/app/models/user/timeline_methods.rb index 79da93d3..24796954 100644 --- a/app/models/user/timeline_methods.rb +++ b/app/models/user/timeline_methods.rb @@ -8,6 +8,7 @@ module User::TimelineMethods # @return [ActiveRecord::Relation] the user's timeline def timeline Answer + .for_user(self) .then do |query| blocked_and_muted_user_ids = blocked_user_ids_cached + muted_user_ids_cached next query if blocked_and_muted_user_ids.empty? diff --git a/app/views/actions/_answer.html.haml b/app/views/actions/_answer.html.haml index 78b81363..e029fe19 100644 --- a/app/views/actions/_answer.html.haml +++ b/app/views/actions/_answer.html.haml @@ -1,5 +1,5 @@ .dropdown-menu.dropdown-menu-end{ role: :menu } - - if subscribed_answer_ids&.include?(answer.id) + - if answer.is_subscribed = render "subscriptions/destroy", answer: answer - else = render "subscriptions/create", answer: answer diff --git a/app/views/answer/show.html.haml b/app/views/answer/show.html.haml index 136fcfbe..4cc12576 100644 --- a/app/views/answer/show.html.haml +++ b/app/views/answer/show.html.haml @@ -1,4 +1,4 @@ - provide(:title, answer_title(@answer)) - provide(:og, answer_opengraph(@answer)) .container-lg.container--main - = render "answerbox", a: @answer, display_all: @display_all, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a: @answer, display_all: @display_all diff --git a/app/views/answerbox/_actions.html.haml b/app/views/answerbox/_actions.html.haml index 170f1ec9..6dafce38 100644 --- a/app/views/answerbox/_actions.html.haml +++ b/app/views/answerbox/_actions.html.haml @@ -1,4 +1,4 @@ -%button.btn.btn-link.answerbox__action{ type: :button, name: "ab-smile", data: { a_id: a.id, action: reacted_answer_ids&.include?(a.id) ? :unsmile : :smile, selection_hotkey: "s" }, disabled: !user_signed_in? } +%button.btn.btn-link.answerbox__action{ type: :button, name: "ab-smile", data: { a_id: a.id, action: a.has_reacted ? :unsmile : :smile, selection_hotkey: "s" }, disabled: !user_signed_in? } %i.fa.fa-fw.fa-smile-o %span{ id: "ab-smile-count-#{a.id}" }= a.smile_count - unless display_all @@ -13,4 +13,4 @@ .btn-group %button.btn.btn-default.btn-sm.dropdown-toggle{ data: { bs_toggle: :dropdown }, aria: { expanded: false } } %span.caret - = render "actions/answer", answer: a, subscribed_answer_ids: + = render "actions/answer", answer: a diff --git a/app/views/application/_answerbox.html.haml b/app/views/application/_answerbox.html.haml index fb11033d..bfe29e4e 100644 --- a/app/views/application/_answerbox.html.haml +++ b/app/views/application/_answerbox.html.haml @@ -22,7 +22,7 @@ .answerbox__answer-date = link_to(raw(t("time.distance_ago", time: time_tooltip(a))), answer_path(a.user.screen_name, a.id), data: { selection_hotkey: "l" }) .col-md-6.d-flex.d-md-block.answerbox__actions - = render "answerbox/actions", a:, display_all:, subscribed_answer_ids:, reacted_answer_ids: + = render "answerbox/actions", a:, display_all: - else .row .col-md-6.text-start.text-muted @@ -34,7 +34,7 @@ %i.fa.fa-thumbtack = t(".pinned") .col-md-6.d-md-flex.answerbox__actions - = render "answerbox/actions", a:, display_all:, subscribed_answer_ids:, reacted_answer_ids: + = render "answerbox/actions", a:, display_all: .card-footer{ id: "ab-comments-section-#{a.id}", class: display_all.nil? ? "d-none" : nil } = turbo_frame_tag("ab-reactions-list-#{a.id}", src: reactions_path(a.question, a), loading: :lazy) do .d-flex.smiles diff --git a/app/views/discover/tab/_answers.html.haml b/app/views/discover/tab/_answers.html.haml index 952004d4..cef9a25a 100644 --- a/app/views/discover/tab/_answers.html.haml +++ b/app/views/discover/tab/_answers.html.haml @@ -1,3 +1,3 @@ .tab-pane.active.fade.show{ role: :tabpanel, id: "answers" } - answers.each do |a| - = render "answerbox", a:, subscribed_answer_ids:, reacted_answer_ids: + = render "answerbox", a: diff --git a/app/views/discover/tab/_discussed.html.haml b/app/views/discover/tab/_discussed.html.haml index 0eeeb94c..ed288036 100644 --- a/app/views/discover/tab/_discussed.html.haml +++ b/app/views/discover/tab/_discussed.html.haml @@ -1,3 +1,3 @@ .tab-pane.fade{ role: :tabpanel, id: "comments" } - comments.each do |a| - = render "answerbox", a:, subscribed_answer_ids:, reacted_answer_ids: + = render "answerbox", a: diff --git a/app/views/question/show.html.haml b/app/views/question/show.html.haml index a2d9d48d..cb24fbd1 100644 --- a/app/views/question/show.html.haml +++ b/app/views/question/show.html.haml @@ -8,7 +8,7 @@ %button.d-none{ data: { hotkey: "j", action: "navigation#down" } } %button.d-none{ data: { hotkey: "k", action: "navigation#up" } } - @answers.each do |a| - = render "answerbox", a:, show_question: false, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a:, show_question: false - if @more_data_available .d-flex.justify-content-center.justify-content-sm-start#paginator diff --git a/app/views/question/show.turbo_stream.haml b/app/views/question/show.turbo_stream.haml index f6da225e..335a808b 100644 --- a/app/views/question/show.turbo_stream.haml +++ b/app/views/question/show.turbo_stream.haml @@ -1,6 +1,6 @@ = turbo_stream.append "answers" do - @answers.each do |a| - = render "answerbox", a:, show_question: false, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a:, show_question: false = turbo_stream.update "paginator" do - if @more_data_available diff --git a/app/views/timeline/timeline.html.haml b/app/views/timeline/timeline.html.haml index bffd66cf..2b3089b8 100644 --- a/app/views/timeline/timeline.html.haml +++ b/app/views/timeline/timeline.html.haml @@ -2,7 +2,7 @@ %button.d-none{ data: { hotkey: "j", action: "navigation#down" } } %button.d-none{ data: { hotkey: "k", action: "navigation#up" } } - @timeline.each do |answer| - = render "answerbox", a: answer, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a: answer - if @more_data_available .d-flex.justify-content-center#paginator diff --git a/app/views/timeline/timeline.turbo_stream.haml b/app/views/timeline/timeline.turbo_stream.haml index ccbc5211..561309e6 100644 --- a/app/views/timeline/timeline.turbo_stream.haml +++ b/app/views/timeline/timeline.turbo_stream.haml @@ -1,6 +1,6 @@ = turbo_stream.append "timeline" do - @timeline.each do |answer| - = render "answerbox", a: answer, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a: answer = turbo_stream.update "paginator" do - if @more_data_available diff --git a/app/views/user/show.html.haml b/app/views/user/show.html.haml index 382ed8b8..98f36de8 100644 --- a/app/views/user/show.html.haml +++ b/app/views/user/show.html.haml @@ -4,11 +4,11 @@ %button.d-none{ data: { hotkey: "k", action: "navigation#up" } } #pinned-answers - @pinned_answers.each do |a| - = render "answerbox", a:, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a: #answers - @answers.each do |a| - = render "answerbox", a:, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a: - if @more_data_available .d-flex.justify-content-center.justify-content-sm-start#paginator diff --git a/app/views/user/show.turbo_stream.haml b/app/views/user/show.turbo_stream.haml index 977cbe4c..ae7889ad 100644 --- a/app/views/user/show.turbo_stream.haml +++ b/app/views/user/show.turbo_stream.haml @@ -1,6 +1,6 @@ = turbo_stream.append "answers" do - @answers.each do |a| - = render "answerbox", a:, subscribed_answer_ids: @subscribed_answer_ids, reacted_answer_ids: @reacted_answer_ids + = render "answerbox", a: = turbo_stream.update "paginator" do - if @more_data_available