Closed Bug 245375 Opened 20 years ago Closed 12 years ago

Scheduled whining needs custom columns

Categories

(Bugzilla :: Whining, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 4.4

People

(Reporter: erik, Assigned: mrbball)

References

Details

Attachments

(1 file, 3 obsolete files)

There should be a way for the new whine scheduler to specify which columns are listed in a message. This could get a little bit tricky, and isn't critical to landing the scheduled whines, so I'm putting it in its own bug.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.20
Component: Email Notifications → Whining
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004. Anything flagged enhancement that hasn't already landed is being pushed out. If this bug is otherwise ready to land, we'll handle it on a case-by-case basis, please set the blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
*** Bug 277708 has been marked as a duplicate of this bug. ***
Attached patch robzilla_v1 (obsolete) (deleted) — Splinter Review
patch attached. Moved column definition, display column determiniation, select column determination, and sort order determination into Search.pm, and used those functions in whine.pl to generate the search. To see the custom columns, the "columnlist" cgi parameter needs to be in the saved search URL (but there is no interface for this yet).
Assignee: erik → robzilla
Attachment #199964 - Flags: review?
Whiteboard: patch awaiting review
too late for 2.22
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
It would be great if whine mails could send "Full Text Bug Listing" of bugs that match query.
Comment on attachment 199964 [details] [diff] [review] robzilla_v1 Cool patch, unfortunately it has bitrotten because of changes to buglist. Few general comments for an updated patch: 1) Maybe DefineColumn sub should be a private one? This means it should start with a _, if I'm not mistaken. 2) Search.pm is already very long and adding these subs there make it even longer. Maybe we should instead add them to some sub namespace, like Bugzilla::Search::Columns or something like that.
Attachment #199964 - Flags: review? → review-
mkanat, any ideas about the module namespace this should use?
(In reply to comment #7) > mkanat, any ideas about the module namespace this should use? Yeah. The whole thing should be one Bugzilla::Search::ColumnList object. It shouldn't be a module with subroutines, it should be an object that you can pass into "new Bugzilla::Search".
I don't know when I'll get the time to update the patch, so if someone else wants to take it, go right ahead.
Whiteboard: patch awaiting review
QA Contact: mattyt-bugzilla → default-qa
*** Bug 344658 has been marked as a duplicate of this bug. ***
This bug is retargetted to Bugzilla 3.2 for one of the following reasons: - it has no assignee (except the default one) - we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1) - it's not a blocker If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0. If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug. If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Untargeting this bug until it sees some action.
Target Milestone: Bugzilla 3.2 → ---
*** Bug 359463 has been marked as a duplicate of this bug. ***
Fixing this bug would improve our work team's production as it will help management in notification of bug status. I tried setting the flag to "yes" for blocking 3.0 to have someone continue to work this bug but the flag failed to post because I do not have the permission to post a flag, to your bug.(In reply to comment #11) > This bug is retargetted to Bugzilla 3.2 for one of the following reasons: > - it has no assignee (except the default one) > - we don't expect someone to fix it in the next two weeks (i.e. before we > freeze the trunk to prepare Bugzilla 3.0 RC1) > - it's not a blocker > If you are working on this bug and you think you will be able to submit a patch > in the next two weeks, retarget this bug to 3.0. > If this bug is something you would like to see implemented in 3.0 but you are > not a developer or you don't think you will be able to fix this bug yourself in > the next two weeks, please *do not* retarget this bug. > If you think this bug should absolutely be fixed before we release 3.0, either > ask on IRC or use the "blocking3.0 flag".
(In reply to comment #14) > I tried setting the flag to "yes" for > blocking 3.0 to have someone continue to work this bug but the flag failed to > post because I do not have the permission to post a flag, to your bug. That's because we don't let you decide whether this bug should block 3.0 or not. Normal users can only do requests (blocking3.0?). Anyway, the two weeks deadline is now over, meaning this enhancement will definitely not go into Bugzilla 3.0.
I agree completely that having custom format capability with whining would be fantastic. If nothing else, I think it's important to see a last changed date as part of the columns so readers can see when something changed versus their last whine. Rob - if you're still working on this issue, would you mind updating? I'm reassigning to default because your last patch is more than two years old.
Assignee: robzilla2 → whining
Status: ASSIGNED → NEW
Priority: P3 → P1
(In reply to comment #5) > It would be great if whine mails could send "Full Text Bug Listing" of bugs > that match query. i second this; would be great to be able to send a "Full Text Bug Listing" to someone who is not in the office.
Assignee: whining → wicked
Depends on: 344878
Depends on: 297382
No longer depends on: 297382
when will solve this problem?
I'm also in need of this fix, when is it possible to have another update since the last comment please?
Come on, can't be that difficult.
(In reply to comment #23) > Come on, can't be that difficult. Patches welcome: http://wiki.mozilla.org/Bugzilla:Developers Alternately, you're welcome to hire somebody to work on this. None of us get paid to work on Bugzilla. http://www.bugzilla.org/support/consulting.html
Attached patch v2 (obsolete) (deleted) — Splinter Review
Instead of supporting just custom columns, I think a better solution is to take the column list as defined in the saved query. This also addresses supporting custom fields. Attached is a patch that I think goes in the right direction. One issue I see is that the abbreviation table is currently hardcoded in list/table.html.tmpl. Instead, this should be defined in perhaps fielddefs so that it can be reused. One could then define abbreviations through the editfields admin page.
Attachment #199964 - Attachment is obsolete: true
Attachment #650703 - Flags: review?(wicked)
Comment on attachment 650703 [details] [diff] [review] v2 >+++ whine.pl 2012-08-09 13:46:36.746252000 -0700 >+ push @searchfields, "bug_id"; Did you make sure bug_id is not already in @searchfields? Duplicating columns is going to make Oracle unhappy. >+ $bug->{columnlist} = \@searchfields; You should rather store the column list as part of $thisquery as all bugs belonging to the same query will have the same columns. >+++ template/en/default/whine/mail.txt.tmpl 2012-08-09 15:33:58.068348000 -0700 >+ [% FOREACH col=bug.columnlist %] >+ [% IF col.length > padding; padding = col.length; END %] You should look at the length of field_descs.$col, not $col. Also, write this as: [% padding = field_descs.${col}.length IF field_descs.${col}.length > padding %] Otherwise your patch looks good. Note that your patch doesn't apply cleanly on the current code (4.3.2+): Hunk #1 FAILED at 47. 1 out of 1 hunk FAILED -- saving rejects to file template/en/default/whine/mail.txt.tmpl.rej patching file template/en/default/whine/mail.html.tmpl Hunk #1 FAILED at 61. 1 out of 1 hunk FAILED -- saving rejects to file template/en/default/whine/mail.html.tmpl.rej
Attachment #650703 - Flags: review?(wicked) → review-
Assignee: wicked → altlist
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 4.4
Attached patch v3 (obsolete) (deleted) — Splinter Review
I'm very interested in seeing this bug resolved and it seemed like only a few tweaks to the previous patch were necessary. Here is a new version with the changes that were suggested, although the third suggestion didn't work for me so I stuck with the original logic (although I switched the variable name). I hope I'm not stepping on Albert's toes by submitting this. It just seemed so close ...
Attachment #663325 - Flags: review?(LpSolit)
Attached patch v3 (deleted) — Splinter Review
Ack, the previous attachment was the wrong version. This one is correct.
Attachment #663325 - Attachment is obsolete: true
Attachment #663325 - Flags: review?(LpSolit)
Attachment #663327 - Flags: review?(LpSolit)
Thanks Kent. Glad that you can help! Feel free to take over. I still think the abbreviation table in list/table.html.tmpl should be moved to a more central place so that it can be used in this patch. Although that may be a separate ticket. There is likely an additional patch needed to support custom abbreviations via the custom field admin page.
I thought about it and felt that not having abbreviations in the email shouldn't stop this bug. Moving the abbreviation table to a more central place is probably worth considering, but I think it is a separate bug.
Attachment #650703 - Attachment is obsolete: true
Comment on attachment 663327 [details] [diff] [review] v3 >=== modified file 'template/en/default/whine/mail.html.tmpl' >+ [% FOREACH col=query.columnlist %] Nit: add whitespaces around "=". >=== modified file 'template/en/default/whine/mail.txt.tmpl' >+ >+ [% largest_title = 0 %] Don't add an empty line, else it will appear in the email. >+ [% FOREACH col = query.columnlist %] >+ [% NEXT IF col == 'bug_id' %] >+ [% IF field_descs.${col}.length > largest_title %] >+ [% largest_title = field_descs.${col}.length %] The indentation is 2 characters in templates. Also, as it's a plain text email, the indentation matters in the output. Also, both templates miss 'columnlist' in their INTERFACE section at the top of the template. >=== modified file 'whine.pl' >+ if (defined $searchparams->param('columnlist')) { 'defined' is not needed. If it's defined but empty, we should ignore it. I tested your patch and it works great. I will fix the comments above on checkin. Thanks a lot for your patch! r=LpSolit
Attachment #663327 - Flags: review?(LpSolit) → review+
Reassigning the bug to Kent as his patch is the one which is going to be checked in, but I will mention both Kent and Albert as authors in the commit message. Just on time for Bugzilla 4.4! :)
Assignee: altlist → kar
Flags: approval4.4+
Flags: approval+
Keywords: relnote
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified whine.pl modified template/en/default/whine/mail.html.tmpl modified template/en/default/whine/mail.txt.tmpl Committed revision 8426. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/ modified whine.pl modified template/en/default/whine/mail.html.tmpl modified template/en/default/whine/mail.txt.tmpl Committed revision 8419.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Added to relnotes for 4.4.
Keywords: relnote
The change to whine/mail.txt.tmpl has problems on saved searches where 'short_short_desc' is one of the columns. I see these errors: Argument "" isn't numeric in numeric gt (>) at template/en/default/whine/mail.txt.tmpl line 58, <DATA> line 522. Argument "" isn't numeric in subtraction (-) at template/en/default/whine/mail.txt.tmpl line 66, <DATA> line 522. . . I believe this is due to 'short_short_desc' (i.e. first 60 characters) not being one of the fields in vars.field_descs in global/field-descs.none.tmpl. What is the proper way to fix this? 1. Add 'short_short_desc' to vars.field_descs definition in global/field-descs.none.tmpl 2. Define field_descs.short_short_desc in whine/mail.txt.tmpl (similar to how it is in list/change-columns.html.tmpl) Both appear to work.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
short_short_desc is already in field-descs.none.tmpl.
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Yes it is! I didn't have that change in my tree yet. Sorry! :-o
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: