Closed Bug 975943 Opened 10 years ago Closed 10 years ago

add special support for a "deleted" comment tag

Categories

(bugzilla.mozilla.org :: General, defect)

Production
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: glob, Assigned: glob)

References

Details

Attachments

(1 file)

we need a way to delete comments in bulk when bmo is attacked by a spammer.
to do this we'll add special processing for a "deleted" comment tag.

comments tagged as "deleted" wouldn't actually be removed from the database, as this offsets comment numbering causing issues.  instead the comment will be marked as private and bug.comments() will be updated to no longer return "deleted" comments.

using a comment tag means:
- don't need to develop a new ui
- can 'delete' comments in bug via the api
- if you know a comment's id, you'd be able to 'undelete' a comment via the api
- can extend ability to 'delete' comments beyond just admins with group membership
Attached patch 975943_1.patchSplinter Review
marking the comment as private wasn't viable as i'd have to update the value of the 'is private' checkbox on show_bug at the same time as adding the tag.  instead i updated the sanitisation code.
Attachment #8380479 - Flags: review?(dkl)
bug.comments() surely needs to return a blank comment rather than nothing for deleted comments, otherwise remote clients will also have comment numbering issues?

Gerv
(In reply to Gervase Markham [:gerv] from comment #2)
> bug.comments() surely needs to return a blank comment rather than nothing
> for deleted comments, otherwise remote clients will also have comment
> numbering issues?

the code doesn't "return nothing" for deleted comments -- deleted comments are not returned at all.
private comments currently result in gaps in the comment numbers, so that isn't an issue.
Comment on attachment 8380479 [details] [diff] [review]
975943_1.patch

Review of attachment 8380479 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and works as expected except for for the couple changes mentioned which can be applied on commit. I did not try the sanitizeme.pl script althiough it looks correct. r=dkl

::: extensions/BMO/Extension.pm
@@ +1395,5 @@
> +}
> +
> +sub bug_comments {
> +    my ($self, $args) = @_;
> +    my $comments = $args->{comments};

I think we should still show the deleted comments (maybe have them collapsed by default) if the current user is in the delete_comments_group.

    my $group_name = Bugzilla->params->{delete_comments_group};
    return if Bugzilla->user->in_group($group_name);

::: extensions/BMO/template/en/default/hook/global/user-error-auth_failure_object.html.tmpl
@@ +11,5 @@
>  [% ELSIF object == 'email_queue' %]
>    the email queue status report
>  [% ELSIF object == 'product_security' %]
>    the product security report
> +[% ELSIF object == 'comment' %]

s/comment/comments/ or this part never matches.
Attachment #8380479 - Flags: review?(dkl) → review+
(In reply to David Lawrence [:dkl] from comment #4) 
> ::: extensions/BMO/Extension.pm
> @@ +1395,5 @@
> > +}
> > +
> > +sub bug_comments {
> > +    my ($self, $args) = @_;
> > +    my $comments = $args->{comments};
> 
> I think we should still show the deleted comments (maybe have them collapsed
> by default) if the current user is in the delete_comments_group.
> 
>     my $group_name = Bugzilla->params->{delete_comments_group};
>     return if Bugzilla->user->in_group($group_name);

Another reason I forgot to add before that I would like this to be added, how would we undelete a comment that has been marked as deleted otherwise without DBA intervention?

dkl
(In reply to David Lawrence [:dkl] from comment #5)
> Another reason I forgot to add before that I would like this to be added,
> how would we undelete a comment that has been marked as deleted otherwise
> without DBA intervention?

if you know a comment's id, you'd be able to 'undelete' a comment via the api.  a DBA isn't required to determine a comment's id.
both review points addressed..

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified Bugzilla/Bug.pm
modified Bugzilla/Comment.pm
modified contrib/sanitizeme.pl
modified extensions/BMO/Extension.pm
added extensions/BMO/template/en/default/hook/admin/params
added extensions/BMO/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl
modified extensions/BMO/template/en/default/hook/global/user-error-auth_failure_object.html.tmpl
Committed revision 9248.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: