From 150f7f4a352f8820f759fa3cab3cb8d1c33a395d Mon Sep 17 00:00:00 2001 From: Lhc_fl Date: Thu, 5 Sep 2024 17:55:02 +0800 Subject: [PATCH 01/10] DEV: Move the post and topic custom fields into a table This PR does some preparatory work to prepare for the front-end display of who marked an answer as solved. It migrates the custom fields from discourse-solved of Topic and Post to a new table, except "accepted_answer_post_id" ref: /t/-/95318 --- app/lib/before_head_close.rb | 5 +- .../assigned_reminder_exclude_solved.rb | 7 +- app/models/discourse-solved/solution.rb | 10 +++ ...tom_field_to_discourse_solved_solutions.rb | 52 +++++++++++++ plugin.rb | 78 ++++++++----------- spec/fabricators/extend_topic_fabricator.rb | 4 + spec/fabricators/solution_fabricator.rb | 6 ++ spec/integration/solved_spec.rb | 26 +++---- spec/requests/topics_controller_spec.rb | 13 ++-- 9 files changed, 124 insertions(+), 77 deletions(-) create mode 100644 app/models/discourse-solved/solution.rb create mode 100644 db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb create mode 100644 spec/fabricators/solution_fabricator.rb 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..611a3763 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.pluck(: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/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..04ba0448 --- /dev/null +++ b/db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true +class MoveSolvedTopicCustomFieldToDiscourseSolvedSolutions < ActiveRecord::Migration[7.1] + 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, null: true + t.integer :topic_timer_id, null: true + t.timestamps + end + + add_index :discourse_solved_solutions, :topic_id, unique: true + add_index :discourse_solved_solutions, :answer_post_id, unique: true + + 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 + + # execute <<-SQL + # DELETE FROM post_custom_fields + # WHERE name = 'is_accepted_answer' + # SQL + end + + def down + # raise ActiveRecord::IrreversibleMigration + remove_index :discourse_solved_solutions, :topic_id + remove_index :discourse_solved_solutions, :answer_post_id + + drop_table :discourse_solved_solutions + end +end diff --git a/plugin.rb b/plugin.rb index dc8f32b3..a7e435c8 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,18 @@ 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 + old_solution = topic.solution + + if old_solution.present? + UserAction.where( + action_type: UserAction::SOLVED, + target_post_id: old_solution.answer_post_id, + ).destroy_all + old_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 +117,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 +141,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 +182,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 +244,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 +262,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 +279,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 +298,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 +317,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 +400,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..ca38a27d 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}" From 56f83272347147e7866e75f3ce110dc6d584c992 Mon Sep 17 00:00:00 2001 From: Lhc_fl Date: Thu, 5 Sep 2024 19:35:03 +0800 Subject: [PATCH 02/10] remove unused post/topic custom fields --- ...tom_field_to_discourse_solved_solutions.rb | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) 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 index 04ba0448..59829c99 100644 --- 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 @@ -36,14 +36,51 @@ def up AND ua.action_type = #{UserAction::SOLVED} SQL - # execute <<-SQL - # DELETE FROM post_custom_fields - # WHERE name = 'is_accepted_answer' - # SQL + 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 + execute <<-SQL + INSERT INTO topic_custom_fields ( + topic_id, + name, + value, + created_at, + updated_at + ) SELECT DISTINCT + topic_id, + 'solved_auto_close_topic_timer_id', + CAST(topic_timer_id AS TEXT), + created_at, + updated_at + FROM discourse_solved_solutions tc + WHERE tc.topic_timer_id IS NOT NULL + SQL + + execute <<-SQL + INSERT INTO post_custom_fields ( + post_id, + name, + value, + created_at, + updated_at + ) SELECT DISTINCT + answer_post_id, + 'is_accepted_answer', + 'true', + created_at, + updated_at + FROM discourse_solved_solutions + SQL + remove_index :discourse_solved_solutions, :topic_id remove_index :discourse_solved_solutions, :answer_post_id From 29f744e4101a27176795d65b143aad945c9fb0f3 Mon Sep 17 00:00:00 2001 From: Lhc_fl Date: Thu, 5 Sep 2024 20:16:14 +0800 Subject: [PATCH 03/10] fix badges SQL --- db/fixtures/001_badges.rb | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) 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} From c00ca89b202b3916761b1994648e4fbbc744dd1e Mon Sep 17 00:00:00 2001 From: Lhc_fl Date: Fri, 6 Sep 2024 19:22:58 +0800 Subject: [PATCH 04/10] remove down sql --- ...tom_field_to_discourse_solved_solutions.rb | 38 +------------------ 1 file changed, 1 insertion(+), 37 deletions(-) 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 index 59829c99..43261590 100644 --- 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 @@ -48,42 +48,6 @@ def up end def down - execute <<-SQL - INSERT INTO topic_custom_fields ( - topic_id, - name, - value, - created_at, - updated_at - ) SELECT DISTINCT - topic_id, - 'solved_auto_close_topic_timer_id', - CAST(topic_timer_id AS TEXT), - created_at, - updated_at - FROM discourse_solved_solutions tc - WHERE tc.topic_timer_id IS NOT NULL - SQL - - execute <<-SQL - INSERT INTO post_custom_fields ( - post_id, - name, - value, - created_at, - updated_at - ) SELECT DISTINCT - answer_post_id, - 'is_accepted_answer', - 'true', - created_at, - updated_at - FROM discourse_solved_solutions - SQL - - remove_index :discourse_solved_solutions, :topic_id - remove_index :discourse_solved_solutions, :answer_post_id - - drop_table :discourse_solved_solutions + raise ActiveRecord::IrreversibleMigration end end From ba796897ad8b70dbebe88b856e666942c3fda99e Mon Sep 17 00:00:00 2001 From: Lhc_fl Date: Fri, 6 Sep 2024 20:09:12 +0800 Subject: [PATCH 05/10] review --- ...ed_topic_custom_field_to_discourse_solved_solutions.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 index 43261590..3e91f76d 100644 --- 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 @@ -1,5 +1,7 @@ # 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 @@ -9,9 +11,6 @@ def up t.timestamps end - add_index :discourse_solved_solutions, :topic_id, unique: true - add_index :discourse_solved_solutions, :answer_post_id, unique: true - execute <<-SQL INSERT INTO discourse_solved_solutions ( topic_id, @@ -36,6 +35,9 @@ def up 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' From 2c0aff5a21a91cfc79a9618f22f9ef760bf861f6 Mon Sep 17 00:00:00 2001 From: Linca <41134017+Lhcfl@users.noreply.github.com> Date: Tue, 10 Sep 2024 14:25:23 +0800 Subject: [PATCH 06/10] Update app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb Co-authored-by: Bianca Nenciu --- app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb b/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb index 611a3763..63a513f9 100644 --- a/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb +++ b/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb @@ -18,7 +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: DiscourseSolved::Solution.pluck(:topic_id)) + query.where.not(id: DiscourseSolved::Solution.select(:topic_id)) end end end From 1a07c87e7189f0b5e6197cca378a015c160496c6 Mon Sep 17 00:00:00 2001 From: Linca <41134017+Lhcfl@users.noreply.github.com> Date: Tue, 10 Sep 2024 14:26:11 +0800 Subject: [PATCH 07/10] Update spec/fabricators/extend_topic_fabricator.rb Co-authored-by: Bianca Nenciu --- spec/fabricators/extend_topic_fabricator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fabricators/extend_topic_fabricator.rb b/spec/fabricators/extend_topic_fabricator.rb index ca38a27d..a26ba843 100644 --- a/spec/fabricators/extend_topic_fabricator.rb +++ b/spec/fabricators/extend_topic_fabricator.rb @@ -3,7 +3,7 @@ transient :custom_topic_name transient :value after_create do |top, transients| - if (transients[:custom_topic_name] == DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) + 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 From 15bc18248f064f98294630198faf786cc744ee55 Mon Sep 17 00:00:00 2001 From: Linca <41134017+Lhcfl@users.noreply.github.com> Date: Tue, 10 Sep 2024 14:26:18 +0800 Subject: [PATCH 08/10] Update db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb Co-authored-by: Bianca Nenciu --- ...e_solved_topic_custom_field_to_discourse_solved_solutions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 3e91f76d..be9c3b38 100644 --- 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 @@ -6,7 +6,7 @@ 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, null: true + t.integer :accepter_user_id t.integer :topic_timer_id, null: true t.timestamps end From d10c7c3f1f89a87c7d9ccd54bef5efdf8733c4b9 Mon Sep 17 00:00:00 2001 From: Linca <41134017+Lhcfl@users.noreply.github.com> Date: Tue, 10 Sep 2024 14:27:01 +0800 Subject: [PATCH 09/10] Update db/migrate/20240905024953_move_solved_topic_custom_field_to_discourse_solved_solutions.rb Co-authored-by: Bianca Nenciu --- ...e_solved_topic_custom_field_to_discourse_solved_solutions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index be9c3b38..b4139a51 100644 --- 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 @@ -7,7 +7,7 @@ def up t.integer :topic_id, null: false t.integer :answer_post_id, null: false t.integer :accepter_user_id - t.integer :topic_timer_id, null: true + t.integer :topic_timer_id t.timestamps end From ca19bb71faff113d129a89058d9a0e755b29b11c Mon Sep 17 00:00:00 2001 From: Lhc_fl Date: Tue, 10 Sep 2024 14:33:26 +0800 Subject: [PATCH 10/10] review: remove old_solution --- plugin.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/plugin.rb b/plugin.rb index a7e435c8..d4e75a83 100644 --- a/plugin.rb +++ b/plugin.rb @@ -51,14 +51,12 @@ def self.accept_answer!(post, acting_user, topic: nil) topic ||= post.topic DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do - old_solution = topic.solution - - if old_solution.present? + if topic.solution.present? UserAction.where( action_type: UserAction::SOLVED, - target_post_id: old_solution.answer_post_id, + target_post_id: topic.solution.answer_post_id, ).destroy_all - old_solution.destroy! + topic.solution.destroy! end solution = DiscourseSolved::Solution.create(topic:, post:, accepter_user_id: acting_user.id)