Skip to content

Commit ee3e225

Browse files
committed
Make "new topics count" async
We had a "there are X new topics" notification already, but it was only refreshed during page load - so unless a user started scrolling, never. We also calculated this for every request, which could be constly for some search queries, greatly increasing page load time. This commit extracts it to work asynchronously, polling regularly, which decouples it from the main page load time and also makes it more useful.
1 parent 3180b93 commit ee3e225

6 files changed

Lines changed: 97 additions & 31 deletions

File tree

app/controllers/topics_controller.rb

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ class TopicsController < ApplicationController
44

55
def index
66
@search_query = nil
7-
base_query = Topic.includes(:creator)
8-
base_query = apply_filters(base_query)
7+
base_query = apply_filters(Topic.includes(:creator))
98

109
apply_cursor_pagination(base_query)
10+
@new_topics_count = 0
1111

1212
preload_topic_states if user_signed_in?
1313
preload_note_counts if user_signed_in?
@@ -20,6 +20,15 @@ def index
2020
end
2121
end
2222

23+
def new_topics_count
24+
@viewing_since = viewing_since_param
25+
base_query = topics_base_query(search_query: params[:q])
26+
@new_topics_count = count_new_topics(base_query, @viewing_since)
27+
refresh_path = params[:q].present? ? search_topics_path(q: params[:q]) : topics_path
28+
29+
render partial: "new_topics_banner", locals: { count: @new_topics_count, viewing_since: @viewing_since, refresh_path: refresh_path }
30+
end
31+
2332
def show
2433
messages_scope = @topic.messages.includes(:sender, reply_to: :sender)
2534

@@ -81,20 +90,10 @@ def read_all
8190
def search
8291
@search_query = params[:q].to_s.strip
8392

84-
base_query = if @search_query.present?
85-
search_pattern = "%#{ActiveRecord::Base.sanitize_sql_like(@search_query)}%"
86-
87-
title_sql = Topic.select(:id).where("title ILIKE ?", search_pattern).to_sql
88-
message_sql = Message.select(:topic_id).where("body ILIKE ?", search_pattern).to_sql
89-
union_sql = "(#{title_sql}) UNION (#{message_sql})"
90-
91-
Topic.where("topics.id IN (#{union_sql})")
92-
.includes(:creator)
93-
else
94-
Topic.none
95-
end
93+
base_query = topics_base_query(search_query: @search_query)
9694

9795
apply_cursor_pagination(base_query)
96+
@new_topics_count = 0
9897

9998
preload_participation_flags if user_signed_in?
10099

@@ -220,11 +219,7 @@ def assign_branch_segments!
220219
end
221220

222221
def apply_cursor_pagination(base_query)
223-
@viewing_since = if params[:viewing_since].present?
224-
Time.zone.parse(params[:viewing_since])
225-
else
226-
Time.current
227-
end
222+
@viewing_since = viewing_since_param
228223

229224
query_with_window = base_query.joins(:messages)
230225
.group('topics.id')
@@ -242,12 +237,6 @@ def apply_cursor_pagination(base_query)
242237
@topics = @topics.order('MAX(messages.created_at) DESC, topics.id DESC')
243238
.limit(25)
244239
.load
245-
246-
@new_topics_count = base_query.joins(:messages)
247-
.group('topics.id')
248-
.having('MAX(messages.created_at) > ?', @viewing_since)
249-
.count
250-
.size
251240
end
252241

253242
def preload_topic_states
@@ -551,6 +540,45 @@ def filter_team_readers(topic, team_id:)
551540
readers.select { |r| r[:team_id] == team_id }
552541
end
553542

543+
def topics_base_query(search_query: nil)
544+
return apply_filters(Topic.includes(:creator)) if search_query.nil?
545+
546+
cleaned_query = search_query.to_s.strip
547+
return Topic.none if cleaned_query.blank?
548+
549+
build_search_query(cleaned_query)
550+
end
551+
552+
def build_search_query(query)
553+
cleaned_query = query.to_s.strip
554+
return Topic.none if cleaned_query.blank?
555+
556+
search_pattern = "%#{ActiveRecord::Base.sanitize_sql_like(cleaned_query)}%"
557+
558+
title_sql = Topic.select(:id).where("title ILIKE ?", search_pattern).to_sql
559+
message_sql = Message.select(:topic_id).where("body ILIKE ?", search_pattern).to_sql
560+
union_sql = "(#{title_sql}) UNION (#{message_sql})"
561+
562+
Topic.where("topics.id IN (#{union_sql})")
563+
.includes(:creator)
564+
end
565+
566+
def viewing_since_param
567+
if params[:viewing_since].present?
568+
Time.zone.parse(params[:viewing_since])
569+
else
570+
Time.current
571+
end
572+
end
573+
574+
def count_new_topics(base_query, viewing_since)
575+
base_query.joins(:messages)
576+
.group('topics.id')
577+
.having('MAX(messages.created_at) > ?', viewing_since)
578+
.count
579+
.size
580+
end
581+
554582
def load_notes
555583
notes = Note.active.visible_to(current_user)
556584
.where(topic: @topic)
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { Controller } from "@hotwired/stimulus"
2+
3+
export default class extends Controller {
4+
static values = {
5+
url: String,
6+
intervalMs: { type: Number, default: 180000 },
7+
}
8+
9+
connect() {
10+
this.startPolling()
11+
}
12+
13+
disconnect() {
14+
this.stopPolling()
15+
}
16+
17+
startPolling() {
18+
this.fetchBanner()
19+
this.poller = setInterval(() => this.fetchBanner(), this.intervalMsValue)
20+
}
21+
22+
stopPolling() {
23+
if (this.poller) {
24+
clearInterval(this.poller)
25+
this.poller = null
26+
}
27+
}
28+
29+
fetchBanner() {
30+
if (!this.urlValue) return
31+
32+
fetch(this.urlValue, { headers: { "Accept": "text/html" } })
33+
.then(response => response.ok ? response.text() : Promise.reject(response))
34+
.then(html => {
35+
this.element.innerHTML = html
36+
})
37+
.catch(error => {
38+
console.warn("Failed to refresh new topics banner", error)
39+
})
40+
}
41+
}

app/views/topics/index.html.slim

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
= render partial: "sidebar", locals: { available_note_tags: @available_note_tags, search_query: @search_query }
44

5-
#new-topics-banner
5+
#new-topics-banner data-controller="new-topics-banner" data-new-topics-banner-url-value=new_topics_count_topics_path(viewing_since: @viewing_since.iso8601, filter: params[:filter], team_id: params[:team_id]) data-new-topics-banner-interval-ms-value="180000"
66
= render partial: "new_topics_banner", locals: { count: @new_topics_count, viewing_since: @viewing_since }
77

88
- if @active_note_tag.present?

app/views/topics/index.turbo_stream.slim

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
= turbo_stream.append "topics" do
22
= render partial: "topics", locals: { topics: @topics }
33

4-
= turbo_stream.replace "new-topics-banner" do
5-
- refresh_path = params[:q].present? ? search_topics_path(q: params[:q]) : topics_path
6-
= render partial: "new_topics_banner", locals: { count: @new_topics_count, viewing_since: @viewing_since, refresh_path: refresh_path }
7-
84
- if @topics.size == 25
95
- last_topic = @topics.last
106
- cursor = "#{last_topic.last_activity.iso8601}_#{last_topic.id}"

app/views/topics/search.html.slim

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
= render partial: "sidebar", locals: { available_note_tags: @available_note_tags, search_query: @search_query }
44

5-
#new-topics-banner
5+
#new-topics-banner data-controller="new-topics-banner" data-new-topics-banner-url-value=new_topics_count_topics_path(q: @search_query, viewing_since: @viewing_since.iso8601) data-new-topics-banner-interval-ms-value="180000"
66
= render partial: "new_topics_banner", locals: { count: @new_topics_count, viewing_since: @viewing_since, refresh_path: search_topics_path(q: @search_query) }
77

88
- if @search_query.present?

config/routes.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
resources :topics, only: [:index, :show] do
2525
collection do
2626
get :search
27+
get :new_topics_count
2728
post :aware_bulk
2829
post :aware_all
2930
end

0 commit comments

Comments
 (0)