Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

FIX: delete solution with post #256

Merged
merged 1 commit into from
Oct 13, 2023
Merged

FIX: delete solution with post #256

merged 1 commit into from
Oct 13, 2023

Conversation

ZogStriP
Copy link
Member

Ensures we remove the solution when the post marked as the solution is deleted.

DEV: Added IS_ACCEPTED_ASNWER_CUSTOM_FIELD constant.
DEV: Refactored the PostSerializer for better readability.
PERF: Improved the TopicViewSerializer's performance by looking up the accepted_answer_post_info from the stream first.

Internal ref. dev/112251

plugin.rb Outdated
@@ -77,6 +77,7 @@ class Engine < ::Rails::Engine
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_ASNWER_CUSTOM_FIELD = "is_accepted_answer"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this constant so we don't keep referencing the "is_accepted_answer" string everywhere.

@@ -257,12 +259,18 @@ def limit_accepts

Discourse::Application.routes.append { mount ::DiscourseSolved::Engine, at: "solution" }

on(:post_destroyed) do |post|
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the gist of the PR. We listen to the post_destroyer event so we can properly "unaccept" an answer.

postInfo[2] = if SiteSetting.solved_quote_length > 0
PrettyText.excerpt(postInfo[2], SiteSetting.solved_quote_length, keep_emoji_images: true)
post_info =
if post = object.posts.find { |p| p.post_number == accepted_answer_post_id }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the "performance" change which looks up the stream for the post that is marked as the solution before doing a request to the database.

end
)
postInfo
post_info[3] = nil if !SiteSetting.enable_names || !SiteSetting.display_name_on_posts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like it's clearer to "just" set to nil when names are disabled.

@@ -268,6 +268,23 @@
expect(p1.custom_fields["is_accepted_answer"]).to eq("true")
end

it "removes the solution when the post is deleted" do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test for the gist of the PR

topic&.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].present?
end

def topic
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny refactor for improved readability.

@ZogStriP ZogStriP force-pushed the delete-solution-with-post branch from e85055c to 812aa95 Compare October 13, 2023 16:20
Ensures we remove the solution when the post marked as the solution is deleted.

DEV: Added `IS_ACCEPTED_ANSWER_CUSTOM_FIELD` constant.
DEV: Refactored the `PostSerializer` for better readability.
PERF: Improved the `TopicViewSerializer`'s performance by looking up the `accepted_answer_post_info` from the stream first.

Internal ref. dev/112251
@ZogStriP ZogStriP force-pushed the delete-solution-with-post branch from 812aa95 to f939606 Compare October 13, 2023 16:21
@ZogStriP ZogStriP merged commit b269689 into main Oct 13, 2023
@ZogStriP ZogStriP deleted the delete-solution-with-post branch October 13, 2023 17:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants