From 92f5fd1d235adf2cfd64578b706eec5ce0c7b3d2 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 10 Aug 2026 16:46:17 +0200 Subject: [PATCH 01/12] Bug 1883428 - Improve the needinfo email situation (use HTML, drop unnecessary emails) Remove Flag::notify()'s separate text-only flag emails and instead add flag requestee/requester and per-flagtype cc_list addresses to the recipient list built by BugMail.pm, so flag grant/deny/needinfo notices ride along with the normal bug-change email instead of arriving as a second, plaintext-only message (also fixes bug 1410772). - Add REL_FLAG_REQUESTEE/REL_FLAG_REQUESTER/REL_FLAG_TYPE_CC relationships; requestee/requester respect the existing EVT_FLAG_REQUESTED/EVT_REQUESTED_FLAG opt-in, type_cc is unconditional (matches notify()'s prior behavior) - Add a flag-events section to bugmail.txt.tmpl/bugmail.html.tmpl, with more descriptive wording for the needinfo requestee - Preserve cc_list addresses with no Bugzilla account via a small side path (bugmail-flagtype-cc.txt.tmpl) sent directly through MessageToMTA --- Bugzilla/BugMail.pm | 193 +++++++++++++++++- Bugzilla/Constants.pm | 14 +- Bugzilla/Flag.pm | 156 +------------- .../email/bugmail-flagtype-cc.txt.tmpl | 32 +++ template/en/default/email/bugmail.html.tmpl | 24 +++ template/en/default/email/bugmail.txt.tmpl | 12 ++ .../en/default/global/reason-descs.none.tmpl | 3 + template/en/default/request/email.txt.tmpl | 107 ---------- 8 files changed, 271 insertions(+), 270 deletions(-) create mode 100644 template/en/default/email/bugmail-flagtype-cc.txt.tmpl delete mode 100644 template/en/default/request/email.txt.tmpl diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index a8cd371129..aa5f694b76 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -16,7 +16,9 @@ use Bugzilla::User; use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Bug; +use Bugzilla::Attachment; use Bugzilla::Comment; +use Bugzilla::FlagType; use Bugzilla::Logging; use Bugzilla::Mailer; use Bugzilla::Hook; @@ -199,6 +201,30 @@ sub Send { } } + # Bug 1883428: someone a flag was requested of, or who requested a flag + # that's now been granted/denied, becomes a recipient even without any + # other role on the bug. Flag-type cc_list addresses (admin-configured, + # bypasses per-user opt-in) are added the same way notify() used to. + my @flag_events + = $params->{dep_only} + ? () + : _get_flag_mail_events($bug, $start, $end, \%user_cache); + + foreach my $event (@flag_events) { + if ($event->{action} eq 'requested') { + $recipients{$event->{requestee_id}}->{+REL_FLAG_REQUESTEE} = BIT_DIRECT; + } + elsif ($event->{action} eq 'answered') { + $recipients{$event->{requester_id}}->{+REL_FLAG_REQUESTER} = BIT_DIRECT; + } + } + + my ($flag_type_cc, $flag_type_cc_raw) + = _get_flag_type_cc($bug, \@flag_events, \%user_cache); + foreach my $user_id (keys %$flag_type_cc) { + $recipients{$user_id}->{+REL_FLAG_TYPE_CC} = BIT_DIRECT; + } + # Make sure %user_cache has every user in it so far referenced foreach my $user_id (keys %recipients) { $user_cache{$user_id} ||= new Bugzilla::User({id => $user_id, cache => 1}); @@ -272,11 +298,29 @@ sub Send { # Go through each role the user has and see if they want mail in # that role. foreach my $relationship (keys %{$recipients{$user_id}}) { - if ($user->wants_bug_mail( - $bug, $relationship, $start ? \@diffs : [], - $comments, $params->{dep_only}, $changer - )) - { + my $wants_mail; + if ($relationship == REL_FLAG_REQUESTEE) { + + # Same opt-in flag "requested of me" has always used, not the + # normal per-field wants_bug_mail() logic (flags aren't a diffed + # bug field from this recipient's point of view). + $wants_mail = $user->wants_mail([EVT_FLAG_REQUESTED], REL_ANY); + } + elsif ($relationship == REL_FLAG_REQUESTER) { + $wants_mail = $user->wants_mail([EVT_REQUESTED_FLAG], REL_ANY); + } + elsif ($relationship == REL_FLAG_TYPE_CC) { + + # Admin-configured list; always notified, like notify() did. + $wants_mail = 1; + } + else { + $wants_mail = $user->wants_bug_mail( + $bug, $relationship, $start ? \@diffs : [], + $comments, $params->{dep_only}, $changer + ); + } + if ($wants_mail) { $rels_which_want{$relationship} = $recipients{$user_id}->{$relationship}; } } @@ -330,6 +374,16 @@ sub Send { $blocker_short_desc = $blocker_entry->{short_desc} if $blocker_entry; } + # Flag events relevant to this recipient: they were asked, or they + # asked and someone else answered (no need to tell people about + # their own actions). + my @user_flag_events = grep { + ($_->{action} eq 'requested' && $_->{requestee_id} == $user_id) + || ($_->{action} eq 'answered' + && $_->{requester_id} == $user_id + && $_->{setter}->id != $user_id) + } @flag_events; + my $sent_mail = sendMail({ to => $user, bug => $bug, @@ -342,12 +396,15 @@ sub Send { referenced_bugs => $referenced_bugs, dep_only => $params->{dep_only}, blocker_short_desc => $blocker_short_desc, + flag_events => \@user_flag_events, }); push(@sent, $user->login) if $sent_mail; } } } + _send_flag_type_cc_raw_mail($bug, $flag_type_cc_raw, $date); + # When sending bugmail about a blocker being reopened or resolved, # we say nothing about changes in the bug being blocked, so we must # not update lastdiffed in this case. @@ -373,6 +430,7 @@ sub sendMail { my $referenced_bugs = $params->{referenced_bugs}; my $dep_only = $params->{dep_only}; my $blocker_short_desc = $params->{blocker_short_desc}; + my $flag_events = $params->{flag_events} || []; my $attach_id; # Only display changes the user is allowed see. @@ -482,6 +540,7 @@ sub sendMail { referenced_bugs => $referenced_bugs, bugmailtype => $bugmailtype, blocker_short_desc => $blocker_short_desc, + flag_events => $flag_events, }; if (Bugzilla->get_param_with_override('use_mailer_queue')) { @@ -701,6 +760,130 @@ sub _generate_bugmail { return $email; } +# Find flags that were requested of, or answered by, someone during this +# window, so sendMail() can add a note at the top of the mail for that +# person even if they hold no other role on the bug (bug 1883428). +sub _get_flag_mail_events { + my ($bug, $start, $end, $user_cache) = @_; + my $dbh = Bugzilla->dbh; + + my @args = ($bug->id); + my $when_restriction = ''; + if ($start) { + $when_restriction = ' AND flag_when > ? AND flag_when <= ?'; + push @args, ($start, $end); + } + + my $rows = $dbh->selectall_arrayref( + "SELECT flag_id, flag_when, type_id, status, setter_id, requestee_id, + attachment_id + FROM flag_activity + WHERE bug_id = ?$when_restriction + ORDER BY flag_when", {Slice => {}}, @args + ); + + my @events; + foreach my $row (@$rows) { + $user_cache->{$row->{setter_id}} + ||= Bugzilla::User->new({id => $row->{setter_id}, cache => 1}); + + if ($row->{status} eq '?' && $row->{requestee_id}) { + push @events, + { + action => 'requested', + type => Bugzilla::FlagType->new({id => $row->{type_id}, cache => 1}), + attachment_id => $row->{attachment_id}, + requestee_id => $row->{requestee_id}, + setter => $user_cache->{$row->{setter_id}}, + }; + } + elsif ($row->{status} eq '+' || $row->{status} eq '-') { + + # flags.setter_id gets overwritten to whoever granted/denied the + # flag, so the requester has to be found by looking back at this + # flag's most recent '?' activity row. + my ($requester_id) = $dbh->selectrow_array( + "SELECT setter_id FROM flag_activity + WHERE flag_id = ? AND status = '?' AND flag_when <= ? + ORDER BY flag_when DESC LIMIT 1", undef, $row->{flag_id}, + $row->{flag_when} + ); + next unless $requester_id; + + push @events, + { + action => 'answered', + type => Bugzilla::FlagType->new({id => $row->{type_id}, cache => 1}), + attachment_id => $row->{attachment_id}, + requester_id => $requester_id, + status => $row->{status}, + setter => $user_cache->{$row->{setter_id}}, + }; + } + } + return @events; +} + +# A flag type's cc_list is an admin-configured list of addresses notified +# on any status change of that type, independent of role on the bug -- +# this is the same computation notify() used to do per flag change. +# Returns (\%account_recipients keyed by user id, \%raw_addresses keyed by +# email for addresses with no Bugzilla account, each valued with the +# flag_events relevant to that address). +sub _get_flag_type_cc { + my ($bug, $flag_events, $user_cache) = @_; + my @bug_in_groups = grep { $_->{ison} || $_->{mandatory} } @{$bug->groups}; + + my (%account_recipients, %raw_addresses); + foreach my $event (@$flag_events) { + my $cc_list = $event->{type}->cc_list; + next unless $cc_list; + + my $attachment_is_private + = $event->{attachment_id} + ? Bugzilla::Attachment->new({id => $event->{attachment_id}, cache => 1}) + ->isprivate + : 0; + + foreach my $cc (split(/[, ]+/, $cc_list)) { + my $ccuser = Bugzilla::User->new({name => $cc, cache => 1}); + next if @bug_in_groups && (!$ccuser || !$ccuser->can_see_bug($bug->id)); + next if $attachment_is_private && (!$ccuser || !$ccuser->is_insider); + + if ($ccuser) { + $account_recipients{$ccuser->id} = 1; + $user_cache->{$ccuser->id} ||= $ccuser; + } + else { + push @{$raw_addresses{$cc}}, $event; + } + } + } + return (\%account_recipients, \%raw_addresses); +} + +# cc_list can list addresses with no Bugzilla account. The normal recipient +# pipeline above is keyed on user id throughout (wants_bug_mail, language +# prefs, mailer-queue enqueue/dequeue...), so these get a minimal, separate +# notice instead of going through it. +sub _send_flag_type_cc_raw_mail { + my ($bug, $raw_addresses, $date) = @_; + return unless %$raw_addresses; + + my $lang = Bugzilla::User->new()->setting('lang'); + my $template = Bugzilla->template_inner($lang); + + foreach my $to (keys %$raw_addresses) { + my $message; + $template->process( + "email/bugmail-flagtype-cc.txt.tmpl", + {to => $to, bug => $bug, date => $date, flag_events => $raw_addresses->{$to}}, + \$message + ) || ThrowTemplateError($template->error()); + MessageToMTA($message); + } +} + sub _get_diffs { my ($bug, $end, $user_cache) = @_; my $dbh = Bugzilla->dbh; diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 2890df0ca6..b87350386d 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -89,6 +89,7 @@ use Memoize; RELATIONSHIPS REL_ASSIGNEE REL_QA REL_REPORTER REL_CC REL_GLOBAL_WATCHER + REL_FLAG_REQUESTEE REL_FLAG_REQUESTER REL_FLAG_TYPE_CC REL_ANY POS_EVENTS @@ -355,14 +356,21 @@ use constant REL_CC => 3; # REL 4 was REL_VOTER, before it was moved ino an extension. use constant REL_GLOBAL_WATCHER => 5; +# Bug 1883428: recipients added because of a flag change, independently of +# any other role they may hold on the bug. +use constant REL_FLAG_REQUESTEE => 6; # A flag was requested of them +use constant REL_FLAG_REQUESTER => 7; # Their flag request was granted/denied +use constant REL_FLAG_TYPE_CC => 8; # On the flag type's admin-configured cc_list + # We need these strings for the X-Bugzilla-Reasons header # Note: this hash uses "," rather than "=>" to avoid auto-quoting of the LHS. # This should be accessed through Bugzilla::BugMail::relationships() instead # of being accessed directly. use constant RELATIONSHIPS => { - REL_ASSIGNEE, "AssignedTo", REL_REPORTER, "Reporter", - REL_QA, "QAcontact", REL_CC, "CC", - REL_GLOBAL_WATCHER, "GlobalWatcher" + REL_ASSIGNEE, "AssignedTo", REL_REPORTER, "Reporter", + REL_QA, "QAcontact", REL_CC, "CC", + REL_GLOBAL_WATCHER, "GlobalWatcher", REL_FLAG_REQUESTEE, "FlagRequestee", + REL_FLAG_REQUESTER, "FlagRequester", REL_FLAG_TYPE_CC, "FlagTypeCC" }; # Used for global events like EVT_FLAG_REQUESTED diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 9d90970636..f582bfcaa0 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -48,7 +48,6 @@ use Bugzilla::Hook; use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Error; -use Bugzilla::Mailer; use Bugzilla::Constants; use Bugzilla::Field; @@ -69,8 +68,6 @@ use constant AUDIT_REMOVES => 0; use constant SKIP_REQUESTEE_ON_ERROR => 1; -our $disable_flagmail = 0; - sub DB_COLUMNS { my $dbh = Bugzilla->dbh; return qw( @@ -580,20 +577,15 @@ sub update_flags { $new_flag->{id} = $flag->id; $new_flag->{creation_date} = format_time($timestamp, '%Y-%m-%d %H:%i:%s'); $new_flag->{modification_date} = format_time($timestamp, '%Y-%m-%d %H:%i:%s'); - $class->notify($new_flag, undef, $self, $timestamp); } else { - my $changes = $new_flag->update($timestamp); - if (scalar(keys %$changes)) { - $class->notify($new_flag, $old_flags{$new_flag->id}, $self, $timestamp); - } + $new_flag->update($timestamp); delete $old_flags{$new_flag->id}; } } # These flags have been deleted. foreach my $old_flag (values %old_flags) { - $class->notify(undef, $old_flag, $self, $timestamp); # BMO - provide a hook which passes the timestamp, # because that isn't passed to remove_from_db(). @@ -714,7 +706,6 @@ sub force_retarget { else { # Track deleted attachment flags. push(@removed, $class->snapshot([$flag])) if $flag->attach_id; - $class->notify(undef, $flag, $bug || $flag->bug); # BMO - provide a hook which passes the timestamp, # because that isn't passed to remove_from_db(). @@ -1066,151 +1057,6 @@ sub extract_flags_from_cgi { return (\@flags, \@new_flags); } -=pod - -=over - -=item C - -Sends an email notification about a flag being created, fulfilled -or deleted. - -=back - -=cut - -sub notify { - my ($class, $flag, $old_flag, $obj, $timestamp) = @_; - - if ($disable_flagmail) { - return; - } - - my ($bug, $attachment); - if (blessed($obj) && $obj->isa('Bugzilla::Attachment')) { - $attachment = $obj; - $bug = $attachment->bug; - } - elsif (blessed($obj) && $obj->isa('Bugzilla::Bug')) { - $bug = $obj; - } - else { - # Not a good time to throw an error. - return; - } - - my $addressee; - - # If the flag is set to '?', maybe the requestee wants a notification. - if ( $flag - && $flag->requestee_id - && (!$old_flag || ($old_flag->requestee_id || 0) != $flag->requestee_id)) - { - if ($flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) { - $addressee = $flag->requestee; - } - } - elsif ($old_flag - && $old_flag->status eq '?' - && (!$flag || $flag->status ne '?')) - { - if ($old_flag->setter->wants_mail([EVT_REQUESTED_FLAG])) { - $addressee = $old_flag->setter; - } - } - - my $cc_list = $flag ? $flag->type->cc_list : $old_flag->type->cc_list; - $cc_list //= ''; - - # Is there someone to notify? - return unless ($addressee || $cc_list); - - # The email client will display the Date: header in the desired timezone, - # so we can always use UTC here. - $timestamp ||= Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); - $timestamp = format_time($timestamp, '%a, %d %b %Y %T %z', 'UTC'); - - # If the target bug is restricted to one or more groups, then we need - # to make sure we don't send email about it to unauthorized users - # on the request type's CC: list, so we have to trawl the list for users - # not in those groups or email addresses that don't have an account. - my @bug_in_groups = grep { $_->{'ison'} || $_->{'mandatory'} } @{$bug->groups}; - my $attachment_is_private = $attachment ? $attachment->isprivate : undef; - - my %recipients; - foreach my $cc (split(/[, ]+/, $cc_list)) { - my $ccuser = new Bugzilla::User({name => $cc, cache => 1}); - next - if (scalar(@bug_in_groups) - && (!$ccuser || !$ccuser->can_see_bug($bug->bug_id))); - next if $attachment_is_private && (!$ccuser || !$ccuser->is_insider); - - # Prevent duplicated entries due to case sensitivity. - $cc = $ccuser ? $ccuser->email : $cc; - $recipients{$cc} = $ccuser; - } - - # Only notify if the addressee is allowed to receive the email - # and can see the bug (prevents short_desc leaking via Subject/body). - if ( - $addressee - && $addressee->email_enabled - && ( (!scalar(@bug_in_groups) || $addressee->can_see_bug($bug->bug_id)) - && (!$attachment_is_private || $addressee->is_insider)) - ) - { - $recipients{$addressee->email} = $addressee; - } - - return unless keys %recipients; - - # Process and send notification for each recipient. - # If there are users in the CC list who don't have an account, - # use the default language for email notifications. - my $default_lang; - if (grep { !$_ } values %recipients) { - $default_lang = Bugzilla::User->new()->setting('lang'); - } - - # Get comments on the bug - my $all_comments = $bug->comments({after => $bug->lastdiffed}); - @$all_comments = grep { $_->type || $_->body =~ /\S/ } @$all_comments; - - # Get public only comments - my $public_comments = [grep { !$_->is_private } @$all_comments]; - - foreach my $to (keys %recipients) { - - # Add threadingmarker to allow flag notification emails to be the - # threaded similar to normal bug change emails. - my $thread_user_id = $recipients{$to} ? $recipients{$to}->id : 0; - - # We only want to show private comments to users in the is_insider group - my $comments = $recipients{$to} - && $recipients{$to}->is_insider ? $all_comments : $public_comments; - - my $vars = { - flag => $flag, - old_flag => $old_flag, - to => $to, - date => $timestamp, - bug => $bug, - attachment => $attachment, - threadingmarker => build_thread_marker($bug->id, $thread_user_id), - new_comments => $comments, - }; - - my $lang = $recipients{$to} ? $recipients{$to}->setting('lang') : $default_lang; - - my $template = Bugzilla->template_inner($lang); - my $message; - $template->process("request/email.txt.tmpl", $vars, \$message) - || ThrowTemplateError($template->error()); - - MessageToMTA($message); - } -} - # This is an internal function used by $bug->flag_types # and $attachment->flag_types to collect data about available # flag types and existing flags set on them. You should never diff --git a/template/en/default/email/bugmail-flagtype-cc.txt.tmpl b/template/en/default/email/bugmail-flagtype-cc.txt.tmpl new file mode 100644 index 0000000000..48b5f63c5b --- /dev/null +++ b/template/en/default/email/bugmail-flagtype-cc.txt.tmpl @@ -0,0 +1,32 @@ +[%# The contents of this file are subject to the Mozilla Public + # License Version 1.1 (the "License"); you may not use this file + # except in compliance with the License. You may obtain a copy of + # the License at http://www.mozilla.org/MPL/ + # + # Software distributed under the License is distributed on an "AS + # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or + # implied. See the License for the specific language governing + # rights and limitations under the License. + #%] + +[%# ponytail: minimal fallback for flag type cc_list addresses with no + # Bugzilla account -- the normal bugmail pipeline is keyed on user id + # throughout and has no path to a raw address. Revisit if this needs to + # carry more than a one-line-per-event summary (diffs, comments, HTML). %] + +From: [% Param('mailfrom') %] +To: [% to %] +Subject: [[% terms.Bug %] [%+ bug.id %]] [% bug.short_desc %] +Date: [% date %] +X-Bugzilla-Type: request +[%+ INCLUDE "email/header-common.txt.tmpl" %] + +[%+ urlbase %]show_bug.cgi?id=[% bug.id %] + +[% FOREACH event = flag_events %] +[% IF event.action == 'requested' %] +[%+ event.setter.identity %] requested [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %]. +[% ELSIF event.action == 'answered' %] +[%+ event.setter.identity %] [% event.status == '+' ? 'granted' : 'denied' %] [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %]. +[% END %] +[% END %] diff --git a/template/en/default/email/bugmail.html.tmpl b/template/en/default/email/bugmail.html.tmpl index 3f7d3d246b..83525f4ab1 100644 --- a/template/en/default/email/bugmail.html.tmpl +++ b/template/en/default/email/bugmail.html.tmpl @@ -26,6 +26,30 @@ + [% IF flag_events.size %] + +
+ [% END %] [% PROCESS generate_diffs %]

[% SET comment_anchor = '' %] diff --git a/template/en/default/email/bugmail.txt.tmpl b/template/en/default/email/bugmail.txt.tmpl index 9bfbc15bf7..c4a529f9eb 100644 --- a/template/en/default/email/bugmail.txt.tmpl +++ b/template/en/default/email/bugmail.txt.tmpl @@ -25,6 +25,18 @@ [% isnew = bug.lastdiffed ? 0 : 1 %] +[% FOREACH event = flag_events %] +[% IF event.action == 'requested' %] +[% IF event.type.name == 'needinfo' %] +[%+ event.setter.identity %] needs more information from you to work on this [% terms.bug %]. Please answer any open questions or provide the requested details in a comment below. +[% ELSE %] +[%+ event.setter.identity %] has requested [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] from you on this [% terms.bug %]. +[% END %] +[% ELSIF event.action == 'answered' %] +Your request for [% event.type.name %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] has been [% event.status == '+' ? 'granted' : 'denied' %] by [% event.setter.identity %]. +[% END %] + +[% END %] [%+ PROCESS generate_diffs -%] [% FOREACH comment = new_comments %] diff --git a/template/en/default/global/reason-descs.none.tmpl b/template/en/default/global/reason-descs.none.tmpl index b79a5bbb4f..df0c14a144 100644 --- a/template/en/default/global/reason-descs.none.tmpl +++ b/template/en/default/global/reason-descs.none.tmpl @@ -24,6 +24,9 @@ ${constants.REL_QA} => "You are the QA Contact for the ${terms.bug}.", ${constants.REL_CC} => "You are on the CC list for the ${terms.bug}.", ${constants.REL_GLOBAL_WATCHER} => "You are watching all $terms.bug changes.", + ${constants.REL_FLAG_REQUESTEE} => "A flag was requested of you on this ${terms.bug}.", + ${constants.REL_FLAG_REQUESTER} => "You requested a flag that was set on this ${terms.bug}.", + ${constants.REL_FLAG_TYPE_CC} => "You are on the CC list for this flag type.", } %] [% SET watch_reason_descs => { diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl deleted file mode 100644 index 2b9ea37827..0000000000 --- a/template/en/default/request/email.txt.tmpl +++ /dev/null @@ -1,107 +0,0 @@ -[%# The contents of this file are subject to the Mozilla Public - # License Version 1.1 (the "License"); you may not use this file - # except in compliance with the License. You may obtain a copy of - # the License at http://www.mozilla.org/MPL/ - # - # Software distributed under the License is distributed on an "AS - # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or - # implied. See the License for the specific language governing - # rights and limitations under the License. - # - # The Original Code is the Bugzilla Bug Tracking System. - # - # The Initial Developer of the Original Code is Netscape Communications - # Corporation. Portions created by Netscape are - # Copyright (C) 1998 Netscape Communications Corporation. All - # Rights Reserved. - # - # Contributor(s): Myk Melez - # Jeff Hedlund - # Frédéric Buclin - #%] - -[% PROCESS global/variables.none.tmpl %] - -[% bugidsummary = bug.bug_id _ ': ' _ bug.short_desc %] -[% attidsummary = attachment.id _ ': ' _ attachment.description %] -[% flagtype_name = flag ? flag.type.name : old_flag.type.name %] -[% statuses = { '+' => "granted" , '-' => 'not granted' , 'X' => "canceled" , '?' => "asked" } %] - -[% to_identity = "" %] -[% on_behalf_of = 0 %] -[% action = flag.status || 'X' %] - -[% - attach_url = { title => "", href => "" }; - Hook.process("start"); -%] - -[% IF flag && flag.status == '?' %] - [% subject_status = "requested" %] - [% IF flag.setter_id == user.id %] - [% to_identity = flag.requestee.identity _ " for" %] - [% ELSE %] - [% on_behalf_of = 1 %] - [% IF flag.requestee %][% to_identity = " to " _ flag.requestee.identity %][% END %] - [% END %] -[% ELSE %] - [% IF old_flag && old_flag.status == '?' %] - [% to_identity = old_flag.setter.identity _ "'s request for" %] - [% END %] - [% subject_status = statuses.$action %] -[% END %] -From: [% Param('mailfrom') %] -To: [% to %] -Subject: [% flagtype_name %] [%+ subject_status %]: [[% terms.Bug %] [%+ bug.bug_id %]] [% bug.short_desc %] -[%- IF attachment %] : - [Attachment [% attachment.id %]] [% attachment.description FILTER clean_text %][% END %] -Date: [% date %] -X-Bugzilla-Type: request -[%- IF flag.requestee %] -X-Bugzilla-Flag-Requestee: [% flag.requestee.email %] -[% END %] -[%+ INCLUDE "email/header-common.txt.tmpl" %] -[%+ threadingmarker %] - -[%+ USE wrap -%] -[%- FILTER bullet = wrap(80) -%] - -[% IF on_behalf_of %] -[% user.identity %] has reassigned [% flag.setter.identity %]'s request for [% flagtype_name %] -[% to_identity %]: -[% ELSE %] -[% user.identity %] has [% statuses.$action %] [%+ to_identity %] [%+ flagtype_name %]: -[% END %] - -[% terms.Bug %] [%+ bugidsummary %] -[% END %] -[%+ urlbase %]show_bug.cgi?id=[% bug.bug_id %] -[% IF attachment %] - -[% FILTER bullet = wrap(80) %] -Attachment [% attidsummary %] -[%- END %] -[% IF attach_url.href %] -[%+ attach_url.title _ ": " _ attach_url.href %] -[%- END %] -[%+ urlbase %]attachment.cgi?id=[% attachment.id %]&action=edit -[%- END %] - -[%- Hook.process('after_summary') -%] - -[%- FILTER bullet = wrap(80) %] - -[% FOREACH comment = new_comments %] - -[%- IF comment.count %] ---- Comment #[% comment.count %] from [% comment.author.identity %] --- -[% ELSE %] ---- Description --- -[% END %] -[%+ comment.body_full({ is_bugmail => 1, wrap => 1 }) FILTER strip_control_chars %] -[% END %] - -[%- END %] - --- [%# Protect the trailing space of the signature marker %] -@@body-headers@@ From 36bcec8d49e36fa943dbfe23c9e7d8e75e5b7142 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Tue, 11 Aug 2026 04:04:43 +0200 Subject: [PATCH 02/12] Bug 1883428 - Port flag-event bugmail content into the BMO extension templates The previous commit added a flag-event section to the core bugmail templates, but extensions/BMO/template/.../email/bugmail.{txt,html}.tmpl fully overrides those core templates, so the new section was never executed. Also fixes a real gap the core-only version missed: needinfo's normal resolution (auto-clear to flag status 'X' when the requestee replies) wasn't handled (only +/- were) so the single most common needinfo outcome silently produced no "your request was answered" notice. notify() covered this case; now BugMail.pm does too, with matching "cleared" wording alongside "granted" and "denied". - Port the flag-event section into the BMO override templates (txt + html), with a Hook.process('flag_event', ...) extension point mirroring the old request/email.txt.tmpl hook mechanism - Add Needinfo and Splinter hook fragments preserving their previous content (reporter-aware wording + wiki guide link; Splinter review-tool link, now gated on attachment.can_review so it also covers GitHub PR/Phabricator attachments, not just ispatch) - Inline BMO's own external-redirect (GitHub PR) attachment link directly in the BMO templates rather than hooking it: hooking it caused two fragments (BMO + Splinter) to concatenate with no separator when both fire for the same event, since TT's TRIM strips whitespace at each independently compiled hook fragment's own edges. The original design avoided this by having the parent template render that piece directly instead of through the shared print-hook slot - Resolve the attachment object in _get_flag_mail_events() so hooks can check can_review/external_redirect - Remove the three hook fragments orphaned by the previous commit's deletion of request/email.txt.tmpl --- Bugzilla/BugMail.pm | 24 +++++++++---- .../en/default/email/bugmail.html.tmpl | 33 +++++++++++++++++ .../en/default/email/bugmail.txt.tmpl | 18 ++++++++++ .../hook/email/bugmail-flag_event.html.tmpl | 18 ++++++++++ .../hook/email/bugmail-flag_event.txt.tmpl | 16 +++++++++ .../hook/request/email-after_summary.txt.tmpl | 35 ------------------- .../hook/email/bugmail-flag_event.html.tmpl | 14 ++++++++ .../hook/email/bugmail-flag_event.txt.tmpl} | 13 ++++--- .../hook/request/email-after_summary.txt.tmpl | 9 ----- template/en/default/email/bugmail.html.tmpl | 5 +-- template/en/default/email/bugmail.txt.tmpl | 6 ++-- 11 files changed, 129 insertions(+), 62 deletions(-) create mode 100644 extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.html.tmpl create mode 100644 extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.txt.tmpl delete mode 100644 extensions/Needinfo/template/en/default/hook/request/email-after_summary.txt.tmpl create mode 100644 extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.html.tmpl rename extensions/{BMO/template/en/default/hook/request/email-start.txt.tmpl => Splinter/template/en/default/hook/email/bugmail-flag_event.txt.tmpl} (54%) delete mode 100644 extensions/Splinter/template/en/default/hook/request/email-after_summary.txt.tmpl diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index aa5f694b76..a1a0d351af 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -793,13 +793,20 @@ sub _get_flag_mail_events { action => 'requested', type => Bugzilla::FlagType->new({id => $row->{type_id}, cache => 1}), attachment_id => $row->{attachment_id}, - requestee_id => $row->{requestee_id}, - setter => $user_cache->{$row->{setter_id}}, + attachment => $row->{attachment_id} + ? Bugzilla::Attachment->new({id => $row->{attachment_id}, cache => 1}) + : undef, + requestee_id => $row->{requestee_id}, + setter => $user_cache->{$row->{setter_id}}, }; } - elsif ($row->{status} eq '+' || $row->{status} eq '-') { + elsif ($row->{status} eq '+' || $row->{status} eq '-' || $row->{status} eq 'X') { - # flags.setter_id gets overwritten to whoever granted/denied the + # 'X' is a flag cleared without +/- (e.g. needinfo auto-cleared when + # the requestee replies) -- notify.() treated that the same as an + # explicit answer, so we do too. + # + # flags.setter_id gets overwritten to whoever granted/denied/cleared the # flag, so the requester has to be found by looking back at this # flag's most recent '?' activity row. my ($requester_id) = $dbh->selectrow_array( @@ -815,9 +822,12 @@ sub _get_flag_mail_events { action => 'answered', type => Bugzilla::FlagType->new({id => $row->{type_id}, cache => 1}), attachment_id => $row->{attachment_id}, - requester_id => $requester_id, - status => $row->{status}, - setter => $user_cache->{$row->{setter_id}}, + attachment => $row->{attachment_id} + ? Bugzilla::Attachment->new({id => $row->{attachment_id}, cache => 1}) + : undef, + requester_id => $requester_id, + status => $row->{status}, + setter => $user_cache->{$row->{setter_id}}, }; } } diff --git a/extensions/BMO/template/en/default/email/bugmail.html.tmpl b/extensions/BMO/template/en/default/email/bugmail.html.tmpl index 82053792be..9db1c7edc2 100644 --- a/extensions/BMO/template/en/default/email/bugmail.html.tmpl +++ b/extensions/BMO/template/en/default/email/bugmail.html.tmpl @@ -16,6 +16,39 @@ + [% IF flag_events.size %] +

+
    + [% FOREACH event = flag_events %] +
  • + [% IF event.action == 'requested' %] + [% IF event.type.name == 'needinfo' %] + [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %] + needs more information from you to work on this [% terms.bug %]. + [% ELSE %] + [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %] + has requested [% event.type.name FILTER html %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] + from you on this [% terms.bug %]. + [% END %] + [% IF event.attachment && event.attachment.external_redirect %] + [% external = event.attachment.external_redirect %] +
    + [% external.title FILTER html %] + [% END %] + [% ELSIF event.action == 'answered' %] + [% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] + Your request for [% event.type.name FILTER html %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] + has been [% status_word %] by + [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %]. + [% END %] + [% Hook.process('flag_event', 'email/bugmail.html.tmpl') %] +
    • + [% END %] +
+
+
+ [% END %] + [% IF !to_user.in_group('editbugs') %]
Do not reply to this email. You can add comments to this [% terms.bug %] at diff --git a/extensions/BMO/template/en/default/email/bugmail.txt.tmpl b/extensions/BMO/template/en/default/email/bugmail.txt.tmpl index 6fefa2295d..00fa39eff7 100644 --- a/extensions/BMO/template/en/default/email/bugmail.txt.tmpl +++ b/extensions/BMO/template/en/default/email/bugmail.txt.tmpl @@ -11,6 +11,24 @@ [% isnew = bug.lastdiffed ? 0 : 1 %] +[% FOREACH event = flag_events %] +[% IF event.action == 'requested' %] +[% IF event.type.name == 'needinfo' %] +[%+ event.setter.identity %] needs more information from you to work on this [% terms.bug %]. +[% ELSE %] +[%+ event.setter.identity %] has requested [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] from you on this [% terms.bug %]. +[% END %] +[% IF event.attachment && event.attachment.external_redirect %] +[% external = event.attachment.external_redirect %] +[%+ external.title _ ": " _ event.attachment.data %] +[% END %] +[% ELSIF event.action == 'answered' %] +[% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] +Your request for [% event.type.name %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] has been [% status_word %] by [% event.setter.identity %]. +[% END %] +[%+ Hook.process('flag_event', 'email/bugmail.txt.tmpl') %] + +[% END %] [% IF !to_user.in_group('editbugs') %] Do not reply to this email. You can add comments to this [% terms.bug %] at [% END %] diff --git a/extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.html.tmpl b/extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.html.tmpl new file mode 100644 index 0000000000..0e479a9c74 --- /dev/null +++ b/extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.html.tmpl @@ -0,0 +1,18 @@ +[%# 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. + #%] + +[% RETURN UNLESS event.action == 'requested' && event.type.name == 'needinfo' %] +
+[% IF event.requestee_id == bug.reporter.id %] +Since you reported this [% terms.bug %], the person asking needs your input to understand it. +Responding to the question will enable developers to take further action on this [% terms.bug %]. +[% ELSE %] +Please respond as soon as possible so developers may take action on this [% terms.bug %]. +[% END %] +If you have questions about responding to needinfo requests, please see the +needinfo user guide. diff --git a/extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.txt.tmpl b/extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.txt.tmpl new file mode 100644 index 0000000000..6bf288b3e3 --- /dev/null +++ b/extensions/Needinfo/template/en/default/hook/email/bugmail-flag_event.txt.tmpl @@ -0,0 +1,16 @@ +[%# 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. + #%] + +[% RETURN UNLESS event.action == 'requested' && event.type.name == 'needinfo' %] +[% IF event.requestee_id == bug.reporter.id %] +Since you reported this [% terms.bug %], the person asking needs your input to understand it. Responding to the question will enable developers to take further action on this [% terms.bug %]. +[% ELSE %] +Please respond as soon as possible so developers may take action on this [% terms.bug %]. +[% END %] +If you have questions about responding to needinfo requests, please see +https://wiki.mozilla.org/BMO/UserGuide#Needinfo_Flag. diff --git a/extensions/Needinfo/template/en/default/hook/request/email-after_summary.txt.tmpl b/extensions/Needinfo/template/en/default/hook/request/email-after_summary.txt.tmpl deleted file mode 100644 index 66a6d1ad16..0000000000 --- a/extensions/Needinfo/template/en/default/hook/request/email-after_summary.txt.tmpl +++ /dev/null @@ -1,35 +0,0 @@ -[%# 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. - #%] - -[% RETURN UNLESS flag && flag.type.name == 'needinfo' && flag.status == '?' %] ---- ---- Hello from [% terms.Bugzilla %], the [% terms.bug %] tracker for Mozilla and Firefox: ---- ---- Someone has asked you for information about this [% terms.bug %]. ---- ---- Please go to [% urlbase %]show_bug.cgi?id=[% bug.bug_id %] to respond ---- to the question below. -[% IF flag.requestee.login_name == bug.reporter.login_name %] ---- ---- Since you reported this [% terms.bug %], then the person asking the ---- question needs more information from you to understand it. ---- ---- Responding to the question will enable developers to take further ---- action on this [% terms.bug %]. -[% ELSE %] ---- ---- Please log into [% terms.Bugzilla %] and respond as soon as possible ---- so developers may take action on this [% terms.bug %]. -[% END %] ---- ---- If you have questions about responding to needinfo requests, please ---- see https://wiki.mozilla.org/BMO/UserGuide#Needinfo_Flag. ---- ---- Thank you ---- ---- diff --git a/extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.html.tmpl b/extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.html.tmpl new file mode 100644 index 0000000000..280924c822 --- /dev/null +++ b/extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.html.tmpl @@ -0,0 +1,14 @@ +[%# 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. + #%] + +[% USE Bugzilla %] +[% RETURN UNLESS event.action == 'requested' + && (event.type.name == 'review' || event.type.name == 'feedback') + && event.attachment && event.attachment.can_review %] +
+Review diff --git a/extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl b/extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.txt.tmpl similarity index 54% rename from extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl rename to extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.txt.tmpl index bd2af3d6fa..ff027ac55e 100644 --- a/extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl +++ b/extensions/Splinter/template/en/default/hook/email/bugmail-flag_event.txt.tmpl @@ -6,10 +6,9 @@ # defined by the Mozilla Public License, v. 2.0. #%] -[% - RETURN UNLESS attachment; - external = attachment.external_redirect; - RETURN UNLESS external; - attach_url.title = external.title; - attach_url.href = attachment.data; -%] +[% USE Bugzilla %] +[% RETURN UNLESS event.action == 'requested' + && (event.type.name == 'review' || event.type.name == 'feedback') + && event.attachment && event.attachment.can_review %] + +Review [%+ Bugzilla.splinter_review_url(bug.id, event.attachment_id, 1) FILTER none %] diff --git a/extensions/Splinter/template/en/default/hook/request/email-after_summary.txt.tmpl b/extensions/Splinter/template/en/default/hook/request/email-after_summary.txt.tmpl deleted file mode 100644 index 20a56dbaad..0000000000 --- a/extensions/Splinter/template/en/default/hook/request/email-after_summary.txt.tmpl +++ /dev/null @@ -1,9 +0,0 @@ -[% USE Bugzilla %] -[% IF flag && flag.status == '?' - && (flag.type.name == 'review' || flag.type.name == 'feedback') - && attachment && attachment.ispatch %] - -Review -[%+ Bugzilla.splinter_review_url(bug.bug_id, attachment.id, 1) FILTER none %] -[%- END %] - diff --git a/template/en/default/email/bugmail.html.tmpl b/template/en/default/email/bugmail.html.tmpl index 83525f4ab1..b780ef8b3c 100644 --- a/template/en/default/email/bugmail.html.tmpl +++ b/template/en/default/email/bugmail.html.tmpl @@ -34,17 +34,18 @@ [% IF event.type.name == 'needinfo' %] [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %] needs more information from you to work on this [% terms.bug %]. - Please answer any open questions or provide the requested details in a comment below. [% ELSE %] [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %] has requested [% event.type.name FILTER html %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] from you on this [% terms.bug %]. [% END %] [% ELSIF event.action == 'answered' %] + [% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] Your request for [% event.type.name FILTER html %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] - has been [% event.status == '+' ? 'granted' : 'denied' %] by + has been [% status_word %] by [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %]. [% END %] + [% Hook.process('flag_event', 'email/bugmail.html.tmpl') %] [% END %] diff --git a/template/en/default/email/bugmail.txt.tmpl b/template/en/default/email/bugmail.txt.tmpl index c4a529f9eb..56f76c4dc5 100644 --- a/template/en/default/email/bugmail.txt.tmpl +++ b/template/en/default/email/bugmail.txt.tmpl @@ -28,13 +28,15 @@ [% FOREACH event = flag_events %] [% IF event.action == 'requested' %] [% IF event.type.name == 'needinfo' %] -[%+ event.setter.identity %] needs more information from you to work on this [% terms.bug %]. Please answer any open questions or provide the requested details in a comment below. +[%+ event.setter.identity %] needs more information from you to work on this [% terms.bug %]. [% ELSE %] [%+ event.setter.identity %] has requested [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] from you on this [% terms.bug %]. [% END %] [% ELSIF event.action == 'answered' %] -Your request for [% event.type.name %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] has been [% event.status == '+' ? 'granted' : 'denied' %] by [% event.setter.identity %]. +[% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] +Your request for [% event.type.name %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] has been [% status_word %] by [% event.setter.identity %]. [% END %] +[%+ Hook.process('flag_event', 'email/bugmail.txt.tmpl') %] [% END %] [%+ PROCESS generate_diffs -%] From a2ea1b7314cc15a28e876a7c9a1e0b1bf71d19a6 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 12 Aug 2026 16:48:51 +0200 Subject: [PATCH 03/12] Bug 1883428 - Fix missing FILTER directives to flag-event bugmail HTML template --- extensions/BMO/template/en/default/email/bugmail.html.tmpl | 6 +++--- template/en/default/email/bugmail.html.tmpl | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/BMO/template/en/default/email/bugmail.html.tmpl b/extensions/BMO/template/en/default/email/bugmail.html.tmpl index 9db1c7edc2..5282418db2 100644 --- a/extensions/BMO/template/en/default/email/bugmail.html.tmpl +++ b/extensions/BMO/template/en/default/email/bugmail.html.tmpl @@ -27,7 +27,7 @@ needs more information from you to work on this [% terms.bug %]. [% ELSE %] [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %] - has requested [% event.type.name FILTER html %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] + has requested [% event.type.name FILTER html %][% " for attachment ${event.attachment_id}" FILTER none IF event.attachment_id %] from you on this [% terms.bug %]. [% END %] [% IF event.attachment && event.attachment.external_redirect %] @@ -37,8 +37,8 @@ [% END %] [% ELSIF event.action == 'answered' %] [% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] - Your request for [% event.type.name FILTER html %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] - has been [% status_word %] by + Your request for [% event.type.name FILTER html %][% " on attachment ${event.attachment_id}" FILTER none IF event.attachment_id %] + has been [% status_word FILTER html %] by [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %]. [% END %] [% Hook.process('flag_event', 'email/bugmail.html.tmpl') %] diff --git a/template/en/default/email/bugmail.html.tmpl b/template/en/default/email/bugmail.html.tmpl index b780ef8b3c..b828768a9f 100644 --- a/template/en/default/email/bugmail.html.tmpl +++ b/template/en/default/email/bugmail.html.tmpl @@ -36,13 +36,13 @@ needs more information from you to work on this [% terms.bug %]. [% ELSE %] [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %] - has requested [% event.type.name FILTER html %][% " for attachment ${event.attachment_id}" IF event.attachment_id %] + has requested [% event.type.name FILTER html %][% " for attachment ${event.attachment_id}" FILTER none IF event.attachment_id %] from you on this [% terms.bug %]. [% END %] [% ELSIF event.action == 'answered' %] [% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] - Your request for [% event.type.name FILTER html %][% " on attachment ${event.attachment_id}" IF event.attachment_id %] - has been [% status_word %] by + Your request for [% event.type.name FILTER html %][% " on attachment ${event.attachment_id}" FILTER none IF event.attachment_id %] + has been [% status_word FILTER html %] by [% INCLUDE global/user.html.tmpl user = to_user, who = event.setter %]. [% END %] [% Hook.process('flag_event', 'email/bugmail.html.tmpl') %] From 98250c62f5531a5f5c05407881674162849e4492 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 19 Aug 2026 17:12:37 +0200 Subject: [PATCH 04/12] Bug 1883428 - Only treat an X/answer flag_activity row as answering the immediately preceding requester --- Bugzilla/BugMail.pm | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index a1a0d351af..8700ac37be 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -808,14 +808,18 @@ sub _get_flag_mail_events { # # flags.setter_id gets overwritten to whoever granted/denied/cleared the # flag, so the requester has to be found by looking back at this - # flag's most recent '?' activity row. - my ($requester_id) = $dbh->selectrow_array( - "SELECT setter_id FROM flag_activity - WHERE flag_id = ? AND status = '?' AND flag_when <= ? + # flag's activity. Only treat it as an answer if the immediately + # preceding row was still '?' -- old notify() gated on that too, so + # e.g. an already-answered flag later cleared by an attachment being + # obsoleted (also logged as status 'X') doesn't re-notify the + # original requester. + my ($prev_status, $requester_id) = $dbh->selectrow_array( + "SELECT status, setter_id FROM flag_activity + WHERE flag_id = ? AND flag_when < ? ORDER BY flag_when DESC LIMIT 1", undef, $row->{flag_id}, $row->{flag_when} ); - next unless $requester_id; + next unless $requester_id && $prev_status && $prev_status eq '?'; push @events, { From c69dc88308d1341db2c5b7ef45357afcd0f8a476 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 19 Aug 2026 17:50:06 +0200 Subject: [PATCH 05/12] Bug 1883428 - Gate flag-event recipients and mail content on private-attachment insider access --- Bugzilla/BugMail.pm | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 8700ac37be..89834f9431 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -211,11 +211,25 @@ sub Send { : _get_flag_mail_events($bug, $start, $end, \%user_cache); foreach my $event (@flag_events) { + # notify() used to skip addressees who couldn't see a private + # attachment; match that here so a non-insider doesn't get added as a + # recipient purely to be told about a flag on an attachment they can't + # see (and can't be shown to them in the mail body either). + my $attachment_private = $event->{attachment} && $event->{attachment}->isprivate; + if ($event->{action} eq 'requested') { - $recipients{$event->{requestee_id}}->{+REL_FLAG_REQUESTEE} = BIT_DIRECT; + my $requestee_id = $event->{requestee_id}; + my $requestee + = $user_cache{$requestee_id} ||= Bugzilla::User->new({id => $requestee_id, cache => 1}); + $recipients{$requestee_id}->{+REL_FLAG_REQUESTEE} = BIT_DIRECT + if !$attachment_private || ($requestee && $requestee->is_insider); } elsif ($event->{action} eq 'answered') { - $recipients{$event->{requester_id}}->{+REL_FLAG_REQUESTER} = BIT_DIRECT; + my $requester_id = $event->{requester_id}; + my $requester + = $user_cache{$requester_id} ||= Bugzilla::User->new({id => $requester_id, cache => 1}); + $recipients{$requester_id}->{+REL_FLAG_REQUESTER} = BIT_DIRECT + if !$attachment_private || ($requester && $requester->is_insider); } } @@ -376,12 +390,17 @@ sub Send { # Flag events relevant to this recipient: they were asked, or they # asked and someone else answered (no need to tell people about - # their own actions). + # their own actions). A recipient who isn't an insider doesn't get + # told about a flag on a private attachment even if they qualify + # for the mail some other way (e.g. they're on the CC list). my @user_flag_events = grep { - ($_->{action} eq 'requested' && $_->{requestee_id} == $user_id) + (!$_->{attachment} || !$_->{attachment}->isprivate || $user->is_insider) + && ( + ($_->{action} eq 'requested' && $_->{requestee_id} == $user_id) || ($_->{action} eq 'answered' - && $_->{requester_id} == $user_id - && $_->{setter}->id != $user_id) + && $_->{requester_id} == $user_id + && $_->{setter}->id != $user_id) + ) } @flag_events; my $sent_mail = sendMail({ From c335c181c892e4268d9df8d0b86c3b524c1618a2 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 19 Aug 2026 18:10:33 +0200 Subject: [PATCH 06/12] Bug 1883428 - Don't re-fetch a possibly-deleted attachment in _get_flag_type_cc --- Bugzilla/BugMail.pm | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 89834f9431..4aa4ec6395 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -872,11 +872,12 @@ sub _get_flag_type_cc { my $cc_list = $event->{type}->cc_list; next unless $cc_list; + # Reuse the attachment already loaded by _get_flag_mail_events rather + # than re-fetching: flag_activity is a historical log, so the + # attachment it points at can have since been deleted, and re-fetching + # by id would return undef and die on ->isprivate. my $attachment_is_private - = $event->{attachment_id} - ? Bugzilla::Attachment->new({id => $event->{attachment_id}, cache => 1}) - ->isprivate - : 0; + = $event->{attachment} ? $event->{attachment}->isprivate : 0; foreach my $cc (split(/[, ]+/, $cc_list)) { my $ccuser = Bugzilla::User->new({name => $cc, cache => 1}); From 12eba34d63b61ba829e786bfddc42fe1a57e9b42 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 19 Aug 2026 18:32:48 +0200 Subject: [PATCH 07/12] Bug 1883428 - Fix flag-type cc_list email tmpl: undefined terms, wrong X status wording and missing body-headers - terms was never defined: added `[% PROCESS global/variables.none.tmpl %]` to fix it - status 'X' (cleared) rendered as "denied": now render as "cleared" (like the other templates) - added missing `@@body-headers@@` placeholder: without it, BMO's `_replace_placeholder_in_part` got nothing to substitute and these mails lose the body headers the old request/email.txt.tmpl carried --- template/en/default/email/bugmail-flagtype-cc.txt.tmpl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/template/en/default/email/bugmail-flagtype-cc.txt.tmpl b/template/en/default/email/bugmail-flagtype-cc.txt.tmpl index 48b5f63c5b..bb72cc1d1b 100644 --- a/template/en/default/email/bugmail-flagtype-cc.txt.tmpl +++ b/template/en/default/email/bugmail-flagtype-cc.txt.tmpl @@ -14,6 +14,8 @@ # throughout and has no path to a raw address. Revisit if this needs to # carry more than a one-line-per-event summary (diffs, comments, HTML). %] +[% PROCESS global/variables.none.tmpl %] + From: [% Param('mailfrom') %] To: [% to %] Subject: [[% terms.Bug %] [%+ bug.id %]] [% bug.short_desc %] @@ -27,6 +29,9 @@ X-Bugzilla-Type: request [% IF event.action == 'requested' %] [%+ event.setter.identity %] requested [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %]. [% ELSIF event.action == 'answered' %] -[%+ event.setter.identity %] [% event.status == '+' ? 'granted' : 'denied' %] [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %]. +[% status_word = event.status == '+' ? 'granted' : event.status == '-' ? 'denied' : 'cleared' %] +[%+ event.setter.identity %] [% status_word %] [% event.type.name %][% " for attachment ${event.attachment_id}" IF event.attachment_id %]. [% END %] [% END %] + +@@body-headers@@ From c0e7276da8dc2f553c6a3b186aff152ad6686379 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 19 Aug 2026 19:18:37 +0200 Subject: [PATCH 08/12] Bug 1883428 - Flatten flag_events before the enqueue and re-check attachment visibility at dequeue - enqueue(): flatten flag_events before it hits the job queue - dequeue(): inflate flag_events and re-check attachment visibility at send time --- Bugzilla/BugMail.pm | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 4aa4ec6395..e6e4c05a28 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -599,6 +599,25 @@ sub enqueue { foreach my $reference (@{$vars->{referenced_bugs}}) { $reference->{bug} = _flatten_object($reference->{bug}); } + + # Bug 1883428: flag_events carries live FlagType/Attachment/User objects + # (the setter is a shared %user_cache entry the recipient loop has + # already populated with lazily-built fields) -- flatten them the same + # way, dropping the attachment object (dequeue() re-fetches it by id so + # it can also re-check visibility at send time). + $vars->{flag_events} = [ + map { + { + action => $_->{action}, + attachment_id => $_->{attachment_id}, + requestee_id => $_->{requestee_id}, + requester_id => $_->{requester_id}, + status => $_->{status}, + type => _flatten_object($_->{type}), + setter => _flatten_object($_->{setter}), + } + } @{$vars->{flag_events}} + ]; Bugzilla->job_queue->insert('bug_mail', {vars => $vars}); } @@ -638,6 +657,28 @@ sub dequeue { return; } + # Inflate flag_events, then re-check attachment visibility at send time + # (TOCTOU, bug 1883428) -- the enqueue-time private-attachment gate may + # be stale if the attachment was made private, or the recipient lost + # insider access, between enqueue and dequeue. + $vars->{flag_events} = [ + map { + +{ + %$_, + type => Bugzilla::FlagType->new_from_hash($_->{type}), + setter => Bugzilla::User->new_from_hash($_->{setter}), + attachment => $_->{attachment_id} + ? Bugzilla::Attachment->new({id => $_->{attachment_id}, cache => 1}) + : undef, + } + } @{$vars->{flag_events}} + ]; + $vars->{flag_events} = [ + grep { + !$_->{attachment} || !$_->{attachment}->isprivate || $vars->{to_user}->is_insider + } @{$vars->{flag_events}} + ]; + $vars->{changer} = Bugzilla::User->new_from_hash($vars->{changer}); $vars->{new_comments} = [map { Bugzilla::Comment->new_from_hash($_) } @{$vars->{new_comments}}]; From 9ae8d93eed26ddc9e489f6d7095cdc17abfa8727 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 19 Aug 2026 19:43:17 +0200 Subject: [PATCH 09/12] Bug 1883428 - Do not let watchers inherit flag roles from the person they watch --- Bugzilla/BugMail.pm | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index e6e4c05a28..19296d6e03 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -268,6 +268,17 @@ sub Send { # Mark these people as having the role of the person they are watching foreach my $watch (@$userwatchers) { while (my ($role, $bits) = each %{$recipients{$watch->[1]}}) { + + # Flag roles are addressed to a specific person (someone asked + # them, or they asked and got answered) -- watching that person's + # other bug activity isn't the same thing, and without this the + # watcher got a content-free mail (empty reason line, no flag + # section, since the per-recipient flag_events filter only + # matches the actual requestee/requester's own user id). + next + if $role == REL_FLAG_REQUESTEE + || $role == REL_FLAG_REQUESTER + || $role == REL_FLAG_TYPE_CC; $recipients{$watch->[0]}->{$role} |= BIT_WATCHING if $bits & BIT_DIRECT; } push(@{$watching{$watch->[0]}}, $watch->[1]); From 717ec99fb8d235b3f5ad26f39a1cdbdbe9bd91a9 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Fri, 21 Aug 2026 18:12:15 +0200 Subject: [PATCH 10/12] Bug 1883428 - Fix flag-event recipient/content selection bugs - don't add the requester as recipient when they cleared their own request - give flag-type cc_list account holders the actual event content - don't drop flag-only bugmail when there are no diffs/comments - restore X-Bugzilla-Flag-Requestee header, carrying requestee/requester through the mailer-queue flatten/inflate cycle - break same-second ties on id when resolving a flag's previous status - don't credit a watcher with a role that was never actually inherited --- Bugzilla/BugMail.pm | 90 ++++++++++++------- .../en/default/email/bugmail-header.txt.tmpl | 4 + 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 19296d6e03..ee075f295c 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -212,24 +212,30 @@ sub Send { foreach my $event (@flag_events) { # notify() used to skip addressees who couldn't see a private - # attachment; match that here so a non-insider doesn't get added as a - # recipient purely to be told about a flag on an attachment they can't - # see (and can't be shown to them in the mail body either). - my $attachment_private = $event->{attachment} && $event->{attachment}->isprivate; - + # attachment; _flag_event_visible_to() matches that so a non-insider + # doesn't get added as a recipient purely to be told about a flag on an + # attachment they can't see (and can't be shown to them in the mail + # body either). if ($event->{action} eq 'requested') { my $requestee_id = $event->{requestee_id}; my $requestee = $user_cache{$requestee_id} ||= Bugzilla::User->new({id => $requestee_id, cache => 1}); + $event->{requestee} = $requestee; $recipients{$requestee_id}->{+REL_FLAG_REQUESTEE} = BIT_DIRECT - if !$attachment_private || ($requestee && $requestee->is_insider); + if _flag_event_visible_to($event, $requestee); } elsif ($event->{action} eq 'answered') { my $requester_id = $event->{requester_id}; my $requester = $user_cache{$requester_id} ||= Bugzilla::User->new({id => $requester_id, cache => 1}); + $event->{requester} = $requester; + + # Don't add the requester as a recipient for clearing their own + # request (e.g. cancelling a needinfo they asked for themselves) -- + # old notify() gated on setter != requester for the same reason. $recipients{$requester_id}->{+REL_FLAG_REQUESTER} = BIT_DIRECT - if !$attachment_private || ($requester && $requester->is_insider); + if $event->{setter}->id != $requester_id + && _flag_event_visible_to($event, $requester); } } @@ -267,6 +273,7 @@ sub Send { # Mark these people as having the role of the person they are watching foreach my $watch (@$userwatchers) { + my $role_inherited = 0; while (my ($role, $bits) = each %{$recipients{$watch->[1]}}) { # Flag roles are addressed to a specific person (someone asked @@ -279,9 +286,16 @@ sub Send { if $role == REL_FLAG_REQUESTEE || $role == REL_FLAG_REQUESTER || $role == REL_FLAG_TYPE_CC; - $recipients{$watch->[0]}->{$role} |= BIT_WATCHING if $bits & BIT_DIRECT; + next unless $bits & BIT_DIRECT; + $recipients{$watch->[0]}->{$role} |= BIT_WATCHING; + $role_inherited = 1; } - push(@{$watching{$watch->[0]}}, $watch->[1]); + + # Skip %watching too when nothing was inherited (e.g. the watched + # user's only role was a flag role) -- otherwise this watcher gets a + # spurious "X-Bugzilla-Watch-Reason: None " header for a + # relationship that contributed nothing to the mail. + push(@{$watching{$watch->[0]}}, $watch->[1]) if $role_inherited; } } @@ -401,16 +415,20 @@ sub Send { # Flag events relevant to this recipient: they were asked, or they # asked and someone else answered (no need to tell people about - # their own actions). A recipient who isn't an insider doesn't get - # told about a flag on a private attachment even if they qualify - # for the mail some other way (e.g. they're on the CC list). + # their own actions), or they're on the flag type's cc_list for + # this event. A recipient who isn't an insider doesn't get told + # about a flag on a private attachment even if they qualify for + # the mail some other way (e.g. they're on the CC list). + my $user_cc_events = $flag_type_cc->{$user_id} || []; my @user_flag_events = grep { - (!$_->{attachment} || !$_->{attachment}->isprivate || $user->is_insider) + my $event = $_; + _flag_event_visible_to($event, $user) && ( - ($_->{action} eq 'requested' && $_->{requestee_id} == $user_id) - || ($_->{action} eq 'answered' - && $_->{requester_id} == $user_id - && $_->{setter}->id != $user_id) + ($event->{action} eq 'requested' && $event->{requestee_id} == $user_id) + || ($event->{action} eq 'answered' + && $event->{requester_id} == $user_id + && $event->{setter}->id != $user_id) + || (grep { $_ == $event } @$user_cc_events) ) } @flag_events; @@ -512,7 +530,7 @@ sub sendMail { @send_comments = grep { !$_->is_private } @send_comments; } - if (!scalar(@display_diffs) && !scalar(@send_comments)) { + if (!scalar(@display_diffs) && !scalar(@send_comments) && !scalar(@$flag_events)) { # Whoops, no differences! return 0; @@ -626,6 +644,8 @@ sub enqueue { status => $_->{status}, type => _flatten_object($_->{type}), setter => _flatten_object($_->{setter}), + requestee => _flatten_object($_->{requestee}), + requester => _flatten_object($_->{requester}), } } @{$vars->{flag_events}} ]; @@ -678,17 +698,16 @@ sub dequeue { %$_, type => Bugzilla::FlagType->new_from_hash($_->{type}), setter => Bugzilla::User->new_from_hash($_->{setter}), + requestee => $_->{requestee} ? Bugzilla::User->new_from_hash($_->{requestee}) : undef, + requester => $_->{requester} ? Bugzilla::User->new_from_hash($_->{requester}) : undef, attachment => $_->{attachment_id} ? Bugzilla::Attachment->new({id => $_->{attachment_id}, cache => 1}) : undef, } } @{$vars->{flag_events}} ]; - $vars->{flag_events} = [ - grep { - !$_->{attachment} || !$_->{attachment}->isprivate || $vars->{to_user}->is_insider - } @{$vars->{flag_events}} - ]; + $vars->{flag_events} + = [grep { _flag_event_visible_to($_, $vars->{to_user}) } @{$vars->{flag_events}}]; $vars->{changer} = Bugzilla::User->new_from_hash($vars->{changer}); $vars->{new_comments} @@ -831,6 +850,16 @@ sub _generate_bugmail { return $email; } +# True if $user is allowed to see $event's attachment, i.e. it isn't +# private or $user is an insider. Flag events with no attachment are always +# visible. Used everywhere a flag event's exposure to a specific user is +# decided: adding recipients, filtering an already-built recipient's +# flag_events, and the dequeue-time TOCTOU re-check (bug 1883428). +sub _flag_event_visible_to { + my ($event, $user) = @_; + return !$event->{attachment} || !$event->{attachment}->isprivate || ($user && $user->is_insider); +} + # Find flags that were requested of, or answered by, someone during this # window, so sendMail() can add a note at the top of the mail for that # person even if they hold no other role on the bug (bug 1883428). @@ -846,11 +875,11 @@ sub _get_flag_mail_events { } my $rows = $dbh->selectall_arrayref( - "SELECT flag_id, flag_when, type_id, status, setter_id, requestee_id, + "SELECT id, flag_id, flag_when, type_id, status, setter_id, requestee_id, attachment_id FROM flag_activity WHERE bug_id = ?$when_restriction - ORDER BY flag_when", {Slice => {}}, @args + ORDER BY flag_when, id", {Slice => {}}, @args ); my @events; @@ -883,12 +912,13 @@ sub _get_flag_mail_events { # preceding row was still '?' -- old notify() gated on that too, so # e.g. an already-answered flag later cleared by an attachment being # obsoleted (also logged as status 'X') doesn't re-notify the - # original requester. + # original requester. Break ties on id, not just flag_when: a '?' + # and its answer can land in the same second. my ($prev_status, $requester_id) = $dbh->selectrow_array( "SELECT status, setter_id FROM flag_activity - WHERE flag_id = ? AND flag_when < ? - ORDER BY flag_when DESC LIMIT 1", undef, $row->{flag_id}, - $row->{flag_when} + WHERE flag_id = ? AND (flag_when < ? OR (flag_when = ? AND id < ?)) + ORDER BY flag_when DESC, id DESC LIMIT 1", undef, $row->{flag_id}, + $row->{flag_when}, $row->{flag_when}, $row->{id} ); next unless $requester_id && $prev_status && $prev_status eq '?'; @@ -937,7 +967,7 @@ sub _get_flag_type_cc { next if $attachment_is_private && (!$ccuser || !$ccuser->is_insider); if ($ccuser) { - $account_recipients{$ccuser->id} = 1; + push @{$account_recipients{$ccuser->id}}, $event; $user_cache->{$ccuser->id} ||= $ccuser; } else { diff --git a/template/en/default/email/bugmail-header.txt.tmpl b/template/en/default/email/bugmail-header.txt.tmpl index b1a440e036..db292acb7a 100644 --- a/template/en/default/email/bugmail-header.txt.tmpl +++ b/template/en/default/email/bugmail-header.txt.tmpl @@ -34,6 +34,10 @@ X-Bugzilla-Reason: [% reasonsheader %] X-Bugzilla-Type: [% bugmailtype %] X-Bugzilla-Watch-Reason: [% reasonswatchheader %] [%+ INCLUDE "email/header-common.txt.tmpl" %] +[% FOREACH event = flag_events %] +[% NEXT UNLESS event.action == 'requested' %] +X-Bugzilla-Flag-Requestee: [% event.requestee.email %] +[% END %] X-Bugzilla-Changed-Fields: [% changedfields.join(" ") %] X-Bugzilla-Changed-Field-Names: [% changedfieldnames.join(" ") %] [%+ threadingmarker %] From a633689cc589f43a5a7b21fdd3a1cddaddd30679 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Fri, 21 Aug 2026 18:15:19 +0200 Subject: [PATCH 11/12] Bug 1883428 - Add a unit test for flag-event attachment visibility control --- t/bugmail-flag-events.t | 67 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 t/bugmail-flag-events.t diff --git a/t/bugmail-flag-events.t b/t/bugmail-flag-events.t new file mode 100644 index 0000000000..7c4713e714 --- /dev/null +++ b/t/bugmail-flag-events.t @@ -0,0 +1,67 @@ +# 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. + +############################# +#Bug 1883428: BugMail flag events +############################# + +# Covers Bugzilla::BugMail::_flag_event_visible_to(), the private-attachment +# gate shared by recipient selection, per-recipient flag_events filtering, +# and the dequeue-time TOCTOU re-check. This is the one piece of the +# recipient-selection logic (bug 1883428 review) that's pure Perl and +# doesn't need a live DB to exercise; the flag_activity prev-status query +# and the flag-type cc_list DB lookups still need a real Bugzilla instance +# and are covered by the manual test plan in that bug instead. + +use 5.10.1; +use strict; +use warnings; + +use lib qw(. lib local/lib/perl5 t); +use Support::Files; +use Test::More tests => 6; + +BEGIN { use_ok('Bugzilla::BugMail'); } + +package Mock::Attachment; + +sub new { + my ($class, $private) = @_; + return bless {isprivate => $private}, $class; +} +sub isprivate { return $_[0]->{isprivate}; } + +package Mock::User; + +sub new { + my ($class, $insider) = @_; + return bless {is_insider => $insider}, $class; +} +sub is_insider { return $_[0]->{is_insider}; } + +package main; + +my $visible = \&Bugzilla::BugMail::_flag_event_visible_to; + +ok( + $visible->({attachment => undef}, Mock::User->new(0)), + 'no attachment on the event is always visible' +); + +ok($visible->({attachment => Mock::Attachment->new(0)}, Mock::User->new(0)), + 'non-private attachment is visible to a non-insider'); + +ok($visible->({attachment => Mock::Attachment->new(1)}, Mock::User->new(1)), + 'private attachment is visible to an insider'); + +ok(!$visible->({attachment => Mock::Attachment->new(1)}, Mock::User->new(0)), + 'private attachment is hidden from a non-insider'); + +ok( + !$visible->({attachment => Mock::Attachment->new(1)}, undef), + 'private attachment is hidden when there is no user to check' +); From 1d7b04bfbf3866e76819a0d478b3956cb834bd4e Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Fri, 21 Aug 2026 18:45:24 +0200 Subject: [PATCH 12/12] Bug 1883428 - Fix critic violations in the flag-events test - Added `package main;` at top of file - Added `## no critic (Variables::ProtectPrivateVars)` before the private-sub reference --- t/bugmail-flag-events.t | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/bugmail-flag-events.t b/t/bugmail-flag-events.t index 7c4713e714..217ef49b6e 100644 --- a/t/bugmail-flag-events.t +++ b/t/bugmail-flag-events.t @@ -17,6 +17,8 @@ # and the flag-type cc_list DB lookups still need a real Bugzilla instance # and are covered by the manual test plan in that bug instead. +package main; + use 5.10.1; use strict; use warnings; @@ -45,6 +47,7 @@ sub is_insider { return $_[0]->{is_insider}; } package main; +## no critic (Variables::ProtectPrivateVars) my $visible = \&Bugzilla::BugMail::_flag_event_visible_to; ok(