alexames / DeltaBot

GNU General Public License v3.0
65 stars 18 forks source link

New/No OP Reply system (Pull this one, not the other one) #50

Closed Pluckerpluck closed 10 years ago

Pluckerpluck commented 10 years ago

This pull request adds the No OP Replies system. Please test this thoroughly yourself as there's only so much testing a single person can do.

Basically all submissions are marked with the "new_flair" (if they are young enough). This MUST contain a css_class but the text can be blank if you wish it to be (so a hidden flair basically).

When an OP replies a hidden css_class is incremented. This allows the number of replies to be stored online without worrying about keeping count locally.

After a set amount of time the new tag is removed and the comment is check to see if it has more repelies than "replies_required". If so the new tag is removed (though the number of replies is still secretly counted in case you ever want to use this in another way). If not they are marked with the "no_op_replies_flair".

If marked as such, replying enough times will remove the flair.

As far as I can tell this should have 1 API call for the thread search and 2 API call per OP reply (necessary to get the submission + update the flair). So it shouldn't be too bad.

chrisuehlinger commented 10 years ago

Looks great, and thanks for managing the side effects well!

Pluckerpluck commented 10 years ago

Edit: Turns out you can commit changes online when they're still in a pull request. I have made the change so it should work now.


OH GOD! There is a fatal flaw in this! (Quick fix, someone else will need to do it as I'm busy)

check_for_OP_reply() returns a submission even if it shouldn't be incremented. Basically every reply is an OP reply.

the returned submission should be None if it's not an OP reply.

That being said I can't do this as I'm super busy (not enough time to deal with git). It's a really quick fix though.

def check_for_OP_reply(self, comment):
    log = "Not OP, reply count not increased"
    submission = None
    if comment.link_author == comment.author.name:
        log = "OP has replied, increasing reply count"
        submission = comment.submission
    return (log, submission)

Also, it's probably worth checking in this function if comment.parent_id belongs to a comment or a submission (based on the id type). We probably don't want to count top level replies.

PixelOrange commented 10 years ago

This pull can't be automatically merged because of the last pull anyway. I will probably manually implement it and include your fix at the same time.

On Feb 4, 2014, at 12:26 PM, Sammy Gillespie notifications@github.com wrote:

OH GOD!There is a fatal flaw in this! (Quick fix, someone else will need to do it as I'm busy)

check_for_OP_reply() returns a submission even if it shouldn't be incremented. Basically every reply is an OP reply.

the returned submission should be None if it's not an OP reply.

That being said I can't do this as I'm super busy (not enough time to deal with git). It's a really quick fix though.

def check_for_OP_reply(self, comment): log = "Not OP, reply count not increased" submission = None if comment.link_author == comment.author.name: log = "OP has replied, increasing reply count" submission = comment.submission return (log, submission) Also, it's probably worth checking in this function if comment.parent_id belongs to a comment or a submission (based on the id type). We probably don't want to count top level replies.

— Reply to this email directly or view it on GitHub.

PixelOrange commented 10 years ago

Closed due to issue #58