From 8318c4374c0ad95fe7f71275d2455566a7e467c5 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Tue, 8 Aug 2023 15:44:10 -0300 Subject: [PATCH] FIX: Remove muted from Similar list (#127) * FIX: Remove muted from Similar list --- lib/modules/embeddings/entry_point.rb | 25 +--- .../embeddings/semantic_topic_query.rb | 28 +++++ .../modules/embeddings/entry_point_spec.rb | 4 +- .../embeddings/semantic_topic_query_spec.rb | 114 ++++++++++++++++++ 4 files changed, 147 insertions(+), 24 deletions(-) create mode 100644 lib/modules/embeddings/semantic_topic_query.rb create mode 100644 spec/lib/modules/embeddings/semantic_topic_query_spec.rb diff --git a/lib/modules/embeddings/entry_point.rb b/lib/modules/embeddings/entry_point.rb index bc97704a..bcbc7979 100644 --- a/lib/modules/embeddings/entry_point.rb +++ b/lib/modules/embeddings/entry_point.rb @@ -13,6 +13,7 @@ module DiscourseAi require_relative "jobs/regular/generate_embeddings" require_relative "semantic_related" require_relative "semantic_search" + require_relative "semantic_topic_query" end def inject_into(plugin) @@ -31,28 +32,8 @@ module DiscourseAi return nil end - @related_topics ||= TopicQuery.new(@user).list_semantic_related_topics(topic).topics - end - - plugin.add_to_class(:topic_query, :list_semantic_related_topics) do |topic| - query_opts = { - skip_ordering: true, - per_page: SiteSetting.ai_embeddings_semantic_related_topics, - unordered: true, - } - - if !SiteSetting.ai_embeddings_semantic_related_include_closed_topics - query_opts[:status] = "open" - end - - create_list(:semantic_related, query_opts) do |topics| - candidate_ids = DiscourseAi::Embeddings::SemanticRelated.related_topic_ids_for(topic) - - topics - .where.not(id: topic.id) - .where(id: candidate_ids) - .order("array_position(ARRAY#{candidate_ids}, topics.id)") # array_position forces the order of the topics to be preserved - end + @related_topics ||= + SemanticTopicQuery.new(@user).list_semantic_related_topics(topic).topics end %i[topic_view TopicViewPosts].each do |serializer| diff --git a/lib/modules/embeddings/semantic_topic_query.rb b/lib/modules/embeddings/semantic_topic_query.rb new file mode 100644 index 00000000..c1034baf --- /dev/null +++ b/lib/modules/embeddings/semantic_topic_query.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class DiscourseAi::Embeddings::SemanticTopicQuery < TopicQuery + def list_semantic_related_topics(topic) + query_opts = { + skip_ordering: true, + per_page: SiteSetting.ai_embeddings_semantic_related_topics, + unordered: true, + } + + if !SiteSetting.ai_embeddings_semantic_related_include_closed_topics + query_opts[:status] = "open" + end + + list = + create_list(:semantic_related, query_opts) do |topics| + candidate_ids = DiscourseAi::Embeddings::SemanticRelated.related_topic_ids_for(topic) + + list = + topics + .where.not(id: topic.id) + .where(id: candidate_ids) + .order("array_position(ARRAY#{candidate_ids}, topics.id)") # array_position forces the order of the topics to be preserved + + list = remove_muted(list, @user, query_opts) + end + end +end diff --git a/spec/lib/modules/embeddings/entry_point_spec.rb b/spec/lib/modules/embeddings/entry_point_spec.rb index ddc7c4bb..72545005 100644 --- a/spec/lib/modules/embeddings/entry_point_spec.rb +++ b/spec/lib/modules/embeddings/entry_point_spec.rb @@ -29,9 +29,9 @@ describe DiscourseAi::Embeddings::EntryPoint do end end - describe "TopicQuery extensions" do + describe "SemanticTopicQuery extension" do describe "#list_semantic_related_topics" do - subject(:topic_query) { TopicQuery.new(user) } + subject(:topic_query) { DiscourseAi::Embeddings::SemanticTopicQuery.new(user) } fab!(:target) { Fabricate(:topic) } diff --git a/spec/lib/modules/embeddings/semantic_topic_query_spec.rb b/spec/lib/modules/embeddings/semantic_topic_query_spec.rb new file mode 100644 index 00000000..cfc4bc20 --- /dev/null +++ b/spec/lib/modules/embeddings/semantic_topic_query_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe DiscourseAi::Embeddings::EntryPoint do + fab!(:user) { Fabricate(:user) } + + describe "SemanticTopicQuery extension" do + describe "#list_semantic_related_topics" do + subject(:topic_query) { DiscourseAi::Embeddings::SemanticTopicQuery.new(user) } + + fab!(:target) { Fabricate(:topic) } + + def stub_semantic_search_with(results) + DiscourseAi::Embeddings::SemanticRelated.expects(:related_topic_ids_for).returns(results) + end + + context "when the semantic search returns an unlisted topic" do + fab!(:unlisted_topic) { Fabricate(:topic, visible: false) } + + before { stub_semantic_search_with([unlisted_topic.id]) } + + it "filters it out" do + expect(topic_query.list_semantic_related_topics(target).topics).to be_empty + end + end + + context "when the semantic search returns a private topic" do + fab!(:private_topic) { Fabricate(:private_message_topic) } + + before { stub_semantic_search_with([private_topic.id]) } + + it "filters it out" do + expect(topic_query.list_semantic_related_topics(target).topics).to be_empty + end + end + + context "when the semantic search returns a topic from a restricted category" do + fab!(:group) { Fabricate(:group) } + fab!(:category) { Fabricate(:private_category, group: group) } + fab!(:secured_category_topic) { Fabricate(:topic, category: category) } + + before { stub_semantic_search_with([secured_category_topic.id]) } + + it "filters it out" do + expect(topic_query.list_semantic_related_topics(target).topics).to be_empty + end + + it "doesn't filter it out if the user has access to the category" do + group.add(user) + + expect(topic_query.list_semantic_related_topics(target).topics).to contain_exactly( + secured_category_topic, + ) + end + end + + context "when the semantic search returns a closed topic and we explicitly exclude them" do + fab!(:closed_topic) { Fabricate(:topic, closed: true) } + + before do + SiteSetting.ai_embeddings_semantic_related_include_closed_topics = false + stub_semantic_search_with([closed_topic.id]) + end + + it "filters it out" do + expect(topic_query.list_semantic_related_topics(target).topics).to be_empty + end + end + + context "when the semantic search returns a muted topic" do + it "filters it out" do + category = Fabricate(:category_with_definition) + topic = Fabricate(:topic, category: category) + CategoryUser.create!( + user_id: user.id, + category_id: category.id, + notification_level: CategoryUser.notification_levels[:muted], + ) + stub_semantic_search_with([topic.id]) + expect(topic_query.list_semantic_related_topics(target).topics).not_to include(topic) + end + end + + context "when the semantic search returns public topics" do + fab!(:normal_topic_1) { Fabricate(:topic) } + fab!(:normal_topic_2) { Fabricate(:topic) } + fab!(:normal_topic_3) { Fabricate(:topic) } + fab!(:closed_topic) { Fabricate(:topic, closed: true) } + + before do + stub_semantic_search_with( + [closed_topic.id, normal_topic_1.id, normal_topic_2.id, normal_topic_3.id], + ) + end + + it "filters it out" do + expect(topic_query.list_semantic_related_topics(target).topics).to eq( + [closed_topic, normal_topic_1, normal_topic_2, normal_topic_3], + ) + end + + it "returns the plugin limit for the number of results" do + SiteSetting.ai_embeddings_semantic_related_topics = 2 + + expect(topic_query.list_semantic_related_topics(target).topics).to contain_exactly( + closed_topic, + normal_topic_1, + ) + end + end + end + end +end