Closed
Bug 70710
Opened 24 years ago
Closed 19 years ago
More expandable tags within emails
Categories
(Bugzilla :: Email Notifications, enhancement)
Bugzilla
Email Notifications
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: anowak, Assigned: siulung)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
goobix
:
review+
|
Details | Diff | Splinter Review |
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•24 years ago
|
||
Sorry, for confusing you with the field label %project%.
We actually changed the field 'component' to 'project' in our Buzilla
installation.
Comment 2•24 years ago
|
||
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?
Updated•23 years ago
|
Target Milestone: --- → Future
Comment 5•23 years ago
|
||
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?
Comment 6•23 years ago
|
||
-> Bugzilla product, Email component, reassigning.
Assignee: tara → jake
Component: Bugzilla → Email
Product: Webtools → Bugzilla
Version: other → unspecified
Comment 7•23 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 8•23 years ago
|
||
Comment 9•23 years ago
|
||
Comment 11•23 years ago
|
||
These should include the old values as well, but that might not be easy.
Comment 12•23 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•23 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•23 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 16•23 years ago
|
||
*** Bug 107586 has been marked as a duplicate of this bug. ***
Comment 17•23 years ago
|
||
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•23 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 :)
Comment 19•23 years ago
|
||
> 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
Comment 20•23 years ago
|
||
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•23 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•23 years ago
|
Attachment #54485 -
Attachment is obsolete: true
Attachment #54485 -
Attachment is patch: false
Updated•23 years ago
|
Attachment #54486 -
Attachment is obsolete: true
Comment 22•23 years ago
|
||
Comment 23•23 years ago
|
||
Comment 24•23 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•23 years ago
|
||
*** Bug 71560 has been marked as a duplicate of this bug. ***
Comment 26•23 years ago
|
||
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
Comment 27•23 years ago
|
||
*** Bug 108953 has been marked as a duplicate of this bug. ***
Comment 28•23 years ago
|
||
I think that we should leave this until we "templatise" the mail parameters (bug
100089). This should make the interactions between the headers cleaner?
Comment 29•23 years ago
|
||
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•23 years ago
|
||
doesn't the X-Bugzilla-Reason header give you that?
Comment 31•23 years ago
|
||
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•23 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.
Comment 33•23 years ago
|
||
> 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.
Comment 34•23 years ago
|
||
Why not just turn them off in email prefs, then?
Gerv
Comment 35•22 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...
Comment 36•22 years ago
|
||
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•22 years ago
|
Attachment #56049 -
Flags: review?
Updated•22 years ago
|
Attachment #56050 -
Flags: review?
Comment 37•21 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•21 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•21 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•20 years ago
|
||
*** Bug 237907 has been marked as a duplicate of this bug. ***
Comment 41•20 years ago
|
||
*** Bug 239956 has been marked as a duplicate of this bug. ***
Comment 42•20 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•20 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
Assignee | ||
Comment 45•20 years ago
|
||
Now we have BugMail.pm, shan't we just port the old patch and have it reviewed?
Comment 46•20 years ago
|
||
(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•20 years ago
|
||
This is essentially a port of Luis's work to 2.18. Use patch -p1.
Attachment #177077 -
Flags: review?
Comment 48•20 years ago
|
||
We won't be adding any features like this to 2.18. Is there any chance of
porting it to the tip?
Comment 49•20 years ago
|
||
Can you also add %classification% to this patch?
Comment 50•20 years ago
|
||
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•20 years ago
|
||
I like Gerv's idea better than the patch I posted. Unfortunately, I need to move
on to more pressing issues.
Comment 52•19 years ago
|
||
Comment on attachment 177077 [details] [diff] [review]
old patch ported to 2.18
Removing r? per comment 48.
Attachment #177077 -
Flags: review?
Comment 53•19 years ago
|
||
*** Bug 307970 has been marked as a duplicate of this bug. ***
Comment 54•19 years ago
|
||
*** Bug 259926 has been marked as a duplicate of this bug. ***
Comment 55•19 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•19 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•19 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•19 years ago
|
||
(In reply to comment #57)
Can you also include Classification?
Comment 59•19 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•19 years ago
|
Flags: approval?
Target Milestone: Future → Bugzilla 2.24
Comment 60•19 years ago
|
||
downsize --> down side (in the previous comment)
Comment 61•19 years ago
|
||
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•19 years ago
|
||
*** Bug 324794 has been marked as a duplicate of this bug. ***
Comment 63•19 years ago
|
||
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
Comment 64•19 years ago
|
||
*** Bug 284288 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 65•19 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?
Comment 66•19 years ago
|
||
(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•19 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
Updated•19 years ago
|
Attachment #56049 -
Attachment is obsolete: true
Updated•19 years ago
|
Attachment #56050 -
Attachment is obsolete: true
Updated•19 years ago
|
Attachment #177077 -
Attachment is obsolete: true
Comment 68•19 years ago
|
||
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
Updated•19 years ago
|
Flags: approval? → approval+
Comment 69•19 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: 19 years ago
Resolution: --- → FIXED
Comment 70•18 years ago
|
||
*** Bug 152355 has been marked as a duplicate of this bug. ***
Comment 71•18 years ago
|
||
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.
Description
•