More expandable tags within emails

RESOLVED FIXED in Bugzilla 3.0

Status

()

enhancement
RESOLVED FIXED
19 years ago
8 years ago

People

(Reporter: anowak, Assigned: siulung)

Tracking

unspecified
Bugzilla 3.0
Bug Flags:
approval +

Details

Attachments

(1 attachment, 5 obsolete attachments)

Reporter

Description

19 years ago
I receive bugzilla emails for several bugzilla projects.
I would like to folder them automatically in my mailtool.

Therefore, it would be nice to include the projectname in the emails subject. I 
propose to add the new tag %project% so that it can be used within the mail 
params.
E.g. One may specify the subject line like this:

Subject: [Bug %bugid%] [Project: %project] %neworchanged% - %summary%

A more general approach would be to include all fields from the bug form as 
tags.  Therewith, the email content could be tailorized according to individual 
needs.
Reporter

Comment 1

19 years ago
Sorry, for confusing you with the field label %project%.
We actually changed the field 'component' to 'project' in our Buzilla 
installation.
I can tell you right now this isn't going to happen.  We've already had way to 
many complaints that the subject lines were too long already.  Anything that 
makes them longer is going to get complained about very loudly by a lot of 
people.  Most mail clients I know of can filter on just about any header however.  
I certainly would not have a problem with adding some headers to the email with 
basic bug information...

X-Bugzilla-Product: Webtools
X-Bugzilla-Component: Bugzilla
X-Bugzilla-Status: NEW

etc.

Thoughts?

Comment 3

19 years ago
This looks to be an expansion of what requested in bug 26194.

Comment 4

19 years ago
See also bug 71560, which has a very similar proposal.
Target Milestone: --- → Future
This is different from both bug #26194 and bug #71560.  This first is about why
you are receiving the mail, the second what fields have changed, and this about
the current values of certain fields.

They all seem orthogonal, except do we include the new values, the old values or
both?
-> Bugzilla product, Email component, reassigning.
Assignee: tara → jake
Component: Bugzilla → Email
Product: Webtools → Bugzilla
Version: other → unspecified

Comment 7

18 years ago
OK, the functionality for providing this option is trivially easy. I've added it
and tested it in my own location for several days now, where all emails now have
the following headers:

X-Bugzilla-Product: %product%
X-Bugzilla-Component: %component%
X-Bugzilla-Status: %status%
X-Bugzilla-Priority: %priority%
X-Bugzilla-Severity: %severity%
X-Bugzilla-Target-Milestone: %target%
patches incoming.

Comment 10

18 years ago
updating keywords.
Keywords: patch, review
These should include the old values as well, but that might not be easy.

Comment 12

18 years ago
That would be nice. I'll look at the code again but... needless to say, not
nearly as trivial. The other problem is how one represents that in the
editparams code without putting in a massive blob of code like the code that
currently handles changes. 

Comment 13

18 years ago
Actually, looked at the code on the plane today, and it should be pretty easy to:
1) Generate 'current' status as done in the patch currently attached to the bug.
This approach is flexible (the headers can be used for other things as well) and
more easily customizable. 
2) Generate a set of X-Bugzilla-Changed: headers in a block, closing bug 71560.
3) Generate a set of X-Bugzilla-Old: headers in a block, showing only the fields
listed in (2).

I've implemented (1) and (2) (but not tested); now that I've given it some
thought, (3) should take 10-15 minutes to write. I'll attach patches by Monday
at the latest; if anyone has any thoughts or comments over the weekend I'd
appreciate it.

Comment 14

18 years ago
Note that the X-Bugzilla-Old-* headers would ignore long_desc, short_desc, etc.,
since those are way too long to be in a header.

Comment 15

18 years ago
Setting owner to patch author...
Assignee: jake → louie
*** Bug 107586 has been marked as a duplicate of this bug. ***
Asa and I have been discussing this at length before he filed the dupe.
Obviously, there has to be a limit to the number of headers, or we'd have the
whole bug in there. Our view is:

"product" is unnecessary. You should be able to filter just fine on component.
If there's a name clash between products and the two components aren't related,
its your own silly fault for picking bad names. Rename them.

"target-milestone" is bad because people's filters will then go out of date
without active maintenance.

We should add "status whiteboard" because then people can add arbitrary tags to
filter on, reducing the demand for "Oh, I'd like to filter on field X" requests.

Looking at the current patch, it seems somewhat comment-heavy. We use Bugzilla
to track enhancement requests and CVS to track bug numbers relating to checkins.
They are both better tools than comments, and we shouldn't duplicate that
information.

Gerv

Comment 18

18 years ago
In reverse order: I can strip the comments without much problem, but to be
perfectly honest the current code is difficult enough to read as is, due to poor
factorization and lack of comments. Deliberately reducing the amount of comments
is, IMNSHO, _extremely_ poor practice. But, that said, it's your codebase- I'll
happily strip it if that's what you want. I'm just warning you that you're
adding to the barrier to entry to participation unecessarily if that's going to
be the practice.

Adding status whiteboard is trivial; I'll do it tonight.
I'd like to leave target milestone and product in the patch for processmail, but
I'll remove them from the defaults in defparams. b.m.o. doesn't have to turn
them on by default, if you and Asa don't want them, but I personally find them
useful so I'd like to leave them in.

Anyway, I'll try to get the patch updated and in shortly, but it might have to
wait until very late tonight or tomorrow morning (evolution release candidate
one today, plus they might be giants concert :)
> Deliberately reducing the amount of comments
> is, IMNSHO, _extremely_ poor practice.

Adding comments which do not contribute to understanding the code as it is, is
also poor practice. I would suggest that, given that you are doing the same
thing several times in the processmail patch, a comment such as:

# Substitute in values useful for client-side email filtering

would be quite sufficient.

> I'd like to leave target milestone and product in the patch for processmail, >
but I'll remove them from the defaults in defparams.

That sounds like an excellent solution. (As I'm sure you can tell, my point is
to avoid bugmail bloat.)

Gerv





I think the list of fields should be generic rather than a hardcoded list.  If
the user cannot choose, the admin should at least be able to.

Comment 21

18 years ago
Well, if someone has a good suggestion to make as to how to non-hard-code this,
I'm game. 

The best thing I can think of is to ditch %substs altogether and just add things
to %values as necessary, which would contain pretty much all relevant values
already in it. [Note that a good chunk of this patch basically already just
copies values from %values to %substs.] I have no idea what the security
ramifications of that approach would be, though.

Updated

18 years ago
Attachment #54485 - Attachment is obsolete: true
Attachment #54485 - Attachment is patch: false

Updated

18 years ago
Attachment #54486 - Attachment is obsolete: true

Comment 22

18 years ago
Posted patch another whack at processmail (obsolete) — Splinter Review

Comment 23

18 years ago
Posted patch smaller defparams. (obsolete) — Splinter Review

Comment 24

18 years ago
For the moment, I'm excluding the X-Bugzilla-Old stuff because I think it's
hackish and I'd like some feedback on whether or not it is bloat.

Also, I'll note it is still untested- I'm not running 2.1[4|5] here. I do know
the code is syntactically valid (and very similar but not identical code is in
production here) but I can make no guarantees ATM about the functionality.

Comment 25

18 years ago
*** Bug 71560 has been marked as a duplicate of this bug. ***
Er... can we have a diff -u in general, and a diff of defparams in particular? :-)

Also, can we not have changed_by in the default headers, please? I think this
would be extremely bad - what would happen is that some developers would ignore
other developers, and relations would deteriorate.

Thanks :-)

Gerv
*** Bug 108953 has been marked as a duplicate of this bug. ***
I think that we should leave this until we "templatise" the mail parameters (bug
100089).  This should make the interactions between the headers cleaner?
Can we get a different Header for bugmails from people that you are watching ?
(or should I file a new bug about that ?)

Comment 30

18 years ago
doesn't the X-Bugzilla-Reason header give you that?
Alec: 
no, i get the reason from the person that I'm watching watching 
example:I'm watching darin and i got an email: 
bug 117927 and the header is : "X-Bugzilla-Reason: CC" and no additional header

Comment 32

18 years ago
Matti, an addition X-Bugzilla-Reason will be added when bug 74996 is added and
watching is properly supported in the mail prefs rather than just piggy-backed on.
> Also, can we not have changed_by in the default headers, please? I think this
> would be extremely bad - what would happen is that some developers would ignore
> other developers, and relations would deteriorate.

Actually, I'd really like this field. I would like to mark my own changes as
read, since actually reading about them would be a waste of time.
Why not just turn them off in email prefs, then?

Gerv

Updated

17 years ago
Keywords: review

Comment 35

17 years ago
Is any of this checked in?  Recent bugmail still only has the reason header. 
I'll happily take something that is better than just the reason field while the
discussion about what would be perfect continues...
I don't think anything is checked in yet; when mail gets templatised then people
will be able to produce an infinite variety of headers. That depends on bug
84876 happening first.

Gerv

Updated

17 years ago
Attachment #56049 - Flags: review?

Updated

17 years ago
Attachment #56050 - Flags: review?

Comment 37

16 years ago
Comment on attachment 56049 [details] [diff] [review]
another whack at processmail

Processmail is dead, denying review.
Attachment #56049 - Flags: review? → review-

Comment 38

16 years ago
Comment on attachment 56050 [details] [diff] [review]
smaller defparams.

Removing duplicate review request. The remaining review request should probably
be denied as 'needswork' or the like, as the tip defparams.pl has had quite a
few options added and subtracted since this patch was posted.

This attachment will not receive any love in this bugreport, at any rate, as it
has nothing to do with the bug (other than adding an option or two relevant
while changing the format of the entire file).

Comment 39

16 years ago
Comment on attachment 56050 [details] [diff] [review]
smaller defparams.

Bugger all, now it looks like I'm requesting review. Well, I'm not, and as it
wasn't the attachment submitter who requested the reivew in the first place,
removing. Apologies for the spam.
Attachment #56050 - Flags: review?

Comment 40

15 years ago
*** Bug 237907 has been marked as a duplicate of this bug. ***

Comment 41

15 years ago
*** Bug 239956 has been marked as a duplicate of this bug. ***

Comment 42

15 years ago
If this ever gets resolved, some another things that would be nice to have is
"priority" integer value as well as an "imporance" string, based on the
bug_severity and bug_priority values.  This way, my email headers can include
these two fields:

   X-Priority: %priority%
   Importance: %importance%

I'm not a Microsoft fan, but both of these are used by MS-Outlook to highlight
"important" emails. I ended up doing this at my site to appease upper management. 

Comment 43

15 years ago
Here's the patch against BugMail.pm I attached to bug 239956
https://bugzilla.mozilla.org/attachment.cgi?id=145656&action=view

it was made against 2.17.6, but applies cleanly against 2.18rc3

It's actually pretty easy to add more fields, if these don't suit your needs.

An example of usage of the fields I added is in my original bug description:
https://bugzilla.mozilla.org/show_bug.cgi?id=239956#c0
This should be fairly easy after we templatize BugMail.
Depends on: 275636
Assignee

Comment 45

15 years ago
Now we have BugMail.pm, shan't we just port the old patch and have it reviewed?
(In reply to comment #45)
> Now we have BugMail.pm, shan't we just port the old patch and have it reviewed?

  Sure, you're always free to do that. If you want to port the patch, you can
set "review?" on it and somebody will look it over and decide what we want to do
about it.

Comment 47

15 years ago
Posted patch old patch ported to 2.18 (obsolete) — Splinter Review
This is essentially a port of Luis's work to 2.18. Use patch -p1.
Attachment #177077 - Flags: review?
We won't be adding any features like this to 2.18. Is there any chance of
porting it to the tip?

Comment 49

15 years ago
Can you also add %classification% to this patch?
Now that Bug.pm has been speeded up, can we instead simplify the patch by
looking anything in %...% markers up in a Bug object hash? Generality and
simplicity - and  no more bugs like this one :-)

Gerv

Comment 51

15 years ago
I like Gerv's idea better than the patch I posted. Unfortunately, I need to move
on to more pressing issues.
Comment on attachment 177077 [details] [diff] [review]
old patch ported to 2.18

Removing r? per comment 48.
Attachment #177077 - Flags: review?

Comment 53

14 years ago
*** Bug 307970 has been marked as a duplicate of this bug. ***

Comment 54

14 years ago
*** Bug 259926 has been marked as a duplicate of this bug. ***

Comment 55

14 years ago
(In reply to comment #2)
> I can tell you right now this isn't going to happen.  We've already had way to 
> many complaints that the subject lines were too long already.

You know, all that has to happen is to make it POSSIBLE for these fields to be added to the emails. Then the administrators can configure their sites any way the like. What a concept.

Comment 56

14 years ago
(In reply to comment #55)
> (In reply to comment #2)
> > I can tell you right now this isn't going to happen.  We've already had way to 
> > many complaints that the subject lines were too long already.
> 
> You know, all that has to happen is to make it POSSIBLE for these fields to be
> added to the emails. Then the administrators can configure their sites any way
> the like. What a concept.

So go ahead and write a patch for current CVS. Comment #2 was a reaction to put projects name in the subject.
Assignee

Comment 57

14 years ago
Bugzilla 2.21.1 already has Product, Component, Keywords and Severity.
I've also changed a bit to make names consistent.
Attachment #206240 - Flags: review?

Comment 58

14 years ago
(In reply to comment #57)
Can you also include Classification?

Comment 59

14 years ago
Comment on attachment 206240 [details] [diff] [review]
diff against cvs head

Classifications are tricky since they could be disabled, we might split them in another bug.

Target milestone could be bad (comment #17) but at the same time it could be useful for release managers and such.

I certainly don't mind patching BugMail.pm with those additional values in $substs. Regarding the MTA.pm patch, the only downsize would be the increased size of the headers, but 100 bytes or so aren't a big issue (even if those get multiplied with the huge amount of mail that gets sent by some Bugzillas).

Status Whiteboard would have been cool but that could be another bug as well.
Attachment #206240 - Flags: review? → review+

Updated

14 years ago
Flags: approval?
Target Milestone: Future → Bugzilla 2.24

Comment 60

14 years ago
downsize --> down side (in the previous comment)
I kinda like Gerv's idea of looking up stuff in the Bug object in addition to the replacement list and params.  I think that'd be a lot cleaner and more flexible for admins.  Breaking the dependency as it's not really necessary.
No longer depends on: 275636
Flags: approval?

Comment 62

14 years ago
*** Bug 324794 has been marked as a duplicate of this bug. ***
Morphing, presuming comment 61 to be a decision. Tam, are you interested in modifying your patch?
Assignee: luis.villa → Tam
Summary: More expandable tags within emails → Make the Bug object available to newchangedmail substitutions
*** Bug 284288 has been marked as a duplicate of this bug. ***
Assignee

Comment 65

13 years ago
I have no problem with modifying my patch. But I'd like to first clarify on what to do and how to do it.

* Does it mean we want to allow accessing all values stored in the Bugzilla::Bug hash, including Bugzilla->custom_field_names?
* Using the raw database field names like %bug_severity%?
* Do we allow backward compatibility to the old name, like %severity%?
* Do we check for nonexistent names (e.g. %nonexistent%) and report error, or simply remove the tag, or ignore it?

BTW, do we really create the Bugzilla::Bug object? This will result in one SQL query per bug?
(In reply to comment #65)
> BTW, do we really create the Bugzilla::Bug object? This will result in one SQL
> query per bug?

  Well, actually, the answer to this question answers the whole question of the patch. Right now Bugzilla::BugMail can't use Bugzilla::Bug, because of dependency loops. I think that will pretty much always be the case. So what we need to do is rearchitecture Bugzilla::BugMail to take a Bug object as a parameter instead of a bug_id, but that's more part of bug 301447, and is a large project.
Assignee

Comment 67

13 years ago
Then I don't see I have the ability to modify my patch.
IMHO, the bug should not have changed to this new summary,
since the new summary pursues something immature.
Instead one should fire a new bug for this greater vision,
and keep this bug fixing what we originally want.
As a result, I hope my patch should get into trunk first.
Assignee: Tam → luis.villa
Summary: Make the Bug object available to newchangedmail substitutions → More expandable tags within emails
Attachment #56049 - Attachment is obsolete: true
Attachment #56050 - Attachment is obsolete: true
Attachment #177077 - Attachment is obsolete: true
Per comment #66, re-requesting approval. Let's leave the bug object thingy to bug 301447 and for now just add these ready-to-go additional fields.
Assignee: luis.villa → Tam
Flags: approval?
QA Contact: mattyt-bugzilla → default-qa
Flags: approval? → approval+

Comment 69

13 years ago
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.69; previous revision: 1.68
done
Checking in Bugzilla/Config/MTA.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/MTA.pm,v  <--  MTA.pm
new revision: 1.7; previous revision: 1.6
done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Keywords: relnote

Comment 70

13 years ago
*** Bug 152355 has been marked as a duplicate of this bug. ***
Added to relnotes in bug 349423. Please let me know if I missed any critical information about this bug in the release notes.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.