Fixes bug when deleted user's note lacks comments #5609
+9
−5
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
AntonKhorev
reviewed
Feb 5, 2025
app/views/notes/show.html.erb
Outdated
| @@ -52,10 +52,10 @@ | |||
| <% end %> | |||
| </div> | |||
|
|
|||
| <% if @note_comments.length > 1 %> | |||
| <% if @note_comments.length > (!@note.author.nil? && @note.author.status == "deleted" ? 0 : 1) %> | |||
There was a problem hiding this comment.
I think it's better to make @note_comments contain true comments in the controller instead of having this condition checked twice here.
@note_comments were used in three places of this view: description, anonymous warning and discussion.
- Description is handled differently now.
- Discussion is going to work with all
@note_commentswithout deciding which to drop. - Anonymous warning will need to check if either note author is nil or any comment's author is nil. Maybe this check is also better done in the controller and saved to some variable.
There was a problem hiding this comment.
Fixed in the last push. Thanks!
Q: Do we need to replace current anonymous warning to something like "This note is created by anonymous user or includes comments from anonymous users so should be independently verified." or current message is good enough?
Switches from using note's .comments to .all_comments. Fixes bug with notes of deleted users without comments, which are filtered out, but the code tries to access the first comment.
Removes dropping note's first visible comment in case of deleted note's author. After adding displaying "deleted" as note's description, first visible comment is now displayed as note's comment. Moves logic of calculating which note comments will be displayed and if note contains anonymous author / comments to the controller.
nenad-vujicic
force-pushed
the
issue_replace_opening_comment_task_6
branch
from
0c08a73 to
592b28f
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.
Description
PR fixes bug where a note of deleted user has no comments, but the code tries to access the first comment by switching to using
.all_comments(instead of.comments) and removing dropping first comment (if author is deleted) when displaying notes comments in sidebar.Suggested here.
How has this been tested?
Automated tests, manual testing before and after migration.