diff --git a/app/serializers/topic_summary_serializer.rb b/app/serializers/topic_summary_serializer.rb index e2980d88d00..ec57d9a723b 100644 --- a/app/serializers/topic_summary_serializer.rb +++ b/app/serializers/topic_summary_serializer.rb @@ -8,6 +8,11 @@ class TopicSummarySerializer < ApplicationSerializer end def new_posts_since_summary - object.target.highest_post_number.to_i - object.content_range&.end.to_i + # Postgres uses discrete range types for int4range, which means + # an inclusive [1,2] ranges is stored as [1,3). To work around this + # an provide an accurate count, we do the following: + range_end = [object.content_range&.end.to_i - 1, 0].max + + object.target.highest_post_number.to_i - range_end end end diff --git a/app/services/topic_summarization.rb b/app/services/topic_summarization.rb index e0c9ee87474..3f271116f65 100644 --- a/app/services/topic_summarization.rb +++ b/app/services/topic_summarization.rb @@ -116,7 +116,10 @@ class TopicSummarization ) end - main_summary + # Calling reload here ensures Postgres' discrete range type is applied. + # an inclusive [1,2] ranges is stored as [1,3). + # Read more about this here: https://www.postgresql.org/docs/current/rangetypes.html#RANGETYPES-DISCRETE + main_summary.reload end def build_sha(ids) diff --git a/plugins/chat/spec/requests/chat/api/summaries_controller_spec.rb b/plugins/chat/spec/requests/chat/api/summaries_controller_spec.rb index 19f9f7d1df7..8835bb5ed9a 100644 --- a/plugins/chat/spec/requests/chat/api/summaries_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/summaries_controller_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Chat::Api::SummariesController do before do group.add(current_user) - strategy = DummyCustomSummarization.new("dummy") + strategy = DummyCustomSummarization.new({ summary: "dummy", chunks: [] }) plugin.register_summarization_strategy(strategy) SiteSetting.summarization_strategy = strategy.model SiteSetting.custom_summarization_allowed_groups = group.id @@ -18,6 +18,8 @@ RSpec.describe Chat::Api::SummariesController do sign_in(current_user) end + after { DiscoursePluginRegistry.reset_register!(:summarization_strategies) } + describe "#get_summary" do context "when the user is not allowed to join the channel" do fab!(:channel) { Fabricate(:private_category_channel) } diff --git a/spec/jobs/regular/stream_topic_summary_spec.rb b/spec/jobs/regular/stream_topic_summary_spec.rb index d92ad1081e6..d9433cf0797 100644 --- a/spec/jobs/regular/stream_topic_summary_spec.rb +++ b/spec/jobs/regular/stream_topic_summary_spec.rb @@ -4,7 +4,9 @@ RSpec.describe Jobs::StreamTopicSummary do subject(:job) { described_class.new } describe "#execute" do - fab!(:topic) { Fabricate(:topic) } + fab!(:topic) { Fabricate(:topic, highest_post_number: 2) } + fab!(:post_1) { Fabricate(:post, topic: topic, post_number: 1) } + fab!(:post_2) { Fabricate(:post, topic: topic, post_number: 2) } let(:plugin) { Plugin::Instance.new } let(:strategy) { DummyCustomSummarization.new({ summary: "dummy", chunks: [] }) } fab!(:user) { Fabricate(:leader) } @@ -16,6 +18,8 @@ RSpec.describe Jobs::StreamTopicSummary do SiteSetting.summarization_strategy = strategy.model end + after { DiscoursePluginRegistry.reset_register!(:summarization_strategies) } + describe "validates params" do it "does nothing if there is no topic" do messages = diff --git a/spec/lib/summarization/base_spec.rb b/spec/lib/summarization/base_spec.rb index 34021e07445..e4773bd2584 100644 --- a/spec/lib/summarization/base_spec.rb +++ b/spec/lib/summarization/base_spec.rb @@ -10,11 +10,13 @@ describe Summarization::Base do before do group.add(user) - strategy = DummyCustomSummarization.new("dummy") + strategy = DummyCustomSummarization.new({ summary: "dummy", chunks: [] }) plugin.register_summarization_strategy(strategy) SiteSetting.summarization_strategy = strategy.model end + after { DiscoursePluginRegistry.reset_register!(:summarization_strategies) } + describe "#can_see_summary?" do context "when the user cannot generate a summary" do before { SiteSetting.custom_summarization_allowed_groups = "" } diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 04e85e916bd..065d1dffab9 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -5506,7 +5506,9 @@ RSpec.describe TopicsController do end describe "#summary" do - fab!(:topic) { Fabricate(:topic) } + fab!(:topic) { Fabricate(:topic, highest_post_number: 2) } + fab!(:post_1) { Fabricate(:post, topic: topic, post_number: 1) } + fab!(:post_2) { Fabricate(:post, topic: topic, post_number: 2) } let(:plugin) { Plugin::Instance.new } let(:strategy) { DummyCustomSummarization.new({ summary: "dummy", chunks: [] }) } @@ -5515,6 +5517,8 @@ RSpec.describe TopicsController do SiteSetting.summarization_strategy = strategy.model end + after { DiscoursePluginRegistry.reset_register!(:summarization_strategies) } + context "for anons" do it "returns a 404 if there is no cached summary" do get "/t/#{topic.id}/strategy-summary.json" @@ -5577,6 +5581,20 @@ RSpec.describe TopicsController do expect(summary["can_regenerate"]).to eq(true) expect(summary["new_posts_since_summary"]).to be_zero end + + it "signals the summary is outdated" do + get "/t/#{topic.id}/strategy-summary.json" + + Fabricate(:post, topic: topic, post_number: 3) + topic.update!(highest_post_number: 3) + + get "/t/#{topic.id}/strategy-summary.json" + expect(response.status).to eq(200) + summary = response.parsed_body["topic_summary"] + + expect(summary["outdated"]).to eq(true) + expect(summary["new_posts_since_summary"]).to eq(1) + end end context "when the user is not a member of an allowlisted group" do diff --git a/spec/services/topic_summarization_spec.rb b/spec/services/topic_summarization_spec.rb index 16339bc0738..3446013f1e9 100644 --- a/spec/services/topic_summarization_spec.rb +++ b/spec/services/topic_summarization_spec.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true describe TopicSummarization do - fab!(:topic) { Fabricate(:topic) } fab!(:user) { Fabricate(:admin) } - fab!(:post_1) { Fabricate(:post, topic: topic) } - fab!(:post_2) { Fabricate(:post, topic: topic) } + fab!(:topic) { Fabricate(:topic, highest_post_number: 2) } + fab!(:post_1) { Fabricate(:post, topic: topic, post_number: 1) } + fab!(:post_2) { Fabricate(:post, topic: topic, post_number: 2) } shared_examples "includes only public-visible topics" do subject { described_class.new(DummyCustomSummarization.new({})) }