From e7a84948b970284a8a519a4329af17e2ce5a9bbe Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 24 Aug 2022 11:54:01 +0100 Subject: [PATCH] FIX: Avoid duplicate topic-list requests (#18073) When preloading topic_list data we were giving it a 'preload key' which was loosely based on the parameters of the list. However, it did not include all parameters, and mismatches between client/server-side logic would cause the preloaded data to be ignored. This commit simplifies things by using a single key for all topic_list preloading. This works on the assumption that "The first topic_list the JS app will load is the one which was preloaded". That assumption also existed to some extent in the old design, so we don't expect any regressions here. --- .../discourse/app/adapters/topic-list.js | 2 +- app/models/topic_list.rb | 12 +----------- spec/lib/topic_query_spec.rb | 2 +- spec/models/topic_list_spec.rb | 19 ++----------------- spec/requests/categories_controller_spec.rb | 2 +- 5 files changed, 6 insertions(+), 31 deletions(-) diff --git a/app/assets/javascripts/discourse/app/adapters/topic-list.js b/app/assets/javascripts/discourse/app/adapters/topic-list.js index 34073796cd8..31fbd179ddf 100644 --- a/app/assets/javascripts/discourse/app/adapters/topic-list.js +++ b/app/assets/javascripts/discourse/app/adapters/topic-list.js @@ -32,7 +32,7 @@ export default RestAdapter.extend({ const params = findArgs.params; return PreloadStore.getAndRemove( - "topic_list_" + filter, + "topic_list", finderFor(filter, params) ).then(function (result) { result.filter = filter; diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb index 192687d1779..d13c2ab3ecd 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -76,17 +76,7 @@ class TopicList end def preload_key - if @category - if @opts[:no_subcategories] - "topic_list_#{@category.url.sub(/^\//, '')}/none/l/#{@filter}" - else - "topic_list_#{@category.url.sub(/^\//, '')}/l/#{@filter}" - end - elsif @tags && @tags.first.present? - "topic_list_tag/#{@tags.first.name}/l/#{@filter}" - else - "topic_list_#{@filter}" - end + "topic_list" end # Lazy initialization diff --git a/spec/lib/topic_query_spec.rb b/spec/lib/topic_query_spec.rb index b2183f2c980..7ff1fe9bd28 100644 --- a/spec/lib/topic_query_spec.rb +++ b/spec/lib/topic_query_spec.rb @@ -273,7 +273,7 @@ RSpec.describe TopicQuery do list = TopicQuery.new(moderator, category: diff_category.slug).list_latest expect(list.topics.size).to eq(1) - expect(list.preload_key).to eq("topic_list_c/different-category/#{diff_category.id}/l/latest") + expect(list.preload_key).to eq("topic_list") # Defaults to no category filter when slug does not exist expect(TopicQuery.new(moderator, category: 'made up slug').list_latest.topics.size).to eq(2) diff --git a/spec/models/topic_list_spec.rb b/spec/models/topic_list_spec.rb index 89401996d18..f2ba6cfee19 100644 --- a/spec/models/topic_list_spec.rb +++ b/spec/models/topic_list_spec.rb @@ -97,24 +97,9 @@ RSpec.describe TopicList do let(:category) { Fabricate(:category) } let(:tag) { Fabricate(:tag) } - it "generates correct key for categories" do + it "returns topic_list" do topic_list = TopicList.new('latest', nil, nil, category: category, category_id: category.id) - expect(topic_list.preload_key).to eq("topic_list_c/#{category.slug}/#{category.id}/l/latest") - end - - it "generates correct key for 'no subcategories' option" do - topic_list = TopicList.new('latest', nil, nil, category: category, category_id: category.id, no_subcategories: true) - expect(topic_list.preload_key).to eq("topic_list_c/#{category.slug}/#{category.id}/none/l/latest") - end - - it "generates correct key for tag" do - topic_list = TopicList.new('latest', nil, nil, tags: [tag]) - expect(topic_list.preload_key).to eq("topic_list_tag/#{tag.name}/l/latest") - end - - it "generates correct key when both category and tags are missing" do - topic_list = TopicList.new('latest', nil, nil, tags: Tag.none) - expect(topic_list.preload_key).to eq("topic_list_latest") + expect(topic_list.preload_key).to eq("topic_list") end end end diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index c7dd184f741..5b927b7d99e 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -21,7 +21,7 @@ RSpec.describe CategoriesController do expect(response.body).to have_tag("div#data-preloaded") do |element| json = JSON.parse(element.current_scope.attribute('data-preloaded').value) - expect(json['topic_list_latest']).to include(%{"more_topics_url":"/latest"}) + expect(json['topic_list']).to include(%{"more_topics_url":"/latest"}) end end