Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Tabular reports should be sortable

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Reporting/Charting
P2
enhancement
RESOLVED FIXED
15 years ago
4 years ago

People

(Reporter: Jouni Heikniemi, Assigned: Frédéric Buclin)

Tracking

2.15
Bugzilla 4.2
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

15 years ago
The bug counts by engineer view should be sortable through clickable headers or
a similar mechanism.
The new reporting architecture is now in place, and sorting is explicitly not
supported - for architectural and UI reasons. If you need to sort, export the
data as CSV and import it into a spreadsheet.

Gerv
So WONTFIX, then.

Gerv
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 3

15 years ago
Architectural and UI reasons? What on Earth are you talking about?

I have no problem with you wontfixing this (since it's such a minor issue), but 
we should have more than a obscure reference to "architectural and UI reasons" 
to defend this decision against possible future duplicate rfes.
REOPEN

You can sort in the template as a worst case option, but I'm reasonlay certain
that you can do so in the db, too.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Obviously, you can only sort on something you have selected for one of the axis,
though.
The current architecture of report.cgi makes it very difficult to implement
sorting in a sane manner. The axes are alphabetically sorted, and that's what
you get. Sorting in the template is likewise somewhere between very hard and
impossible due to the data structures used. I am actively saying that I will not
fix this bug because it would require a(nother) complete rearchitecting of
report.cgi. I suggest that people who want advanced data processing capabilities
use a dedicated application such as a spreadsheet.

If this bug is to remain open, then it needs a new assignee. :-)

Gerv
Well, its not that tricky. All you need to do is:

a) move the addition into perl, from the template (which is probably better
anyway, since that would be marginally faster)

b) make names.{tbl,col,row} an array, not a hash (so that ordering is preserved)

c) foreach table, foreach column we're sorting on, do:

sort { $a->{$col} <=> $b->{$col} } @rows

Yes, this only allows sorting on coluns, but then we avoid the issues you hinted
at. Adding rows isn't hard, either; allowing both may be.

If you don't want to do this, then future + assign to nobody, but don't close it..
> Well, its not that tricky. All you need to do is:

Before you say how easy it is, check the hoops I'm having to jump through over
in bug 173005 to support graphical charts. Any solution needs to fit the
architecture I'm using over there. :-| 

I'll take your suggestion, and future this and assign it to nobody.

Use a spreadsheet, people :-)

Gerv
Assignee: gerv → nobody
Status: REOPENED → NEW
Target Milestone: --- → Future
(Reporter)

Comment 9

15 years ago
>Use a spreadsheet, people :-)

I'm nobody to judge the triviality of implementing this, so I have faith in what
you say. Still, with all due respect, the architecture is seriously flawed if
_sorting a table_ is that hard. I mean, it's not that odd a need: "Who's got the
most ASSIGNED bugs at the moment" isn't something you should have a spreadsheet
app for. If I really need some extreme stats, I'll dump the MySQL db into my
OLAP application and start digging. But expecting everybody looking for people's
bug counts to do that is ridiculous.

The current solution (future+nobody) is ok with me, of course.
Summary: Bug counts by engineer should be sortable → Bug counts should be sortable
.
Assignee: nobody → nobody
Yeah, this is a pretty serious deficiency.

I don't see why you couldn't sort in multiple dimensions though, you would just
look at the total for that rowset/columnset/table.

The interesting question is whether the ordering has to be the same between
tables (3D).  I'd say yes, because you can't change the ordering of columns for
different rows, so it seems you shouldn't be able to for the third dimension,
although that could perhaps be an option.

Comment 12

12 years ago
This would actually be useful, if it hasn't already happened.
Summary: Bug counts should be sortable → Tabluar and graphical reports should be sortable

Comment 13

12 years ago
*** Bug 256336 has been marked as a duplicate of this bug. ***

Updated

11 years ago
QA Contact: mattyt-bugzilla → default-qa
Target Milestone: Future → ---

Updated

10 years ago
Summary: Tabluar and graphical reports should be sortable → Tabular and graphical reports should be sortable
(Assignee)

Comment 14

10 years ago
That's also something I would like to see implemented, difficult or not to implement.
(Assignee)

Updated

10 years ago
Assignee: nobody → guy.pyrzak
Priority: -- → P2
Target Milestone: --- → Bugzilla 4.0
(Assignee)

Comment 15

9 years ago
Created attachment 312746 [details] [diff] [review]
patch, v1 (using sorttable.js)

I don't know what pyrzak has in mind to implement this, but there is a small 17Kb JS script which does all we need, and is ultra simple to use: sorttable.js. I know b.m.o already uses it to sort columns in buglist.cgi, so I think it would be fine to add sorttable.js to our codebase.

Attached is the unaltered JS file from http://www.kryogenix.org/code/browser/sorttable/sorttable.js
Attachment #312746 - Flags: review?(mkanat)
Attachment #312746 - Flags: review?(gerv)
(Assignee)

Comment 16

9 years ago
To make things clear, what I'm implementing is only about tabular reports, not graphical reports. We shouldn't try to fix both problems in the same bug else we will go nowhere; especially because this patch does the sorting client-side which is something impossible to do with graphical reports.
Blocks: 425883
Anywhere I can try this out?

Gerv

Comment 18

9 years ago
Sure thing. I was planning on also having a filter capability. But this is clean and works so go for it. Assigning it to LpSolit.
Assignee: guy.pyrzak → LpSolit
(Assignee)

Comment 19

9 years ago
gerv, you can test it here:
https://landfill.bugzilla.org/qa32/query.cgi?format=report-table
No longer blocks: 425883

Comment 20

9 years ago
Don't use the unaltered sorttable file, we fixed a few things about it for the bmo code, and you should use the fixed version that's on bugzilla.everythingsolved.com.
(Assignee)

Comment 21

9 years ago
I would prefer if the initial checkin is the original JS file, so that we can more easily track what we changed in it. And without a description of what you really fixed in your customized version, it's unhelpful to me. I can do a diff (and I already did it), but this doesn't tell me why you did such or such customization. Please tell me more.
Status: NEW → ASSIGNED
Looks OK. It would be nice if the initial sort were displayed, and if the headers showed some sign that they were clickable, e.g. by being styled as links.

Gerv

Comment 23

9 years ago
(In reply to comment #21)
> Please tell me more.

A list of comments from the developer of the bmo code:

* "I modified it to handle the in-table headers, so that they stay fixed."
* "I refactored the original sort code a lot, so its easier to read/modify/fix."
* "To indicate multi-column sorting, I used different shades of gray. The 
  darkest one being the most recent sort."

  It also works with the "collapsed" headers on buglist.cgi.
Normally, a refactoring of 3rd party code would be an "!!", but given that this code hasn't changed in years, I guess it's fairly safe.

Gerv
LpSolit: is the ball in your court here, or do you need something from me?

Gerv
(Assignee)

Comment 26

9 years ago
(In reply to comment #25)
> LpSolit: is the ball in your court here, or do you need something from me?

gerv: all I have to do is to replace the original sorttable.js file by the one on b.m.o and to make sure it's working fine. From what I can read in comment 23, the altered JS file uses a gray background in column headers to show which column is used to sort the data. This would fix the first part of your request in comment 22, but wouldn't fix the second part (making it clear that you can click the column header to sort the data). Is that fine anyway, for now?
That sounds fine. My comment #22 was a WIBNI, not a requirement.

Gerv
(Assignee)

Comment 28

9 years ago
Max, sorttable.js at https://bugzilla.mozilla.org/js/sorttable.js and https://bugzilla.everythingsolved.com/attachment.cgi?id=524 do not match. Which one should I take for this bug? Note that bug 438004 should be fixed first.

Comment 29

9 years ago
(In reply to comment #28)
> Which one should I take for this bug?

  Take whatever's in bzr at: bzr://bzr.everythingsolved.com/mozilla/3.0/
(Assignee)

Comment 30

9 years ago
(In reply to comment #29)
>   Take whatever's in bzr at: bzr://bzr.everythingsolved.com/mozilla/3.0/

This one doesn't work as expected. I first had the following JS error:

Erreur : table.sorttable_bodies[0] is undefined
Fichier Source : https://localhost/bugzilla-pg/js/sorttable.js
Ligne : 368

To fix it, I had to add class="sorttable_body" to <tbody> (where is this documented?). But then I got the following JS error:

Erreur : node is undefined
Fichier Source : https://localhost/bugzilla-pg/js/sorttable.js
Ligne : 405

I have no idea how to fix this one. Any help is welcome!


Now, isn't your customized sorttable.js specific to buglist.cgi? Why do I see document.cookie = 'BUGLIST='+BUGLIST; in it?

Comment 31

9 years ago
(In reply to comment #30)
> Now, isn't your customized sorttable.js specific to buglist.cgi? Why do I see
> document.cookie = 'BUGLIST='+BUGLIST; in it?

  It is specific to buglist.cgi, at the moment. I'm sure that you could fix that, if you understand the code.

  You do have to modify the templates for it to work correctly. You can look through the "bzr log" and take all the changes related to it.
(Assignee)

Comment 32

9 years ago
Comment on attachment 312746 [details] [diff] [review]
patch, v1 (using sorttable.js)

Someone (not me) should make sorttable.js less buglist-centric.
Attachment #312746 - Flags: review?(mkanat)
Attachment #312746 - Flags: review?(gerv)
(Assignee)

Comment 33

8 years ago
Created attachment 414079 [details] [diff] [review]
patch, v2

The error I reported last year was easy to fix.

I will attach a diff for sorttable.js which shows changes I made in it in order to work here too.
Attachment #312746 - Attachment is obsolete: true
Attachment #414079 - Flags: review?(mkanat)
(Assignee)

Comment 34

8 years ago
Created attachment 414080 [details] [diff] [review]
diff in sorttable.js

This shows what I changed in sorttable.js compared to what bmo has.

Comment 35

8 years ago
Comment on attachment 414079 [details] [diff] [review]
patch, v2

I'm sorry, but I don't really want to accept sorttable.js as it is. It was written a long time ago, long before we had YUI (or before any real JS library even existed). It needs to be re-architected in modern times to use our libraries.
Attachment #414079 - Flags: review?(mkanat) → review-
And you would prefer that we had no client-side sorting rather than working and useful client-side sorting until someone finds time to do that?

Gerv

Comment 37

8 years ago
(In reply to comment #36)
> And you would prefer that we had no client-side sorting rather than working and
> useful client-side sorting until someone finds time to do that?

  Well, the issue is maintenance. On top of the fact that the architecture is somewhat ancient, in sorttable.js, it's also code that's fundamentally unfamiliar to any of us. As you saw when we put it on bmo, there were various issues, some of which never got resolved, and largely that had to do with architecture and understanding of the code.

  If we're going to have a feature, I want it to work right. The server-side sorting may be slower than client-side sorting, but it's been very thoroughly debugged for many years, and now works very nicely. I understand that any new feature won't be that stable that quickly, and that in fact new features are highly desirable, but adding code that we don't understand and that is poorly architected is a regressive action for Bugzilla to take, particularly when we've worked *so* hard for all these years to correct the architecture of the system, and when we've spent so much effort in 3.6 (and will spend further effort in 3.8) finishing and correcting Bugzilla's incomplete or broken features.

  I'm sure that there is somebody who could contribute to the Bugzilla project who has enough JavaScript expertise to refactor the script appropriately. I could, but I don't know how high on my priority list this will end up. (Really, I would have fun doing it, I just have a lot of things to do.) And if nobody has the expertise to understand the code and JavaScript itself well enough to re-architect it, then I'd also say that nobody has the expertise to maintain the code--a situation we definitely don't want to be in, particularly not with something as important as the bug list.

  So I'm all for feature improvement, I just want it to be maintainable and excellently-designed feature improvement so that Bugzilla is not only an excellent product in the present, but continues to be an excellent product well into the future.
It's true that it's not our code, but it is a stable bit of web plumbing used by a load of sites. Stuart Langridge is a great and responsible guy; he wouldn't let his code rot if there was maintenance required. And there hasn't been since April 2007. Why do we think that we are going to have to maintain it significantly? 

I don't recall these issues on b.m.o. of which you speak; could you point me at the bugs?

I hear your point about taking code you think is poorly architected (although again, I don't think this code is badly written) but there's a difference between taking core Perl code which is a mess (which I agree we've been working against for years) and taking a self-contained external JS library. After all, I haven't seen us audit all the Perl modules we use for code quality (although I guess we pick the most popular ones for any task, which is a vague proxy for quality). I don't think the "slippery slope" between taking sorttable.js and Bugzilla re-descending into a labyrinthine mess of unreadable Perl is actually all that slippery.

It's not just buglist sorting, of course - this bug is about tabular reports, which have no server-side sorting mechanism.

I think you might be letting the best be the enemy of the good, here...?

Gerv
(Assignee)

Comment 39

8 years ago
Nevermind, it seems very easy to do with YUI 2, see http://developer.yahoo.com/yui/examples/datatable/dt_enhanced.html
Target Milestone: Bugzilla 4.0 → Bugzilla 3.8
(Assignee)

Comment 40

7 years ago
Created attachment 456077 [details] [diff] [review]
patch, v3

Now using YUI2. There is only one problem, which should be fixed in YUI 2.9.0: the total row at the bottom of the table cannot be displayed, because DataTable knows nothing about <tfoot>, see http://yuilibrary.com/projects/yui2/ticket/2527635. Maybe we could go with it, and re-add the total row when YUI supports it?
Attachment #414079 - Attachment is obsolete: true
Attachment #414080 - Attachment is obsolete: true
Attachment #456077 - Flags: review?(guy.pyrzak)
(Assignee)

Updated

7 years ago
Target Milestone: Bugzilla 4.0 → Bugzilla 4.2

Comment 41

7 years ago
Created attachment 456376 [details] [diff] [review]
Report table with fancier JS using YUI (no hacks!)
Attachment #456077 - Attachment is obsolete: true
Attachment #456077 - Flags: review?(guy.pyrzak)

Comment 42

7 years ago
Comment on attachment 456376 [details] [diff] [review]
Report table with fancier JS using YUI (no hacks!)

That JS really needs to go into a separate file. And it should probably be an object like the autocompletes and dupTable are, no?
(Assignee)

Comment 43

7 years ago
(In reply to comment #42)
> That JS really needs to go into a separate file. And it should probably be an
> object like the autocompletes and dupTable are, no?

Not sure you can easily move the JS code in a separate js file. It depends on the selected fields and their values. pyrzak, what do you think?

Comment 44

7 years ago
Comment on attachment 456376 [details] [diff] [review]
Report table with fancier JS using YUI (no hacks!)

Hmm. Well, the urlbase can go into the "BUGZILLA" JS constant in header.html.tmpl, and that would let you move some of it, no?
(Assignee)

Comment 45

7 years ago
urlbase is not the main problem. If we can easily pass the row and column names as well as column values + some other extra data, then yes, we can move it outside the template. But I honestly wouldn't block this bug due to that.

pyrzak, your patch is working great. I'm going to remove one line, though, which doesn't work in my browser: console.log(), which is not defined (I guess you need Firebug to make it work).
(Assignee)

Comment 46

7 years ago
Created attachment 456460 [details] [diff] [review]
patch combined with pyrzak's contribution, v4

I simply removed console.log() from the previous patch. mkanat said on IRC it was fine to move the JS code to an external file in a separate bug.
Attachment #456376 - Attachment is obsolete: true
Attachment #456460 - Flags: review?(guy.pyrzak)
(Assignee)

Comment 47

7 years ago
Created attachment 456462 [details] [diff] [review]
patch combined with pyrzak's contribution, v4.1

No code change. I simply added pyrzak and myself to the contributors list.
Attachment #456460 - Attachment is obsolete: true
Attachment #456462 - Flags: review?(guy.pyrzak)
Attachment #456460 - Flags: review?(guy.pyrzak)

Comment 48

7 years ago
Comment on attachment 456462 [details] [diff] [review]
patch combined with pyrzak's contribution, v4.1

Test it on IE6 and if it works then i think we're good to go. If it doesn't work i think it's ok too because it would just fail and look like the current UI.
Attachment #456462 - Flags: review?(guy.pyrzak) → review+
(Assignee)

Comment 49

7 years ago
IE6 displays "null" instead of actual bug counts. No idea why.
(Assignee)

Comment 50

7 years ago
Created attachment 456476 [details] [diff] [review]
patch, v4.2

Fix a regexp which wasn't understood by IE6. Now works with IE6, IE8, Fx, Opera, Safari and Google Chrome. Carrying forward pyrzak's r+.
Attachment #456462 - Attachment is obsolete: true
Attachment #456476 - Flags: review+
(Assignee)

Updated

7 years ago
Flags: approval+
(Assignee)

Comment 51

7 years ago
Updating the bug summary as we can only sort tabular reports (I don't know what "sort a graphical report" means).

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/reports/report-table.html.tmpl
modified template/en/default/reports/report.html.tmpl
Committed revision 7302.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago7 years ago
Keywords: relnote
Resolution: --- → FIXED
Summary: Tabular and graphical reports should be sortable → Tabular reports should be sortable
(Assignee)

Updated

7 years ago
Blocks: 617676
(Assignee)

Comment 52

6 years ago
Added to relnotes in bug 713346.
Keywords: relnote
(Assignee)

Updated

6 years ago
Blocks: 731323
(Assignee)

Updated

5 years ago
Blocks: 734413
(Assignee)

Updated

5 years ago
Blocks: 790296
(Assignee)

Updated

4 years ago
Blocks: 859188
You need to log in before you can comment on or make changes to this bug.