Adding comment trees. #2362
Adding comment trees. #2362
Conversation
dessalines
commented
Jul 14, 2022
dessalines
commented
- Extracted comment replies into its own table.
- Added ltree column to comment
- Added parent_id param to GetComments to fetch a tree branch
- No paging / limiting yet
- Extracted comment replies into its own table. - Added ltree column to comment - Added parent_id param to GetComments to fetch a tree branch - No paging / limiting yet
|
Okay I got this working finally! Demo in the lemmy-ui PR. Might be easy also to see the API changes here. This PR:
Some limitations:
|
| @@ -139,14 +86,71 @@ where ca.comment_id = c.id", | |||
| .get_result::<Self>(conn) | |||
| } | |||
|
|
|||
| pub fn upsert(conn: &PgConnection, comment_form: &CommentForm) -> Result<Comment, Error> { | |||
| pub fn create( | |||
There was a problem hiding this comment.
Should still be called upsert as thats what it does. Maybe consider having separate public methods for create/insert, and a private method which does the ltree related work.
There was a problem hiding this comment.
That'd mean that every Comment::create needs to be changed to comment::upsert, since they can and should do the same thing.
| .order_by(comment_aggregates::score.desc()), | ||
| CommentSortType::New => query.then_order_by(comment::published.desc()), | ||
| CommentSortType::Old => query.then_order_by(comment::published.asc()), | ||
| CommentSortType::Top => query.order_by(comment_aggregates::score.desc()), |
There was a problem hiding this comment.
Any particular reason to remove these sort options for comments? Should probably mention this in the changelog too once released.
There was a problem hiding this comment.
Yep we can do that in the changelog. Post SortType has things like Active and MostComments which don't make sense for comment sorting, so I made another type.
| | SortType::TopWeek | ||
| | SortType::TopYear | ||
| | SortType::TopMonth => CommentSortType::Top, | ||
| } |
There was a problem hiding this comment.
? The SortType is already imported at the top. I also prefer explicit match case handling above the default _, because if we add sorts later, we'll be forced to see them in the compiler.
There was a problem hiding this comment.
With that use statement, you can just write TopWeek instead of SortType::TopWeek etc, making the code shorter and easier to read.
