add X-Bugzilla-Flags to mail headers

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Email Notifications
--
enhancement
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: Noura Elhawary, Assigned: selsky)

Tracking

Bugzilla 4.4
Bug Flags:
approval +

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

9 years ago
Created attachment 351292 [details] [diff] [review]
v1 for adding flag changes to mail headers

Hi,

It would be good if we can have the flag changes that occured in a bug in the mail headers. attached is a patch to enable this.

Noura
Attachment #351292 - Flags: review?(mkanat)

Comment 1

9 years ago
Comment on attachment 351292 [details] [diff] [review]
v1 for adding flag changes to mail headers

This is LpSolit's area.
Attachment #351292 - Flags: review?(mkanat) → review?(LpSolit)

Comment 2

9 years ago
Comment on attachment 351292 [details] [diff] [review]
v1 for adding flag changes to mail headers

>Index: Bugzilla/BugMail.pm

>+    for my $f (@$bug_flags){
>+        if (defined $f->type) {
>+            push(@changed_flags, $f->name . $f->status);
>+        }
>+    }


$f->type cannot be undefined. Also, why do you say "changed_flags" when you list all flags?
Attachment #351292 - Flags: review?(LpSolit) → review-
(Reporter)

Comment 3

9 years ago
Created attachment 351850 [details] [diff] [review]
v2 for adding flags to mail headers

Thanks for the review Frederic, here is another modified patch.

Noura
Attachment #351292 - Attachment is obsolete: true
Attachment #351850 - Flags: review?(LpSolit)

Updated

9 years ago
Attachment #351850 - Flags: review?(LpSolit) → review-

Comment 4

9 years ago
Comment on attachment 351850 [details] [diff] [review]
v2 for adding flags to mail headers

>+    my $bug_flags = Bugzilla::Flag->match({ 'bug_id'      => $id,
>+                                            'target_type' => 'bug' });
>+
>+    my @flags;
>+    foreach my $f (@$bug_flags){
>+        push(@flags, $f->name . $f->status);
>+    }

This code must not be in sendMail(), else it would be called again and again for each user getting bugmail. It must go in the first half of Send(), where other %values fields are populated.


>+        flags => join(", ", @flags),

Don't include the comma. I think blocking3.2+ approval+ is nicer than blocking3.2+, approval+



>Index: template/en/default/email/newchangedmail.txt.tmpl

>+X-Bugzilla-Flags: [% flags %]

You should enclose this in [% Bugzilla.has_flags %] [% END %] or [% IF flags %] [% END %]. Maybe the latter is better.

Comment 5

9 years ago
too late for 3.4.
Severity: normal → enhancement
Target Milestone: Bugzilla 3.4 → Bugzilla 4.0

Updated

7 years ago
Assignee: nelhawar → email-notifications
Status: ASSIGNED → NEW
(Assignee)

Comment 6

6 years ago
Created attachment 631413 [details] [diff] [review]
v3

I updated Noura's patch as per the comments.
Assignee: email-notifications → selsky
Attachment #351850 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #631413 - Flags: review?(LpSolit)

Comment 7

6 years ago
Comment on attachment 631413 [details] [diff] [review]
v3

Since this patch was written in 2008, the code about flags has evolved a lot and you can now simply call $bug->flags which will return an arrayref of flag objects. As the backend code already caches bug flags, you don't need to cache the result in BugMail.pm again, and you simply need to generate the desired output in the template itself (it will be regenerated once per recipient, but that's better than passing a flag variable; the perf impact should be minimal).
Attachment #631413 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 8

6 years ago
Created attachment 637124 [details] [diff] [review]
v4

Ah.  That's much easier.
Attachment #631413 - Attachment is obsolete: true
Attachment #637124 - Flags: review?(LpSolit)

Comment 9

6 years ago
Comment on attachment 637124 [details] [diff] [review]
v4

r=LpSolit
Attachment #637124 - Flags: review?(LpSolit) → review+

Updated

6 years ago
Flags: approval+
Keywords: relnote
Summary: add flag changes to mail headers → add X-Bugzilla-Flags to mail headers

Comment 10

6 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/email/bugmail-header.txt.tmpl
Committed revision 8281.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 11

5 years ago
Added to relnotes for 4.4.
Keywords: relnote

Updated

5 years ago
Duplicate of this bug: 846484
You need to log in before you can comment on or make changes to this bug.