-
Notifications
You must be signed in to change notification settings - Fork 365
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
Remove reply button on old comments #2009
Conversation
Do other people have opinions on this being a pure UI change? |
It looks to be a trivial change in the backend, so let's do it instead of letting people deliberately necropost to "show their 1337 h4x0r skills". |
0cf694f
to
3b97d1c
Compare
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks fine.
if not (self.request.user.has_perm('judge.change_comment') or | ||
parent_comment.time > timezone.now() - settings.DMOJ_COMMENT_REPLY_TIMEFRAME): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be the only one, but I feel like
if timezone.now() - parent_comment.time > settings.DMOJ_COMMENT_REPLY_TIMEFRAME and not self.request.user.has_perm('judge.change_comment'):
Is the more readable way of writing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
fixes #823
judge.change_comment
.