diff --git a/app/lib/before_head_close.rb b/app/lib/before_head_close.rb index b07b9bfc..945284da 100644 --- a/app/lib/before_head_close.rb +++ b/app/lib/before_head_close.rb @@ -40,10 +40,7 @@ def html }, } - if accepted_answer = - Post.find_by( - id: topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD], - ) + if accepted_answer = topic.solution&.post question_json["answerCount"] = 1 question_json[:acceptedAnswer] = { "@type" => "Answer", diff --git a/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb b/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb index f3aa8f85..63a513f9 100644 --- a/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb +++ b/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb @@ -18,12 +18,7 @@ class AssignsReminderForTopicsQuery < PluginInitializer def apply_plugin_api plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query| next query if !SiteSetting.ignore_solved_topics_in_assigned_reminder - query.where.not( - id: - TopicCustomField.where( - name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, - ).pluck(:topic_id), - ) + query.where.not(id: DiscourseSolved::Solution.select(:topic_id)) end end end diff --git a/app/models/discourse-solved/solution.rb b/app/models/discourse-solved/solution.rb new file mode 100644 index 00000000..d4315d49 --- /dev/null +++ b/app/models/discourse-solved/solution.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module ::DiscourseSolved + class Solution < ActiveRecord::Base + belongs_to :accepter, foreign_key: :accepter_user_id, class_name: :user + belongs_to :post, foreign_key: :answer_post_id + belongs_to :topic + belongs_to :topic_timer, dependent: :destroy + end +end diff --git a/db/fixtures/001_badges.rb b/db/fixtures/001_badges.rb index 4bd6abe3..013c901e 100644 --- a/db/fixtures/001_badges.rb +++ b/db/fixtures/001_badges.rb @@ -3,14 +3,13 @@ first_solution_query = <<~SQL SELECT post_id, user_id, created_at AS granted_at FROM ( - SELECT p.id AS post_id, p.user_id, pcf.created_at, - ROW_NUMBER() OVER (PARTITION BY p.user_id ORDER BY pcf.created_at) AS row_number - FROM post_custom_fields pcf - JOIN badge_posts p ON pcf.post_id = p.id - JOIN topics t ON p.topic_id = t.id - WHERE pcf.name = 'is_accepted_answer' - AND p.user_id <> t.user_id -- ignore topics solved by OP - AND (:backfill OR p.id IN (:post_ids)) + SELECT p.id AS post_id, p.user_id, dss.created_at, + ROW_NUMBER() OVER (PARTITION BY p.user_id ORDER BY dss.created_at) AS row_number + FROM discourse_solved_solutions dss + JOIN badge_posts p ON dss.answer_post_id = p.id + JOIN topics t ON p.topic_id = t.id + WHERE p.user_id <> t.user_id -- ignore topics solved by OP + AND (:backfill OR p.id IN (:post_ids)) ) x WHERE row_number = 1 SQL @@ -32,12 +31,11 @@ def solved_query_with_count(min_count) <<~SQL - SELECT p.user_id, MAX(pcf.created_at) AS granted_at - FROM post_custom_fields pcf - JOIN badge_posts p ON pcf.post_id = p.id + SELECT p.user_id, MAX(dss.created_at) AS granted_at + FROM discourse_solved_solutions dss + JOIN badge_posts p ON dss.answer_post_id = p.id JOIN topics t ON p.topic_id = t.id - WHERE pcf.name = 'is_accepted_answer' - AND p.user_id <> t.user_id -- ignore topics solved by OP + WHERE p.user_id <> t.user_id -- ignore topics solved by OP AND (:backfill OR p.id IN (:post_ids)) GROUP BY p.user_id HAVING COUNT(*) >= #{min_count} diff --git a/db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb b/db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb new file mode 100644 index 00000000..b4139a51 --- /dev/null +++ b/db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true +class MoveSolvedTopicCustomFieldToDiscourseSolvedSolutions < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + def up + create_table :discourse_solved_solutions do |t| + t.integer :topic_id, null: false + t.integer :answer_post_id, null: false + t.integer :accepter_user_id + t.integer :topic_timer_id + t.timestamps + end + + execute <<-SQL + INSERT INTO discourse_solved_solutions ( + topic_id, + answer_post_id, + topic_timer_id, + accepter_user_id, + created_at, + updated_at + ) SELECT DISTINCT + tc.topic_id, + CAST(tc.value AS INTEGER), + CAST(tc2.value AS INTEGER), + ua.acting_user_id, + tc.created_at, + tc.updated_at + FROM topic_custom_fields tc + LEFT JOIN topic_custom_fields tc2 + ON tc2.topic_id = tc.topic_id AND tc2.name = 'solved_auto_close_topic_timer_id' + LEFT JOIN user_actions ua + ON ua.target_topic_id = tc.topic_id + WHERE tc.name = 'accepted_answer_post_id' + AND ua.action_type = #{UserAction::SOLVED} + SQL + + add_index :discourse_solved_solutions, :topic_id, unique: true, algorithm: :concurrently + add_index :discourse_solved_solutions, :answer_post_id, unique: true, algorithm: :concurrently + + execute <<-SQL + DELETE FROM post_custom_fields + WHERE name = 'is_accepted_answer' + SQL + + execute <<-SQL + DELETE FROM topic_custom_fields + WHERE name = 'solved_auto_close_topic_timer_id' + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugin.rb b/plugin.rb index dc8f32b3..d4e75a83 100644 --- a/plugin.rb +++ b/plugin.rb @@ -19,10 +19,8 @@ after_initialize do module ::DiscourseSolved PLUGIN_NAME = "discourse-solved" - AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD = "solved_auto_close_topic_timer_id" ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD = "accepted_answer_post_id" ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD = "enable_accepted_answers" - IS_ACCEPTED_ANSWER_CUSTOM_FIELD = "is_accepted_answer" class Engine < ::Rails::Engine engine_name PLUGIN_NAME @@ -44,6 +42,7 @@ class Engine < ::Rails::Engine require_relative "app/lib/user_summary_extension" require_relative "app/lib/web_hook_extension" require_relative "app/serializers/concerns/topic_answer_mixin" + require_relative "app/models/discourse-solved/solution.rb" require_relative "app/lib/plugin_initializers/assigned_reminder_exclude_solved" DiscourseSolved::AssignsReminderForTopicsQuery.new(self).apply_plugin_api @@ -52,18 +51,16 @@ def self.accept_answer!(post, acting_user, topic: nil) topic ||= post.topic DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do - accepted_id = topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].to_i - - if accepted_id > 0 - if p2 = Post.find_by(id: accepted_id) - p2.custom_fields.delete(IS_ACCEPTED_ANSWER_CUSTOM_FIELD) - p2.save! - - UserAction.where(action_type: UserAction::SOLVED, target_post_id: p2.id).destroy_all - end + if topic.solution.present? + UserAction.where( + action_type: UserAction::SOLVED, + target_post_id: topic.solution.answer_post_id, + ).destroy_all + topic.solution.destroy! end - post.custom_fields[IS_ACCEPTED_ANSWER_CUSTOM_FIELD] = "true" + solution = DiscourseSolved::Solution.create(topic:, post:, accepter_user_id: acting_user.id) + topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD] = post.id UserAction.log_action!( @@ -118,13 +115,13 @@ def self.accept_answer!(post, acting_user, topic: nil) duration_minutes: auto_close_hours * 60, ) - topic.custom_fields[AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD] = topic_timer.id + solution.topic_timer_id = topic_timer.id MessageBus.publish("/topic/#{topic.id}", reload_topic: true) end topic.save! - post.save! + solution.save! if WebHook.active_web_hooks(:accepted_solution).exists? payload = WebHook.generate_payload(:post, post) @@ -142,17 +139,10 @@ def self.unaccept_answer!(post, topic: nil) return if topic.nil? DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do - post.custom_fields.delete(IS_ACCEPTED_ANSWER_CUSTOM_FIELD) + topic.solution&.destroy! topic.custom_fields.delete(ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) - if timer_id = topic.custom_fields[AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD] - topic_timer = TopicTimer.find_by(id: timer_id) - topic_timer.destroy! if topic_timer - topic.custom_fields.delete(AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD) - end - topic.save! - post.save! # TODO remove_action! does not allow for this type of interface UserAction.where(action_type: UserAction::SOLVED, target_post_id: post.id).destroy_all @@ -190,6 +180,12 @@ def self.skip_db? ::PostSerializer.prepend(DiscourseSolved::PostSerializerExtension) ::UserSummary.prepend(DiscourseSolved::UserSummaryExtension) ::Topic.attr_accessor(:accepted_answer_user_id) + ::Topic.has_one(:solution, class_name: ::DiscourseSolved::Solution.to_s) + ::Post.has_one( + :solution, + class_name: ::DiscourseSolved::Solution.to_s, + foreign_key: :answer_post_id, + ) ::TopicPostersSummary.alias_method(:old_user_ids, :user_ids) ::TopicPostersSummary.prepend(DiscourseSolved::TopicPostersSummaryExtension) [ @@ -246,19 +242,13 @@ def self.skip_db? Discourse::Application.routes.append { mount ::DiscourseSolved::Engine, at: "solution" } - on(:post_destroyed) do |post| - if post.custom_fields[::DiscourseSolved::IS_ACCEPTED_ANSWER_CUSTOM_FIELD] == "true" - ::DiscourseSolved.unaccept_answer!(post) - end - end + on(:post_destroyed) { |post| ::DiscourseSolved.unaccept_answer!(post) if post.solution } add_api_key_scope( :solved, { answer: { actions: %w[discourse_solved/answer#accept discourse_solved/answer#unaccept] } }, ) - topic_view_post_custom_fields_allowlister { [::DiscourseSolved::IS_ACCEPTED_ANSWER_CUSTOM_FIELD] } - register_html_builder("server:before-head-close-crawler") do |controller| DiscourseSolved::BeforeHeadClose.new(controller).html end @@ -270,8 +260,7 @@ def self.skip_db? Report.add_report("accepted_solutions") do |report| report.data = [] - accepted_solutions = - TopicCustomField.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) + accepted_solutions = DiscourseSolved::Solution category_id, include_subcategories = report.add_category_filter if category_id @@ -288,17 +277,17 @@ def self.skip_db? end accepted_solutions - .where("topic_custom_fields.created_at >= ?", report.start_date) - .where("topic_custom_fields.created_at <= ?", report.end_date) - .group("DATE(topic_custom_fields.created_at)") - .order("DATE(topic_custom_fields.created_at)") + .where("discourse_solved_solutions.created_at >= ?", report.start_date) + .where("discourse_solved_solutions.created_at <= ?", report.end_date) + .group("DATE(discourse_solved_solutions.created_at)") + .order("DATE(discourse_solved_solutions.created_at)") .count .each { |date, count| report.data << { x: date, y: count } } report.total = accepted_solutions.count report.prev30Days = accepted_solutions - .where("topic_custom_fields.created_at >= ?", report.start_date - 30.days) - .where("topic_custom_fields.created_at <= ?", report.start_date) + .where("discourse_solved_solutions.created_at >= ?", report.start_date - 30.days) + .where("discourse_solved_solutions.created_at <= ?", report.start_date) .count end @@ -307,10 +296,8 @@ def self.skip_db? condition = <<~SQL EXISTS ( SELECT 1 - FROM topic_custom_fields + FROM discourse_solved_solutions WHERE topic_id = topics.id - AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' - AND value IS NOT NULL ) SQL @@ -328,7 +315,10 @@ def self.skip_db? scope.can_accept_answer?(topic, object) && accepted_answer end add_to_serializer(:post, :accepted_answer) do - post_custom_fields[::DiscourseSolved::IS_ACCEPTED_ANSWER_CUSTOM_FIELD] == "true" + if topic&.custom_fields&.[](::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD).nil? + return false + end + topic.solution.answer_post_id == object.id end add_to_serializer(:post, :topic_accepted_answer) do topic&.custom_fields&.[](::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD).present? @@ -408,10 +398,8 @@ def self.skip_db? sql = <<~SQL NOT EXISTS ( SELECT 1 - FROM topic_custom_fields + FROM discourse_solved_solutions WHERE topic_id = topics.id - AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' - AND value IS NOT NULL ) SQL diff --git a/spec/fabricators/extend_topic_fabricator.rb b/spec/fabricators/extend_topic_fabricator.rb index c41e4b6e..a26ba843 100644 --- a/spec/fabricators/extend_topic_fabricator.rb +++ b/spec/fabricators/extend_topic_fabricator.rb @@ -3,6 +3,10 @@ transient :custom_topic_name transient :value after_create do |top, transients| + if transients[:custom_topic_name] == DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD + post = Fabricate(:post) + Fabricate(:solution, topic_id: top.id, answer_post_id: post.id) + end custom_topic = TopicCustomField.new( topic_id: top.id, diff --git a/spec/fabricators/solution_fabricator.rb b/spec/fabricators/solution_fabricator.rb new file mode 100644 index 00000000..868dc815 --- /dev/null +++ b/spec/fabricators/solution_fabricator.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +Fabricator(:solution, from: DiscourseSolved::Solution) do + topic_id + answer_post_id +end diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 75e4a900..9906df20 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -248,15 +248,13 @@ post "/solution/accept.json", params: { id: p1.id } expect(response.status).to eq(200) - expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") + expect(p1.reload.solution.present?).to eq(true) topic.reload expect(topic.public_topic_timer.status_type).to eq(TopicTimer.types[:silent_close]) - expect(topic.custom_fields["solved_auto_close_topic_timer_id"].to_i).to eq( - topic.public_topic_timer.id, - ) + expect(topic.solution.topic_timer_id).to eq(topic.public_topic_timer.id) expect(topic.public_topic_timer.execute_at).to eq_time(Time.zone.now + 2.hours) @@ -274,15 +272,13 @@ post "/solution/accept.json", params: { id: post_2.id } expect(response.status).to eq(200) - expect(post_2.reload.custom_fields["is_accepted_answer"]).to eq("true") + expect(post_2.reload.solution.present?).to eq(true) topic_2.reload expect(topic_2.public_topic_timer.status_type).to eq(TopicTimer.types[:silent_close]) - expect(topic_2.custom_fields["solved_auto_close_topic_timer_id"].to_i).to eq( - topic_2.public_topic_timer.id, - ) + expect(topic_2.solution.topic_timer_id).to eq(topic_2.public_topic_timer.id) expect(topic_2.public_topic_timer.execute_at).to eq_time(Time.zone.now + 4.hours) @@ -322,7 +318,7 @@ p1.reload topic.reload - expect(p1.custom_fields["is_accepted_answer"]).to eq("true") + expect(p1.solution.present?).to eq(true) expect(topic.public_topic_timer).to eq(nil) expect(topic.closed).to eq(true) end @@ -338,7 +334,7 @@ expect(response.status).to eq(200) p1.reload - expect(p1.custom_fields["is_accepted_answer"]).to eq("true") + expect(p1.solution.present?).to eq(true) end it "removes the solution when the post is deleted" do @@ -348,13 +344,13 @@ expect(response.status).to eq(200) reply.reload - expect(reply.custom_fields["is_accepted_answer"]).to eq("true") + expect(reply.solution.present?).to eq(true) expect(reply.topic.custom_fields["accepted_answer_post_id"].to_i).to eq(reply.id) PostDestroyer.new(Discourse.system_user, reply).destroy reply.reload - expect(reply.custom_fields["is_accepted_answer"]).to eq(nil) + expect(reply.solution).to eq(nil) expect(reply.topic.custom_fields["accepted_answer_post_id"]).to eq(nil) end @@ -395,7 +391,7 @@ expect(response.status).to eq(200) p1.reload - expect(p1.custom_fields["is_accepted_answer"]).to eq(nil) + expect(p1.solution).to eq(nil) expect(p1.topic.custom_fields["accepted_answer_post_id"]).to eq(nil) end end @@ -457,7 +453,7 @@ expect(p1.topic.assignment.status).to eq("New") DiscourseSolved.accept_answer!(p1, user) - expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") + expect(p1.reload.solution.present?).to eq(true) expect(p1.topic.assignment.reload.status).to eq("Done") end @@ -472,7 +468,7 @@ DiscourseSolved.unaccept_answer!(p1) - expect(p1.reload.custom_fields["is_accepted_answer"]).to eq(nil) + expect(p1.reload.solution).to eq(nil) expect(p1.reload.topic.assignment.reload.status).to eq("New") end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 215b06aa..88408305 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -45,11 +45,11 @@ def schema_json(answerCount) expect(response.body).to include(schema_json(0)) - p2.custom_fields["is_accepted_answer"] = true - p2.save_custom_fields topic.custom_fields["accepted_answer_post_id"] = p2.id topic.save_custom_fields + Fabricate(:solution, topic_id: topic.id, answer_post_id: p2.id) + get "/t/#{topic.slug}/#{topic.id}" expect(response.body).to include(schema_json(1)) @@ -68,10 +68,9 @@ def schema_json(answerCount) it "should include user name in output with the corresponding site setting" do SiteSetting.display_name_on_posts = true - p2.custom_fields["is_accepted_answer"] = true - p2.save_custom_fields topic.custom_fields["accepted_answer_post_id"] = p2.id topic.save_custom_fields + Fabricate(:solution, topic_id: topic.id, answer_post_id: p2.id) get "/t/#{topic.slug}/#{topic.id}.json" @@ -87,10 +86,9 @@ def schema_json(answerCount) it "should not include user name when site setting is disabled" do SiteSetting.display_name_on_posts = false - p2.custom_fields["is_accepted_answer"] = true - p2.save_custom_fields topic.custom_fields["accepted_answer_post_id"] = p2.id topic.save_custom_fields + Fabricate(:solution, topic_id: topic.id, answer_post_id: p2.id) get "/t/#{topic.slug}/#{topic.id}.json" @@ -106,10 +104,9 @@ def schema_json(answerCount) it "includes the correct schema information" do DiscourseTagging.add_or_create_tags_by_name(topic, [tag.name]) - p2.custom_fields["is_accepted_answer"] = true - p2.save_custom_fields topic.custom_fields["accepted_answer_post_id"] = p2.id topic.save_custom_fields + Fabricate(:solution, topic_id: topic.id, answer_post_id: p2.id) get "/t/#{topic.slug}/#{topic.id}"