Skip to content

Commit 4b21a51

Browse files
authored
Cache welcome thread query (forem#22741)
* Cache welcome thread query * Fix query * Fix private keyword * Fix spec
1 parent d6d4183 commit 4b21a51

File tree

7 files changed

+136
-14
lines changed

7 files changed

+136
-14
lines changed

app/controllers/community_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ def index
4848
@key_pages = Page.where(subforem_id: RequestStore.store[:subforem_id]).where(is_top_level_path: true).limit(6)
4949

5050
# Get welcome article if it exists
51-
@welcome_article = Article.from_subforem.admin_published_with("welcome").first ||
52-
Article.admin_published_with("welcome").first
51+
@welcome_article = Article.cached_admin_published_with("welcome", subforem_id: RequestStore.store[:subforem_id]) ||
52+
Article.cached_admin_published_with("welcome")
5353

5454
set_surrogate_key_header "community_hub"
5555
end

app/controllers/pages_controller.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ def robots
107107
end
108108

109109
def welcome
110-
article = Article.from_subforem.admin_published_with("welcome").first
111-
article = Article.admin_published_with("welcome").first unless article
110+
article = Article.cached_admin_published_with("welcome", subforem_id: RequestStore.store[:subforem_id])
111+
article ||= Article.cached_admin_published_with("welcome")
112112
redirect_daily_thread_request(article)
113113
end
114114

115115
def challenge
116-
article = Article.from_subforem.admin_published_with("welcome").first
117-
article = Article.admin_published_with("welcome").first unless article
118-
redirect_daily_thread_request(Article.admin_published_with("challenge").first)
116+
article = Article.cached_admin_published_with("welcome", subforem_id: RequestStore.store[:subforem_id])
117+
article ||= Article.cached_admin_published_with("welcome")
118+
redirect_daily_thread_request(Article.cached_admin_published_with("challenge"))
119119
end
120120

121121
def checkin

app/models/article.rb

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,14 +414,38 @@ def self.unique_url_error
414414
.union(User.with_role(:admin))
415415
.union(id: [Settings::Community.staff_user_id,
416416
Settings::General.mascot_user_id].compact)
417-
.select(:id)).order(published_at: :desc).tagged_with(tag_name)
417+
.select(:id)).order(published_at: :desc).cached_tagged_with(tag_name)
418418
}
419419

420+
def self.cached_admin_published_with(tag_name, subforem_id: nil, expires_in: 6.hours)
421+
cache_key = [
422+
"admin-published-with",
423+
tag_name,
424+
(subforem_id || "all"),
425+
].join(":")
426+
427+
Rails.cache.fetch(cache_key, expires_in: expires_in) do
428+
scope = subforem_id ? from_subforem(subforem_id) : all
429+
scope.admin_published_with(tag_name).first
430+
end
431+
end
432+
433+
def self.bust_cached_admin_published_with(tag_name, subforem_id: nil)
434+
cache_key = [
435+
"admin-published-with",
436+
tag_name,
437+
(subforem_id || "all"),
438+
].join(":")
439+
Rails.cache.delete(cache_key)
440+
end
441+
442+
after_commit :bust_cached_admin_welcome_thread, on: %i[create update]
443+
420444
scope :user_published_with, lambda { |user_id, tag_name|
421445
published
422446
.where(user_id: user_id)
423447
.order(published_at: :desc)
424-
.tagged_with(tag_name)
448+
.cached_tagged_with(tag_name)
425449
}
426450

427451
scope :active_help, lambda {
@@ -1576,6 +1600,15 @@ def record_field_test_event
15761600

15771601
private
15781602

1603+
def bust_cached_admin_welcome_thread
1604+
return unless published?
1605+
return unless cached_tag_list.to_s.match?(/(?:^|,)\s*welcome(?:\s*,|$)/)
1606+
return unless user&.admin?
1607+
1608+
self.class.bust_cached_admin_published_with("welcome", subforem_id: subforem_id)
1609+
self.class.bust_cached_admin_published_with("welcome")
1610+
end
1611+
15791612
def should_add_urls_from_title?
15801613
# Only add URLs from title for quickie posts (status type) that have a title with URLs
15811614
# Only run this when the title has changed or it's a new record to avoid duplicating embed tags

app/services/broadcasts/welcome_notification/generator.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def received_notification?(broadcast)
9595
end
9696

9797
def commented_on_welcome_thread?
98-
welcome_thread = Article.admin_published_with("welcome").first
98+
welcome_thread = Article.cached_admin_published_with("welcome")
9999
Comment.where(commentable: welcome_thread, user: user).any?
100100
end
101101

app/services/scheduled_automations/warm_welcome_badge_awarder.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ def find_current_welcome_thread
7070
# Try from subforem context if available, otherwise just get the most recent one
7171
subforem_id = RequestStore.store[:subforem_id] if defined?(RequestStore) && RequestStore.store
7272
if subforem_id
73-
Article.from_subforem(subforem_id).admin_published_with("welcome").first ||
74-
Article.admin_published_with("welcome").first
73+
Article.cached_admin_published_with("welcome", subforem_id: subforem_id) ||
74+
Article.cached_admin_published_with("welcome")
7575
else
76-
Article.admin_published_with("welcome").first
76+
Article.cached_admin_published_with("welcome")
7777
end
7878
end
7979

app/views/onboardings/_task_card.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
<h2 class="task-card-title" id="task-card-title"><%= t("views.onboardings.heading") %></h2>
3131
<p id="task-card-subtitle" class="task-card-subtitle"><%= t("views.onboardings.subtitle") %></p>
3232
<ul class="task-card-actions">
33-
<% if Article.admin_published_with("welcome").any? %>
33+
<% if Article.cached_admin_published_with("welcome").present? %>
3434
<li class="task-card-action">
3535
<a class="task-card-link" href="/welcome">
3636
<p><%= t("views.onboardings.welcome_html") %></p>

spec/models/article_spec.rb

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,6 +1634,95 @@ def build_and_validate_article(*args)
16341634
end
16351635
end
16361636

1637+
describe ".cached_admin_published_with" do
1638+
let(:admin) { create(:user, :admin) }
1639+
let(:cache_store) { ActiveSupport::Cache::MemoryStore.new }
1640+
1641+
around do |example|
1642+
original_cache = Rails.cache
1643+
Rails.cache = cache_store
1644+
example.run
1645+
ensure
1646+
Rails.cache = original_cache
1647+
end
1648+
1649+
it "caches the admin published article by tag" do
1650+
article = create(:article, user: admin, tags: "welcome")
1651+
1652+
cached = described_class.cached_admin_published_with("welcome")
1653+
1654+
expect(cached).to eq(article)
1655+
expect(Rails.cache.read("admin-published-with:welcome:all").id).to eq(article.id)
1656+
end
1657+
1658+
it "caches the admin published article by tag and subforem" do
1659+
subforem = create(:subforem)
1660+
article = create(:article, user: admin, tags: "welcome", subforem_id: subforem.id)
1661+
1662+
cached = described_class.cached_admin_published_with("welcome", subforem_id: subforem.id)
1663+
1664+
expect(cached).to eq(article)
1665+
expect(Rails.cache.read("admin-published-with:welcome:#{subforem.id}").id).to eq(article.id)
1666+
end
1667+
end
1668+
1669+
describe "admin welcome cache busting" do
1670+
let(:admin) { create(:user, :admin) }
1671+
let(:cache_store) { ActiveSupport::Cache::MemoryStore.new }
1672+
1673+
around do |example|
1674+
original_cache = Rails.cache
1675+
Rails.cache = cache_store
1676+
example.run
1677+
ensure
1678+
Rails.cache = original_cache
1679+
end
1680+
1681+
it "busts cached welcome keys when an admin publishes a welcome article" do
1682+
subforem = create(:subforem)
1683+
article = create(:article, user: admin, tags: "welcome", subforem_id: subforem.id)
1684+
1685+
Rails.cache.write("admin-published-with:welcome:all", "cached")
1686+
Rails.cache.write("admin-published-with:welcome:#{subforem.id}", "cached")
1687+
1688+
article.update!(title: "Updated title")
1689+
1690+
expect(Rails.cache.read("admin-published-with:welcome:all")).to be_nil
1691+
expect(Rails.cache.read("admin-published-with:welcome:#{subforem.id}")).to be_nil
1692+
end
1693+
1694+
it "does not bust cached welcome keys for non-admin authors" do
1695+
user = create(:user)
1696+
article = create(:article, user: user, tags: "welcome")
1697+
1698+
Rails.cache.write("admin-published-with:welcome:all", "cached")
1699+
1700+
article.update!(title: "Updated title")
1701+
1702+
expect(Rails.cache.read("admin-published-with:welcome:all")).to eq("cached")
1703+
end
1704+
1705+
it "busts the cache when welcome is in a comma-separated tag list" do
1706+
article = create(:article, user: admin, tags: "ruby, welcome, rails")
1707+
1708+
Rails.cache.write("admin-published-with:welcome:all", "cached")
1709+
1710+
article.update!(title: "Updated title")
1711+
1712+
expect(Rails.cache.read("admin-published-with:welcome:all")).to be_nil
1713+
end
1714+
1715+
it "does not bust the cache for tags that only contain welcome as a substring" do
1716+
article = create(:article, user: admin, tags: "welcome2, ruby")
1717+
1718+
Rails.cache.write("admin-published-with:welcome:all", "cached")
1719+
1720+
article.update!(title: "Updated title")
1721+
1722+
expect(Rails.cache.read("admin-published-with:welcome:all")).to eq("cached")
1723+
end
1724+
end
1725+
16371726
describe ".seo_boostable" do
16381727
let!(:top_article) do
16391728
create(:article, organic_page_views_past_month_count: 20, score: 30, tags: "good, greatalicious", user: user)

0 commit comments

Comments
 (0)