Closed
Bug 245375
Opened 20 years ago
Closed 12 years ago
Scheduled whining needs custom columns
Categories
(Bugzilla :: Whining, enhancement, P1)
Bugzilla
Whining
Tracking
()
RESOLVED
FIXED
Bugzilla 4.4
People
(Reporter: erik, Assigned: mrbball)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.20
Updated•20 years ago
|
Component: Email Notifications → Whining
Comment 1•20 years ago
|
||
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
Reporter | ||
Comment 2•20 years ago
|
||
*** Bug 277708 has been marked as a duplicate of this bug. ***
Comment 3•19 years ago
|
||
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
Updated•19 years ago
|
Attachment #199964 -
Flags: review?
Updated•19 years ago
|
Whiteboard: patch awaiting review
Comment 5•19 years ago
|
||
It would be great if whine mails could send "Full Text Bug Listing" of bugs that match query.
Comment 6•19 years ago
|
||
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-
Comment 7•19 years ago
|
||
mkanat, any ideas about the module namespace this should use?
Comment 8•19 years ago
|
||
(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".
Comment 9•19 years ago
|
||
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
Updated•18 years ago
|
QA Contact: mattyt-bugzilla → default-qa
Comment 10•18 years ago
|
||
*** Bug 344658 has been marked as a duplicate of this bug. ***
Comment 11•18 years ago
|
||
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
Comment 12•18 years ago
|
||
Untargeting this bug until it sees some action.
Target Milestone: Bugzilla 3.2 → ---
Comment 13•18 years ago
|
||
*** Bug 359463 has been marked as a duplicate of this bug. ***
Comment 14•18 years ago
|
||
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".
Comment 15•18 years ago
|
||
(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.
Comment 17•17 years ago
|
||
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
Updated•17 years ago
|
Priority: P3 → P1
Comment 18•17 years ago
|
||
(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.
Updated•16 years ago
|
Assignee: whining → wicked
Comment 21•15 years ago
|
||
when will solve this problem?
Comment 22•14 years ago
|
||
I'm also in need of this fix, when is it possible to have another update since the last comment please?
Comment 23•14 years ago
|
||
Come on, can't be that difficult.
Comment 24•14 years ago
|
||
(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
Comment 25•12 years ago
|
||
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 26•12 years ago
|
||
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-
Updated•12 years ago
|
Assignee: wicked → altlist
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 4.4
Assignee | ||
Comment 27•12 years ago
|
||
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)
Assignee | ||
Comment 28•12 years ago
|
||
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)
Comment 29•12 years ago
|
||
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.
Assignee | ||
Comment 30•12 years ago
|
||
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.
Updated•12 years ago
|
Attachment #650703 -
Attachment is obsolete: true
Comment 31•12 years ago
|
||
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+
Comment 32•12 years ago
|
||
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! :)
Comment 33•12 years ago
|
||
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
Assignee | ||
Comment 35•12 years ago
|
||
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 → ---
Comment 36•12 years ago
|
||
short_short_desc is already in field-descs.none.tmpl.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 37•12 years ago
|
||
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.
Description
•