FEATURE: Make hot topic gists opt-in. (#846)
This change restricts gists to members of specific groups. It also fixes a bug where other lists could display the gist if available.
This commit is contained in:
parent
37c2930fbf
commit
6d504ab80d
|
@ -9,9 +9,7 @@ module DiscourseAi
|
||||||
topic = Topic.find(params[:topic_id])
|
topic = Topic.find(params[:topic_id])
|
||||||
guardian.ensure_can_see!(topic)
|
guardian.ensure_can_see!(topic)
|
||||||
|
|
||||||
if !guardian.can_see_summary?(topic, AiSummary.summary_types[:complete])
|
raise Discourse::NotFound if !guardian.can_see_summary?(topic)
|
||||||
raise Discourse::NotFound
|
|
||||||
end
|
|
||||||
|
|
||||||
RateLimiter.new(current_user, "summary", 6, 5.minutes).performed! if current_user
|
RateLimiter.new(current_user, "summary", 6, 5.minutes).performed! if current_user
|
||||||
|
|
||||||
|
|
|
@ -9,10 +9,7 @@ module Jobs
|
||||||
return unless user = User.find_by(id: args[:user_id])
|
return unless user = User.find_by(id: args[:user_id])
|
||||||
|
|
||||||
strategy = DiscourseAi::Summarization.topic_summary(topic)
|
strategy = DiscourseAi::Summarization.topic_summary(topic)
|
||||||
if strategy.nil? ||
|
return if strategy.nil? || !Guardian.new(user).can_see_summary?(topic)
|
||||||
!Guardian.new(user).can_see_summary?(topic, AiSummary.summary_types[:complete])
|
|
||||||
return
|
|
||||||
end
|
|
||||||
|
|
||||||
guardian = Guardian.new(user)
|
guardian = Guardian.new(user)
|
||||||
return unless guardian.can_see?(topic)
|
return unless guardian.can_see?(topic)
|
||||||
|
|
|
@ -85,6 +85,7 @@ en:
|
||||||
ai_custom_summarization_allowed_groups: "Groups allowed to use create new summaries."
|
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_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_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_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"
|
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"
|
||||||
|
|
|
@ -380,6 +380,10 @@ discourse_ai:
|
||||||
default: 0
|
default: 0
|
||||||
min: 0
|
min: 0
|
||||||
max: 1000
|
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
|
ai_summarization_strategy: # TODO(roman): Deprecated. Remove by Sept 2024
|
||||||
type: enum
|
type: enum
|
||||||
default: ""
|
default: ""
|
||||||
|
|
|
@ -2,7 +2,7 @@
|
||||||
|
|
||||||
module DiscourseAi
|
module DiscourseAi
|
||||||
module GuardianExtensions
|
module GuardianExtensions
|
||||||
def can_see_summary?(target, summary_type)
|
def can_see_summary?(target)
|
||||||
return false if !SiteSetting.ai_summarization_enabled
|
return false if !SiteSetting.ai_summarization_enabled
|
||||||
|
|
||||||
if target.class == Topic && target.private_message?
|
if target.class == Topic && target.private_message?
|
||||||
|
@ -14,12 +14,24 @@ module DiscourseAi
|
||||||
return false if !allowed
|
return false if !allowed
|
||||||
end
|
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?
|
return has_cached_summary if user.nil?
|
||||||
|
|
||||||
has_cached_summary || can_request_summary?
|
has_cached_summary || can_request_summary?
|
||||||
end
|
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?
|
def can_request_summary?
|
||||||
return false if anonymous?
|
return false if anonymous?
|
||||||
|
|
||||||
|
|
|
@ -10,11 +10,11 @@ module DiscourseAi
|
||||||
end
|
end
|
||||||
|
|
||||||
plugin.add_to_serializer(:topic_view, :summarizable) do
|
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
|
end
|
||||||
|
|
||||||
plugin.add_to_serializer(:web_hook_topic_view, :summarizable) do
|
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
|
end
|
||||||
|
|
||||||
plugin.register_modifier(:topic_query_create_list_topics) do |topics, options|
|
plugin.register_modifier(:topic_query_create_list_topics) do |topics, options|
|
||||||
|
@ -32,12 +32,11 @@ module DiscourseAi
|
||||||
plugin.add_to_serializer(
|
plugin.add_to_serializer(
|
||||||
:topic_list_item,
|
:topic_list_item,
|
||||||
:ai_topic_gist,
|
:ai_topic_gist,
|
||||||
include_condition: -> do
|
include_condition: -> { scope.can_see_gists? },
|
||||||
SiteSetting.ai_summarization_enabled &&
|
|
||||||
SiteSetting.ai_summarize_max_hot_topics_gists_per_batch > 0 &&
|
|
||||||
options[:filter] == :hot
|
|
||||||
end,
|
|
||||||
) do
|
) 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 = object.ai_summaries.to_a
|
||||||
|
|
||||||
# Summaries should always have one or zero elements here.
|
# Summaries should always have one or zero elements here.
|
||||||
|
|
|
@ -9,18 +9,20 @@ describe DiscourseAi::GuardianExtensions do
|
||||||
group.add(user)
|
group.add(user)
|
||||||
assign_fake_provider_to(:ai_summarization_model)
|
assign_fake_provider_to(:ai_summarization_model)
|
||||||
SiteSetting.ai_summarization_enabled = true
|
SiteSetting.ai_summarization_enabled = true
|
||||||
|
SiteSetting.ai_summarize_max_hot_topics_gists_per_batch = 1
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "#can_see_summary?" do
|
let(:anon_guardian) { Guardian.new }
|
||||||
let(:guardian) { Guardian.new(user) }
|
let(:guardian) { Guardian.new(user) }
|
||||||
|
|
||||||
|
describe "#can_see_summary?" do
|
||||||
context "when the user cannot generate a summary" do
|
context "when the user cannot generate a summary" do
|
||||||
before { SiteSetting.ai_custom_summarization_allowed_groups = "" }
|
before { SiteSetting.ai_custom_summarization_allowed_groups = "" }
|
||||||
|
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
SiteSetting.ai_custom_summarization_allowed_groups = ""
|
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
|
end
|
||||||
|
|
||||||
it "returns true if there is a cached summary" do
|
it "returns true if there is a cached summary" do
|
||||||
|
@ -32,7 +34,7 @@ describe DiscourseAi::GuardianExtensions do
|
||||||
summary_type: AiSummary.summary_types[:complete],
|
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
|
end
|
||||||
|
|
||||||
|
@ -40,7 +42,7 @@ describe DiscourseAi::GuardianExtensions do
|
||||||
before { SiteSetting.ai_custom_summarization_allowed_groups = group.id }
|
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
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -49,20 +51,18 @@ describe DiscourseAi::GuardianExtensions do
|
||||||
let(:pm) { Fabricate(:private_message_topic) }
|
let(:pm) { Fabricate(:private_message_topic) }
|
||||||
|
|
||||||
it "returns false" do
|
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
|
end
|
||||||
|
|
||||||
it "returns true if user is in a group that is allowed summaries" do
|
it "returns true if user is in a group that is allowed summaries" do
|
||||||
SiteSetting.ai_pm_summarization_allowed_groups = group.id
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when there is no user" do
|
context "when there is no user" do
|
||||||
let(:guardian) { Guardian.new }
|
|
||||||
|
|
||||||
it "returns false for anons" do
|
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
|
end
|
||||||
|
|
||||||
it "returns true for anons when there is a cached summary" do
|
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],
|
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
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -49,7 +49,13 @@ RSpec.describe DiscourseAi::Summarization::EntryPoint do
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when hot topics summarization is enabled" do
|
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
|
it "includes the summary" do
|
||||||
gist_topic = topic_query.list_hot.topics.find { |t| t.id == topic_ai_gist.target_id }
|
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 =
|
serialized =
|
||||||
TopicListItemSerializer.new(
|
TopicListItemSerializer.new(
|
||||||
gist_topic,
|
gist_topic,
|
||||||
scope: Guardian.new,
|
scope: Guardian.new(user),
|
||||||
root: false,
|
root: false,
|
||||||
filter: :hot,
|
filter: :hot,
|
||||||
).as_json
|
).as_json
|
||||||
|
@ -71,13 +77,29 @@ RSpec.describe DiscourseAi::Summarization::EntryPoint do
|
||||||
serialized =
|
serialized =
|
||||||
TopicListItemSerializer.new(
|
TopicListItemSerializer.new(
|
||||||
gist_topic,
|
gist_topic,
|
||||||
scope: Guardian.new,
|
scope: Guardian.new(user),
|
||||||
root: false,
|
root: false,
|
||||||
filter: :latest,
|
filter: :latest,
|
||||||
).as_json
|
).as_json
|
||||||
|
|
||||||
expect(serialized[:ai_topic_gist]).to be_nil
|
expect(serialized[:ai_topic_gist]).to be_nil
|
||||||
end
|
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
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue