Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Add new setting to force user edit last post. #6571

Merged
merged 1 commit into from Nov 14, 2018

Conversation

nbianca
Copy link
Member

@nbianca nbianca commented Nov 6, 2018

No description provided.

@discoursebot
Copy link

You've signed the CLA, nbianca. Thank you! This pull request is ready for review.

@coding-horror
Copy link
Contributor

coding-horror commented Nov 6, 2018

I am not sure this is what was discussed.. there should be a site setting for the maximum number of consecutive replies in the same topic by the same user?

https://meta.discourse.org/t/allow-only-x-consecutive-replies-in-the-same-topic/69756

So the alert after reaching the threshold would be

No more than {%n} consecutive replies are allowed. Please edit your previous reply instead, or wait for someone to reply to you.

@nbianca nbianca force-pushed the force_edit_last branch 2 times, most recently from 584a9f3 to c846c16 Compare November 9, 2018 17:10
@coding-horror
Copy link
Contributor

coding-horror commented Nov 9, 2018 via email

@nbianca nbianca force-pushed the force_edit_last branch 2 times, most recently from 954a6e0 to 40ee76e Compare November 13, 2018 10:26
@nbianca
Copy link
Member Author

nbianca commented Nov 13, 2018

I do not think it's possible to write a query that is doing this and is also readable. In theory, the best query should check if the last X posts are all made by the same user AND then fetch only the last one.

What I do now is:

  • fetch at most SiteSetting.max_consecutive_replies user IDs (which is not that much) that have last posted in the current topic
  • iterate over those IDs in Ruby and count how many of them were made by current user (usually a few cheap iterations)
  • if user has reached the threshold, do another query to fetch the last post that would be passed to the Guardian

If we consider that in most communities users will not post many consecutive replies, in that case this validator is going to query the database only once and the result will usually be small (few integers).

@ZogStriP
Copy link
Member

In theory, the best query should check if the last X posts are all made by the same user AND then fetch only the last one.

No, that's not what I meant. I meant you could do one query to "check whether the last poster has made X consecutive replies" ;)

@nbianca
Copy link
Member Author

nbianca commented Nov 13, 2018

No, that's not what I meant. I meant you could do one query to "check whether the last poster has made X consecutive replies" ;)

Yes, I understand that. That would be the "gaps and islands" problem and I believe the query will be way too complicated and the benefit will be very small. I will look into it.

@coding-horror
Copy link
Contributor

coding-horror commented Nov 13, 2018 via email

@ZogStriP
Copy link
Member

ZogStriP commented Nov 13, 2018

Exactly, we only care about knowing whether the last X posts in a topic were done by the current user.

Hence, me recommending we can do this in one SQL query ;)

@nbianca
Copy link
Member Author

nbianca commented Nov 13, 2018

I was considering current user's posts as "islands" and other posts as "gaps" and it kind of made sense. Since the maximum length of an island is known (SiteSetting.max_consecutive_replies), I was able to simplify the query.

I believe it should be good.

@nbianca nbianca force-pushed the force_edit_last branch 2 times, most recently from 6212a2e to 328f2b4 Compare November 14, 2018 13:31
@ZogStriP ZogStriP merged commit b6576d9 into discourse:master Nov 14, 2018
@nbianca nbianca deleted the force_edit_last branch July 2, 2019 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants