From 1d96fa1689470945545ac8e0f239357185e832a7 Mon Sep 17 00:00:00 2001 From: Pami Ketolainen Date: Fri, 13 Mar 2015 14:23:54 -0400 Subject: Bug 1062718 - add the ability to disable sending of mail when updating bugs r=dylan,a=sgreen --- Bugzilla/Bug.pm | 25 ++++++--- Bugzilla/BugMail.pm | 6 ++- Bugzilla/Config/GroupSecurity.pm | 8 +++ Bugzilla/Constants.pm | 5 +- Bugzilla/User.pm | 7 ++- Bugzilla/WebService/Bug.pm | 62 +++++++++++++++++++--- attachment.cgi | 12 ++++- process_bug.cgi | 4 +- skins/standard/bug.css | 3 ++ template/en/default/account/prefs/email.html.tmpl | 2 + template/en/default/admin/groups/list.html.tmpl | 3 +- .../default/admin/params/groupsecurity.html.tmpl | 7 ++- template/en/default/attachment/create.html.tmpl | 11 ++++ template/en/default/attachment/edit.html.tmpl | 6 +++ template/en/default/bug/edit.html.tmpl | 7 +++ template/en/default/list/edit-multiple.html.tmpl | 7 +++ 16 files changed, 151 insertions(+), 24 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 73ba62a3b..2bd6fb646 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1265,7 +1265,7 @@ sub remove_from_db { ##################################################################### sub send_changes { - my ($self, $changes, $vars) = @_; + my ($self, $changes, $vars, $minor_update) = @_; my $user = Bugzilla->user; @@ -1283,15 +1283,15 @@ sub send_changes { changer => $user, ); - _send_bugmail({ id => $self->id, type => 'bug', forced => \%forced }, - $vars); + _send_bugmail({ id => $self->id, type => 'bug', forced => \%forced, + minor_update => $minor_update }, $vars); # If the bug was marked as a duplicate, we need to notify users on the # other bug of any changes to that bug. my $new_dup_id = $changes->{'dup_id'} ? $changes->{'dup_id'}->[1] : undef; if ($new_dup_id) { _send_bugmail({ forced => { changer => $user }, type => "dupe", - id => $new_dup_id }, $vars); + id => $new_dup_id, minor_update => $minor_update }, $vars); } # If there were changes in dependencies, we need to notify those @@ -1306,7 +1306,8 @@ sub send_changes { type => 'dep', dep_only => 1, blocker => $self, - changes => $changes }; + changes => $changes, + minor_update => $minor_update }; foreach my $id (@{ $self->blocked }) { $params->{id} = $id; @@ -1329,13 +1330,13 @@ sub send_changes { foreach my $id (sort { $a <=> $b } (keys %changed_deps)) { _send_bugmail({ forced => { changer => $user }, type => "dep", - id => $id }, $vars); + id => $id, minor_update => $minor_update }, $vars); } # Sending emails for the referenced bugs. foreach my $ref_bug_id (uniq @{ $self->{see_also_changes} || [] }) { _send_bugmail({ forced => { changer => $user }, - id => $ref_bug_id }, $vars); + id => $ref_bug_id, minor_update => $minor_update }, $vars); } } @@ -4221,6 +4222,12 @@ sub get_activity { return(\@operations, $incomplete_data); } +sub has_unsent_changes { + my $self = shift; + return 1 if !defined $self->lastdiffed; + return datetime_from($self->lastdiffed) < datetime_from($self->delta_ts) ? 1 : 0; +} + # Update the bugs_activity table to reflect changes made in bugs. sub LogActivityEntry { my ($bug_id, $field, $removed, $added, $user_id, $timestamp, $comment_id, @@ -4625,6 +4632,10 @@ call L to make the changes permanent. Creates or updates a L for this bug and the supplied $user, the timestamp given as $last_visit. +=item C + +Checks if this bug has changes for which bug mail has not been sent. + =back =head1 B diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 8b4219787..7dfb48238 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -225,6 +225,9 @@ sub Send { my $date = $params->{dep_only} ? $end : $bug->delta_ts; $date = format_time($date, '%a, %d %b %Y %T %z', 'UTC'); + my $minor_update = $changer->in_group(Bugzilla->params->{minor_update_group}) + && $params->{minor_update}; + foreach my $user_id (keys %recipients) { my %rels_which_want; my $user = $user_cache{$user_id} ||= new Bugzilla::User($user_id); @@ -244,7 +247,8 @@ sub Send { $start ? \@diffs : [], $comments, $params->{dep_only}, - $changer)) + $changer, + $minor_update)) { $rels_which_want{$relationship} = $recipients{$user_id}->{$relationship}; diff --git a/Bugzilla/Config/GroupSecurity.pm b/Bugzilla/Config/GroupSecurity.pm index e827834a0..4fa21e72f 100644 --- a/Bugzilla/Config/GroupSecurity.pm +++ b/Bugzilla/Config/GroupSecurity.pm @@ -66,6 +66,14 @@ sub get_param_list { checker => \&check_comment_taggers_group }, + { + name => 'minor_update_group', + type => 's', + choices => \&_get_all_group_names, + default => '', + checker => \&check_group + }, + { name => 'debug_group', type => 's', diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 414cd2fed..323fb151c 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -94,7 +94,7 @@ use Memoize; EVT_BUG_CREATED EVT_COMPONENT NEG_EVENTS - EVT_UNCONFIRMED EVT_CHANGED_BY_ME + EVT_UNCONFIRMED EVT_CHANGED_BY_ME EVT_MINOR_UPDATE GLOBAL_EVENTS EVT_FLAG_REQUESTED EVT_REQUESTED_FLAG @@ -384,8 +384,9 @@ use constant POS_EVENTS => EVT_OTHER, EVT_ADDED_REMOVED, EVT_COMMENT, use constant EVT_UNCONFIRMED => 50; use constant EVT_CHANGED_BY_ME => 51; +use constant EVT_MINOR_UPDATE => 52; -use constant NEG_EVENTS => EVT_UNCONFIRMED, EVT_CHANGED_BY_ME; +use constant NEG_EVENTS => EVT_UNCONFIRMED, EVT_CHANGED_BY_ME, EVT_MINOR_UPDATE; # These are the "global" flags, which aren't tied to a particular relationship. # and so use REL_ANY. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index b28d09323..fdc54de04 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -2037,7 +2037,8 @@ our %names_to_events = ( # Note: the "+" signs before the constants suppress bareword quoting. sub wants_bug_mail { my $self = shift; - my ($bug, $relationship, $fieldDiffs, $comments, $dep_mail, $changer) = @_; + my ($bug, $relationship, $fieldDiffs, $comments, $dep_mail, $changer, + $minor_update) = @_; # Make a list of the events which have happened during this bug change, # from the point of view of this user. @@ -2115,6 +2116,10 @@ sub wants_bug_mail { if ($wants_mail && $bug->bug_status eq 'UNCONFIRMED') { $wants_mail &= $self->wants_mail([EVT_UNCONFIRMED], $relationship); } + + if ($wants_mail && $minor_update) { + $wants_mail &= $self->wants_mail([EVT_MINOR_UPDATE], $relationship); + } return $wants_mail; } diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 23d385cd4..b3cb631fb 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -644,6 +644,7 @@ sub update { my @bugs = map { Bugzilla::Bug->check_for_edit($_) } @$ids; + my $minor_update = delete $params->{minor_update} ? 1 : 0; my %values = %$params; $values{other_bugs} = \@bugs; @@ -677,14 +678,16 @@ sub update { } my %all_changes; + my %minor_updates; $dbh->bz_start_transaction(); foreach my $bug (@bugs) { + $minor_updates{$bug->id} = $bug->has_unsent_changes ? 0 : $minor_update; $all_changes{$bug->id} = $bug->update(); } $dbh->bz_commit_transaction(); foreach my $bug (@bugs) { - $bug->send_changes($all_changes{$bug->id}); + $bug->send_changes($all_changes{$bug->id}, undef, $minor_updates{$bug->id}); } my %api_name = reverse %{ Bugzilla::Bug::FIELD_MAP() }; @@ -820,6 +823,7 @@ sub add_attachment { || ThrowCodeError('param_required', { param => 'data' }); my @bugs = map { Bugzilla::Bug->check_for_edit($_) } @{ $params->{ids} }; + my $minor_update = delete $params->{minor_update} ? 1 : 0; my @created; $dbh->bz_start_transaction(); @@ -863,10 +867,17 @@ sub add_attachment { extra_data => $attachment->id }); push(@created, $attachment); } - $_->bug->update($timestamp) foreach @created; + my %minor_updates; + foreach my $attachment (@created) { + my $bug = $attachment->bug; + $minor_updates{$bug->id} = $bug->has_unsent_changes ? 0 : $minor_update; + $bug->update($timestamp); + } $dbh->bz_commit_transaction(); - $_->send_changes() foreach @bugs; + foreach my $bug (@bugs) { + $bug->send_changes(undef, undef, $minor_updates{$bug->id}); + } my @created_ids = map { $_->id } @created; @@ -882,6 +893,7 @@ sub update_attachment { my $ids = delete $params->{ids}; defined $ids || ThrowCodeError('param_required', { param => 'ids' }); + my $req_minor_update = delete $params->{minor_update} ? 1 : 0; # Some fields cannot be sent to set_all foreach my $key (qw(login password token)) { delete $params->{$key}; @@ -967,8 +979,9 @@ sub update_attachment { # Email users about the change foreach my $bug (values %bugs) { + my $minor_update = $bug->has_unsent_changes ? 0 : $req_minor_update; $bug->update(); - $bug->send_changes(); + $bug->send_changes(undef, undef, $minor_update); } # Return the information to the user @@ -989,6 +1002,8 @@ sub add_comment { || ThrowCodeError('param_required', { param => 'comment' }); my $bug = Bugzilla::Bug->check_for_edit($params->{id}); + my $minor_update = delete $params->{minor_update} ? 1 : 0; + $minor_update = $bug->has_unsent_changes ? 0 : $minor_update; # Backwards-compatibility for versions before 3.6 if (defined $params->{private}) { @@ -1007,7 +1022,8 @@ sub add_comment { my $new_comment_id = $bug->{added_comments}[0]->id; # Send mail. - Bugzilla::BugMail::Send($bug->bug_id, { changer => $user }); + Bugzilla::BugMail::Send($bug->bug_id, { changer => $user }, + { minor_update => $minor_update }); return { id => $self->type('int', $new_comment_id) }; } @@ -1023,6 +1039,7 @@ sub update_see_also { my ($add, $remove) = @$params{qw(add remove)}; ($add || $remove) or ThrowCodeError('params_required', { params => ['add', 'remove'] }); + my $req_minor_update = delete $params->{minor_update} ? 1 : 0; my @bugs; foreach my $id (@{ $params->{ids} }) { @@ -1038,6 +1055,7 @@ sub update_see_also { my %changes; foreach my $bug (@bugs) { + my $minor_update = $bug->has_unsent_changes ? 0 : $req_minor_update; my $change = $bug->update(); if (my $see_also = $change->{see_also}) { $changes{$bug->id}->{see_also} = { @@ -1050,7 +1068,8 @@ sub update_see_also { $changes{$bug->id}->{see_also} = { added => [], removed => [] }; } - Bugzilla::BugMail::Send($bug->id, { changer => $user }); + Bugzilla::BugMail::Send($bug->id, { changer => $user }, + { minor_update => $minor_update }); } return { changes => \%changes }; @@ -3414,6 +3433,12 @@ C The login of the requestee if the flag type is requestable to a specif =back +=item C + +C If set to true, this is considered a minor update and no mail is sent +to users who do not want minor update emails. If current user is not in the +minor_update_group, this parameter is simply ignored. + =back =item B @@ -3609,6 +3634,14 @@ C Set to true if you specifically want a new flag to be created. =back +=item C + +C If set to true, this is considered a minor update and no mail is sent +to users who do not want minor update emails. If current user is not in the +minor_update_group, this parameter is simply ignored. + +=back + =item B A C with a single field, "attachments". This points to an array of hashes @@ -3729,8 +3762,6 @@ You did not specify a value for the C argument. =back -=back - =head2 add_comment B @@ -3771,6 +3802,9 @@ structures, otherwise it is a normal text. on the bug. If you are not in the time tracking group, this value will be ignored. +=item C (boolean) - If set to true, this is considered a minor update +and no mail is sent to users who do not want minor update emails. If current user +is not in the minor_update_group, this parameter is simply ignored. =back @@ -3872,6 +3906,12 @@ pulled from the URL path. Array of Cs or Cs. The ids or aliases of the bugs that you want to modify. +=item C + +C If set to true, this is considered a minor update and no mail is sent +to users who do not want minor update emails. If current user is not in the +minor_update_group, this parameter is simply ignored. + =back B: All following fields specify the values you want to set on the @@ -4442,6 +4482,12 @@ If you specify a URL that is not in the See Also field of a particular bug, it will just be silently ignored. Invaild URLs are currently silently ignored, though this may change in some future version of Bugzilla. +=item C + +C If set to true, this is considered a minor update and no mail is sent +to users who do not want minor update emails. If current user is not in the +minor_update_group, this parameter is simply ignored. + =back NOTE: If you specify the same URL in both C and C, it will diff --git a/attachment.cgi b/attachment.cgi index 61e6f58d8..7df230d79 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -511,6 +511,9 @@ sub insert { @obsolete_attachments = Bugzilla::Attachment->validate_obsolete($bug, \@obsolete); } + my $minor_update = $cgi->param('minor_update') ? 1 : 0; + $minor_update = $bug->has_unsent_changes ? 0 : $minor_update; + # Must be called before create() as it may alter $cgi->param('ispatch'). my $content_type = Bugzilla::Attachment::get_content_type(); @@ -586,7 +589,8 @@ sub insert { $vars->{'contenttypemethod'} = $cgi->param('contenttypemethod'); my $recipients = { 'changer' => $user, 'owner' => $owner }; - $vars->{'sent_bugmail'} = Bugzilla::BugMail::Send($bugid, $recipients); + my $params = { 'minor_update' => $minor_update }; + $vars->{'sent_bugmail'} = Bugzilla::BugMail::Send($bugid, $recipients, $params); print $cgi->header(); # Generate and return the UI (HTML page) from the appropriate template. @@ -677,6 +681,9 @@ sub update { my $token = $cgi->param('token'); check_hash_token($token, [$attachment->id, $attachment->modification_time]); + my $minor_update = $cgi->param('minor_update') ? 1 : 0; + $minor_update = $bug->has_unsent_changes ? 0 : $minor_update; + # If the user submitted a comment while editing the attachment, # add the comment to the bug. Do this after having validated isprivate! my $comment = $cgi->param('comment'); @@ -739,7 +746,8 @@ sub update { $vars->{'bugs'} = [$bug]; $vars->{'header_done'} = 1; $vars->{'sent_bugmail'} = - Bugzilla::BugMail::Send($bug->id, { 'changer' => $user }); + Bugzilla::BugMail::Send($bug->id, { 'changer' => $user }, + {'minor_update' => $minor_update }); print $cgi->header(); diff --git a/process_bug.cgi b/process_bug.cgi index b3d979960..448b42c40 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -373,7 +373,9 @@ else { ############################## # Do Actual Database Updates # ############################## +my $req_minor_update = $cgi->param('minor_update') ? 1 : 0; foreach my $bug (@bug_objects) { + my $minor_update = $bug->has_unsent_changes ? 0 : $req_minor_update; my $changes = $bug->update(); if ($changes->{'bug_status'}) { @@ -386,7 +388,7 @@ foreach my $bug (@bug_objects) { } } - $bug->send_changes($changes, $vars); + $bug->send_changes($changes, $vars, $minor_update); } # Delete the session token used for the mass-change. diff --git a/skins/standard/bug.css b/skins/standard/bug.css index 851b1bdba..35d169c1e 100644 --- a/skins/standard/bug.css +++ b/skins/standard/bug.css @@ -226,6 +226,9 @@ table#flags { .knob-buttons { float: right; + text-align: right; + font-size: small; + font-weight: normal; } .text_input, .bz_userfield, #keywords_container, #tag_container { diff --git a/template/en/default/account/prefs/email.html.tmpl b/template/en/default/account/prefs/email.html.tmpl index 052484174..ab25ff8bb 100644 --- a/template/en/default/account/prefs/email.html.tmpl +++ b/template/en/default/account/prefs/email.html.tmpl @@ -123,6 +123,8 @@ function SetCheckboxes(setting) { description = "The $terms.bug is in the UNCONFIRMED state" }, { id = constants.EVT_CHANGED_BY_ME, description = "The change was made by me" }, + { id = constants.EVT_MINOR_UPDATE, + description = "The change was marked as a minor update" }, ] %] [% relationships = [ diff --git a/template/en/default/admin/groups/list.html.tmpl b/template/en/default/admin/groups/list.html.tmpl index 673c053cb..33732f957 100644 --- a/template/en/default/admin/groups/list.html.tmpl +++ b/template/en/default/admin/groups/list.html.tmpl @@ -76,7 +76,8 @@ %] [% FOREACH group IN ["chartgroup", "comment_taggers_group", "debug_group", - "insidergroup", "querysharegroup", "timetrackinggroup"] %] + "insidergroup", "minor_update_group", "querysharegroup", + "timetrackinggroup"] %] [% special_group = Param(group) %] [% IF special_group %] diff --git a/template/en/default/admin/params/groupsecurity.html.tmpl b/template/en/default/admin/params/groupsecurity.html.tmpl index 590f4da02..19d78de5a 100644 --- a/template/en/default/admin/params/groupsecurity.html.tmpl +++ b/template/en/default/admin/params/groupsecurity.html.tmpl @@ -51,6 +51,11 @@ "view it. If it is off, a user needs to be a member of all " _ "the $terms.bug's groups. Note that in either case, if the " _ "user has a role on the $terms.bug (e.g. reporter) that may " _ - "also affect their permissions." + "also affect their permissions.", + + minor_update_group => "The name of the group of users who are allowed to " _ + "use the 'minor update'-option on $terms.bug changes " _ + "to limit mail sending. " _ + "Setting this to empty disables the feature.", } %] diff --git a/template/en/default/attachment/create.html.tmpl b/template/en/default/attachment/create.html.tmpl index e566b428e..05223ede4 100644 --- a/template/en/default/attachment/create.html.tmpl +++ b/template/en/default/attachment/create.html.tmpl @@ -119,6 +119,17 @@ TUI_hide_default('attachment_text_field'); [% Hook.process('form_before_submit') %] + [% IF Param('minor_update_group') && user.in_group(Param('minor_update_group')) %] + +   + + + + + + [% END %]   diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl index 184cdde05..d2554dfb8 100644 --- a/template/en/default/attachment/edit.html.tmpl +++ b/template/en/default/attachment/edit.html.tmpl @@ -283,6 +283,12 @@ [% IF user.id %]
+ [% IF Param('minor_update_group') && user.in_group(Param('minor_update_group')) %] + +
+ [% END %]
[% END %] diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl index 6bb75b985..f23c6b85c 100644 --- a/template/en/default/bug/edit.html.tmpl +++ b/template/en/default/bug/edit.html.tmpl @@ -1240,6 +1240,13 @@
+ [% IF Param('minor_update_group') && user.in_group(Param('minor_update_group')) %] +
+ + + [% END %]
[% END %] [% END %] diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl index d956fa62b..4ef067201 100644 --- a/template/en/default/list/edit-multiple.html.tmpl +++ b/template/en/default/list/edit-multiple.html.tmpl @@ -395,6 +395,13 @@ [%+ Hook.process('after_groups') %] +[% IF Param('minor_update_group') && user.in_group(Param('minor_update_group')) %] +
+ + +[% END %] [%############################################################################%] [%# Select Menu Block #%] -- cgit v1.2.3-24-g4f1b