From ba0765bf4dc468815e4fa45509010c0cd675e5b2 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Mon, 25 Nov 2013 16:21:03 +0800 Subject: Bug 793963: add the ability to tag comments with arbitrary tags r=dkl, a=glob --- Bugzilla/Bug.pm | 33 +- Bugzilla/Comment.pm | 195 ++++++++++- Bugzilla/Comment/TagWeights.pm | 74 ++++ Bugzilla/Config/BugFields.pm | 8 +- Bugzilla/Config/Common.pm | 14 + Bugzilla/Config/GroupSecurity.pm | 11 +- Bugzilla/Constants.pm | 7 + Bugzilla/DB/Schema.pm | 48 +++ Bugzilla/Field.pm | 1 + Bugzilla/User.pm | 17 + Bugzilla/WebService/Bug.pm | 211 +++++++++++- Bugzilla/WebService/Constants.pm | 7 +- Bugzilla/WebService/Server/REST/Resources/Bug.pm | 16 + js/comment-tagging.js | 376 +++++++++++++++++++++ js/comments.js | 19 +- js/util.js | 25 +- show_activity.cgi | 2 +- skins/standard/show_bug.css | 39 +++ .../en/default/admin/params/bugfields.html.tmpl | 6 +- .../default/admin/params/groupsecurity.html.tmpl | 3 + template/en/default/bug/comments.html.tmpl | 39 ++- template/en/default/bug/edit.html.tmpl | 25 ++ template/en/default/bug/show-header.html.tmpl | 3 + template/en/default/global/user-error.html.tmpl | 27 +- 24 files changed, 1168 insertions(+), 38 deletions(-) create mode 100644 Bugzilla/Comment/TagWeights.pm create mode 100644 js/comment-tagging.js diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 1e82da30d..9669352cd 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -3822,7 +3822,7 @@ sub _bugs_in_order { # Get the activity of a bug, starting from $starttime (if given). # This routine assumes Bugzilla::Bug->check has been previously called. sub get_activity { - my ($self, $attach_id, $starttime) = @_; + my ($self, $attach_id, $starttime, $include_comment_tags) = @_; my $dbh = Bugzilla->dbh; my $user = Bugzilla->user; @@ -3834,7 +3834,7 @@ sub get_activity { if (defined $starttime) { trick_taint($starttime); push (@args, $starttime); - $datepart = "AND bugs_activity.bug_when > ?"; + $datepart = "AND bug_when > ?"; } my $attachpart = ""; @@ -3854,7 +3854,7 @@ sub get_activity { my $query = "SELECT fielddefs.name, bugs_activity.attach_id, " . $dbh->sql_date_format('bugs_activity.bug_when', '%Y.%m.%d %H:%i:%s') . - ", bugs_activity.removed, bugs_activity.added, profiles.login_name, + " AS bug_when, bugs_activity.removed, bugs_activity.added, profiles.login_name, bugs_activity.comment_id FROM bugs_activity $suppjoins @@ -3865,8 +3865,31 @@ sub get_activity { WHERE bugs_activity.bug_id = ? $datepart $attachpart - $suppwhere - ORDER BY bugs_activity.bug_when, bugs_activity.id"; + $suppwhere "; + + if (Bugzilla->params->{'comment_taggers_group'} + && $include_comment_tags + && !$attach_id) + { + $query .= " + UNION ALL + SELECT 'comment_tag' AS name, + NULL AS attach_id," . + $dbh->sql_date_format('longdescs_tags_activity.bug_when', '%Y.%m.%d %H:%i:%s') . " AS bug_when, + longdescs_tags_activity.removed, + longdescs_tags_activity.added, + profiles.login_name, + longdescs_tags_activity.comment_id as comment_id + FROM longdescs_tags_activity + INNER JOIN profiles ON profiles.userid = longdescs_tags_activity.who + WHERE longdescs_tags_activity.bug_id = ? + $datepart + "; + push @args, $self->id; + push @args, $starttime if defined $starttime; + } + + $query .= "ORDER BY bug_when, comment_id"; my $list = $dbh->selectall_arrayref($query, undef, @args); diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm index d4e3383ea..d1e1e2530 100644 --- a/Bugzilla/Comment.pm +++ b/Bugzilla/Comment.pm @@ -13,11 +13,13 @@ use strict; use parent qw(Bugzilla::Object); use Bugzilla::Attachment; +use Bugzilla::Comment::TagWeights; use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::User; use Bugzilla::Util; +use List::Util qw(first); use Scalar::Util qw(blessed); ############################### @@ -79,21 +81,90 @@ use constant VALIDATOR_DEPENDENCIES => { sub update { my $self = shift; - my $changes = $self->SUPER::update(@_); - $self->bug->_sync_fulltext( update_comments => 1); + my ($changes, $old_comment) = $self->SUPER::update(@_); + + if (exists $changes->{'thetext'} || exists $changes->{'isprivate'}) { + $self->bug->_sync_fulltext( update_comments => 1); + } + + my @old_tags = @{ $old_comment->tags }; + my @new_tags = @{ $self->tags }; + my ($removed_tags, $added_tags) = diff_arrays(\@old_tags, \@new_tags); + + if (@$removed_tags || @$added_tags) { + my $dbh = Bugzilla->dbh; + my $when = $dbh->selectrow_array("SELECT LOCALTIMESTAMP(0)"); + my $sth_delete = $dbh->prepare( + "DELETE FROM longdescs_tags WHERE comment_id = ? AND tag = ?" + ); + my $sth_insert = $dbh->prepare( + "INSERT INTO longdescs_tags(comment_id, tag) VALUES (?, ?)" + ); + my $sth_activity = $dbh->prepare( + "INSERT INTO longdescs_tags_activity + (bug_id, comment_id, who, bug_when, added, removed) + VALUES (?, ?, ?, ?, ?, ?)" + ); + + foreach my $tag (@$removed_tags) { + my $weighted = Bugzilla::Comment::TagWeights->new({ name => $tag }); + if ($weighted) { + if ($weighted->weight == 1) { + $weighted->remove_from_db(); + } else { + $weighted->set_weight($weighted->weight - 1); + $weighted->update(); + } + } + trick_taint($tag); + $sth_delete->execute($self->id, $tag); + $sth_activity->execute( + $self->bug_id, $self->id, Bugzilla->user->id, $when, '', $tag); + } + + foreach my $tag (@$added_tags) { + my $weighted = Bugzilla::Comment::TagWeights->new({ name => $tag }); + if ($weighted) { + $weighted->set_weight($weighted->weight + 1); + $weighted->update(); + } else { + Bugzilla::Comment::TagWeights->create({ tag => $tag, weight => 1 }); + } + trick_taint($tag); + $sth_insert->execute($self->id, $tag); + $sth_activity->execute( + $self->bug_id, $self->id, Bugzilla->user->id, $when, $tag, ''); + } + } + return $changes; } -# Speeds up displays of comment lists by loading all ->author objects -# at once for a whole list. +# Speeds up displays of comment lists by loading all author objects and tags at +# once for a whole list. sub preload { my ($class, $comments) = @_; + # Author my %user_ids = map { $_->{who} => 1 } @$comments; my $users = Bugzilla::User->new_from_list([keys %user_ids]); my %user_map = map { $_->id => $_ } @$users; foreach my $comment (@$comments) { $comment->{author} = $user_map{$comment->{who}}; } + # Tags + if (Bugzilla->params->{'comment_taggers_group'}) { + my $dbh = Bugzilla->dbh; + my @comment_ids = map { $_->id } @$comments; + my %comment_map = map { $_->id => $_ } @$comments; + my $rows = $dbh->selectall_arrayref( + "SELECT comment_id, " . $dbh->sql_group_concat('tag', "','") . " + FROM longdescs_tags + WHERE " . $dbh->sql_in('comment_id', \@comment_ids) . " + GROUP BY comment_id"); + foreach my $row (@$rows) { + $comment_map{$row->[0]}->{tags} = [ split(/,/, $row->[1]) ]; + } + } } ############################### @@ -113,6 +184,39 @@ sub work_time { sub type { return $_[0]->{'type'}; } sub extra_data { return $_[0]->{'extra_data'} } +sub tags { + my ($self) = @_; + return [] unless Bugzilla->params->{'comment_taggers_group'}; + $self->{'tags'} ||= Bugzilla->dbh->selectcol_arrayref( + "SELECT tag + FROM longdescs_tags + WHERE comment_id = ? + ORDER BY tag", + undef, $self->id); + return $self->{'tags'}; +} + +sub collapsed { + my ($self) = @_; + return 0 unless Bugzilla->params->{'comment_taggers_group'}; + return $self->{collapsed} if exists $self->{collapsed}; + $self->{collapsed} = 0; + Bugzilla->request_cache->{comment_tags_collapsed} + ||= [ split(/\s*,\s*/, Bugzilla->params->{'collapsed_comment_tags'}) ]; + my @collapsed_tags = @{ Bugzilla->request_cache->{comment_tags_collapsed} }; + foreach my $my_tag (@{ $self->tags }) { + $my_tag = lc($my_tag); + foreach my $collapsed_tag (@collapsed_tags) { + if ($my_tag eq lc($collapsed_tag)) { + $self->{collapsed} = 1; + last; + } + } + last if $self->{collapsed}; + } + return $self->{collapsed}; +} + sub bug { my $self = shift; require Bugzilla::Bug; @@ -170,6 +274,26 @@ sub set_is_private { $_[0]->set('isprivate', $_[1]); } sub set_type { $_[0]->set('type', $_[1]); } sub set_extra_data { $_[0]->set('extra_data', $_[1]); } +sub add_tag { + my ($self, $tag) = @_; + $tag = $self->_check_tag($tag); + + my $tags = $self->tags; + return if grep { lc($tag) eq lc($_) } @$tags; + push @$tags, $tag; + $self->{'tags'} = [ sort @$tags ]; +} + +sub remove_tag { + my ($self, $tag) = @_; + $tag = $self->_check_tag($tag); + + my $tags = $self->tags; + my $index = first { lc($tags->[$_]) eq lc($tag) } 0..scalar(@$tags) - 1; + return unless defined $index; + splice(@$tags, $index, 1); +} + ############## # Validators # ############## @@ -312,6 +436,17 @@ sub _check_isprivate { return $isprivate ? 1 : 0; } +sub _check_tag { + my ($invocant, $tag) = @_; + length($tag) < MIN_COMMENT_TAG_LENGTH + and ThrowUserError('comment_tag_too_short', { tag => $tag }); + length($tag) > MAX_COMMENT_TAG_LENGTH + and ThrowUserError('comment_tag_too_long', { tag => $tag }); + $tag =~ /^[\w\d\._-]+$/ + or ThrowUserError('comment_tag_invalid', { tag => $tag }); + return $tag; +} + sub count { my ($self) = @_; @@ -326,7 +461,7 @@ sub count { undef, $self->bug_id, $self->creation_ts); return --$self->{'count'}; -} +} 1; @@ -372,7 +507,7 @@ C Time spent as related to this comment. =item C -C Comment is marked as private +C Comment is marked as private. =item C @@ -387,6 +522,54 @@ L who created the comment. C The position this comment is located in the full list of comments for a bug starting from 0. +=item C + +C Comment should be displayed as collapsed by default. + +=item C + +C The tags attached to the comment. + +=item C + +=over + +=item B + +Attaches the specified tag to the comment. + +=item B + +=over + +=item C + +C The tag to attach. + +=back + +=back + +=item C + +=over + +=item B + +Detaches the specified tag from the comment. + +=item B + +=over + +=item C + +C The tag to detach. + +=back + +=back + =item C =over diff --git a/Bugzilla/Comment/TagWeights.pm b/Bugzilla/Comment/TagWeights.pm new file mode 100644 index 000000000..5835efbc4 --- /dev/null +++ b/Bugzilla/Comment/TagWeights.pm @@ -0,0 +1,74 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Comment::TagWeights; + +use 5.10.1; +use strict; + +use parent qw(Bugzilla::Object); + +use Bugzilla::Constants; + +# No auditing required +use constant AUDIT_CREATES => 0; +use constant AUDIT_UPDATES => 0; +use constant AUDIT_REMOVES => 0; + +use constant DB_COLUMNS => qw( + id + tag + weight +); + +use constant UPDATE_COLUMNS => qw( + weight +); + +use constant DB_TABLE => 'longdescs_tags_weights'; +use constant ID_FIELD => 'id'; +use constant NAME_FIELD => 'tag'; +use constant LIST_ORDER => 'weight DESC'; +use constant VALIDATORS => { }; + +sub tag { return $_[0]->{'tag'} } +sub weight { return $_[0]->{'weight'} } + +sub set_weight { $_[0]->set('weight', $_[1]); } + +1; + +=head1 NAME + +Comment::TagWeights - Bugzilla comment weighting class. + +=head1 DESCRIPTION + +TagWeights.pm represents a Comment::TagWeight object. It is an implementation +of L, and thus provides all methods that L +provides. + +TagWeights is used to quickly find tags and order by their usage count. + +=head1 PROPERTIES + +=over + +=item C + +C The tag + +=item C + +C The tag's weight. The value returned corresponds to the number of +comments with this tag attached. + +=item C + +C Set the tag's weight. + +=back diff --git a/Bugzilla/Config/BugFields.pm b/Bugzilla/Config/BugFields.pm index e24f75661..39c43cb92 100644 --- a/Bugzilla/Config/BugFields.pm +++ b/Bugzilla/Config/BugFields.pm @@ -84,7 +84,13 @@ sub get_param_list { choices => ['', @legal_OS], default => '', checker => \&check_opsys - } ); + }, + + { + name => 'collapsed_comment_tags', + type => 't', + default => 'obsolete, spam', + }); return @param_list; } diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index 5872ae1e1..96d7c1c89 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -28,6 +28,7 @@ use parent qw(Exporter); check_mail_delivery_method check_notification check_utf8 check_bug_status check_smtp_auth check_theschwartz_available check_maxattachmentsize check_email check_smtp_ssl + check_comment_taggers_group ); # Checking functions for the various values @@ -367,6 +368,14 @@ sub check_theschwartz_available { return ""; } +sub check_comment_taggers_group { + my $group_name = shift; + if ($group_name && !Bugzilla->feature('jsonrpc')) { + return "Comment tagging requires installation of the JSONRPC feature"; + } + return check_group($group_name); +} + # OK, here are the parameter definitions themselves. # # Each definition is a hash with keys: @@ -465,6 +474,11 @@ Checks that the value is a valid number Checks that the value is a valid regexp +=item C + +Checks that the required modules for comment tagging are installed, and that a +valid group is provided. + =back =head1 B diff --git a/Bugzilla/Config/GroupSecurity.pm b/Bugzilla/Config/GroupSecurity.pm index d57573de3..1d3878415 100644 --- a/Bugzilla/Config/GroupSecurity.pm +++ b/Bugzilla/Config/GroupSecurity.pm @@ -56,7 +56,15 @@ sub get_param_list { default => 'editbugs', checker => \&check_group }, - + + { + name => 'comment_taggers_group', + type => 's', + choices => \&_get_all_group_names, + default => 'editbugs', + checker => \&check_comment_taggers_group + }, + { name => 'debug_group', type => 's', @@ -84,4 +92,5 @@ sub _get_all_group_names { unshift(@group_names, ''); return \@group_names; } + 1; diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 60c7a34e0..de045273b 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -69,6 +69,9 @@ use Memoize; COMMENT_COLS MAX_COMMENT_LENGTH + MIN_COMMENT_TAG_LENGTH + MAX_COMMENT_TAG_LENGTH + CMT_NORMAL CMT_DUPE_OF CMT_HAS_DUPE @@ -291,6 +294,10 @@ use constant COMMENT_COLS => 80; # Used in _check_comment(). Gives the max length allowed for a comment. use constant MAX_COMMENT_LENGTH => 65535; +# The minimum and maximum length of comment tags. +use constant MIN_COMMENT_TAG_LENGTH => 3; +use constant MAX_COMMENT_TAG_LENGTH => 24; + # The type of bug comments. use constant CMT_NORMAL => 0; use constant CMT_DUPE_OF => 1; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index c619a5780..30ed55b9a 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -406,6 +406,54 @@ use constant ABSTRACT_SCHEMA => { ], }, + longdescs_tags => { + FIELDS => [ + id => { TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1 }, + comment_id => { TYPE => 'INT4', + REFERENCES => { TABLE => 'longdescs', + COLUMN => 'comment_id', + DELETE => 'CASCADE' }}, + tag => { TYPE => 'varchar(24)', NOTNULL => 1 }, + ], + INDEXES => [ + longdescs_tags_idx => { FIELDS => ['comment_id', 'tag'], TYPE => 'UNIQUE' }, + ], + }, + + longdescs_tags_weights => { + FIELDS => [ + id => { TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1 }, + tag => { TYPE => 'varchar(24)', NOTNULL => 1 }, + weight => { TYPE => 'INT3', NOTNULL => 1 }, + ], + INDEXES => [ + longdescs_tags_weights_tag_idx => { FIELDS => ['tag'], TYPE => 'UNIQUE' }, + ], + }, + + longdescs_tags_activity => { + FIELDS => [ + id => { TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1 }, + bug_id => { TYPE => 'INT3', NOTNULL => 1, + REFERENCES => { TABLE => 'bugs', + COLUMN => 'bug_id', + DELETE => 'CASCADE' }}, + comment_id => { TYPE => 'INT4', + REFERENCES => { TABLE => 'longdescs', + COLUMN => 'comment_id', + DELETE => 'CASCADE' }}, + who => { TYPE => 'INT3', NOTNULL => 1, + REFERENCES => { TABLE => 'profiles', + COLUMN => 'userid' }}, + bug_when => { TYPE => 'DATETIME', NOTNULL => 1 }, + added => { TYPE => 'varchar(24)' }, + removed => { TYPE => 'varchar(24)' }, + ], + INDEXES => [ + longdescs_tags_activity_bug_id_idx => ['bug_id'], + ], + }, + dependencies => { FIELDS => [ blocked => {TYPE => 'INT3', NOTNULL => 1, diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index cda252542..82c82161a 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -255,6 +255,7 @@ use constant DEFAULT_FIELDS => ( type => FIELD_TYPE_BUG_URLS}, {name => 'tag', desc => 'Tags', buglist => 1, type => FIELD_TYPE_KEYWORDS}, + {name => 'comment_tag', desc => 'Comment Tag'}, ); ################ diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 84487dbbe..7a067fce0 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -1852,6 +1852,17 @@ sub is_timetracker { return $self->{'is_timetracker'}; } +sub can_tag_comments { + my $self = shift; + + if (!defined $self->{'can_tag_comments'}) { + my $group = Bugzilla->params->{'comment_taggers_group'}; + $self->{'can_tag_comments'} = + ($group && $self->in_group($group)) ? 1 : 0; + } + return $self->{'can_tag_comments'}; +} + sub get_userlist { my $self = shift; @@ -2648,6 +2659,12 @@ i.e. if the 'insidergroup' parameter is set and the user belongs to this group. Returns true if the user is a global watcher, i.e. if the 'globalwatchers' parameter contains the user. +=item C + +Returns true if the user can attach tags to comments. +i.e. if the 'comment_taggers_group' parameter is set and the user belongs to +this group. + =back =head1 CLASS FUNCTIONS diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 598316c59..7f012160c 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -13,6 +13,7 @@ use strict; use parent qw(Bugzilla::WebService); use Bugzilla::Comment; +use Bugzilla::Comment::TagWeights; use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Field; @@ -20,7 +21,7 @@ use Bugzilla::WebService::Constants; use Bugzilla::WebService::Util qw(filter filter_wants validate translate); use Bugzilla::Bug; use Bugzilla::BugMail; -use Bugzilla::Util qw(trick_taint trim diff_arrays); +use Bugzilla::Util qw(trick_taint trim diff_arrays detaint_natural); use Bugzilla::Version; use Bugzilla::Milestone; use Bugzilla::Status; @@ -320,7 +321,8 @@ sub _translate_comment { my ($self, $comment, $filters) = @_; my $attach_id = $comment->is_about_attachment ? $comment->extra_data : undef; - return filter $filters, { + + my $comment_hash = { id => $self->type('int', $comment->id), bug_id => $self->type('int', $comment->bug_id), creator => $self->type('email', $comment->author->login), @@ -332,6 +334,16 @@ sub _translate_comment { attachment_id => $self->type('int', $attach_id), count => $self->type('int', $comment->count), }; + + # Don't load comment tags unless enabled + if (Bugzilla->params->{'comment_taggers_group'}) { + $comment_hash->{tags} = [ + map { $self->type('string', $_) } + @{ $comment->tags } + ]; + } + + return filter $filters, $comment_hash; } sub get { @@ -999,6 +1011,70 @@ sub update_tags { return { changes => \%changes }; } +sub update_comment_tags { + my ($self, $params) = @_; + + my $user = Bugzilla->login(LOGIN_REQUIRED); + Bugzilla->params->{'comment_taggers_group'} + || ThrowUserError("comment_tag_disabled"); + $user->can_tag_comments + || ThrowUserError("auth_failure", + { group => Bugzilla->params->{'comment_taggers_group'}, + action => "update", + object => "comment_tags" }); + + my $comment_id = $params->{comment_id} + // ThrowCodeError('param_required', + { function => 'Bug.update_comment_tags', + param => 'comment_id' }); + + my $comment = Bugzilla::Comment->new($comment_id) + || return []; + $comment->bug->check_is_visible(); + if ($comment->is_private && !$user->is_insider) { + ThrowUserError('comment_is_private', { id => $comment_id }); + } + + my $dbh = Bugzilla->dbh; + $dbh->bz_start_transaction(); + foreach my $tag (@{ $params->{add} || [] }) { + $comment->add_tag($tag) if defined $tag; + } + foreach my $tag (@{ $params->{remove} || [] }) { + $comment->remove_tag($tag) if defined $tag; + } + $comment->update(); + $dbh->bz_commit_transaction(); + + return $comment->tags; +} + +sub search_comment_tags { + my ($self, $params) = @_; + + Bugzilla->login(LOGIN_REQUIRED); + Bugzilla->params->{'comment_taggers_group'} + || ThrowUserError("comment_tag_disabled"); + Bugzilla->user->can_tag_comments + || ThrowUserError("auth_failure", { group => Bugzilla->params->{'comment_taggers_group'}, + action => "search", + object => "comment_tags"}); + + my $query = $params->{query}; + $query + // ThrowCodeError('param_required', { param => 'query' }); + my $limit = detaint_natural($params->{limit}) || 7; + + my $tags = Bugzilla::Comment::TagWeights->match({ + WHERE => { + 'tag LIKE ?' => "\%$query\%", + }, + LIMIT => $limit, + }); + return [ map { $_->tag } @$tags ]; +} + + ############################## # Private Helper Subroutines # ############################## @@ -4032,6 +4108,137 @@ This method can throw the same errors as L. =back +=head2 search_comment_tags + +B + +=over + +=item B + +Searches for tags which contain the provided substring. + +=item B + +To search for comment tags: + +GET /bug/comment/tags/ + +=item B + +=over + +=item C + +B C Only tags containg this substring will be returned. + +=item C + +C If provided will return no more than C tags. Defaults to C<10>. + +=back + +=item B + +An C of matching tags. + +=item B + +This method can throw all of the errors that L throws, plus: + +=over + +=item 125 (Comment Tagging Disabled) + +Comment tagging support is not available or enabled. + +=back + +=item B + +=over + +=item Added in Bugzilla B<5.0>. + +=back + +=back + +=head2 update_comment_tags + +B + +=over + +=item B + +Adds or removes tags from a comment. + +=item B + +To update the tags comments attached to a comment: + +PUT /bug/comment/tags + +The params to include in the PUT body as well as the returned data format, +are the same as below. + +=item B + +=over + +=item C + +B C The ID of the comment to update. + +=item C + +C The tags to attach to the comment. + +=item C + +C The tags to detach from the comment. + +=back + +=item B + +An C containing the comment's updated tags. + +=item B + +This method can throw all of the errors that L throws, plus: + +=over + +=item 125 (Comment Tagging Disabled) + +Comment tagging support is not available or enabled. + +=item 126 (Invalid Comment Tag) + +The comment tag provided was not valid (eg. contains invalid characters). + +=item 127 (Comment Tag Too Short) + +The comment tag provided is shorter than the minimum length. + +=item 128 (Comment Tag Too Long) + +The comment tag provided is longer than the maximum length. + +=back + +=item B + +=over + +=item Added in Bugzilla B<5.0>. + +=back + +=back + =head1 B =over diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 1c3929e53..2c5bc31dd 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -98,7 +98,12 @@ use constant WS_ERROR_CODE => { comment_is_private => 110, comment_id_invalid => 111, comment_too_long => 114, - comment_invalid_isprivate => 117, + comment_invalid_isprivate => 117, + # Comment tagging + comment_tag_disabled => 125, + comment_tag_invalid => 126, + comment_tag_too_long => 127, + comment_tag_too_short => 128, # See Also errors bug_url_invalid => 112, bug_url_too_long => 112, diff --git a/Bugzilla/WebService/Server/REST/Resources/Bug.pm b/Bugzilla/WebService/Server/REST/Resources/Bug.pm index 98ae6049c..ea420b4ed 100644 --- a/Bugzilla/WebService/Server/REST/Resources/Bug.pm +++ b/Bugzilla/WebService/Server/REST/Resources/Bug.pm @@ -65,6 +65,22 @@ sub _rest_resources { } } }, + qr{^/bug/comment/tags/([^/]+)$}, { + GET => { + method => 'search_comment_tags', + params => sub { + return { query => $_[0] }; + }, + }, + }, + qr{^/bug/comment/([^/]+)/tags$}, { + PUT => { + method => 'update_comment_tags', + params => sub { + return { comment_id => $_[0] }; + }, + }, + }, qr{^/bug/([^/]+)/history$}, { GET => { method => 'history', diff --git a/js/comment-tagging.js b/js/comment-tagging.js new file mode 100644 index 000000000..b700fe11d --- /dev/null +++ b/js/comment-tagging.js @@ -0,0 +1,376 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. + * + * This Source Code Form is "Incompatible With Secondary Licenses", as + * defined by the Mozilla Public License, v. 2.0. */ + +var Dom = YAHOO.util.Dom; + +YAHOO.bugzilla.commentTagging = { + ctag_div : false, + ctag_add : false, + counter : 0, + min_len : 3, + max_len : 24, + tags_by_no: {}, + nos_by_tag: {}, + current_id: 0, + current_no: -1, + can_edit : false, + pending : {}, + + init : function(can_edit) { + this.can_edit = can_edit; + this.ctag_div = Dom.get('bz_ctag_div'); + this.ctag_add = Dom.get('bz_ctag_add'); + YAHOO.util.Event.on(this.ctag_add, 'keypress', this.onKeyPress); + YAHOO.util.Event.onDOMReady(function() { + YAHOO.bugzilla.commentTagging.updateCollapseControls(); + }); + if (!can_edit) return; + + var ds = new YAHOO.util.XHRDataSource("jsonrpc.cgi"); + ds.connTimeout = 30000; + ds.connMethodPost = true; + ds.connXhrMode = "cancelStaleRequests"; + ds.maxCacheEntries = 5; + ds.responseSchema = { + metaFields : { error: "error", jsonRpcId: "id"}, + resultsList : "result" + }; + + var ac = new YAHOO.widget.AutoComplete('bz_ctag_add', 'bz_ctag_autocomp', ds); + ac.maxResultsDisplayed = 7; + ac.generateRequest = function(query) { + query = YAHOO.lang.trim(query); + YAHOO.bugzilla.commentTagging.last_query = query; + YAHOO.bugzilla.commentTagging.counter = YAHOO.bugzilla.commentTagging.counter + 1; + YAHOO.util.Connect.setDefaultPostHeader('application/json', true); + return YAHOO.lang.JSON.stringify({ + method : "Bug.search_comment_tags", + id : YAHOO.bugzilla.commentTagging.counter, + params : [ { query : query, limit : 10 } ] + }); + }; + ac.minQueryLength = this.min_len; + ac.autoHighlight = false; + ac.typeAhead = true; + ac.queryDelay = 0.5; + ac.dataReturnEvent.subscribe(function(type, args) { + args[0].autoHighlight = args[2].length == 1; + }); + }, + + toggle : function(comment_id, comment_no) { + if (!this.ctag_div) return; + var tags_container = Dom.get('ct_' + comment_no); + + if (this.current_id == comment_id) { + // hide + this.current_id = 0; + this.current_no = -1; + Dom.addClass(this.ctag_div, 'bz_default_hidden'); + this.hideError(); + window.focus(); + + } else { + // show or move + this.rpcRefresh(comment_id, comment_no); + this.current_id = comment_id; + this.current_no = comment_no; + this.ctag_add.value = ''; + tags_container.parentNode.insertBefore(this.ctag_div, tags_container); + Dom.removeClass(this.ctag_div, 'bz_default_hidden'); + Dom.removeClass(tags_container, 'bz_default_hidden'); + var comment = Dom.get('comment_text_' + comment_no); + if (Dom.hasClass(comment, 'collapsed')) { + var link = Dom.get('comment_link_' + comment_no); + expand_comment(link, comment, comment_no); + } + window.setTimeout(function() { + YAHOO.bugzilla.commentTagging.ctag_add.focus(); + }, 50); + } + }, + + hideInput : function() { + if (this.current_id != 0) { + this.toggle(this.current_id, this.current_no); + } + this.hideError(); + }, + + showError : function(comment_id, comment_no, error) { + var bz_ctag_error = Dom.get('bz_ctag_error'); + var tags_container = Dom.get('ct_' + comment_no); + tags_container.parentNode.appendChild(bz_ctag_error, tags_container); + Dom.get('bz_ctag_error_msg').innerHTML = YAHOO.lang.escapeHTML(error); + Dom.removeClass(bz_ctag_error, 'bz_default_hidden'); + }, + + hideError : function() { + Dom.addClass('bz_ctag_error', 'bz_default_hidden'); + }, + + onKeyPress : function(evt) { + evt = evt || window.event; + var charCode = evt.charCode || evt.keyCode; + if (evt.keyCode == 27) { + // escape + YAHOO.bugzilla.commentTagging.hideInput(); + YAHOO.util.Event.stopEvent(evt); + + } else if (evt.keyCode == 13) { + // return + YAHOO.util.Event.stopEvent(evt); + var tags = YAHOO.bugzilla.commentTagging.ctag_add.value.split(/[ ,]/); + var comment_id = YAHOO.bugzilla.commentTagging.current_id; + var comment_no = YAHOO.bugzilla.commentTagging.current_no; + YAHOO.bugzilla.commentTagging.hideInput(); + try { + YAHOO.bugzilla.commentTagging.add(comment_id, comment_no, tags); + } catch(e) { + YAHOO.bugzilla.commentTagging.showError(comment_id, comment_no, e.message); + } + } + }, + + showTags : function(comment_id, comment_no, tags) { + // remove existing tags + var tags_container = Dom.get('ct_' + comment_no); + while (tags_container.hasChildNodes()) { + tags_container.removeChild(tags_container.lastChild); + } + // add tags + if (tags != '') { + if (typeof(tags) == 'string') { + tags = tags.split(','); + } + for (var i = 0, l = tags.length; i < l; i++) { + tags_container.appendChild(this.buildTagHtml(comment_id, comment_no, tags[i])); + } + } + // update tracking array + this.tags_by_no['c' + comment_no] = tags; + this.updateCollapseControls(); + }, + + updateCollapseControls : function() { + var container = Dom.get('comment_tags_collapse_expand_container'); + if (!container) return; + // build list of tags + this.nos_by_tag = {}; + for (var id in this.tags_by_no) { + if (this.tags_by_no.hasOwnProperty(id)) { + for (var i = 0, l = this.tags_by_no[id].length; i < l; i++) { + var tag = this.tags_by_no[id][i].toLowerCase(); + if (!this.nos_by_tag.hasOwnProperty(tag)) { + this.nos_by_tag[tag] = []; + } + this.nos_by_tag[tag].push(id); + } + } + } + var tags = []; + for (var tag in this.nos_by_tag) { + if (this.nos_by_tag.hasOwnProperty(tag)) { + tags.push(tag); + } + } + tags.sort(); + if (tags.length) { + var div = document.createElement('div'); + div.appendChild(document.createTextNode('Comment Tags:')); + var ul = document.createElement('ul'); + ul.id = 'comment_tags_collapse_expand'; + div.appendChild(ul); + for (var i = 0, l = tags.length; i < l; i++) { + var tag = tags[i]; + var li = document.createElement('li'); + ul.appendChild(li); + var a = document.createElement('a'); + li.appendChild(a); + Dom.setAttribute(a, 'href', '#'); + YAHOO.util.Event.addListener(a, 'click', function(evt, tag) { + YAHOO.bugzilla.commentTagging.toggleCollapse(tag); + YAHOO.util.Event.stopEvent(evt); + }, tag); + li.appendChild(document.createTextNode(' (' + this.nos_by_tag[tag].length + ')')); + a.innerHTML = tag; + } + while (container.hasChildNodes()) { + container.removeChild(container.lastChild); + } + container.appendChild(div); + } else { + while (container.hasChildNodes()) { + container.removeChild(container.lastChild); + } + } + }, + + toggleCollapse : function(tag) { + var nos = this.nos_by_tag[tag]; + if (!nos) return; + toggle_all_comments('collapse'); + for (var i = 0, l = nos.length; i < l; i++) { + var comment_no = nos[i].match(/\d+$/)[0]; + var comment = Dom.get('comment_text_' + comment_no); + var link = Dom.get('comment_link_' + comment_no); + expand_comment(link, comment, comment_no); + } + }, + + buildTagHtml : function(comment_id, comment_no, tag) { + var el = document.createElement('span'); + Dom.setAttribute(el, 'id', 'ct_' + comment_no + '_' + tag); + Dom.addClass(el, 'bz_comment_tag'); + if (this.can_edit) { + var a = document.createElement('a'); + Dom.setAttribute(a, 'href', '#'); + YAHOO.util.Event.addListener(a, 'click', function(evt, args) { + YAHOO.bugzilla.commentTagging.remove(args[0], args[1], args[2]) + YAHOO.util.Event.stopEvent(evt); + }, [comment_id, comment_no, tag]); + a.appendChild(document.createTextNode('x')); + el.appendChild(a); + el.appendChild(document.createTextNode("\u00a0")); + } + el.appendChild(document.createTextNode(tag)); + return el; + }, + + add : function(comment_id, comment_no, add_tags) { + // build list of current tags from html + var tags = new Array(); + var spans = Dom.getElementsByClassName('bz_comment_tag', undefined, 'ct_' + comment_no); + for (var i = 0, l = spans.length; i < l; i++) { + tags.push(spans[i].textContent.substr(2)); + } + // add new tags + var new_tags = new Array(); + for (var i = 0, l = add_tags.length; i < l; i++) { + var tag = YAHOO.lang.trim(add_tags[i]); + // validation + if (tag == '') + continue; + if (tag.length < YAHOO.bugzilla.commentTagging.min_len) + throw new Error("Comment tags must be at least " + this.min_len + " characters."); + if (tag.length > YAHOO.bugzilla.commentTagging.max_len) + throw new Error("Comment tags cannot be longer than " + this.min_len + " characters."); + // append new tag + if (bz_isValueInArrayIgnoreCase(tags, tag)) + continue; + new_tags.push(tag); + tags.push(tag); + } + tags.sort(); + // update + this.showTags(comment_id, comment_no, tags); + this.rpcUpdate(comment_id, comment_no, new_tags, undefined); + }, + + remove : function(comment_id, comment_no, tag) { + var el = Dom.get('ct_' + comment_no + '_' + tag); + if (el) { + el.parentNode.removeChild(el); + this.rpcUpdate(comment_id, comment_no, undefined, [ tag ]); + } + }, + + // If multiple updates are triggered quickly, overlapping refresh events + // are generated. We ignore all events except the last one. + incPending : function(comment_id) { + if (this.pending['c' + comment_id] == undefined) { + this.pending['c' + comment_id] = 1; + } else { + this.pending['c' + comment_id]++; + } + }, + + decPending : function(comment_id) { + if (this.pending['c' + comment_id] != undefined) + this.pending['c' + comment_id]--; + }, + + hasPending : function(comment_id) { + return this.pending['c' + comment_id] != undefined + && this.pending['c' + comment_id] > 0; + }, + + rpcRefresh : function(comment_id, comment_no, noRefreshOnError) { + this.incPending(comment_id); + YAHOO.util.Connect.setDefaultPostHeader('application/json', true); + YAHOO.util.Connect.asyncRequest('POST', 'jsonrpc.cgi', + { + success: function(res) { + YAHOO.bugzilla.commentTagging.decPending(comment_id); + data = YAHOO.lang.JSON.parse(res.responseText); + if (data.error) { + YAHOO.bugzilla.commentTagging.handleRpcError( + comment_id, comment_no, data.error.message, noRefreshOnError); + return; + } + + if (!YAHOO.bugzilla.commentTagging.hasPending(comment_id)) + YAHOO.bugzilla.commentTagging.showTags( + comment_id, comment_no, data.result.comments[comment_id].tags); + }, + failure: function(res) { + YAHOO.bugzilla.commentTagging.decPending(comment_id); + YAHOO.bugzilla.commentTagging.handleRpcError( + comment_id, comment_no, res.responseText, noRefreshOnError); + } + }, + YAHOO.lang.JSON.stringify({ + version: "1.1", + method: 'Bug.comments', + params: { + comment_ids: [ comment_id ], + include_fields: [ 'tags' ] + } + }) + ); + }, + + rpcUpdate : function(comment_id, comment_no, add, remove) { + this.incPending(comment_id); + YAHOO.util.Connect.setDefaultPostHeader('application/json', true); + YAHOO.util.Connect.asyncRequest('POST', 'jsonrpc.cgi', + { + success: function(res) { + YAHOO.bugzilla.commentTagging.decPending(comment_id); + data = YAHOO.lang.JSON.parse(res.responseText); + if (data.error) { + YAHOO.bugzilla.commentTagging.handleRpcError(comment_id, comment_no, data.error.message); + return; + } + + if (!YAHOO.bugzilla.commentTagging.hasPending(comment_id)) + YAHOO.bugzilla.commentTagging.showTags(comment_id, comment_no, data.result); + }, + failure: function(res) { + YAHOO.bugzilla.commentTagging.decPending(comment_id); + YAHOO.bugzilla.commentTagging.handleRpcError(comment_id, comment_no, res.responseText); + } + }, + YAHOO.lang.JSON.stringify({ + version: "1.1", + method: 'Bug.update_comment_tags', + params: { + comment_id: comment_id, + add: add, + remove: remove + } + }) + ); + }, + + handleRpcError : function(comment_id, comment_no, message, noRefreshOnError) { + YAHOO.bugzilla.commentTagging.showError(comment_id, comment_no, message); + if (!noRefreshOnError) { + YAHOO.bugzilla.commentTagging.rpcRefresh(comment_id, comment_no, true); + } + } +} diff --git a/js/comments.js b/js/comments.js index acf5dbf9b..8d314de15 100644 --- a/js/comments.js +++ b/js/comments.js @@ -25,9 +25,9 @@ function toggle_comment_display(link, comment_id) { var comment = document.getElementById('comment_text_' + comment_id); var re = new RegExp(/\bcollapsed\b/); if (comment.className.match(re)) - expand_comment(link, comment); + expand_comment(link, comment, comment_id); else - collapse_comment(link, comment); + collapse_comment(link, comment, comment_id); } function toggle_all_comments(action) { @@ -44,20 +44,22 @@ function toggle_all_comments(action) { var id = comments[i].id.match(/\d*$/); var link = document.getElementById('comment_link_' + id); if (action == 'collapse') - collapse_comment(link, comment); + collapse_comment(link, comment, id); else - expand_comment(link, comment); + expand_comment(link, comment, id); } } -function collapse_comment(link, comment) { +function collapse_comment(link, comment, comment_id) { link.innerHTML = "[+]"; YAHOO.util.Dom.addClass(comment, 'collapsed'); + YAHOO.util.Dom.addClass('comment_tag_' + comment_id, 'collapsed'); } -function expand_comment(link, comment) { +function expand_comment(link, comment, comment_id) { link.innerHTML = "[−]"; YAHOO.util.Dom.removeClass(comment, 'collapsed'); + YAHOO.util.Dom.removeClass('comment_tag_' + comment_id, 'collapsed'); } function wrapReplyText(text) { @@ -110,11 +112,12 @@ function wrapReplyText(text) { /* This way, we are sure that browsers which do not support JS * won't display this link */ -function addCollapseLink(count, title) { +function addCollapseLink(count, collapsed, title) { document.write(' [−]<\/a> '); + '); return false;" title="' + title + '">[' + + (collapsed ? '+' : '−') + ']<\/a> '); } function goto_add_comments( anchor ){ diff --git a/js/util.js b/js/util.js index 6d1f88938..d3a34477b 100644 --- a/js/util.js +++ b/js/util.js @@ -125,10 +125,7 @@ function bz_overlayBelow(item, parent) { */ function bz_isValueInArray(aArray, aValue) { - var run = 0; - var len = aArray.length; - - for ( ; run < len; run++) { + for (var run = 0, len = aArray.length ; run < len; run++) { if (aArray[run] == aValue) { return true; } @@ -137,6 +134,26 @@ function bz_isValueInArray(aArray, aValue) return false; } +/** + * Checks if a specified value is in the specified array by performing a + * case-insensitive comparison. + * + * @param aArray Array to search for the value. + * @param aValue Value to search from the array. + * @return Boolean; true if value is found in the array and false if not. + */ +function bz_isValueInArrayIgnoreCase(aArray, aValue) +{ + var re = new RegExp(aValue.replace(/([^A-Za-z0-9])/g, "\\$1"), 'i'); + for (var run = 0, len = aArray.length ; run < len; run++) { + if (aArray[run].match(re)) { + return true; + } + } + + return false; +} + /** * Create wanted options in a select form control. * diff --git a/show_activity.cgi b/show_activity.cgi index 31181a63b..1383dd018 100755 --- a/show_activity.cgi +++ b/show_activity.cgi @@ -38,7 +38,7 @@ my $bug = Bugzilla::Bug->check($id); # visible immediately due to replication lag. Bugzilla->switch_to_shadow_db; -($vars->{'operations'}, $vars->{'incomplete_data'}) = $bug->get_activity; +($vars->{'operations'}, $vars->{'incomplete_data'}) = $bug->get_activity(undef, undef, 1); $vars->{'bug'} = $bug; diff --git a/skins/standard/show_bug.css b/skins/standard/show_bug.css index 2d5ba4309..7c002cc15 100644 --- a/skins/standard/show_bug.css +++ b/skins/standard/show_bug.css @@ -121,3 +121,42 @@ table#flags { .bz_bug .bz_alias_short_desc_container { width: inherit; } + +.bz_comment_tags { + margin-top: 3px; +} + +.bz_comment_tag { + border: 1px solid #c8c8ba; + padding: 1px 3px; + margin-right: 2px; + border-radius: 0.5em; + background-color: #eee; + color: #000; +} + +#bz_ctag_div { + display: inline-block; +} + +#bz_ctag_error { + border: 1px solid #ff6666; + padding: 0px 2px; + border-radius: 0.5em; + margin: 2px; + display: inline-block; +} + +#comment_tags_collapse_expand_container { + padding-top: 1em; +} + +#comment_tags_collapse_expand { + list-style-type: none; + padding-left: 1em; +} + +#comment_tags_collapse_expand li { + margin-bottom: 0px; +} + diff --git a/template/en/default/admin/params/bugfields.html.tmpl b/template/en/default/admin/params/bugfields.html.tmpl index 07b200825..961833d89 100644 --- a/template/en/default/admin/params/bugfields.html.tmpl +++ b/template/en/default/admin/params/bugfields.html.tmpl @@ -41,5 +41,9 @@ "entry form.
" _ "You can leave this empty: " _ "$terms.Bugzilla will then use the operating system that the browser " _ - "reports to be running on as the default." } + "reports to be running on as the default.", + + collapsed_comment_tags => "A comma separated list of tags which, when applied " _ + "to comments, will cause them to be collapsed by default", + } %] diff --git a/template/en/default/admin/params/groupsecurity.html.tmpl b/template/en/default/admin/params/groupsecurity.html.tmpl index 7e30f0723..4f0b4919b 100644 --- a/template/en/default/admin/params/groupsecurity.html.tmpl +++ b/template/en/default/admin/params/groupsecurity.html.tmpl @@ -29,6 +29,9 @@ querysharegroup => "The name of the group of users who can share their " _ "saved searches with others.", + comment_taggers_group => "The name of the group of users who can tag comment." _ + " Setting this to empty disables comment tagging.", + debug_group => "The name of the group of users who can view the actual " _ "SQL query generated when viewing $terms.bug lists and reports.", diff --git a/template/en/default/bug/comments.html.tmpl b/template/en/default/bug/comments.html.tmpl index a2fe4e52f..7aa9a34a6 100644 --- a/template/en/default/bug/comments.html.tmpl +++ b/template/en/default/bug/comments.html.tmpl @@ -13,7 +13,7 @@ [% END %] @@ -175,10 +181,29 @@ [% PROCESS formattimeunit time_unit=comment.work_time %] [% END %] + [% IF user.id && Param('comment_taggers_group') %] +
+ + [% IF comment.tags.size %] + + [% END %] + +
+ [% END %] + [%# Don't indent the 
 block, since then the spaces are displayed in the
   # generated HTML
   #%]
-
   [%- comment_text FILTER quoteUrls(bug, comment) -%]
 

diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index a6857f0d5..202a981ea 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -8,6 +8,31 @@
 
 [% PROCESS bug/time.html.tmpl %]
 
+[% IF Param('comment_taggers_group') %]
+  [% IF user.can_tag_comments %]
+    

+      x
+      

+        
+        
+      

+       
+    

+    

+      x
+      
+    

+  [% END %]
+  [% IF user.id %]
+    
+  [% END %]
+[% END %]
+