DEV: Move solved custom fields into a table (#342)

Related: 
- https://github.com/discourse/discourse-solved/pull/309
- https://github.com/discourse/discourse-solved/pull/341

Requires:
- https://github.com/discourse/discourse/pull/31954

This commit converts all use of post and topic custom fields into a dedicated table:
- migration for copying custom field into table
- swap app usage of custom fields to table

This commit does not attempt to fix issues or optimise, and does not delete old data from custom fields _yet_.
This commit is contained in:
Natalie Tay 2025-03-25 14:51:32 +08:00 committed by GitHub
parent e82c6ae1ca
commit 55c1eb4d60
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 272 additions and 288 deletions

View File

@ -1,3 +1,4 @@
< 3.5.0.beta2-dev: e82c6ae1ca38ccebb34669148f8de93a3028906e
< 3.5.0.beta1-dev: 5450a5ef4e2ae35185320fc6af9678621026e148 < 3.5.0.beta1-dev: 5450a5ef4e2ae35185320fc6af9678621026e148
< 3.4.0.beta4-dev: 3f724bf3114cc7877fa757bc8035f13a7390c739 < 3.4.0.beta4-dev: 3f724bf3114cc7877fa757bc8035f13a7390c739
< 3.4.0.beta2-dev: 1bbdfd8f5681171dc3f0e9ea93cd56997dc7938a < 3.4.0.beta2-dev: 1bbdfd8f5681171dc3f0e9ea93cd56997dc7938a

View File

@ -0,0 +1,34 @@
# frozen_string_literal: true
module DiscourseSolved
class SolvedTopic < ActiveRecord::Base
self.table_name = "discourse_solved_solved_topics"
belongs_to :topic, class_name: "Topic"
belongs_to :answer_post, class_name: "Post", foreign_key: "answer_post_id"
belongs_to :accepter, class_name: "User", foreign_key: "accepter_user_id"
belongs_to :topic_timer
validates :topic_id, presence: true
validates :answer_post_id, presence: true
validates :accepter_user_id, presence: true
end
end
# == Schema Information
#
# Table name: discourse_solved_solved_topics
#
# id :bigint not null, primary key
# topic_id :integer not null
# answer_post_id :integer not null
# accepter_user_id :integer not null
# topic_timer_id :integer
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_discourse_solved_solved_topics_on_answer_post_id (answer_post_id) UNIQUE
# index_discourse_solved_solved_topics_on_topic_id (topic_id) UNIQUE
#

View File

@ -7,7 +7,7 @@ module DiscourseSolved
end end
def has_accepted_answer def has_accepted_answer
object.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].present? object&.solved.present?
end end
def include_has_accepted_answer? def include_has_accepted_answer?

View File

@ -3,14 +3,13 @@
first_solution_query = <<~SQL first_solution_query = <<~SQL
SELECT post_id, user_id, created_at AS granted_at SELECT post_id, user_id, created_at AS granted_at
FROM ( FROM (
SELECT p.id AS post_id, p.user_id, pcf.created_at, SELECT p.id AS post_id, p.user_id, dsst.created_at,
ROW_NUMBER() OVER (PARTITION BY p.user_id ORDER BY pcf.created_at) AS row_number ROW_NUMBER() OVER (PARTITION BY p.user_id ORDER BY dsst.created_at) AS row_number
FROM post_custom_fields pcf FROM discourse_solved_solved_topics dsst
JOIN badge_posts p ON pcf.post_id = p.id JOIN badge_posts p ON dsst.answer_post_id = p.id
JOIN topics t ON p.topic_id = t.id JOIN topics t ON p.topic_id = t.id
WHERE pcf.name = 'is_accepted_answer' WHERE p.user_id <> t.user_id -- ignore topics solved by OP
AND p.user_id <> t.user_id -- ignore topics solved by OP AND (:backfill OR p.id IN (:post_ids))
AND (:backfill OR p.id IN (:post_ids))
) x ) x
WHERE row_number = 1 WHERE row_number = 1
SQL SQL
@ -32,12 +31,11 @@ end
def solved_query_with_count(min_count) def solved_query_with_count(min_count)
<<~SQL <<~SQL
SELECT p.user_id, MAX(pcf.created_at) AS granted_at SELECT p.user_id, MAX(dsst.created_at) AS granted_at
FROM post_custom_fields pcf FROM discourse_solved_solved_topics dsst
JOIN badge_posts p ON pcf.post_id = p.id JOIN badge_posts p ON dsst.answer_post_id = p.id
JOIN topics t ON p.topic_id = t.id JOIN topics t ON p.topic_id = t.id
WHERE pcf.name = 'is_accepted_answer' WHERE p.user_id <> t.user_id -- ignore topics solved by OP
AND p.user_id <> t.user_id -- ignore topics solved by OP
AND (:backfill OR p.id IN (:post_ids)) AND (:backfill OR p.id IN (:post_ids))
GROUP BY p.user_id GROUP BY p.user_id
HAVING COUNT(*) >= #{min_count} HAVING COUNT(*) >= #{min_count}

View File

@ -0,0 +1,13 @@
# frozen_string_literal: true
#
class CreateDiscourseSolvedSolvedTopics < ActiveRecord::Migration[7.2]
def change
create_table :discourse_solved_solved_topics do |t|
t.integer :topic_id, null: false
t.integer :answer_post_id, null: false
t.integer :accepter_user_id, null: false
t.integer :topic_timer_id
t.timestamps
end
end
end

View File

@ -0,0 +1,48 @@
# frozen_string_literal: true
#
class CopySolvedTopicCustomFieldToDiscourseSolvedSolvedTopics < ActiveRecord::Migration[7.2]
disable_ddl_transaction!
BATCH_SIZE = 5000
def up
last_id = 0
loop do
rows = DB.query(<<~SQL, last_id: last_id, batch_size: BATCH_SIZE)
INSERT INTO discourse_solved_solved_topics (
topic_id,
answer_post_id,
topic_timer_id,
accepter_user_id,
created_at,
updated_at
)
SELECT DISTINCT ON (tc.topic_id)
tc.topic_id,
CAST(tc.value AS INTEGER),
CAST(tc2.value AS INTEGER),
COALESCE(ua.acting_user_id, -1),
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
AND ua.action_type = 15
WHERE tc.name = 'accepted_answer_post_id'
AND tc.id > :last_id
ORDER BY tc.topic_id, ua.created_at DESC
LIMIT :batch_size
SQL
break if rows.length == 0
last_id += BATCH_SIZE
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
#
class AddIndexForDiscourseSolvedTopics < ActiveRecord::Migration[7.2]
disable_ddl_transaction!
def change
remove_index :discourse_solved_solved_topics,
:topic_id,
algorithm: :concurrently,
unique: true,
if_exists: true
remove_index :discourse_solved_solved_topics,
:answer_post_id,
algorithm: :concurrently,
unique: true,
if_exists: true
add_index :discourse_solved_solved_topics, :topic_id, unique: true, algorithm: :concurrently
add_index :discourse_solved_solved_topics,
:answer_post_id,
unique: true,
algorithm: :concurrently
end
end

View File

@ -2,15 +2,13 @@
module DiscourseAssign module DiscourseAssign
class EntryPoint class EntryPoint
# TODO: These four plugin api usages should ideally be in the assign plugin, not the solved plugin.
# They have been moved here from plugin.rb as part of the custom fields migration.
def self.inject(plugin) def self.inject(plugin)
plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query| plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query|
next query if !SiteSetting.ignore_solved_topics_in_assigned_reminder next query if !SiteSetting.ignore_solved_topics_in_assigned_reminder
query.where.not( query.where.not(id: DiscourseSolved::SolvedTopic.select(:topic_id))
id:
TopicCustomField.where(
name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
).pluck(:topic_id),
)
end end
plugin.register_modifier(:assigned_count_for_user_query) do |query, user| plugin.register_modifier(:assigned_count_for_user_query) do |query, user|

View File

@ -11,24 +11,24 @@ module DiscourseDev
solved_category = solved_category =
DiscourseDev::Record.random( DiscourseDev::Record.random(
Category.where( ::Category.where(
read_restricted: false, read_restricted: false,
id: records.pluck(:id), id: records.pluck(:id),
parent_category_id: nil, parent_category_id: nil,
), ),
) )
CategoryCustomField.create!( ::CategoryCustomField.create!(
category_id: solved_category.id, category_id: solved_category.id,
name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD,
value: "true", value: "true",
) )
puts "discourse-solved enabled on category '#{solved_category.name}' (#{solved_category.id})." puts "discourse-solved enabled on category '#{solved_category.name}' (#{solved_category.id})."
elsif type == :topic elsif type == :topic
topics = Topic.where(id: records.pluck(:id)) topics = ::Topic.where(id: records.pluck(:id))
unless SiteSetting.allow_solved_on_all_topics unless SiteSetting.allow_solved_on_all_topics
solved_category_id = solved_category_id =
CategoryCustomField ::CategoryCustomField
.where(name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, value: "true") .where(name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, value: "true")
.first .first
.category_id .category_id

View File

@ -40,10 +40,7 @@ class DiscourseSolved::BeforeHeadClose
}, },
} }
if accepted_answer = if accepted_answer = topic.solved&.answer_post
Post.find_by(
id: topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD],
)
question_json["answerCount"] = 1 question_json["answerCount"] = 1
question_json[:acceptedAnswer] = { question_json[:acceptedAnswer] = {
"@type" => "Answer", "@type" => "Answer",

View File

@ -4,24 +4,16 @@ module DiscourseSolved
class RegisterFilters class RegisterFilters
def self.register(plugin) def self.register(plugin)
solved_callback = ->(scope) do solved_callback = ->(scope) do
sql = <<~SQL scope.joins(
topics.id IN ( "INNER JOIN discourse_solved_solved_topics ON discourse_solved_solved_topics.topic_id = topics.id",
SELECT topic_id ).where("topics.archetype <> ?", Archetype.private_message)
FROM topic_custom_fields
WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}'
AND value IS NOT NULL
)
SQL
scope.where(sql).where("topics.archetype <> ?", Archetype.private_message)
end end
unsolved_callback = ->(scope) do unsolved_callback = ->(scope) do
scope = scope.where <<~SQL scope = scope.where(<<~SQL)
topics.id NOT IN ( topics.id NOT IN (
SELECT topic_id SELECT topic_id
FROM topic_custom_fields FROM discourse_solved_solved_topics
WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}'
AND value IS NOT NULL
) )
SQL SQL
@ -29,21 +21,21 @@ module DiscourseSolved
tag_ids = Tag.where(name: SiteSetting.enable_solved_tags.split("|")).pluck(:id) tag_ids = Tag.where(name: SiteSetting.enable_solved_tags.split("|")).pluck(:id)
scope = scope.where <<~SQL, tag_ids scope = scope.where <<~SQL, tag_ids
topics.id IN ( topics.id IN (
SELECT t.id SELECT t.id
FROM topics t FROM topics t
JOIN category_custom_fields cc JOIN category_custom_fields cc
ON t.category_id = cc.category_id ON t.category_id = cc.category_id
AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}' AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}'
AND cc.value = 'true' AND cc.value = 'true'
) )
OR OR
topics.id IN ( topics.id IN (
SELECT topic_id SELECT topic_id
FROM topic_tags FROM topic_tags
WHERE tag_id IN (?) WHERE tag_id IN (?)
) )
SQL SQL
end end
scope.where("topics.archetype <> ?", Archetype.private_message) scope.where("topics.archetype <> ?", Archetype.private_message)

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
module DiscourseSolved::TopicExtension
extend ActiveSupport::Concern
prepended { has_one :solved, class_name: "DiscourseSolved::SolvedTopic", dependent: :destroy }
end

View File

@ -43,12 +43,6 @@ module DiscourseSolved::TopicViewSerializerExtension
end end
def accepted_answer_post_id def accepted_answer_post_id
id = object.topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD] object.topic.solved&.answer_post_id
# a bit messy but race conditions can give us an array here, avoid
begin
id && id.to_i
rescue StandardError
nil
end
end end
end end

View File

@ -4,6 +4,6 @@ module DiscourseSolved::UserSummaryExtension
extend ActiveSupport::Concern extend ActiveSupport::Concern
def solved_count def solved_count
UserAction.where(user: @user).where(action_type: UserAction::SOLVED).count DiscourseSolved::SolvedTopic.where(accepter: @user).count
end end
end end

184
plugin.rb
View File

@ -18,10 +18,7 @@ register_asset "stylesheets/mobile/solutions.scss", :mobile
module ::DiscourseSolved module ::DiscourseSolved
PLUGIN_NAME = "discourse-solved" 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" ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD = "enable_accepted_answers"
IS_ACCEPTED_ANSWER_CUSTOM_FIELD = "is_accepted_answer"
end end
require_relative "lib/discourse_solved/engine.rb" require_relative "lib/discourse_solved/engine.rb"
@ -34,27 +31,25 @@ after_initialize do
topic ||= post.topic topic ||= post.topic
DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do
accepted_id = topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].to_i solved = topic.solved
if accepted_id > 0 if previous_accepted_post_id = solved&.answer_post_id
if p2 = Post.find_by(id: accepted_id) UserAction.where(
p2.custom_fields.delete(IS_ACCEPTED_ANSWER_CUSTOM_FIELD) action_type: UserAction::SOLVED,
p2.save! target_post_id: previous_accepted_post_id,
).destroy_all
UserAction.where(action_type: UserAction::SOLVED, target_post_id: p2.id).destroy_all else
end UserAction.log_action!(
action_type: UserAction::SOLVED,
user_id: post.user_id,
acting_user_id: acting_user.id,
target_post_id: post.id,
target_topic_id: post.topic_id,
)
end end
post.custom_fields[IS_ACCEPTED_ANSWER_CUSTOM_FIELD] = "true" solved ||=
topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD] = post.id DiscourseSolved::SolvedTopic.new(topic:, answer_post: post, accepter: acting_user)
UserAction.log_action!(
action_type: UserAction::SOLVED,
user_id: post.user_id,
acting_user_id: acting_user.id,
target_post_id: post.id,
target_topic_id: post.topic_id,
)
notification_data = { notification_data = {
message: "solved.accepted_notification", message: "solved.accepted_notification",
@ -99,14 +94,12 @@ after_initialize do
based_on_last_post: true, based_on_last_post: true,
duration_minutes: auto_close_hours * 60, duration_minutes: auto_close_hours * 60,
) )
solved.topic_timer = topic_timer
topic.custom_fields[AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD] = topic_timer.id
MessageBus.publish("/topic/#{topic.id}", reload_topic: true) MessageBus.publish("/topic/#{topic.id}", reload_topic: true)
end end
topic.save! solved.save!
post.save!
if WebHook.active_web_hooks(:accepted_solution).exists? if WebHook.active_web_hooks(:accepted_solution).exists?
payload = WebHook.generate_payload(:post, post) payload = WebHook.generate_payload(:post, post)
@ -120,41 +113,28 @@ after_initialize do
def self.unaccept_answer!(post, topic: nil) def self.unaccept_answer!(post, topic: nil)
topic ||= post.topic topic ||= post.topic
topic ||= Topic.unscoped.find_by(id: post.topic_id) topic ||= Topic.unscoped.find_by(id: post.topic_id)
return if topic.nil? return if topic.nil?
return if topic.solved.nil?
DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do
post.custom_fields.delete(IS_ACCEPTED_ANSWER_CUSTOM_FIELD) solved = topic.solved
topic.custom_fields.delete(ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD)
if timer_id = topic.custom_fields[AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD] ActiveRecord::Base.transaction do
topic_timer = TopicTimer.find_by(id: timer_id) UserAction.where(action_type: UserAction::SOLVED, target_post_id: post.id).destroy_all
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
# yank notification
notification =
Notification.find_by( Notification.find_by(
notification_type: Notification.types[:custom], notification_type: Notification.types[:custom],
user_id: post.user_id, user_id: post.user_id,
topic_id: post.topic_id, topic_id: post.topic_id,
post_number: post.post_number, post_number: post.post_number,
) )&.destroy!
solved.topic_timer.destroy! if solved.topic_timer
notification.destroy! if notification solved.destroy!
end
if WebHook.active_web_hooks(:unaccepted_solution).exists? if WebHook.active_web_hooks(:unaccepted_solution).exists?
payload = WebHook.generate_payload(:post, post) payload = WebHook.generate_payload(:post, post)
WebHook.enqueue_solved_hooks(:unaccepted_solution, post, payload) WebHook.enqueue_solved_hooks(:unaccepted_solution, post, payload)
end end
DiscourseEvent.trigger(:unaccepted_solution, post) DiscourseEvent.trigger(:unaccepted_solution, post)
end end
end end
@ -168,6 +148,7 @@ after_initialize do
::Guardian.prepend(DiscourseSolved::GuardianExtensions) ::Guardian.prepend(DiscourseSolved::GuardianExtensions)
::WebHook.prepend(DiscourseSolved::WebHookExtension) ::WebHook.prepend(DiscourseSolved::WebHookExtension)
::TopicViewSerializer.prepend(DiscourseSolved::TopicViewSerializerExtension) ::TopicViewSerializer.prepend(DiscourseSolved::TopicViewSerializerExtension)
::Topic.prepend(DiscourseSolved::TopicExtension)
::Category.prepend(DiscourseSolved::CategoryExtension) ::Category.prepend(DiscourseSolved::CategoryExtension)
::PostSerializer.prepend(DiscourseSolved::PostSerializerExtension) ::PostSerializer.prepend(DiscourseSolved::PostSerializerExtension)
::UserSummary.prepend(DiscourseSolved::UserSummaryExtension) ::UserSummary.prepend(DiscourseSolved::UserSummaryExtension)
@ -183,49 +164,10 @@ after_initialize do
].each { |klass| klass.include(DiscourseSolved::TopicAnswerMixin) } ].each { |klass| klass.include(DiscourseSolved::TopicAnswerMixin) }
end end
# we got to do a one time upgrade register_category_list_topics_preloader_associations(:solved) if SiteSetting.solved_enabled
if !::DiscourseSolved.skip_db? register_topic_preloader_associations(:solved) if SiteSetting.solved_enabled
unless Discourse.redis.get("solved_already_upgraded") Search.custom_topic_eager_load { [:solved] } if SiteSetting.solved_enabled
unless UserAction.where(action_type: UserAction::SOLVED).exists?
Rails.logger.info("Upgrading storage for solved")
sql = <<~SQL
INSERT INTO user_actions(action_type,
user_id,
target_topic_id,
target_post_id,
acting_user_id,
created_at,
updated_at)
SELECT :solved,
p.user_id,
p.topic_id,
p.id,
t.user_id,
pc.created_at,
pc.updated_at
FROM
post_custom_fields pc
JOIN
posts p ON p.id = pc.post_id
JOIN
topics t ON t.id = p.topic_id
WHERE
pc.name = 'is_accepted_answer' AND
pc.value = 'true' AND
p.user_id IS NOT NULL
SQL
DB.exec(sql, solved: UserAction::SOLVED)
end
Discourse.redis.set("solved_already_upgraded", "true")
end
end
topic_view_post_custom_fields_allowlister { [::DiscourseSolved::IS_ACCEPTED_ANSWER_CUSTOM_FIELD] }
TopicList.preloaded_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD
Site.preloaded_category_custom_fields << ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_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
CategoryList.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD
add_api_key_scope( add_api_key_scope(
:solved, :solved,
@ -244,10 +186,10 @@ after_initialize do
report.data = [] report.data = []
accepted_solutions = accepted_solutions =
TopicCustomField DiscourseSolved::SolvedTopic.joins(:topic).where(
.joins(:topic) "topics.archetype <> ?",
.where("topics.archetype <> ?", Archetype.private_message) Archetype.private_message,
.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) )
category_id, include_subcategories = report.add_category_filter category_id, include_subcategories = report.add_category_filter
if category_id if category_id
@ -263,17 +205,17 @@ after_initialize do
end end
accepted_solutions accepted_solutions
.where("topic_custom_fields.created_at >= ?", report.start_date) .where("discourse_solved_solved_topics.created_at >= ?", report.start_date)
.where("topic_custom_fields.created_at <= ?", report.end_date) .where("discourse_solved_solved_topics.created_at <= ?", report.end_date)
.group("DATE(topic_custom_fields.created_at)") .group("DATE(discourse_solved_solved_topics.created_at)")
.order("DATE(topic_custom_fields.created_at)") .order("DATE(discourse_solved_solved_topics.created_at)")
.count .count
.each { |date, count| report.data << { x: date, y: count } } .each { |date, count| report.data << { x: date, y: count } }
report.total = accepted_solutions.count report.total = accepted_solutions.count
report.prev30Days = report.prev30Days =
accepted_solutions accepted_solutions
.where("topic_custom_fields.created_at >= ?", report.start_date - 30.days) .where("discourse_solved_solved_topics.created_at >= ?", report.start_date - 30.days)
.where("topic_custom_fields.created_at <= ?", report.start_date) .where("discourse_solved_solved_topics.created_at <= ?", report.start_date)
.count .count
end end
@ -282,10 +224,8 @@ after_initialize do
condition = <<~SQL condition = <<~SQL
EXISTS ( EXISTS (
SELECT 1 SELECT 1
FROM topic_custom_fields FROM discourse_solved_solved_topics
WHERE topic_id = topics.id WHERE discourse_solved_solved_topics.topic_id = topics.id
AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}'
AND value IS NOT NULL
) )
SQL SQL
@ -315,18 +255,10 @@ after_initialize do
add_to_serializer(:post, :can_unaccept_answer) do add_to_serializer(:post, :can_unaccept_answer) do
scope.can_accept_answer?(topic, object) && accepted_answer scope.can_accept_answer?(topic, object) && accepted_answer
end end
add_to_serializer(:post, :accepted_answer) do add_to_serializer(:post, :accepted_answer) { topic&.solved&.answer_post_id == object.id }
post_custom_fields[::DiscourseSolved::IS_ACCEPTED_ANSWER_CUSTOM_FIELD] == "true" add_to_serializer(:post, :topic_accepted_answer) { topic&.solved&.present? }
end
add_to_serializer(:post, :topic_accepted_answer) do
topic&.custom_fields&.[](::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD).present?
end
on(:post_destroyed) do |post| on(:post_destroyed) { |post| ::DiscourseSolved.unaccept_answer!(post) }
if post.custom_fields[::DiscourseSolved::IS_ACCEPTED_ANSWER_CUSTOM_FIELD] == "true"
::DiscourseSolved.unaccept_answer!(post)
end
end
on(:filter_auto_bump_topics) do |_category, filters| on(:filter_auto_bump_topics) do |_category, filters|
filters.push( filters.push(
@ -334,10 +266,8 @@ after_initialize do
sql = <<~SQL sql = <<~SQL
NOT EXISTS ( NOT EXISTS (
SELECT 1 SELECT 1
FROM topic_custom_fields FROM discourse_solved_solved_topics
WHERE topic_id = topics.id WHERE discourse_solved_solved_topics.topic_id = topics.id
AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}'
AND value IS NOT NULL
) )
SQL SQL
@ -390,7 +320,7 @@ after_initialize do
add_to_class(:composer_messages_finder, :check_topic_is_solved) do add_to_class(:composer_messages_finder, :check_topic_is_solved) do
return if !SiteSetting.solved_enabled || SiteSetting.disable_solved_education_message return if !SiteSetting.solved_enabled || SiteSetting.disable_solved_education_message
return if !replying? || @topic.blank? || @topic.private_message? return if !replying? || @topic.blank? || @topic.private_message?
return if @topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].blank? return if @topic.solved.nil?
{ {
id: "solved_topic", id: "solved_topic",
@ -402,15 +332,17 @@ after_initialize do
} }
end end
register_topic_list_preload_user_ids do |topics, user_ids, topic_list| register_topic_list_preload_user_ids do |topics, user_ids|
answer_post_ids = # [{ topic_id => answer_user_id }, ... ]
TopicCustomField topics_with_answer_poster =
.select("value::INTEGER") DiscourseSolved::SolvedTopic
.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) .joins(:answer_post)
.where(topic_id: topics.map(&:id)) .where(topic_id: topics.map(&:id))
answer_user_ids = Post.where(id: answer_post_ids).pluck(:topic_id, :user_id).to_h .pluck(:topic_id, "posts.user_id")
topics.each { |topic| topic.accepted_answer_user_id = answer_user_ids[topic.id] } .to_h
user_ids.concat(answer_user_ids.values)
topics.each { |topic| topic.accepted_answer_user_id = topics_with_answer_poster[topic.id] }
user_ids.concat(topics_with_answer_poster.values)
end end
DiscourseSolved::RegisterFilters.register(self) DiscourseSolved::RegisterFilters.register(self)

View File

@ -0,0 +1,6 @@
# frozen_string_literal: true
Fabricator(:solved_topic, from: DiscourseSolved::SolvedTopic) do
topic
answer_post { Fabricate(:post) }
accepter { Fabricate(:user) }
end

View File

@ -30,31 +30,21 @@ RSpec.describe "Managing Posts solved status" do
fab!(:solvable_tag) { Fabricate(:tag) } fab!(:solvable_tag) { Fabricate(:tag) }
fab!(:solved_in_category) do fab!(:solved_in_category) do
Fabricate( topic = Fabricate(:topic, category: solvable_category)
:custom_topic, Fabricate(:solved_topic, topic:, answer_post: Fabricate(:post, topic:))
category: solvable_category, topic
custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
value: "42",
)
end end
fab!(:solved_in_tag) do fab!(:solved_in_tag) do
Fabricate( topic = Fabricate(:topic, tags: [solvable_tag])
:custom_topic, Fabricate(:solved_topic, topic:, answer_post: Fabricate(:post, topic:))
tags: [solvable_tag], topic
custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
value: "42",
)
end end
fab!(:solved_pm) do fab!(:solved_pm) do
Fabricate( topic = Fabricate(:topic, archetype: Archetype.private_message, category_id: nil)
:custom_topic, Fabricate(:solved_topic, topic:, answer_post: Fabricate(:post, topic:))
category_id: nil, topic
archetype: Archetype.private_message,
custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
value: "42",
)
end end
fab!(:unsolved_in_category) { Fabricate(:topic, category: solvable_category) } fab!(:unsolved_in_category) { Fabricate(:topic, category: solvable_category) }
@ -136,39 +126,15 @@ RSpec.describe "Managing Posts solved status" do
category category
end end
fab!(:tag) fab!(:tag)
fab!(:topic_unsolved) do fab!(:topic_unsolved) { Fabricate(:topic, user:, category: category_enabled) }
Fabricate(
:custom_topic,
user: user,
category: category_enabled,
custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
)
end
fab!(:topic_unsolved_2) { Fabricate(:topic, user: user, tags: [tag]) } fab!(:topic_unsolved_2) { Fabricate(:topic, user: user, tags: [tag]) }
fab!(:topic_solved) do fab!(:topic_solved) do
Fabricate( topic = Fabricate(:topic, category: category_enabled)
:custom_topic, Fabricate(:solved_topic, topic:, answer_post: Fabricate(:post, topic:))
user: user, topic
category: category_enabled,
custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
)
end
fab!(:topic_disabled_1) do
Fabricate(
:custom_topic,
user: user,
category: category_disabled,
custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
)
end
fab!(:topic_disabled_2) do
Fabricate(
:custom_topic,
user: user,
category: category_disabled,
custom_topic_name: "another_custom_field",
)
end end
fab!(:topic_disabled_1) { Fabricate(:topic, category: category_disabled) }
fab!(:topic_disabled_2) { Fabricate(:topic, category: category_disabled) }
fab!(:post_unsolved) { Fabricate(:post, topic: topic_unsolved) } fab!(:post_unsolved) { Fabricate(:post, topic: topic_unsolved) }
fab!(:post_unsolved_2) { Fabricate(:post, topic: topic_unsolved_2) } fab!(:post_unsolved_2) { Fabricate(:post, topic: topic_unsolved_2) }
fab!(:post_solved) do fab!(:post_solved) do
@ -258,18 +224,14 @@ RSpec.describe "Managing Posts solved status" do
post "/solution/accept.json", params: { id: p1.id } post "/solution/accept.json", params: { id: p1.id }
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") expect(topic.solved.answer_post_id).to eq(p1.id)
topic.reload topic.reload
expect(topic.public_topic_timer.status_type).to eq(TopicTimer.types[:silent_close]) 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( expect(topic.solved.topic_timer).to eq(topic.public_topic_timer)
topic.public_topic_timer.id,
)
expect(topic.public_topic_timer.execute_at).to eq_time(Time.zone.now + 2.hours) expect(topic.public_topic_timer.execute_at).to eq_time(Time.zone.now + 2.hours)
expect(topic.public_topic_timer.based_on_last_post).to eq(true) expect(topic.public_topic_timer.based_on_last_post).to eq(true)
end end
@ -284,18 +246,14 @@ RSpec.describe "Managing Posts solved status" do
post "/solution/accept.json", params: { id: post_2.id } post "/solution/accept.json", params: { id: post_2.id }
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(post_2.reload.custom_fields["is_accepted_answer"]).to eq("true") expect(topic_2.solved.answer_post_id).to eq(post_2.id)
topic_2.reload topic_2.reload
expect(topic_2.public_topic_timer.status_type).to eq(TopicTimer.types[:silent_close]) 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( expect(topic_2.solved.topic_timer).to eq(topic_2.public_topic_timer)
topic_2.public_topic_timer.id,
)
expect(topic_2.public_topic_timer.execute_at).to eq_time(Time.zone.now + 4.hours) expect(topic_2.public_topic_timer.execute_at).to eq_time(Time.zone.now + 4.hours)
expect(topic_2.public_topic_timer.based_on_last_post).to eq(true) expect(topic_2.public_topic_timer.based_on_last_post).to eq(true)
end end
@ -332,7 +290,7 @@ RSpec.describe "Managing Posts solved status" do
p1.reload p1.reload
topic.reload topic.reload
expect(p1.custom_fields["is_accepted_answer"]).to eq("true") expect(topic.solved.answer_post_id).to eq(p1.id)
expect(topic.public_topic_timer).to eq(nil) expect(topic.public_topic_timer).to eq(nil)
expect(topic.closed).to eq(true) expect(topic.closed).to eq(true)
end end
@ -348,7 +306,7 @@ RSpec.describe "Managing Posts solved status" do
expect(response.status).to eq(200) expect(response.status).to eq(200)
p1.reload p1.reload
expect(p1.custom_fields["is_accepted_answer"]).to eq("true") expect(topic.solved.answer_post_id).to eq(p1.id)
end end
it "removes the solution when the post is deleted" do it "removes the solution when the post is deleted" do
@ -357,15 +315,12 @@ RSpec.describe "Managing Posts solved status" do
post "/solution/accept.json", params: { id: reply.id } post "/solution/accept.json", params: { id: reply.id }
expect(response.status).to eq(200) expect(response.status).to eq(200)
reply.reload expect(topic.solved.answer_post_id).to eq(reply.id)
expect(reply.custom_fields["is_accepted_answer"]).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 PostDestroyer.new(Discourse.system_user, reply).destroy
reply.topic.reload
reply.reload expect(topic.solved).to be(nil)
expect(reply.custom_fields["is_accepted_answer"]).to eq(nil)
expect(reply.topic.custom_fields["accepted_answer_post_id"]).to eq(nil)
end end
it "does not allow you to accept a whisper" do it "does not allow you to accept a whisper" do
@ -395,6 +350,7 @@ RSpec.describe "Managing Posts solved status" do
before do before do
SiteSetting.solved_topics_auto_close_hours = 2 SiteSetting.solved_topics_auto_close_hours = 2
DiscourseSolved.accept_answer!(p1, user) DiscourseSolved.accept_answer!(p1, user)
topic.reload
end end
it "should unmark the post as solved" do it "should unmark the post as solved" do
@ -405,12 +361,13 @@ RSpec.describe "Managing Posts solved status" do
expect(response.status).to eq(200) expect(response.status).to eq(200)
p1.reload p1.reload
expect(p1.custom_fields["is_accepted_answer"]).to eq(nil) expect(topic.solved).to be(nil)
expect(p1.topic.custom_fields["accepted_answer_post_id"]).to eq(nil)
end end
end end
it "triggers a webhook" do it "triggers a webhook" do
DiscourseSolved.accept_answer!(p1, user)
Fabricate(:solved_web_hook) Fabricate(:solved_web_hook)
post "/solution/unaccept.json", params: { id: p1.id } post "/solution/unaccept.json", params: { id: p1.id }
@ -466,8 +423,9 @@ RSpec.describe "Managing Posts solved status" do
expect(p1.topic.assignment.status).to eq("New") expect(p1.topic.assignment.status).to eq("New")
DiscourseSolved.accept_answer!(p1, user) DiscourseSolved.accept_answer!(p1, user)
topic.reload
expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") expect(topic.solved.answer_post_id).to eq(p1.id)
expect(p1.topic.assignment.reload.status).to eq("Done") expect(p1.topic.assignment.reload.status).to eq("Done")
end end
@ -482,7 +440,6 @@ RSpec.describe "Managing Posts solved status" do
DiscourseSolved.unaccept_answer!(p1) DiscourseSolved.unaccept_answer!(p1)
expect(p1.reload.custom_fields["is_accepted_answer"]).to eq(nil)
expect(p1.reload.topic.assignment.reload.status).to eq("New") expect(p1.reload.topic.assignment.reload.status).to eq("New")
end end

View File

@ -45,10 +45,7 @@ RSpec.describe TopicsController do
expect(response.body).to include(schema_json(0)) expect(response.body).to include(schema_json(0))
p2.custom_fields["is_accepted_answer"] = true Fabricate(:solved_topic, topic: topic, answer_post: p2)
p2.save_custom_fields
topic.custom_fields["accepted_answer_post_id"] = p2.id
topic.save_custom_fields
get "/t/#{topic.slug}/#{topic.id}" get "/t/#{topic.slug}/#{topic.id}"
@ -68,10 +65,7 @@ RSpec.describe TopicsController do
it "should include user name in output with the corresponding site setting" do it "should include user name in output with the corresponding site setting" do
SiteSetting.display_name_on_posts = true SiteSetting.display_name_on_posts = true
p2.custom_fields["is_accepted_answer"] = true Fabricate(:solved_topic, topic: topic, answer_post: p2)
p2.save_custom_fields
topic.custom_fields["accepted_answer_post_id"] = p2.id
topic.save_custom_fields
get "/t/#{topic.slug}/#{topic.id}.json" get "/t/#{topic.slug}/#{topic.id}.json"
@ -87,10 +81,7 @@ RSpec.describe TopicsController do
it "should not include user name when site setting is disabled" do it "should not include user name when site setting is disabled" do
SiteSetting.display_name_on_posts = false SiteSetting.display_name_on_posts = false
p2.custom_fields["is_accepted_answer"] = true Fabricate(:solved_topic, topic: topic, answer_post: p2)
p2.save_custom_fields
topic.custom_fields["accepted_answer_post_id"] = p2.id
topic.save_custom_fields
get "/t/#{topic.slug}/#{topic.id}.json" get "/t/#{topic.slug}/#{topic.id}.json"
@ -106,10 +97,7 @@ RSpec.describe TopicsController do
it "includes the correct schema information" do it "includes the correct schema information" do
DiscourseTagging.add_or_create_tags_by_name(topic, [tag.name]) DiscourseTagging.add_or_create_tags_by_name(topic, [tag.name])
p2.custom_fields["is_accepted_answer"] = true Fabricate(:solved_topic, topic: topic, answer_post: p2)
p2.save_custom_fields
topic.custom_fields["accepted_answer_post_id"] = p2.id
topic.save_custom_fields
get "/t/#{topic.slug}/#{topic.id}" get "/t/#{topic.slug}/#{topic.id}"
@ -120,10 +108,7 @@ RSpec.describe TopicsController do
another_tag = Fabricate(:tag) another_tag = Fabricate(:tag)
DiscourseTagging.add_or_create_tags_by_name(topic, [another_tag.name]) DiscourseTagging.add_or_create_tags_by_name(topic, [another_tag.name])
p2.custom_fields["is_accepted_answer"] = true Fabricate(:solved_topic, topic: topic, answer_post: p2)
p2.save_custom_fields
topic.custom_fields["accepted_answer_post_id"] = p2.id
topic.save_custom_fields
get "/t/#{topic.slug}/#{topic.id}" get "/t/#{topic.slug}/#{topic.id}"

View File

@ -7,10 +7,7 @@ describe DiscourseSolved::TopicAnswerMixin do
let(:post) { Fabricate(:post, topic: topic) } let(:post) { Fabricate(:post, topic: topic) }
let(:guardian) { Guardian.new } let(:guardian) { Guardian.new }
before do before { Fabricate(:solved_topic, topic: topic, answer_post: post) }
topic.custom_fields["accepted_answer_post_id"] = post.id
topic.save_custom_fields
end
it "should have true for `has_accepted_answer` field in each serializer" do it "should have true for `has_accepted_answer` field in each serializer" do
[ [

View File

@ -12,6 +12,7 @@ describe UserCardSerializer do
post1 = Fabricate(:post, user: user) post1 = Fabricate(:post, user: user)
DiscourseSolved.accept_answer!(post1, Discourse.system_user) DiscourseSolved.accept_answer!(post1, Discourse.system_user)
post1.topic.reload
expect(serializer.as_json[:accepted_answers]).to eq(1) expect(serializer.as_json[:accepted_answers]).to eq(1)
post2 = Fabricate(:post, user: user) post2 = Fabricate(:post, user: user)