FIX: delete solution with post #256
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZogStriP
commented
Oct 13, 2023
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" | |||
There was a problem hiding this comment.
Added this constant so we don't keep referencing the "is_accepted_answer" string everywhere.
ZogStriP
commented
Oct 13, 2023
| @@ -257,12 +259,18 @@ def limit_accepts | |||
|
|
|||
| Discourse::Application.routes.append { mount ::DiscourseSolved::Engine, at: "solution" } | |||
|
|
|||
| on(:post_destroyed) do |post| | |||
There was a problem hiding this comment.
This is the gist of the PR. We listen to the post_destroyer event so we can properly "unaccept" an answer.
ZogStriP
commented
Oct 13, 2023
| 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 } |
There was a problem hiding this comment.
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.
ZogStriP
commented
Oct 13, 2023
| end | ||
| ) | ||
| postInfo | ||
| post_info[3] = nil if !SiteSetting.enable_names || !SiteSetting.display_name_on_posts |
There was a problem hiding this comment.
Feels like it's clearer to "just" set to nil when names are disabled.
ZogStriP
commented
Oct 13, 2023
| @@ -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 | |||
There was a problem hiding this comment.
The test for the gist of the PR
ZogStriP
commented
Oct 13, 2023
| topic&.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].present? | ||
| end | ||
|
|
||
| def topic |
There was a problem hiding this comment.
Tiny refactor for improved readability.
pmusaraj
approved these changes
Oct 13, 2023
ZogStriP
force-pushed
the
delete-solution-with-post
branch
from
e85055c
to
812aa95
Compare
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
force-pushed
the
delete-solution-with-post
branch
from
812aa95
to
f939606
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensures we remove the solution when the post marked as the solution is deleted.
DEV: Added
IS_ACCEPTED_ASNWER_CUSTOM_FIELDconstant.DEV: Refactored the
PostSerializerfor better readability.PERF: Improved the
TopicViewSerializer's performance by looking up theaccepted_answer_post_infofrom the stream first.Internal ref. dev/112251