When getting the updated patch from Kiko, I got only the patches for summarize_time.cgi, and not all the mods for summarize-time.html.tmpl. This means that there's a line of information missing from the detailed reports. Patch to follow.
Adds missing information to the Time Tracking Summary Reports that should have been there in the first place.
Attachment #177637 - Flags: review?
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.20
Flags: blocking2.20? → blocking2.20+
"If it's not a regression from 2.18 and it's not a critical problem with something that's already landed, let's push it off." - Dave
Comment on attachment 177637 [details] [diff] [review] Code patch for tip + <tr class="[% tr_class %]"> + <td> </td> + <td colspan="3"> + <table width="100%" cellpadding="0" cellspacing="0"> + <tr><td width="33%"> + Estimated: [% detail_data.$id.estimated_time FILTER html %]h + </td><td width="33%"> + Remaining: [% detail_data.$id.remaining_time FILTER html %]h + </td><td width="33%"> + Deadline: [% detail_data.$id.deadline || "<b>Not set</b>" %] + </td></tr></table> + </tr></td> The last line should probably be "</td></tr>". Due to it, the current patch would invalidate the HTML. Also the whole identation style of this block could be improved, in the lines of: <tr> <td width="33%"> .... </td> <td width="33%"> .... </td> and so on.
Attachment #177637 - Flags: review? → review-
Updating status whiteboard according to blocking2.20 flag.
Whiteboard: [wanted for 2.20]
Didn't do any real work. Fixed comments and made sure it passes runtests.pl.
*** Bug 304421 has been marked as a duplicate of this bug. ***
Comment on attachment 191576 [details] [diff] [review] Shane v1.1 Vlad, if you're reviewing this patch at the moment, please leave a comment -- you seem to be busy, so I'm opening the r? to the wind for now.
Attachment #191576 - Flags: review?(vladd) → review?
Comment on attachment 191576 [details] [diff] [review] Shane v1.1 >Index: template/en/default/bug/summarize-time.html.tmpl >=================================================================== >@@ -110,7 +110,7 @@ >- <tr><td colspan="5" class="owner_header"> >+ <tr><td colspan="4" class="owner_header"> This change looks incorrect. This makes the last column empty and thus it's background changes color. >+[% BLOCK do_tt_header %] >+ <tr class="[% tr_class FILTER html %]"> >+ <td> </td> >+ <td colspan="3"> >+ <table width="100%" cellpadding="0" cellspacing="0"> >+ <tr> >+ <td width="33%"> >+ Estimated: [% detail_data.$id.estimated_time FILTER html %]h >+ </td> >+ <td width="33%"> >+ Remaining: [% detail_data.$id.remaining_time FILTER html %]h >+ </td> >+ <td width="33%"> >+ Deadline: [% detail_data.$id.deadline || "<b>Not set</b>" %] >+ </td> >+ </tr> >+ </table> >+ </td> >+ </tr> >+[% END %] Looks like this only covers 4 columns when there's sometimes 5 columns. This also makes even rows to have incorrect background color in last column. In addition Deadline field shows unnecessary time portion. Also, please move this BLOCK after the two subsequent BLOCKs so that the comment just before this BLOCK still makes sense. >@@ -167,7 +190,12 @@ >+ [% INCLUDE bug_header id=id bug_status=status colspan=2 >+ short_desc=items.join(";") %] I can't see that this change does anything. Param colspan isn't used in bug_header BLOCK.
Attachment #191576 - Flags: review? → review-
Only security and dataloss fixes will be accepted on the 2.20 branch.
Assignee: shane.h.w.travis → gerv
Severity: normal → minor
Status: ASSIGNED → NEW
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Maybe is that a better component?
Assignee: gerv → query-and-buglist
Component: Reporting/Charting → Query/Bug List
We are freezing the code for 3.0 in two weeks and we don't expect this bug to be fixed on time.
Target Milestone: Bugzilla 2.22 → ---
This patch puts sums of the "Orig. Est.", "Current Est.", and "Hours Left" fields to the top of the summarize_time report. These totals are not related to the time period specified, only to the list of bug ids selected. I believe it does what Bug #286452 asks for. The template part of this patch is just about the bare minimum it took to support the feature. I wouldn't mind if it got some TLC.
Comment on attachment 270591 [details] [diff] [review] Add totals for Original, Current, and Hours Left estimates to the Time Summary Putting this patch in our radar.
Comment on attachment 270591 [details] [diff] [review] Add totals for Original, Current, and Hours Left estimates to the Time Summary >+ my $sth = $dbh->prepare($q); >+ $sth->execute(); >+ $res = $sth->fetch->; Please write: my $total = $dbh->selectrow_array('SELECT ...'); instead. This is shorter and cleaner.
This patch provides the same functionality as my previous patch, but it uses the more idiomatic style requested by Frédéric Buclin.
Comment on attachment 272050 [details] [diff] [review] Revised patch for overall time summaries I'm assuming you want this reviewed to get it checked in someday..
Attachment #272050 - Flags: review?
(In reply to comment #17) > (From update of attachment 272050 [details] [diff] [review]) > I'm assuming you want this reviewed to get it checked in someday.. Yes, that would be great. I get the sense that I'm not the only one interested in this functionality (especially from Bug 304421). Is there something special I have to do to request review and check-in? Feel free to email me privately if you don't want to clutter up the bug comments with instructions.
> in this functionality (especially from Bug 304421). Is there something special > I have to do to request review and check-in? Yes, you need to request a review by setting the review flag for the patch but I took care of that when I made my comment. I did ask it from wind (iow, from no specific reviewer) so it might take a while to get it reviewed. Review process is documented in http://wiki.mozilla.org/Bugzilla:Review, which you can refer for details and also for information of how to select a specific reviewer.
Comment on attachment 272050 [details] [diff] [review] Revised patch for overall time summaries This patch doesn't display time fields per bug, which is what travis wanted in his original patch. Before updating your patch, wait for the one from bug 389313, which will make your life much easier as you won't have to edit the .cgi script anymore.
Attachment #272050 - Flags: review?(kiko) → review-
That's true, it doesn't display time fields per bug. It's actually more appropriately a patch to bug #383240, but since that bug was marked as a duplicate of this one, I was assuming that this bug was meant to serve as an umbrella for a couple of different time-tracking improvements. If that's not the case, then bug #383240 should probably be re-opened since it's not really a duplicate of this one - it asks for overall summaries of time remaining, not per-bug information.
What's the link between this bug and bug 316425 ? I have the feeling they depend on each other since bug 316425, comment 16 .
This is a combination of patches made by Shane and Dennis. This makes following changes to summary report when "detailed summaries" is enabled: 1) Adds estimated, remaining and deadline information to each shown bug. 2) Calculates and shows total of estimated and remaining hours as well as latest deadline for all summarized bugs. Patch adds filter exceptions so two reviews will be needed for that part.
Comment on attachment 354674 [details] [diff] [review] Add more detail information, V2 The patch looks good and works fine, but I have some additional comments, which could lead to new separate bugs: - When grouped by bug number, it would make sense (IMHO) to not exclude bugs which have some remaining time, but which got no traction yet. This way, the global remaining time would be more accurate (we shouldn't exclude resolved bugs here, because QA may work on RESOLVED bugs, before marking them as VERIFIED). - Also, the global deadline should not take resolved bugs deadline into account, IMO, because if the bug is already resolved, then that's not something you have to worry about anymore, and so you would get a false "too far in the future" deadline. Or in case QA still needs to verify the resolved bugs before the given deadline, then we should still take them into account assuming there is some remaining time left. - We should add some text at the top of the "worked time" column to describe what the sub-totals represent. Now that we have many fields displayed, this may look confusing otherwise. Otherwise, I like the patch. This summarize_time page still has a large place for improvements, and this patch is a start in this direction. So r=LpSolit, and I will file new bugs for each of the comment above.
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 3.4
Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <-- filterexceptions.pl new revision: 1.124; previous revision: 1.123 done Checking in template/en/default/bug/summarize-time.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/summarize-time.html.tmpl,v <-- summarize-time.html.tmpl new revision: 1.14; previous revision: 1.13 done (Yay! This was my very first checking!)
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Added to the release notes for Bugzilla 3.4 in bug 494037.
You need to log in before you can comment on or make changes to this bug.