Closed Bug 70710 Opened 24 years ago Closed 19 years ago

More expandable tags within emails

Categories

(Bugzilla :: Email Notifications, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: anowak, Assigned: siulung)

References

Details

Attachments

(1 file, 5 obsolete files)

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.
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?
This looks to be an expansion of what requested in bug 26194.
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
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.
updating keywords.
Keywords: patch, review
These should include the old values as well, but that might not be easy.
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.
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.
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.
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
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.
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.
Attachment #54485 - Attachment is obsolete: true
Attachment #54485 - Attachment is patch: false
Attachment #54486 - Attachment is obsolete: true
Attached patch another whack at processmail (obsolete) (deleted) — Splinter Review
Attached patch smaller defparams. (obsolete) (deleted) — Splinter Review
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.
*** 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 ?)
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
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
Keywords: review
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
Attachment #56049 - Flags: review?
Attachment #56050 - Flags: review?
Comment on attachment 56049 [details] [diff] [review] another whack at processmail Processmail is dead, denying review.
Attachment #56049 - Flags: review? → review-
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 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?
*** Bug 237907 has been marked as a duplicate of this bug. ***
*** Bug 239956 has been marked as a duplicate of this bug. ***
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.
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
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.
Attached patch old patch ported to 2.18 (obsolete) (deleted) — 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?
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
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?
*** Bug 307970 has been marked as a duplicate of this bug. ***
*** Bug 259926 has been marked as a duplicate of this bug. ***
(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.
(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.
Attached patch diff against cvs head (deleted) — Splinter Review
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?
(In reply to comment #57) Can you also include Classification?
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+
Flags: approval?
Target Milestone: Future → Bugzilla 2.24
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?
*** 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. ***
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.
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+
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
Keywords: relnote
*** 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.

Attachment

General

Creator:
Created:
Updated:
Size: