Closed
Bug 94303
Opened 23 years ago
Closed 22 years ago
Group together changes made at the same time in activity log
Categories
(Bugzilla :: Creating/Changing Bugs, enhancement, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: bugzilla, Assigned: bugzilla)
References
Details
(Whiteboard: [blocker will fix])
Attachments
(3 files, 2 obsolete files)
9.32 KB,
patch
|
justdave
:
review-
|
Details | Diff | Splinter Review |
2.28 KB,
patch
|
Details | Diff | Splinter Review | |
5.94 KB,
text/html
|
Details |
It would be nice in the Activity Log for a bug, if some fields (i.e. who and when) could be grouped together (rowspan=X) for activities which all resulted from one change.
Comment 1•23 years ago
|
||
There's precedent. Bonsai does this with the cvs change logs. I would like to see this implemented :-)
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
Comment 2•23 years ago
|
||
Would this involve a schema change or would we just interpret the same user/date/time? I'd like to see this grouped in the schema eventually.
Comment 3•23 years ago
|
||
no schema change required. You just read the data internally ahead of time, and don't output anything until the data in the first two columns changes, then you output the first two columns with the rowspan= parameter and the as many rows as necessary for the other two columns. This is how Bonsai does it, and this is how we do it on the patch/installation listing on the main landfill page if there's more than one patch installed on the same test installation.
So, I think this would look best if the last column ('when') was moved to be the 2nd column, after the 'who' column, yes?. Would this be OK with people? And, if the same person has made many many changes, at many different times, but without anyone else making any changes inbetween, would you want the 'who' to span all those changes, or to just span the changes made at one time?
Comment 5•23 years ago
|
||
I would say just the changes made at that time. It would be more distict where the different times were that way.
I've got a patch for this, which I'll attach when 2.14 is out, so it is as up-to-date as possible. It will be a simple one line change to swap between the 2 possible ways the 'who' cell can span stuff, so you can check it out and see which you prefer. I think it looks simpler (less data, less cells) when the 'who' cell spans consecutive 'when' cells, and I think it is still possible to see the grouping of dates.
Assignee: justdave → gavins
Comment 7•23 years ago
|
||
please upload your patch based on the current cvs tip.
Component: Bugzilla → Creating/Changing Bugs
OS: SunOS → All
Product: Webtools → Bugzilla
Hardware: Sun → All
Version: Bugzilla 2.12 → 2.10
Comment 9•23 years ago
|
||
The following needs to be discussed: 1. Currently the cells are aligned vertically centred. This would mean that if a user made lots of changes, there could be a substantial vertical distance between the first row and the user name. See http://bugzilla.mozilla.org/show_activity.cgi?id=7954 for an example of what will be a problem. Top alignment might be a better policy here if we stick with two categorisations. 2. A variety of variables are named *_href. To me this seems to imply HTML links, yet they seems to be normal refs. I'll guess this means hash ref but I'm not sure it's a convention I like. I think field_* would be clearer than what_* here too. 3. This: foreach (sort(keys(%$what_href))) { my $date = $_; should, AFAICT, be: foreach my $date (sort(keys(%$what_href))) { 4. I'm not sure I think the extra code for the second categorisation is worth it in this instance. However I can see us getting benefit if this code were reusable, and you could specify the first N columns were categorised. It might be simpler too. Beyond that, I don't have a 100% understanding of this patch, and might have other issues in future. Is this the code Bonsai is using or based on it?
Comment 10•23 years ago
|
||
Updated•23 years ago
|
Attachment #47647 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #52228 -
Flags: review-
Assignee | ||
Comment 11•23 years ago
|
||
1) Yes, top alignment is much better. (whatever grouping we end up with). 2) Yes, href == hash ref, but I will change that (and the other name change.) 3) Yes. 4) OK, I'll have a think about that. Misc things: No, this is not the bonsai code - I haven't looked at that. I'd probably agree that I got carried away with hashes and lists, and I suspect it is probably easier to just read the data into 5 lists, for each column, so I will look at re-doing the patch like that, which may be easier to understand.
Comment 12•23 years ago
|
||
Any updates to this? Last patch is marked needs-work, and it's been a couple months. If we don't have something on here in the next couple days it'll get pushed to 2.18
Assignee | ||
Comment 13•23 years ago
|
||
Updated patch, still only against 2.14, I'm afraid. 1) This now aligns the user and data at the top of the cell, as suggested. 2) Variable name improvements 3) Done 4) I tried to make it simpler, and I tried a few ways of making the grouping more generic and applicable to many columns, but I failed :( We've been using this for 5 months, with no problems, so it would be nice to see it at least tested on landfill for a bit, maybe???
Attachment #52228 -
Attachment is obsolete: true
Comment 14•23 years ago
|
||
Comment on attachment 67420 [details] [diff] [review] Updated patch after suggestions. Trying to view bug activity with this patch applied gets me this: Bizarre copy of HASH in leave at CGI.pl line 1432. Line 1432 of CGI.pl is here: foreach (keys(%$field_details)) { my $tmp_details_list = ${%$field_details}{$_}; # <= 1432 $user_span += scalar(@$tmp_details_list); } Mac OS X 10.1.2, Perl 5..6.0
Attachment #67420 -
Flags: review-
Comment 15•23 years ago
|
||
This one just stashes the HTML for the constantly-changing rows in a temporary string, counting the number of rows as it goes, then outputs the first two columns with a rowspan followed by the previously stashed data any time the name or the date changes.
Comment 16•23 years ago
|
||
Assignee | ||
Comment 17•23 years ago
|
||
Yup, that's about a zillion times simpler :) But, the user cell doesn't span many date cells, which I quite like now I've got used to it here :( The 'bizarre copy of hash' is fixed by removing the % from '${%$' , if anyone does want to try that patch out. Later perls are more or less picky about that construct - I can't remember which way it is. I also added a 'No activity found' row in my patch, instead of just having a table with headers but no data, which never looks any good. If someone checks in the more simple patch, could they add that?
Comment 18•23 years ago
|
||
the patch attached here works against the existing code... but is pretty much voided since we're templatizing this now. We'll have to handle this in the template. See bug 124937
Whiteboard: [blocker will fix]
Comment 19•23 years ago
|
||
The latest patch on bug 124937 now contains a fix for this bug. Gerv
Comment 20•22 years ago
|
||
This is fixed by the checkin on bug 124937. Gerv
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•