Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-emptively fetch answer reaction state #1459

Merged
merged 3 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions app/controllers/answer_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +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
@subscribed_answer_ids = []

return unless user_signed_in?

@subscribed_answer_ids = Subscription.where(user: current_user, answer: @answer).pluck(:answer_id)
mark_notifications_as_read
end

Expand Down
4 changes: 1 addition & 3 deletions app/controllers/concerns/paginates_answers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ def paginate_answers
@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?
@subscribed_answer_ids = Subscription.where(user: current_user, answer_id: answer_ids).pluck(:answer_id) if user_signed_in?
@more_data_available = !yield(last_id: @answers_last_id, size: 1).select("answers.id").count.zero?
end
end
9 changes: 3 additions & 6 deletions app/controllers/discover_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@

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.utc.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.utc.ago(1.week)).order(:comment_count).reverse_order.limit(top_x).includes(:question, :user, :comments)

Check warning on line 12 in app/controllers/discover_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/discover_controller.rb#L11-L12

Added lines #L11 - L12 were not covered by tests
@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)
@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').
Expand All @@ -28,7 +25,7 @@
# .user = the user
# .answer_count = how many questions did the user answer
@users_with_most_answers = Answer.select('user_id, COUNT(*) AS answer_count').
where("created_at > ?", Time.now.ago(1.week)).
where("created_at > ?", week_ago).

Check notice on line 28 in app/controllers/discover_controller.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] app/controllers/discover_controller.rb#L28 <Layout/MultilineMethodCallIndentation>

Align `where` with `Answer.select('user_id, COUNT(*) AS answer_count').` on line 27.
Raw output
app/controllers/discover_controller.rb:28:9: C: Layout/MultilineMethodCallIndentation: Align `where` with `Answer.select('user_id, COUNT(*) AS answer_count').` on line 27.

Check notice on line 28 in app/controllers/discover_controller.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] app/controllers/discover_controller.rb#L28 <Layout/DotPosition>

Place the . on the next line, together with the method name.
Raw output
app/controllers/discover_controller.rb:28:42: C: Layout/DotPosition: Place the . on the next line, together with the method name.
group(:user_id).
order('answer_count').
reverse_order.limit(top_x)
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/question_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ def show
@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 = [email protected]_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 = [email protected]_answers(last_id: @answers_last_id, size: 1, current_user:).select("answers.id").count.zero?

respond_to do |format|
format.html
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/timeline_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ def set_lists

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?
@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" }
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/user_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions app/models/answer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@
# 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",
)

Check notice on line 31 in app/models/answer.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] app/models/answer.rb#L31 <Layout/ClosingParenthesisIndentation>

Align `)` with `(`.
Raw output
app/models/answer.rb:31:12: C: Layout/ClosingParenthesisIndentation: Align `)` with `(`.

Check notice on line 31 in app/models/answer.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] app/models/answer.rb#L31 <Layout/MultilineMethodCallBraceLayout>

Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.
Raw output
app/models/answer.rb:31:12: C: Layout/MultilineMethodCallBraceLayout: Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.
}

SHORT_ANSWER_MAX_LENGTH = 640

Expand Down
3 changes: 2 additions & 1 deletion app/models/answer/timeline_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions app/models/list/timeline_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module List::TimelineMethods
# @return [ActiveRecord::Relation<Answer>] 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
Expand Down
1 change: 1 addition & 0 deletions app/models/question/answer_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/models/user/answer_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ module User::AnswerMethods
define_cursor_paginator :cursored_answers, :ordered_answers

# @return [ActiveRecord::Relation<Answer>] 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] })
Expand Down
1 change: 1 addition & 0 deletions app/models/user/timeline_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module User::TimelineMethods
# @return [ActiveRecord::Relation<Answer>] 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?
Expand Down
2 changes: 1 addition & 1 deletion app/views/actions/_answer.html.haml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/answer/show.html.haml
Original file line number Diff line number Diff line change
@@ -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
= render "answerbox", a: @answer, display_all: @display_all
4 changes: 2 additions & 2 deletions app/views/answerbox/_actions.html.haml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
%button.btn.btn-link.answerbox__action{ type: :button, name: "ab-smile", data: { a_id: a.id, action: current_user&.smiled?(a) ? :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? }

Check warning on line 1 in app/views/answerbox/_actions.html.haml

View workflow job for this annotation

GitHub Actions / haml-lint

[haml-lint] app/views/answerbox/_actions.html.haml#L1

LineLength: Line is too long. [189/160]
Raw output
app/views/answerbox/_actions.html.haml:1 [W] LineLength: Line is too long. [189/160]
%i.fa.fa-fw.fa-smile-o
%span{ id: "ab-smile-count-#{a.id}" }= a.smile_count
- unless display_all
Expand 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
4 changes: 2 additions & 2 deletions app/views/application/_answerbox.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
= render "answerbox/actions", a:, display_all:
- else
.row
.col-md-6.text-start.text-muted
Expand All @@ -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:
= 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
Expand Down
2 changes: 1 addition & 1 deletion app/views/discover/tab/_answers.html.haml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
.tab-pane.active.fade.show{ role: :tabpanel, id: "answers" }
- answers.each do |a|
= render "answerbox", a:, subscribed_answer_ids:
= render "answerbox", a:
2 changes: 1 addition & 1 deletion app/views/discover/tab/_discussed.html.haml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
.tab-pane.fade{ role: :tabpanel, id: "comments" }
- comments.each do |a|
= render "answerbox", a:, subscribed_answer_ids:
= render "answerbox", a:
2 changes: 1 addition & 1 deletion app/views/question/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
= render "answerbox", a:, show_question: false

- if @more_data_available
.d-flex.justify-content-center.justify-content-sm-start#paginator
Expand Down
2 changes: 1 addition & 1 deletion app/views/question/show.turbo_stream.haml
Original file line number Diff line number Diff line change
@@ -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
= render "answerbox", a:, show_question: false

= turbo_stream.update "paginator" do
- if @more_data_available
Expand Down
2 changes: 1 addition & 1 deletion app/views/timeline/timeline.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
= render "answerbox", a: answer

- if @more_data_available
.d-flex.justify-content-center#paginator
Expand Down
2 changes: 1 addition & 1 deletion app/views/timeline/timeline.turbo_stream.haml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
= turbo_stream.append "timeline" do
- @timeline.each do |answer|
= render "answerbox", a: answer, subscribed_answer_ids: @subscribed_answer_ids
= render "answerbox", a: answer

= turbo_stream.update "paginator" do
- if @more_data_available
Expand Down
4 changes: 2 additions & 2 deletions app/views/user/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
= render "answerbox", a:

#answers
- @answers.each do |a|
= render "answerbox", a:, subscribed_answer_ids: @subscribed_answer_ids
= render "answerbox", a:

- if @more_data_available
.d-flex.justify-content-center.justify-content-sm-start#paginator
Expand Down
2 changes: 1 addition & 1 deletion app/views/user/show.turbo_stream.haml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
= turbo_stream.append "answers" do
- @answers.each do |a|
= render "answerbox", a:, subscribed_answer_ids: @subscribed_answer_ids
= render "answerbox", a:

= turbo_stream.update "paginator" do
- if @more_data_available
Expand Down