From cb0958fceae58544ba42440f01b5691ddc5808a9 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Fri, 29 Oct 2021 17:52:23 +0300 Subject: [PATCH] FIX: Hide links to muted topics and in categories list (#14761) * FIX: Hide links to muted topics * FIX: Hide muted topics in categories list on mobile --- app/models/category_list.rb | 7 +++++++ app/models/topic_link.rb | 6 ++++++ spec/models/category_list_spec.rb | 13 +++++++++++++ spec/models/topic_link_spec.rb | 16 ++++++++++++++++ 4 files changed, 42 insertions(+) diff --git a/app/models/category_list.rb b/app/models/category_list.rb index 655edfa194e..e092dabf42d 100644 --- a/app/models/category_list.rb +++ b/app/models/category_list.rb @@ -63,6 +63,13 @@ class CategoryList category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank) @all_topics = Topic.where(id: category_featured_topics.map(&:topic_id)) + + if @guardian.authenticated? + @all_topics = @all_topics + .joins("LEFT JOIN topic_users tu ON topics.id = tu.topic_id AND tu.user_id = #{@guardian.user.id.to_i}") + .where('COALESCE(tu.notification_level,1) > :muted', muted: TopicUser.notification_levels[:muted]) + end + @all_topics = @all_topics.includes(:last_poster) if @options[:include_topics] @all_topics.each do |t| # hint for the serializer diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 6758689c712..977de9c2ac8 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -85,12 +85,18 @@ class TopicLink < ActiveRecord::Base FROM topic_links l LEFT JOIN topics t ON t.id = l.link_topic_id LEFT JOIN categories AS c ON c.id = t.category_id + /*left_join*/ /*where*/ ORDER BY reflection ASC, clicks DESC") builder.where('t.deleted_at IS NULL') builder.where("COALESCE(t.archetype, 'regular') <> :archetype", archetype: Archetype.private_message) + if guardian.authenticated? + builder.left_join("topic_users AS tu ON (t.id = tu.topic_id AND tu.user_id = #{guardian.user.id.to_i})") + builder.where('COALESCE(tu.notification_level,1) > :muted', muted: TopicUser.notification_levels[:muted]) + end + # not certain if pluck is right, cause it may interfere with caching builder.where('l.post_id in (:post_ids)', post_ids: posts.map(&:id)) builder.secure_category(guardian.secure_category_ids) diff --git a/spec/models/category_list_spec.rb b/spec/models/category_list_spec.rb index 81a0fe33101..5e8b8adcd97 100644 --- a/spec/models/category_list_spec.rb +++ b/spec/models/category_list_spec.rb @@ -53,6 +53,19 @@ describe CategoryList do expect(CategoryList.new(Guardian.new(nil), include_topics: true).categories.find { |x| x.name == private_cat.name }).to eq(nil) end + it "doesn't show muted topics" do + cat = Fabricate(:category_with_definition) # public category + topic = Fabricate(:topic, category: cat) + + CategoryFeaturedTopic.feature_topics + + expect(CategoryList.new(Guardian.new(user), include_topics: true).categories.find { |x| x.name == cat.name }.displayable_topics.count).to eq(1) + + TopicUser.change(user.id, topic.id, notification_level: TopicUser.notification_levels[:muted]) + + expect(CategoryList.new(Guardian.new(user), include_topics: true).categories.find { |x| x.name == cat.name }.displayable_topics.count).to eq(0) + end + end context "when mute_all_categories_by_default enabled" do diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index 099aa5a587a..2b2ee26f7d8 100644 --- a/spec/models/topic_link_spec.rb +++ b/spec/models/topic_link_spec.rb @@ -472,6 +472,22 @@ describe TopicLink do expect(TopicLink.topic_map(Guardian.new, post.topic_id).count).to eq(0) end + + it 'secures internal links correctly' do + other_topic = Fabricate(:topic) + other_user = Fabricate(:user) + + url = "http://#{test_uri.host}/t/topic-slug/#{other_topic.id}" + post = Fabricate(:post, raw: "hello test topic #{url}") + TopicLink.extract_from(post) + TopicLinkClick.create!(topic_link: post.topic.topic_links.first, ip_address: '192.168.1.1') + + expect(TopicLink.counts_for(Guardian.new(other_user), post.topic, [post]).length).to eq(1) + + TopicUser.change(other_user.id, other_topic.id, notification_level: TopicUser.notification_levels[:muted]) + + expect(TopicLink.counts_for(Guardian.new(other_user), post.topic, [post]).length).to eq(0) + end end describe ".duplicate_lookup" do