diff --git a/app/controllers/discourse_ai/summarization/summary_controller.rb b/app/controllers/discourse_ai/summarization/summary_controller.rb index a7900549..67e7ec82 100644 --- a/app/controllers/discourse_ai/summarization/summary_controller.rb +++ b/app/controllers/discourse_ai/summarization/summary_controller.rb @@ -9,9 +9,7 @@ module DiscourseAi topic = Topic.find(params[:topic_id]) guardian.ensure_can_see!(topic) - if !guardian.can_see_summary?(topic, AiSummary.summary_types[:complete]) - raise Discourse::NotFound - end + raise Discourse::NotFound if !guardian.can_see_summary?(topic) RateLimiter.new(current_user, "summary", 6, 5.minutes).performed! if current_user diff --git a/app/jobs/regular/stream_topic_ai_summary.rb b/app/jobs/regular/stream_topic_ai_summary.rb index d22a7b81..51e4be63 100644 --- a/app/jobs/regular/stream_topic_ai_summary.rb +++ b/app/jobs/regular/stream_topic_ai_summary.rb @@ -9,10 +9,7 @@ module Jobs return unless user = User.find_by(id: args[:user_id]) strategy = DiscourseAi::Summarization.topic_summary(topic) - if strategy.nil? || - !Guardian.new(user).can_see_summary?(topic, AiSummary.summary_types[:complete]) - return - end + return if strategy.nil? || !Guardian.new(user).can_see_summary?(topic) guardian = Guardian.new(user) return unless guardian.can_see?(topic) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ce1e1680..538da773 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -85,6 +85,7 @@ en: ai_custom_summarization_allowed_groups: "Groups allowed to use create new summaries." ai_pm_summarization_allowed_groups: "Groups allowed to create and view summaries in PMs." ai_summarize_max_hot_topics_gists_per_batch: "After updating topics in the hot list, we'll generate brief summaries of the first N ones. (Disabled when 0)" + ai_hot_topic_gists_allowed_groups: "Groups allowed to see gists in the hot topics list." ai_bot_enabled: "Enable the AI Bot module." ai_bot_enable_chat_warning: "Display a warning when PM chat is initiated. Can be overriden by editing the translation string: discourse_ai.ai_bot.pm_warning" diff --git a/config/settings.yml b/config/settings.yml index cba62a7a..07abc071 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -380,6 +380,10 @@ discourse_ai: default: 0 min: 0 max: 1000 + ai_hot_topic_gists_allowed_groups: + type: group_list + list_type: compact + default: "" ai_summarization_strategy: # TODO(roman): Deprecated. Remove by Sept 2024 type: enum default: "" diff --git a/lib/guardian_extensions.rb b/lib/guardian_extensions.rb index 075ae96c..b22ddbb4 100644 --- a/lib/guardian_extensions.rb +++ b/lib/guardian_extensions.rb @@ -2,7 +2,7 @@ module DiscourseAi module GuardianExtensions - def can_see_summary?(target, summary_type) + def can_see_summary?(target) return false if !SiteSetting.ai_summarization_enabled if target.class == Topic && target.private_message? @@ -14,12 +14,24 @@ module DiscourseAi return false if !allowed end - has_cached_summary = AiSummary.exists?(target: target, summary_type: summary_type) + has_cached_summary = + AiSummary.exists?(target: target, summary_type: AiSummary.summary_types[:complete]) return has_cached_summary if user.nil? has_cached_summary || can_request_summary? end + def can_see_gists? + return false if !SiteSetting.ai_summarization_enabled + return false if SiteSetting.ai_summarize_max_hot_topics_gists_per_batch.zero? + return false if anonymous? + return false if SiteSetting.ai_hot_topic_gists_allowed_groups_map.empty? + + SiteSetting.ai_hot_topic_gists_allowed_groups_map.any? do |group_id| + user.group_ids.include?(group_id) + end + end + def can_request_summary? return false if anonymous? diff --git a/lib/summarization/entry_point.rb b/lib/summarization/entry_point.rb index 769b2df4..07fc3ef4 100644 --- a/lib/summarization/entry_point.rb +++ b/lib/summarization/entry_point.rb @@ -10,11 +10,11 @@ module DiscourseAi end plugin.add_to_serializer(:topic_view, :summarizable) do - scope.can_see_summary?(object.topic, AiSummary.summary_types[:complete]) + scope.can_see_summary?(object.topic) end plugin.add_to_serializer(:web_hook_topic_view, :summarizable) do - scope.can_see_summary?(object.topic, AiSummary.summary_types[:complete]) + scope.can_see_summary?(object.topic) end plugin.register_modifier(:topic_query_create_list_topics) do |topics, options| @@ -32,12 +32,11 @@ module DiscourseAi plugin.add_to_serializer( :topic_list_item, :ai_topic_gist, - include_condition: -> do - SiteSetting.ai_summarization_enabled && - SiteSetting.ai_summarize_max_hot_topics_gists_per_batch > 0 && - options[:filter] == :hot - end, + include_condition: -> { scope.can_see_gists? }, ) do + # Options is defined at the instance level so we cannot run this check inside "include_condition". + return if options[:filter] != :hot + summaries = object.ai_summaries.to_a # Summaries should always have one or zero elements here. diff --git a/spec/lib/guardian_extensions_spec.rb b/spec/lib/guardian_extensions_spec.rb index 38268ce1..1e287be4 100644 --- a/spec/lib/guardian_extensions_spec.rb +++ b/spec/lib/guardian_extensions_spec.rb @@ -9,18 +9,20 @@ describe DiscourseAi::GuardianExtensions do group.add(user) assign_fake_provider_to(:ai_summarization_model) SiteSetting.ai_summarization_enabled = true + SiteSetting.ai_summarize_max_hot_topics_gists_per_batch = 1 end - describe "#can_see_summary?" do - let(:guardian) { Guardian.new(user) } + let(:anon_guardian) { Guardian.new } + let(:guardian) { Guardian.new(user) } + describe "#can_see_summary?" do context "when the user cannot generate a summary" do before { SiteSetting.ai_custom_summarization_allowed_groups = "" } it "returns false" do SiteSetting.ai_custom_summarization_allowed_groups = "" - expect(guardian.can_see_summary?(topic, AiSummary.summary_types[:complete])).to eq(false) + expect(guardian.can_see_summary?(topic)).to eq(false) end it "returns true if there is a cached summary" do @@ -32,7 +34,7 @@ describe DiscourseAi::GuardianExtensions do summary_type: AiSummary.summary_types[:complete], ) - expect(guardian.can_see_summary?(topic, AiSummary.summary_types[:complete])).to eq(true) + expect(guardian.can_see_summary?(topic)).to eq(true) end end @@ -40,7 +42,7 @@ describe DiscourseAi::GuardianExtensions do before { SiteSetting.ai_custom_summarization_allowed_groups = group.id } it "returns true if the user group is present in the ai_custom_summarization_allowed_groups_map setting" do - expect(guardian.can_see_summary?(topic, AiSummary.summary_types[:complete])).to eq(true) + expect(guardian.can_see_summary?(topic)).to eq(true) end end @@ -49,20 +51,18 @@ describe DiscourseAi::GuardianExtensions do let(:pm) { Fabricate(:private_message_topic) } it "returns false" do - expect(guardian.can_see_summary?(pm, AiSummary.summary_types[:complete])).to eq(false) + expect(guardian.can_see_summary?(pm)).to eq(false) end it "returns true if user is in a group that is allowed summaries" do SiteSetting.ai_pm_summarization_allowed_groups = group.id - expect(guardian.can_see_summary?(pm, AiSummary.summary_types[:complete])).to eq(true) + expect(guardian.can_see_summary?(pm)).to eq(true) end end context "when there is no user" do - let(:guardian) { Guardian.new } - it "returns false for anons" do - expect(guardian.can_see_summary?(topic, AiSummary.summary_types[:complete])).to eq(false) + expect(anon_guardian.can_see_summary?(topic)).to eq(false) end it "returns true for anons when there is a cached summary" do @@ -74,7 +74,32 @@ describe DiscourseAi::GuardianExtensions do summary_type: AiSummary.summary_types[:complete], ) - expect(guardian.can_see_summary?(topic, AiSummary.summary_types[:complete])).to eq(true) + expect(guardian.can_see_summary?(topic)).to eq(true) + end + end + end + + describe "#can_see_gists?" do + before { SiteSetting.ai_hot_topic_gists_allowed_groups = group.id } + let(:guardian) { Guardian.new(user) } + + context "when there is no user" do + it "returns false for anons" do + expect(anon_guardian.can_see_gists?).to eq(false) + end + end + + context "when there is a user but it's not a member of the allowed groups" do + before { SiteSetting.ai_hot_topic_gists_allowed_groups = "" } + + it "returns false" do + expect(guardian.can_see_gists?).to eq(false) + end + end + + context "when there is a user who is a member of an allowed group" do + it "returns false" do + expect(guardian.can_see_gists?).to eq(true) end end end diff --git a/spec/lib/modules/summarization/entry_point_spec.rb b/spec/lib/modules/summarization/entry_point_spec.rb index b467cb04..f7e1f56f 100644 --- a/spec/lib/modules/summarization/entry_point_spec.rb +++ b/spec/lib/modules/summarization/entry_point_spec.rb @@ -49,7 +49,13 @@ RSpec.describe DiscourseAi::Summarization::EntryPoint do end context "when hot topics summarization is enabled" do - before { SiteSetting.ai_summarize_max_hot_topics_gists_per_batch = 100 } + fab!(:group) + + before do + group.add(user) + SiteSetting.ai_hot_topic_gists_allowed_groups = group.id + SiteSetting.ai_summarize_max_hot_topics_gists_per_batch = 100 + end it "includes the summary" do gist_topic = topic_query.list_hot.topics.find { |t| t.id == topic_ai_gist.target_id } @@ -57,7 +63,7 @@ RSpec.describe DiscourseAi::Summarization::EntryPoint do serialized = TopicListItemSerializer.new( gist_topic, - scope: Guardian.new, + scope: Guardian.new(user), root: false, filter: :hot, ).as_json @@ -71,13 +77,29 @@ RSpec.describe DiscourseAi::Summarization::EntryPoint do serialized = TopicListItemSerializer.new( gist_topic, - scope: Guardian.new, + scope: Guardian.new(user), root: false, filter: :latest, ).as_json expect(serialized[:ai_topic_gist]).to be_nil end + + it "doesn't include the summary when the user is not a member of the opt-in group" do + SiteSetting.ai_hot_topic_gists_allowed_groups = "" + + gist_topic = topic_query.list_hot.topics.find { |t| t.id == topic_ai_gist.target_id } + + serialized = + TopicListItemSerializer.new( + gist_topic, + scope: Guardian.new(user), + root: false, + filter: :hot, + ).as_json + + expect(serialized[:ai_topic_gist]).to be_nil + end end end end