mirror of
https://github.com/discourse/discourse.git
synced 2024-11-25 18:13:38 +08:00
SECURITY: add pagination to post replies
When a post has some replies, and the user click on the button to show them, we would load ALL the replies. This could lead to DoS if there were a very large number of replies. This adds support for pagination to these post replies. Internal ref t/129773
This commit is contained in:
Jan Cernik
Alan Guo Xiang Tan
parent
e9e9ae37a9
commit
cd9d0d7c17
|
|
@ -2,12 +2,9 @@ import RestAdapter from "discourse/adapters/rest";
|
|||
import { ajax } from "discourse/lib/ajax";
|
||||
|
||||
export default class PostReplyHistoryAdapter extends RestAdapter {
|
||||
find(store, type, findArgs) {
|
||||
const maxReplies = this.siteSettings.max_reply_history;
|
||||
return ajax(
|
||||
`/posts/${findArgs.postId}/reply-history?max_replies=${maxReplies}`
|
||||
).then((replies) => {
|
||||
return { post_reply_histories: replies };
|
||||
});
|
||||
find(_store, _type, { postId }) {
|
||||
return ajax(`/posts/${postId}/reply-history`).then(
|
||||
(post_reply_histories) => ({ post_reply_histories })
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,9 +2,9 @@ import RestAdapter from "discourse/adapters/rest";
|
|||
import { ajax } from "discourse/lib/ajax";
|
||||
|
||||
export default class PostReplyAdapter extends RestAdapter {
|
||||
find(store, type, findArgs) {
|
||||
return ajax(`/posts/${findArgs.postId}/replies`).then((replies) => {
|
||||
return { post_replies: replies };
|
||||
});
|
||||
find(_store, _type, { postId, after = 1 }) {
|
||||
return ajax(`/posts/${postId}/replies?after=${after || 1}`).then(
|
||||
(post_replies) => ({ post_replies })
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -100,10 +100,7 @@ registerButton("read-count", (attrs, state) => {
|
|||
if (attrs.showReadIndicator) {
|
||||
const count = attrs.readCount;
|
||||
if (count > 0) {
|
||||
let ariaPressed = "false";
|
||||
if (state?.readers && state.readers.length > 0) {
|
||||
ariaPressed = "true";
|
||||
}
|
||||
let ariaPressed = (state?.readers?.length > 0).toString();
|
||||
return {
|
||||
action: "toggleWhoRead",
|
||||
title: "post.controls.read_indicator",
|
||||
|
|
@ -151,10 +148,8 @@ function likeCount(attrs, state) {
|
|||
addContainer = true;
|
||||
}
|
||||
|
||||
let ariaPressed = "false";
|
||||
if (state?.likedUsers && state.likedUsers.length > 0) {
|
||||
ariaPressed = "true";
|
||||
}
|
||||
let ariaPressed = (state?.likedUsers?.length > 0).toString();
|
||||
|
||||
return {
|
||||
action: "toggleWhoLiked",
|
||||
title,
|
||||
|
|
@ -287,53 +282,52 @@ registerButton("wiki-edit", (attrs) => {
|
|||
});
|
||||
|
||||
registerButton("replies", (attrs, state, siteSettings) => {
|
||||
const replyCount = attrs.replyCount;
|
||||
if (!replyCount) {
|
||||
const count = attrs.replyCount;
|
||||
|
||||
if (!count) {
|
||||
return;
|
||||
}
|
||||
|
||||
let action = "toggleRepliesBelow",
|
||||
icon = state.repliesShown ? "chevron-up" : "chevron-down";
|
||||
let title;
|
||||
let action = "toggleRepliesBelow";
|
||||
let icon = state.repliesShown ? "chevron-up" : "chevron-down";
|
||||
|
||||
if (siteSettings.enable_filtered_replies_view) {
|
||||
action = "toggleFilteredRepliesView";
|
||||
icon = state.filteredRepliesShown ? "chevron-up" : "chevron-down";
|
||||
title = state.filteredRepliesShown
|
||||
? "post.view_all_posts"
|
||||
: "post.filtered_replies_hint";
|
||||
}
|
||||
|
||||
// Omit replies if the setting `suppress_reply_directly_below` is enabled
|
||||
if (
|
||||
replyCount === 1 &&
|
||||
count === 1 &&
|
||||
attrs.replyDirectlyBelow &&
|
||||
siteSettings.suppress_reply_directly_below
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
let ariaPressed;
|
||||
let ariaPressed, ariaExpanded;
|
||||
|
||||
if (!siteSettings.enable_filtered_replies_view) {
|
||||
ariaPressed = state.repliesShown ? "true" : "false";
|
||||
ariaPressed = state.repliesShown.toString();
|
||||
ariaExpanded = state.repliesShown.toString();
|
||||
}
|
||||
|
||||
return {
|
||||
action,
|
||||
icon,
|
||||
className: "show-replies",
|
||||
titleOptions: { count: replyCount },
|
||||
title: siteSettings.enable_filtered_replies_view
|
||||
? state.filteredRepliesShown
|
||||
? "post.view_all_posts"
|
||||
: "post.filtered_replies_hint"
|
||||
: "",
|
||||
labelOptions: { count: replyCount },
|
||||
titleOptions: { count },
|
||||
title,
|
||||
labelOptions: { count },
|
||||
label: attrs.mobileView ? "post.has_replies_count" : "post.has_replies",
|
||||
iconRight: !siteSettings.enable_filtered_replies_view || attrs.mobileView,
|
||||
disabled: !!attrs.deleted,
|
||||
translatedAriaLabel: I18n.t("post.sr_expand_replies", {
|
||||
count: replyCount,
|
||||
}),
|
||||
ariaExpanded:
|
||||
!siteSettings.enable_filtered_replies_view && state.repliesShown
|
||||
? "true"
|
||||
: "false",
|
||||
translatedAriaLabel: I18n.t("post.sr_expand_replies", { count }),
|
||||
ariaExpanded,
|
||||
ariaPressed,
|
||||
ariaControls: `embedded-posts__bottom--${attrs.post_number}`,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -31,9 +31,23 @@ import getURL, {
|
|||
import { iconNode } from "discourse-common/lib/icon-library";
|
||||
import I18n from "discourse-i18n";
|
||||
|
||||
function transformWithCallbacks(post) {
|
||||
function transformWithCallbacks(post, topicUrl, store) {
|
||||
let transformed = transformBasicPost(post);
|
||||
postTransformCallbacks(transformed);
|
||||
|
||||
// We don't want to overwrite CPs
|
||||
// We are doing something a bit weird here by creating a post object from a transformed post.
|
||||
// They aren't 100% the same.
|
||||
delete transformed.new_user;
|
||||
delete transformed.deleted;
|
||||
delete transformed.shareUrl;
|
||||
delete transformed.firstPost;
|
||||
delete transformed.usernameUrl;
|
||||
delete transformed.topicNotificationLevel;
|
||||
|
||||
transformed.customShare = `${topicUrl}/${post.post_number}`;
|
||||
transformed.asPost = store.createRecord("post", transformed);
|
||||
|
||||
return transformed;
|
||||
}
|
||||
|
||||
|
|
@ -162,12 +176,10 @@ createWidget("reply-to-tab", {
|
|||
tabindex: "0",
|
||||
};
|
||||
|
||||
if (!this.attrs.mobileView) {
|
||||
if (!attrs.mobileView) {
|
||||
result["role"] = "button";
|
||||
result["aria-controls"] = `embedded-posts__top--${attrs.post_number}`;
|
||||
result["aria-expanded"] = this.attrs.repliesAbove.length
|
||||
? "true"
|
||||
: "false";
|
||||
result["aria-expanded"] = (attrs.repliesAbove.length > 0).toString();
|
||||
}
|
||||
|
||||
return result;
|
||||
|
|
@ -526,7 +538,7 @@ createWidget("post-contents", {
|
|||
|
||||
const extraState = {
|
||||
state: {
|
||||
repliesShown: !!state.repliesBelow.length,
|
||||
repliesShown: state.repliesBelow.length > 0,
|
||||
filteredRepliesShown: state.filteredRepliesShown,
|
||||
},
|
||||
};
|
||||
|
|
@ -534,31 +546,49 @@ createWidget("post-contents", {
|
|||
|
||||
const repliesBelow = state.repliesBelow;
|
||||
if (repliesBelow.length) {
|
||||
let children = [];
|
||||
|
||||
repliesBelow.forEach((p) => {
|
||||
children.push(
|
||||
this.attach("embedded-post", p, {
|
||||
model: p.asPost,
|
||||
state: {
|
||||
role: "region",
|
||||
"aria-label": I18n.t("post.sr_embedded_reply_description", {
|
||||
post_number: attrs.post_number,
|
||||
username: p.username,
|
||||
}),
|
||||
},
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
children.push(
|
||||
this.attach("button", {
|
||||
title: "post.collapse",
|
||||
icon: "chevron-up",
|
||||
action: "toggleRepliesBelow",
|
||||
actionParam: true,
|
||||
className: "btn collapse-up",
|
||||
translatedAriaLabel: I18n.t("post.sr_collapse_replies"),
|
||||
})
|
||||
);
|
||||
|
||||
if (repliesBelow.length < this.attrs.replyCount) {
|
||||
children.push(
|
||||
this.attach("button", {
|
||||
label: "post.load_more_replies",
|
||||
action: "loadMoreReplies",
|
||||
actionParam: repliesBelow[repliesBelow.length - 1]?.post_number,
|
||||
className: "btn load-more-replies",
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
result.push(
|
||||
h(
|
||||
`section.embedded-posts.bottom#embedded-posts__bottom--${this.attrs.post_number}`,
|
||||
[
|
||||
repliesBelow.map((p) => {
|
||||
return this.attach("embedded-post", p, {
|
||||
model: p.asPost,
|
||||
state: {
|
||||
role: "region",
|
||||
"aria-label": I18n.t("post.sr_embedded_reply_description", {
|
||||
post_number: attrs.post_number,
|
||||
username: p.username,
|
||||
}),
|
||||
},
|
||||
});
|
||||
}),
|
||||
this.attach("button", {
|
||||
title: "post.collapse",
|
||||
icon: "chevron-up",
|
||||
action: "toggleRepliesBelow",
|
||||
actionParam: "true",
|
||||
className: "btn collapse-up",
|
||||
translatedAriaLabel: I18n.t("post.sr_collapse_replies"),
|
||||
}),
|
||||
]
|
||||
children
|
||||
)
|
||||
);
|
||||
}
|
||||
|
|
@ -599,40 +629,30 @@ createWidget("post-contents", {
|
|||
}
|
||||
},
|
||||
|
||||
toggleRepliesBelow(goToPost = "false") {
|
||||
if (this.state.repliesBelow.length) {
|
||||
this.state.repliesBelow = [];
|
||||
if (goToPost === "true") {
|
||||
DiscourseURL.routeTo(
|
||||
`${this.attrs.topicUrl}/${this.attrs.post_number}`
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const post = this.findAncestorModel();
|
||||
const topicUrl = post ? post.get("topic.url") : null;
|
||||
loadMoreReplies(after = 1) {
|
||||
return this.store
|
||||
.find("post-reply", { postId: this.attrs.id })
|
||||
.then((posts) => {
|
||||
this.state.repliesBelow = posts.map((p) => {
|
||||
let result = transformWithCallbacks(p);
|
||||
|
||||
// these would conflict with computed properties with identical names
|
||||
// in the post model if we kept them.
|
||||
delete result.new_user;
|
||||
delete result.deleted;
|
||||
delete result.shareUrl;
|
||||
delete result.firstPost;
|
||||
delete result.usernameUrl;
|
||||
|
||||
result.customShare = `${topicUrl}/${p.post_number}`;
|
||||
result.asPost = this.store.createRecord("post", result);
|
||||
return result;
|
||||
.find("post-reply", { postId: this.attrs.id, after })
|
||||
.then((replies) => {
|
||||
replies.forEach((reply) => {
|
||||
this.state.repliesBelow.push(
|
||||
transformWithCallbacks(reply, this.attrs.topicUrl, this.store)
|
||||
);
|
||||
});
|
||||
});
|
||||
},
|
||||
|
||||
toggleRepliesBelow(goToPost = false) {
|
||||
if (this.state.repliesBelow.length) {
|
||||
this.state.repliesBelow = [];
|
||||
if (goToPost === true) {
|
||||
const { topicUrl, post_number } = this.attrs;
|
||||
DiscourseURL.routeTo(`${topicUrl}/${post_number}`);
|
||||
}
|
||||
} else {
|
||||
return this.loadMoreReplies();
|
||||
}
|
||||
},
|
||||
|
||||
expandFirstPost() {
|
||||
const post = this.findAncestorModel();
|
||||
return post.expand().then(() => (this.state.expandedFirstPost = true));
|
||||
|
|
@ -807,7 +827,7 @@ createWidget("post-article", {
|
|||
title: "post.collapse",
|
||||
icon: "chevron-down",
|
||||
action: "toggleReplyAbove",
|
||||
actionParam: "true",
|
||||
actionParam: true,
|
||||
className: "btn collapse-down",
|
||||
}),
|
||||
replies,
|
||||
|
|
@ -843,7 +863,7 @@ createWidget("post-article", {
|
|||
return post ? post.get("topic.url") : null;
|
||||
},
|
||||
|
||||
toggleReplyAbove(goToPost = "false") {
|
||||
toggleReplyAbove(goToPost = false) {
|
||||
const replyPostNumber = this.attrs.reply_to_post_number;
|
||||
|
||||
if (this.siteSettings.enable_filtered_replies_view) {
|
||||
|
|
@ -868,10 +888,9 @@ createWidget("post-article", {
|
|||
|
||||
if (this.state.repliesAbove.length) {
|
||||
this.state.repliesAbove = [];
|
||||
if (goToPost === "true") {
|
||||
DiscourseURL.routeTo(
|
||||
`${this.attrs.topicUrl}/${this.attrs.post_number}`
|
||||
);
|
||||
if (goToPost === true) {
|
||||
const { topicUrl, post_number } = this.attrs;
|
||||
DiscourseURL.routeTo(`${topicUrl}/${post_number}`);
|
||||
}
|
||||
return Promise.resolve();
|
||||
} else {
|
||||
|
|
@ -879,22 +898,10 @@ createWidget("post-article", {
|
|||
return this.store
|
||||
.find("post-reply-history", { postId: this.attrs.id })
|
||||
.then((posts) => {
|
||||
this.state.repliesAbove = posts.map((p) => {
|
||||
let result = transformWithCallbacks(p);
|
||||
|
||||
// We don't want to overwrite CPs - we are doing something a bit weird
|
||||
// here by creating a post object from a transformed post. They aren't
|
||||
// 100% the same.
|
||||
delete result.new_user;
|
||||
delete result.deleted;
|
||||
delete result.shareUrl;
|
||||
delete result.firstPost;
|
||||
delete result.usernameUrl;
|
||||
delete result.topicNotificationLevel;
|
||||
|
||||
result.customShare = `${topicUrl}/${p.post_number}`;
|
||||
result.asPost = this.store.createRecord("post", result);
|
||||
return result;
|
||||
posts.forEach((post) => {
|
||||
this.state.repliesAbove.push(
|
||||
transformWithCallbacks(post, topicUrl, this.store)
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -205,7 +205,6 @@ pre.codeblock-buttons:hover {
|
|||
border: none;
|
||||
> div {
|
||||
position: relative;
|
||||
margin-bottom: 1.5em;
|
||||
&:last-of-type {
|
||||
margin-bottom: 0;
|
||||
.row {
|
||||
|
|
@ -296,6 +295,13 @@ pre.codeblock-buttons:hover {
|
|||
}
|
||||
}
|
||||
}
|
||||
.load-more-replies {
|
||||
font-size: var(--font-down-1);
|
||||
position: absolute;
|
||||
left: 55%;
|
||||
transform: translate(-50%, 50%);
|
||||
padding: 0.35em 0.5em;
|
||||
}
|
||||
|
||||
.topic-avatar {
|
||||
padding-left: 1em;
|
||||
|
|
|
|||
|
|
@ -217,6 +217,13 @@ nav.post-controls button.reply .d-icon {
|
|||
}
|
||||
}
|
||||
}
|
||||
.load-more-replies {
|
||||
font-size: var(--font-down-1);
|
||||
position: absolute;
|
||||
left: 50%;
|
||||
transform: translate(-50%, 150%);
|
||||
padding: 0.35em 0.5em;
|
||||
}
|
||||
}
|
||||
|
||||
.post-actions {
|
||||
|
|
|
|||
|
|
@ -310,13 +310,16 @@ class PostsController < ApplicationController
|
|||
def reply_history
|
||||
post = find_post_from_params
|
||||
|
||||
reply_history = post.reply_history(params[:max_replies].to_i, guardian)
|
||||
user_custom_fields = {}
|
||||
if (added_fields = User.allowed_user_custom_fields(guardian)).present?
|
||||
user_custom_fields = User.custom_fields_for_ids(reply_history.pluck(:user_id), added_fields)
|
||||
end
|
||||
topic_view =
|
||||
TopicView.new(
|
||||
post.topic,
|
||||
current_user,
|
||||
include_suggested: false,
|
||||
include_related: false,
|
||||
reply_history_for: post.id,
|
||||
)
|
||||
|
||||
render_serialized(reply_history, PostSerializer, user_custom_fields: user_custom_fields)
|
||||
render_json_dump(TopicViewPostsSerializer.new(topic_view, scope: guardian).post_stream[:posts])
|
||||
end
|
||||
|
||||
def reply_ids
|
||||
|
|
@ -426,17 +429,38 @@ class PostsController < ApplicationController
|
|||
render_json_error(e.message)
|
||||
end
|
||||
|
||||
# Direct replies to this post
|
||||
MAX_POST_REPLIES ||= 20
|
||||
|
||||
def replies
|
||||
params.permit(:after)
|
||||
|
||||
after = [params[:after].to_i, 1].max
|
||||
post = find_post_from_params
|
||||
replies = post.replies.secured(guardian)
|
||||
|
||||
user_custom_fields = {}
|
||||
if (added_fields = User.allowed_user_custom_fields(guardian)).present?
|
||||
user_custom_fields = User.custom_fields_for_ids(replies.pluck(:user_id), added_fields)
|
||||
post_ids =
|
||||
post
|
||||
.replies
|
||||
.secured(guardian)
|
||||
.where(post_number: after + 1..)
|
||||
.limit(MAX_POST_REPLIES)
|
||||
.pluck(:id)
|
||||
|
||||
if post_ids.blank?
|
||||
render_json_dump []
|
||||
else
|
||||
topic_view =
|
||||
TopicView.new(
|
||||
post.topic,
|
||||
current_user,
|
||||
post_ids:,
|
||||
include_related: false,
|
||||
include_suggested: false,
|
||||
)
|
||||
|
||||
render_json_dump(
|
||||
TopicViewPostsSerializer.new(topic_view, scope: guardian).post_stream[:posts],
|
||||
)
|
||||
end
|
||||
|
||||
render_serialized(replies, PostSerializer, user_custom_fields: user_custom_fields)
|
||||
end
|
||||
|
||||
def revisions
|
||||
|
|
@ -991,20 +1015,15 @@ class PostsController < ApplicationController
|
|||
# A deleted post can be seen by staff or a category group moderator for the topic.
|
||||
# But we must find the deleted post to determine which category it belongs to, so
|
||||
# we must find.with_deleted
|
||||
post = finder.with_deleted.first
|
||||
raise Discourse::NotFound unless post
|
||||
raise Discourse::NotFound unless post = finder.with_deleted.first
|
||||
raise Discourse::NotFound unless post.topic ||= Topic.with_deleted.find_by(id: post.topic_id)
|
||||
|
||||
post.topic = Topic.with_deleted.find_by(id: post.topic_id)
|
||||
|
||||
if !post.topic ||
|
||||
(
|
||||
(post.deleted_at.present? || post.topic.deleted_at.present?) &&
|
||||
!guardian.can_moderate_topic?(post.topic)
|
||||
)
|
||||
raise Discourse::NotFound
|
||||
if post.deleted_at.present? || post.topic.deleted_at.present?
|
||||
raise Discourse::NotFound unless guardian.can_moderate_topic?(post.topic)
|
||||
end
|
||||
|
||||
guardian.ensure_can_see!(post)
|
||||
|
||||
post
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -971,27 +971,6 @@ class Post < ActiveRecord::Base
|
|||
.count
|
||||
end
|
||||
|
||||
def reply_history(max_replies = 100, guardian = nil)
|
||||
post_ids = DB.query_single(<<~SQL, post_id: id, topic_id: topic_id)
|
||||
WITH RECURSIVE breadcrumb(id, reply_to_post_number) AS (
|
||||
SELECT p.id, p.reply_to_post_number FROM posts AS p
|
||||
WHERE p.id = :post_id
|
||||
UNION
|
||||
SELECT p.id, p.reply_to_post_number FROM posts AS p, breadcrumb
|
||||
WHERE breadcrumb.reply_to_post_number = p.post_number
|
||||
AND p.topic_id = :topic_id
|
||||
)
|
||||
SELECT id from breadcrumb
|
||||
WHERE id <> :post_id
|
||||
ORDER by id
|
||||
SQL
|
||||
|
||||
# [1,2,3][-10,-1] => nil
|
||||
post_ids = (post_ids[(0 - max_replies)..-1] || post_ids)
|
||||
|
||||
Post.secured(guardian).where(id: post_ids).includes(:user, :topic).order(:id).to_a
|
||||
end
|
||||
|
||||
MAX_REPLY_LEVEL ||= 1000
|
||||
|
||||
def reply_ids(guardian = nil, only_replies_to_single_post: true)
|
||||
|
|
|
|||
|
|
@ -3660,7 +3660,9 @@ en:
|
|||
show_hidden: "View ignored content."
|
||||
deleted_by_author_simple: "(post deleted by author)"
|
||||
collapse: "collapse"
|
||||
load_more_replies: "load more replies"
|
||||
sr_collapse_replies: "Collapse embedded replies"
|
||||
sr_load_more_replies: "Load more embedded replies"
|
||||
sr_date: "Post date"
|
||||
sr_expand_replies:
|
||||
one: "This post has %{count} reply"
|
||||
|
|
|
|||
|
|
@ -870,6 +870,29 @@ class TopicView
|
|||
Topic.with_deleted.includes(:category).find_by(id: topic_or_topic_id)
|
||||
end
|
||||
|
||||
def find_post_replies_ids(post_id)
|
||||
DB.query_single(<<~SQL, post_id: post_id)
|
||||
WITH RECURSIVE breadcrumb(id, reply_to_post_number, topic_id, level) AS (
|
||||
SELECT id, reply_to_post_number, topic_id, 0
|
||||
FROM posts
|
||||
WHERE id = :post_id
|
||||
|
||||
UNION
|
||||
|
||||
SELECT p.id, p.reply_to_post_number, p.topic_id, b.level + 1
|
||||
FROM posts AS p
|
||||
, breadcrumb AS b
|
||||
WHERE b.reply_to_post_number = p.post_number
|
||||
AND b.topic_id = p.topic_id
|
||||
AND b.level < #{SiteSetting.max_reply_history}
|
||||
)
|
||||
SELECT id
|
||||
FROM breadcrumb
|
||||
WHERE id <> :post_id
|
||||
ORDER BY id
|
||||
SQL
|
||||
end
|
||||
|
||||
def unfiltered_posts
|
||||
result = filter_post_types(@topic.posts)
|
||||
result = result.with_deleted if @guardian.can_see_deleted_posts?(@topic.category)
|
||||
|
|
@ -969,33 +992,23 @@ class TopicView
|
|||
)
|
||||
end
|
||||
|
||||
# Reply history
|
||||
if @reply_history_for.present?
|
||||
post_ids = find_post_replies_ids(@reply_history_for)
|
||||
|
||||
@filtered_posts = @filtered_posts.where("posts.id IN (:post_ids)", post_ids:)
|
||||
@contains_gaps = true
|
||||
end
|
||||
|
||||
# Filtering upwards
|
||||
if @filter_upwards_post_id.present?
|
||||
post = Post.find(@filter_upwards_post_id)
|
||||
post_ids = DB.query_single(<<~SQL, post_id: post.id, topic_id: post.topic_id)
|
||||
WITH RECURSIVE breadcrumb(id, reply_to_post_number) AS (
|
||||
SELECT p.id, p.reply_to_post_number FROM posts AS p
|
||||
WHERE p.id = :post_id
|
||||
UNION
|
||||
SELECT p.id, p.reply_to_post_number FROM posts AS p, breadcrumb
|
||||
WHERE breadcrumb.reply_to_post_number = p.post_number
|
||||
AND p.topic_id = :topic_id
|
||||
)
|
||||
SELECT id from breadcrumb
|
||||
WHERE id <> :post_id
|
||||
ORDER by id
|
||||
SQL
|
||||
|
||||
post_ids = (post_ids[(0 - SiteSetting.max_reply_history)..-1] || post_ids)
|
||||
post_ids.push(post.id)
|
||||
post_ids = find_post_replies_ids(@filter_upwards_post_id) | [@filter_upwards_post_id.to_i]
|
||||
|
||||
@filtered_posts =
|
||||
@filtered_posts.where(
|
||||
"
|
||||
posts.post_number = 1
|
||||
OR posts.id IN (:post_ids)
|
||||
OR posts.id > :max_post_id",
|
||||
{ post_ids: post_ids, max_post_id: post_ids.max },
|
||||
"posts.post_number = 1 OR posts.id IN (:post_ids) OR posts.id > :max_post_id",
|
||||
post_ids:,
|
||||
max_post_id: post_ids.max,
|
||||
)
|
||||
|
||||
@contains_gaps = true
|
||||
|
|
|
|||
|
|
@ -1027,20 +1027,6 @@ RSpec.describe Post do
|
|||
end
|
||||
end
|
||||
|
||||
describe "reply_history" do
|
||||
let!(:p1) { Fabricate(:post, post_args) }
|
||||
let!(:p2) { Fabricate(:post, post_args.merge(reply_to_post_number: p1.post_number)) }
|
||||
let!(:p3) { Fabricate(:post, post_args) }
|
||||
let!(:p4) { Fabricate(:post, post_args.merge(reply_to_post_number: p2.post_number)) }
|
||||
|
||||
it "returns the posts in reply to this post" do
|
||||
expect(p4.reply_history).to eq([p1, p2])
|
||||
expect(p4.reply_history(1)).to eq([p2])
|
||||
expect(p3.reply_history).to be_blank
|
||||
expect(p2.reply_history).to eq([p1])
|
||||
end
|
||||
end
|
||||
|
||||
describe "reply_ids" do
|
||||
fab!(:topic)
|
||||
let!(:p1) { Fabricate(:post, topic: topic, post_number: 1) }
|
||||
|
|
|
|||
|
|
@ -207,6 +207,31 @@ RSpec.describe PostsController do
|
|||
expect(json[0]["user_custom_fields"]["hello"]).to eq("world")
|
||||
expect(json[0]["user_custom_fields"]["hidden"]).to be_blank
|
||||
end
|
||||
|
||||
it "supports pagination" do
|
||||
parent = Fabricate(:post)
|
||||
30.times do
|
||||
reply = Fabricate(:post, topic: parent.topic, reply_to_post_number: parent.post_number)
|
||||
PostReply.create!(post: parent, reply:)
|
||||
end
|
||||
|
||||
get "/posts/#{parent.id}/replies.json", params: { after: parent.post_number }
|
||||
expect(response.status).to eq(200)
|
||||
replies = response.parsed_body
|
||||
expect(replies.size).to eq(20)
|
||||
|
||||
after = replies.last["post_number"]
|
||||
|
||||
get "/posts/#{parent.id}/replies.json", params: { after: }
|
||||
expect(response.status).to eq(200)
|
||||
replies = response.parsed_body
|
||||
expect(replies.size).to eq(10)
|
||||
expect(replies[0][:post_number]).to eq(after + 1)
|
||||
|
||||
get "/posts/#{parent.id}/replies.json", params: { after: 999_999 }
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.parsed_body.size).to eq(0)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#destroy" do
|
||||
|
|
|
|||
43
spec/system/page_objects/pages/post.rb
Normal file
43
spec/system/page_objects/pages/post.rb
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module PageObjects
|
||||
module Components
|
||||
class Post < PageObjects::Components::Base
|
||||
def initialize(post_number)
|
||||
@post_number = post_number
|
||||
end
|
||||
|
||||
def show_replies
|
||||
find("#post_#{@post_number} .show-replies").click
|
||||
end
|
||||
|
||||
def load_more_replies
|
||||
find("#embedded-posts__bottom--#{@post_number} .load-more-replies").click
|
||||
end
|
||||
|
||||
def has_replies?(count: nil)
|
||||
find("#embedded-posts__bottom--#{@post_number}").has_css?(".reply", count:)
|
||||
end
|
||||
|
||||
def has_more_replies?
|
||||
find("#embedded-posts__bottom--#{@post_number}").has_css?(".load-more-replies")
|
||||
end
|
||||
|
||||
def has_loaded_all_replies?
|
||||
find("#embedded-posts__bottom--#{@post_number}").has_no_css?(".load-more-replies")
|
||||
end
|
||||
|
||||
def show_parent_posts
|
||||
find("#post_#{@post_number} .reply-to-tab").click
|
||||
end
|
||||
|
||||
def has_parent_posts?(count: nil)
|
||||
find("#embedded-posts__top--#{@post_number}").has_css?(".reply", count:)
|
||||
end
|
||||
|
||||
def has_no_parent_post_content?(content)
|
||||
find("#embedded-posts__top--#{@post_number}").has_no_content?(content)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
60
spec/system/post_replies_spec.rb
Normal file
60
spec/system/post_replies_spec.rb
Normal file
|
|
@ -0,0 +1,60 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
describe "Post replies", type: :system do
|
||||
fab!(:user)
|
||||
fab!(:topic)
|
||||
fab!(:post) { Fabricate(:post, user:, topic:) }
|
||||
|
||||
let(:first_post) { PageObjects::Components::Post.new(1) }
|
||||
let(:third_chained_reply) { PageObjects::Components::Post.new(7) }
|
||||
|
||||
context "when loading post replies" do
|
||||
before do
|
||||
25.times do
|
||||
reply = Fabricate(:post, topic:, reply_to_post_number: 1)
|
||||
PostReply.create!(post:, reply:)
|
||||
end
|
||||
|
||||
post.update!(reply_count: post.replies.count)
|
||||
end
|
||||
|
||||
it "supports pagination" do
|
||||
sign_in(user)
|
||||
visit(topic.url)
|
||||
|
||||
first_post.show_replies
|
||||
|
||||
expect(first_post).to have_replies(count: 20)
|
||||
expect(first_post).to have_more_replies
|
||||
|
||||
first_post.load_more_replies
|
||||
|
||||
expect(first_post).to have_replies(count: post.replies.count)
|
||||
expect(first_post).to have_loaded_all_replies
|
||||
end
|
||||
end
|
||||
|
||||
context "when loading parent posts" do
|
||||
before do
|
||||
SiteSetting.max_reply_history = 2
|
||||
|
||||
3.times do |i|
|
||||
PostReply.create!(post:, reply: Fabricate(:post, topic:))
|
||||
|
||||
reply = Fabricate(:post, topic:, reply_to_post_number: (i * 2) + 1, raw: "reply #{i + 1}")
|
||||
PostReply.create!(post:, reply:)
|
||||
end
|
||||
|
||||
post.update!(reply_count: post.replies.count)
|
||||
end
|
||||
|
||||
it "does not duplicate replies" do
|
||||
sign_in(user)
|
||||
visit(topic.url)
|
||||
|
||||
third_chained_reply.show_parent_posts
|
||||
expect(third_chained_reply).to have_parent_posts(count: 2)
|
||||
expect(third_chained_reply).to have_no_parent_post_content("reply 3")
|
||||
end
|
||||
end
|
||||
end
|
||||
Loading…
Reference in New Issue
Block a user