Adds comments #42
Open
Adds comments #42
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
Closed
minitech
force-pushed
the
comment_threads
branch
from
76d9dd5 to
179c08a
Compare
minitech
reviewed
Apr 9, 2023
| pub id: ObjectId, | ||
| pub owner: ObjectId, | ||
| pub parent_post: ObjectId, | ||
| pub parent_comments: Vec<ObjectId>, |
minitech
force-pushed
the
comment_threads
branch
from
179c08a to
13f0053
Compare
minitech
suggested changes
Apr 9, 2023
| @@ -69,6 +73,51 @@ async fn initialize_database(db: &Database) -> mongodb::error::Result<()> { | |||
| .build(), | |||
| None, | |||
| ), | |||
| // Is this too many indices? Probably... | |||
There was a problem hiding this comment.
Yep
Suggested change
| // Is this too many indices? Probably... |
| @@ -69,6 +73,51 @@ async fn initialize_database(db: &Database) -> mongodb::error::Result<()> { | |||
| .build(), | |||
| None, | |||
| ), | |||
| // Is this too many indices? Probably... | |||
| comments.create_index( | |||
| IndexModel::builder().keys(doc! {"parent_post": -1}).build(), | |||
There was a problem hiding this comment.
The direction of single-key indexes doesn’t matter, so use 1 for consistency.
Suggested change
| IndexModel::builder().keys(doc! {"parent_post": -1}).build(), | |
| IndexModel::builder().keys(doc! {"parent_post": 1}).build(), |
Comment on lines
+103
to
+129
| comments.create_index( | ||
| IndexModel::builder() | ||
| .keys(doc! {"absolute_score": -1}) | ||
| .build(), | ||
| None, | ||
| ), |
There was a problem hiding this comment.
Only need an index in one direction.
Suggested change
| comments.create_index( | |
| IndexModel::builder() | |
| .keys(doc! {"absolute_score": -1}) | |
| .build(), | |
| None, | |
| ), |
| ), | ||
| comments.create_index( | ||
| IndexModel::builder() | ||
| .keys(doc! {"sequential_id": -1}) |
There was a problem hiding this comment.
Suggested change
| .keys(doc! {"sequential_id": -1}) | |
| .keys(doc! {"sequential_id": 1}) |
| } | ||
|
|
||
| // If the comment is trying to be nested too deep, send a 400 Bad Request. | ||
| if request.parent_comments.len() > conf::COMMENT_MAX_DEPTH { |
There was a problem hiding this comment.
We shouldn’t be trusting the client to send the correct path. Can we store just the parent id instead of the whole list of ancestor ids? I’m not clear on what that’s used for, other than the depth check.
Co-authored-by: Ryan O’Hara <[email protected]>
minitech
force-pushed
the
comment_threads
branch
from
308cdc0 to
b36e4ef
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.
New features
GET /comments/for fetching comments from either a post or another comment. Allows you to sort by: newest, top, worst, most controversial, overall best, and most replies.POST /comments/for creating a comment as either a direct reply to the post, or a reply to another comment.DELETE /comments/{comment_id}/for deleting a comment. A deleted comment will be rendered with text "[deleted]" to indicate it's been deleted, but also not totally removed from the database so the threaded tree structure is not destroyed.PUT /comments/{comment_id}/votefor voting on a comment.Old changes/updates
implto theMaskedObjectIdstructto help with threaded comment sorting.get_trending_score_timemethod into its own file, as I'm now using it for comments as well as posts."votes"collection into"comment_votes"and"post_votes"collections. This was because I added votes for comments and thought comment votes and post votes should be stored separately.Docs
openapi.yamldocs for routes:GET /comments/,POST /comments/,DELETE /comments/{comment_id}/, andPUT /comments/{comment_id}/vote.Notes
Closes #15, #16, #17, and #19.