diff --git a/app/lib/guardian_extensions.rb b/app/lib/guardian_extensions.rb index 1a0ff7c..6ce47a3 100644 --- a/app/lib/guardian_extensions.rb +++ b/app/lib/guardian_extensions.rb @@ -21,7 +21,7 @@ module DiscourseSolved def can_accept_answer?(topic, post) return false if !authenticated? - return false if !topic || !post || post.whisper? + return false if !topic || topic.private_message? || !post || post.whisper? return false if !allow_accepted_answers?(topic.category_id, topic.tags.map(&:name)) return true if is_staff? diff --git a/plugin.rb b/plugin.rb index 20e8334..404dc90 100644 --- a/plugin.rb +++ b/plugin.rb @@ -272,19 +272,20 @@ after_initialize do report.data = [] accepted_solutions = - TopicCustomField.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) + TopicCustomField + .joins(:topic) + .where("topics.archetype <> ?", Archetype.private_message) + .where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) category_id, include_subcategories = report.add_category_filter if category_id if include_subcategories accepted_solutions = - accepted_solutions.joins(:topic).where( - "topics.category_id IN (?)", - Category.subcategory_ids(category_id), - ) + accepted_solutions + .where("topics.category_id IN (?)", Category.subcategory_ids(category_id)) else accepted_solutions = - accepted_solutions.joins(:topic).where("topics.category_id = ?", category_id) + accepted_solutions.where("topics.category_id = ?", category_id) end end @@ -345,7 +346,7 @@ after_initialize do ) SQL - scope.where(sql) + scope.where(sql).where("topics.archetype <> ?", Archetype.private_message) end unsolved_callback = ->(scope) do @@ -379,7 +380,7 @@ after_initialize do SQL end - scope + scope.where("topics.archetype <> ?", Archetype.private_message) end register_custom_filter_by_status("solved", &solved_callback) @@ -398,6 +399,12 @@ after_initialize do end end + register_modifier(:user_action_stream_builder) do |builder| + builder + .where("t.deleted_at IS NULL") + .where("t.archetype <> ?", Archetype.private_message) + end + TopicList.preloaded_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD Site.preloaded_category_custom_fields << ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD Search.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD @@ -477,26 +484,36 @@ after_initialize do end end - query = - " - WITH x AS (SELECT - u.id user_id, - COUNT(DISTINCT ua.id) AS solutions + query = <<~SQL + WITH x AS ( + SELECT u.id user_id, COUNT(DISTINCT ua.id) AS solutions FROM users AS u - LEFT OUTER JOIN user_actions AS ua ON ua.user_id = u.id AND ua.action_type = #{UserAction::SOLVED} AND COALESCE(ua.created_at, :since) > :since - WHERE u.active + LEFT JOIN user_actions AS ua + ON ua.user_id = u.id + AND ua.action_type = #{UserAction::SOLVED} + AND COALESCE(ua.created_at, :since) > :since + JOIN topics AS t + ON t.id = ua.target_topic_id + AND t.archetype <> 'private_message' + AND t.deleted_at IS NULL + JOIN posts AS p + ON p.id = ua.target_post_id + AND p.deleted_at IS NULL + WHERE u.id > 0 + AND u.active AND u.silenced_till IS NULL - AND u.id > 0 + AND u.suspended_till IS NULL GROUP BY u.id ) - UPDATE directory_items di SET - solutions = x.solutions + UPDATE directory_items di + SET solutions = x.solutions FROM x WHERE x.user_id = di.user_id - AND di.period_type = :period_type - AND di.solutions <> x.solutions - " - add_directory_column("solutions", query: query) + AND di.period_type = :period_type + AND di.solutions <> x.solutions + SQL + + add_directory_column("solutions", query:) add_to_class(:composer_messages_finder, :check_topic_is_solved) do return if !SiteSetting.solved_enabled || SiteSetting.disable_solved_education_message @@ -514,7 +531,13 @@ after_initialize do end add_to_serializer(:user_card, :accepted_answers) do - UserAction.where(user_id: object.id).where(action_type: UserAction::SOLVED).count + UserAction + .where(user_id: object.id) + .where(action_type: UserAction::SOLVED) + .joins("JOIN topics ON topics.id = user_actions.target_topic_id") + .where("topics.archetype <> ?", Archetype.private_message) + .where("topics.deleted_at IS NULL") + .count end register_topic_list_preload_user_ids do |topics, user_ids, topic_list| diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index e4120e0..def3beb 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -47,6 +47,16 @@ RSpec.describe "Managing Posts solved status" do ) end + fab!(:solved_pm) do + Fabricate( + :custom_topic, + category_id: nil, + archetype: Archetype.private_message, + custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + value: "42", + ) + end + fab!(:unsolved_in_category) { Fabricate(:topic, category: solvable_category) } fab!(:unsolved_in_tag) { Fabricate(:topic, tags: [solvable_tag]) } @@ -572,4 +582,33 @@ RSpec.describe "Managing Posts solved status" do expect { DiscourseSolved.unaccept_answer!(reply) }.not_to raise_error end end + + describe "user actions stream modifier" do + it "correctly list solutions" do + t1 = Fabricate(:topic) + t2 = Fabricate(:topic) + t3 = Fabricate(:topic) + + p1 = Fabricate(:post, topic: t1, user:) + p2 = Fabricate(:post, topic: t2, user:) + p3 = Fabricate(:post, topic: t3, user:) + + DiscourseSolved.accept_answer!(p1, Discourse.system_user) + DiscourseSolved.accept_answer!(p2, Discourse.system_user) + DiscourseSolved.accept_answer!(p3, Discourse.system_user) + + t1.trash!(Discourse.system_user) + t2.convert_to_private_message(Discourse.system_user) + + expect( + UserAction + .stream( + user_id: user.id, + action_types: [::UserAction::SOLVED], + guardian: user.guardian + ) + .map(&:post_id) + ).to contain_exactly p3.id + end + end end diff --git a/spec/lib/guardian_extensions_spec.rb b/spec/lib/guardian_extensions_spec.rb index 2a12c3b..a3354b5 100644 --- a/spec/lib/guardian_extensions_spec.rb +++ b/spec/lib/guardian_extensions_spec.rb @@ -25,6 +25,11 @@ describe DiscourseSolved::GuardianExtensions do expect(guardian.can_accept_answer?(topic, post)).to eq(false) end + it "returns false for private messages" do + topic.update!(user:, category_id: nil, archetype: Archetype.private_message) + expect(guardian.can_accept_answer?(topic, post)).to eq(false) + end + it "returns false if accepted answers are not allowed" do SiteSetting.allow_solved_on_all_topics = false expect(guardian.can_accept_answer?(topic, post)).to eq(false) diff --git a/spec/serializers/user_card_serializer_spec.rb b/spec/serializers/user_card_serializer_spec.rb index d0a9ce4..5c93b44 100644 --- a/spec/serializers/user_card_serializer_spec.rb +++ b/spec/serializers/user_card_serializer_spec.rb @@ -8,6 +8,8 @@ describe UserCardSerializer do let(:json) { serializer.as_json } it "accepted_answers serializes number of accepted answers" do + expect(serializer.as_json[:accepted_answers]).to eq(0) + post1 = Fabricate(:post, user: user) DiscourseSolved.accept_answer!(post1, Discourse.system_user) expect(serializer.as_json[:accepted_answers]).to eq(1) @@ -16,7 +18,17 @@ describe UserCardSerializer do DiscourseSolved.accept_answer!(post2, Discourse.system_user) expect(serializer.as_json[:accepted_answers]).to eq(2) + post3 = Fabricate(:post, user: user) + DiscourseSolved.accept_answer!(post3, Discourse.system_user) + expect(serializer.as_json[:accepted_answers]).to eq(3) + DiscourseSolved.unaccept_answer!(post1) + expect(serializer.as_json[:accepted_answers]).to eq(2) + + post2.topic.trash!(Discourse.system_user) expect(serializer.as_json[:accepted_answers]).to eq(1) + + post3.topic.convert_to_private_message(Discourse.system_user) + expect(serializer.as_json[:accepted_answers]).to eq(0) end end