Bug 9456

Summary: Add callnumber column to the cart
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Staff interfaceAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P5 - low CC: cedric.vita, gmcharlt, jonathan.druart, katrin.fischer, oleonard, paul.poulain
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart
Screenshot of cart with multi-call-number title
Proposed alternate structure
Bug 9456 [Alternate] Add callnumber column to the cart
Bug 9456 [Alternate] Add callnumber column to the cart - Followup - Sort by callnumbers
Bug 9456 [Alternate] Add callnumber column to the cart - Followup - Sort by callnumbers
Bug 9456 [Alternate] Add callnumber column to the cart - Followup - Sort by callnumbers
Bug 9043: Syspref improvement: add new type "multiple"
Bug 9456 [Alternate] Add callnumber column to the cart
Bug 9456 [Alternate] Add callnumber column to the cart - Followup - Sort by callnumbers
Bug 9456 [Alternate] Add callnumber column to the cart
Bug 9456 [Alternate] Add callnumber column to the cart - Followup - Sort by callnumbers
Bug 9456 [Alternate] Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - Add monospacing and padding for callnumbers
[SIGNED-OFF] Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - QA Followup - Improve individual item presentation in the Items column
Bug 9456 - Add callnumber column to the cart - Add monospacing and padding for callnumbers
Bug 9456 - Add callnumber column to the cart - Improve individual item presentation in the Items column
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - Followup - Move sort filters to datatables.js
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - Followup - Move sort filters to datatables.js
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - Followup - Move sort filters to datatables.js
Bug 9456 - Add callnumber column to the cart - QA Followup - Remove useless CSS
Odd sorting
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - Followup - Move sort filters to datatables.js
Bug 9456 - Add callnumber column to the cart - QA Followup - Remove useless CSS
Bug 9456 - Add callnumber column to the cart - QA Followup 2 - Sort on callnumber only
Bug 9456 - Add callnumber column to the cart - QA Followup 2 - Sort on callnumber only
Bug 9456 - Add callnumber column to the cart - QA Followup 2 - Sort on callnumber only
Bug 9456 - Add callnumber column to the cart
Bug 9456 - Add callnumber column to the cart - add tooltip
Bug 9456 - Add callnumber column to the cart - Followup - Move sort filters to datatables.js
Bug 9456 - Add callnumber column to the cart - QA Followup - Remove useless CSS
Bug 9456 - Add callnumber column to the cart - QA Followup 2 - Sort on callnumber only
Bug 9456: Remove whitespaces and tab

Description Kyle M Hall 2013-01-23 12:53:12 UTC
Adding a separate callnumber column to the cart will allow it to be sorted by callnumber.
Comment 1 Kyle M Hall 2013-01-23 12:54:19 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2013-01-23 13:07:37 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2013-01-23 14:15:25 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2013-01-23 14:17:04 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2013-01-23 16:41:26 UTC
(In reply to comment #4)
> Created attachment 14783 [details]
> Proposed alternate structure
> 
> What about this instead? I didn't test the nested sorting with DataTables,
> but I think it is doable.

If it can do nested sorting that would be perfect! Do you a patch you can upload?
Comment 6 Owen Leonard 2013-01-25 15:04:19 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2013-01-25 17:40:32 UTC
Unfortunately this just doesn't fit the bill. The outer table needs to be sorted by the all inner tables, or it just isn't even useful.

I've tried alternate options such as using rowspans so each callnumber has a table row and all the bib level data is rowspan-ed together, but it breaks both datatables and tablesorter.

One potential solution is the FakeRowSpan plugin ( http://datatables.net/plug-ins/api#fnFakeRowspan ), however, it requires server side processing apparently.
Comment 8 Kyle M Hall 2013-01-28 19:40:42 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2013-01-28 19:41:51 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2013-01-28 19:46:54 UTC
Owen, I think you may find this followup to your patch more acceptable than my original patch. This one looks more like my original patch, but the sorting is actually done using the best suited callnumber for each given row ( by using custom sorting filters for that table column ).

When sorting by Items in ascending fashion, the filter uses the highest valued callnumber from each row for sorting. When sorting by Items descending, it uses the lowest valued callnumber from each row.

Test plan:
1) Apply all the patches.
2) Create a record with two items, make one a very high callnumber ( e.g. '00000' ) and one a very low callnumber ( e.g. 'ZZZZZ' ).
3) Add this record and a number of other records to the cart.
4) View the cart and sort the Items in both ascending and descending fashion.
5) You should see that the item you created always stays on top because it contains both the highest valued and the lowest valued callnumbers in the list!
Comment 11 Paul Poulain 2013-02-19 10:50:12 UTC
Patch tested with a sandbox, by Delaye Stephane <stephane.delaye@biblibre.com>
Comment 12 Paul Poulain 2013-02-19 10:50:33 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2013-02-19 13:54:06 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2013-02-19 13:54:50 UTC
Comment on attachment 15544 [details] [review]
Bug 9043: Syspref improvement: add new type "multiple"

Sorry, wrong number
Comment 15 Jonathan Druart 2013-02-27 15:23:47 UTC
Please resubmit replacing "/intranet-tmpl/prog/en/" with [% themelang %] (in the first patch)

Marked as Failed QA.
Comment 16 Kyle M Hall 2013-02-27 16:08:19 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2013-02-27 16:08:28 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2013-02-28 10:57:17 UTC
QA comment:

Works great.
I don't know if it is relevant to sort on the first value (if there are others one) but I don't have other idea.
A followup could be to move javascript code into the datatable.js file.

Marked as Passed QA.
Comment 19 Jonathan Druart 2013-02-28 10:57:44 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2013-02-28 10:57:51 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2013-02-28 15:55:55 UTC
*** Bug 9449 has been marked as a duplicate of this bug. ***
Comment 22 Jared Camins-Esakov 2013-03-01 00:41:55 UTC
I am a little bit doubtful about pushing this patch as-is. Although the sorting seems to work, it becomes hard to tell what item belongs to which library, and even where the call numbers begin and end: http://screencast.com/t/meDgCBj3Rwm

Is there some way we can group information about each item together so that it's possible to follow?
Comment 23 Owen Leonard 2013-03-01 21:01:44 UTC Comment hidden (obsolete)
Comment 24 Owen Leonard 2013-03-01 21:04:33 UTC
I have attached a rebased copy of my patch because I think git was breaking something in it's auto-merge with the Bootstrap changes. Kyle I'm not sure why you created your patch as a follow-up to mine, since our approaches are so different. I think yours should be constructed as a separate patch and presented as an alternative rather than a follow-up to mine.
Comment 25 Kyle M Hall 2013-03-04 12:12:15 UTC
Owen, the entire point of having the callnumber as a column in the cart is to be able to sort all the results by callnumber. Your patch does not accomplish this. This is the reason I created and submitted this bug.

(In reply to comment #24)
> I have attached a rebased copy of my patch because I think git was breaking
> something in it's auto-merge with the Bootstrap changes. Kyle I'm not sure
> why you created your patch as a follow-up to mine, since our approaches are
> so different. I think yours should be constructed as a separate patch and
> presented as an alternative rather than a follow-up to mine.
Comment 26 Owen Leonard 2013-03-04 13:24:36 UTC
(In reply to comment #25)
> Owen, the entire point of having the callnumber as a column in the cart is
> to be able to sort all the results by callnumber. Your patch does not
> accomplish this.

I realize this. My point was simply that submitting your patch as a follow-up to mine, rather than as a separate patch, doesn't make sense because your solution and my solution are completely different.
Comment 27 Kyle M Hall 2013-03-04 13:30:40 UTC
That's true. I'll make mine a standalone patch. Thanks for the clarification!

(In reply to comment #26)
> (In reply to comment #25)
> > Owen, the entire point of having the callnumber as a column in the cart is
> > to be able to sort all the results by callnumber. Your patch does not
> > accomplish this.
> 
> I realize this. My point was simply that submitting your patch as a
> follow-up to mine, rather than as a separate patch, doesn't make sense
> because your solution and my solution are completely different.
Comment 28 Kyle M Hall 2013-03-04 13:56:48 UTC Comment hidden (obsolete)
Comment 29 Kyle M Hall 2013-03-04 13:57:22 UTC Comment hidden (obsolete)
Comment 30 Kyle M Hall 2013-03-04 13:57:32 UTC Comment hidden (obsolete)
Comment 31 Kyle M Hall 2013-03-04 13:57:44 UTC Comment hidden (obsolete)
Comment 32 Kyle M Hall 2013-03-04 14:00:05 UTC Comment hidden (obsolete)
Comment 33 Kyle M Hall 2013-03-04 14:03:55 UTC
(In reply to comment #22)
> I am a little bit doubtful about pushing this patch as-is. Although the
> sorting seems to work, it becomes hard to tell what item belongs to which
> library, and even where the call numbers begin and end:
> http://screencast.com/t/meDgCBj3Rwm
> 
> Is there some way we can group information about each item together so that
> it's possible to follow?

This presentation-only follow addresses this issue. Each item is on a single line, with the callnumber, library and location given different emphases.
Comment 34 Jared Camins-Esakov 2013-03-07 15:31:47 UTC
Kyle,

The patch you uploaded is the one from Owen with sign-offs by Stephane and Jonathan. If Owen agrees that we should go with your patch, could you please attach the standalone version (you might as well squash the follow-up) and mark it Needs Signoff so that we can move it through the QA process?
Comment 35 Kyle M Hall 2013-03-07 15:41:35 UTC Comment hidden (obsolete)
Comment 36 Kyle M Hall 2013-03-07 15:50:12 UTC Comment hidden (obsolete)
Comment 37 Kyle M Hall 2013-03-07 15:51:56 UTC
(In reply to comment #34)
> Kyle,
> 
> The patch you uploaded is the one from Owen with sign-offs by Stephane and
> Jonathan. If Owen agrees that we should go with your patch, could you please
> attach the standalone version (you might as well squash the follow-up) and
> mark it Needs Signoff so that we can move it through the QA process?

Fixed! Seeing as I cannot let well enough alone, I've included another followup that adds a tooltip to the items sorting explaining it's behavior. If everyone thinks this is a good addition, I can squash it into the primary patch.
Comment 38 Owen Leonard 2013-03-07 16:18:09 UTC
(In reply to comment #35)
> Created attachment 15937 [details] [review] [review]

This patch doesn't apply.
Comment 39 Kyle M Hall 2013-03-07 16:42:44 UTC Comment hidden (obsolete)
Comment 40 Kyle M Hall 2013-03-07 16:45:45 UTC Comment hidden (obsolete)
Comment 41 Kyle M Hall 2013-03-07 16:45:57 UTC Comment hidden (obsolete)
Comment 42 Kyle M Hall 2013-03-07 16:47:16 UTC
Fixed! I've also switched from using the Bootstrap tooltip to the Bootstrap popover as it is visually more pleasing ( though I suppose we could restyle the tooltip to look more pleasing as well ).

(In reply to comment #38)
> (In reply to comment #35)
> > Created attachment 15937 [details] [review] [review] [review]
> 
> This patch doesn't apply.
Comment 43 Jared Camins-Esakov 2013-03-07 16:51:27 UTC
I'm still a bit confused about which patch you've decided to use, but either way it needs to be signed off.
Comment 44 Kyle M Hall 2013-03-07 17:12:34 UTC
(In reply to comment #43)
> I'm still a bit confused about which patch you've decided to use, but either
> way it needs to be signed off.

Sorry, my bad! I still advocate for the patch I submitted as Owen's patch, while visually quite nice, lacks the functionality that is the entire point of this feature ( being able to sort the entire table by callnumber ).

Owen, please give my latest patch a try and let me know what you think!
Comment 45 Jared Camins-Esakov 2013-03-07 17:24:39 UTC
Kyle,

> Sorry, my bad! I still advocate for the patch I submitted as Owen's patch,
> while visually quite nice, lacks the functionality that is the entire point
> of this feature ( being able to sort the entire table by callnumber ).

In that case, please attach it to this bug. Owen's patch is the one you have been attaching to the bug.
Comment 46 Kyle M Hall 2013-03-07 17:28:49 UTC Comment hidden (obsolete)
Comment 47 Kyle M Hall 2013-03-07 17:29:22 UTC Comment hidden (obsolete)
Comment 48 cedric.vita 2013-03-19 09:59:21 UTC
second and third patch doesn't apply.
Comment 49 Kyle M Hall 2013-03-21 13:50:30 UTC Comment hidden (obsolete)
Comment 50 Kyle M Hall 2013-03-21 13:50:43 UTC Comment hidden (obsolete)
Comment 51 Paul Poulain 2013-03-22 09:42:10 UTC
Patch tested with a sandbox, by Cedric Vita <cedric.vita@dracenie.com>
Comment 52 Paul Poulain 2013-03-22 09:42:30 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2013-05-03 08:43:21 UTC
Could you export the 2 dataTable sorting functions into the datatables.js file please?

Marked as Failed QA.
Comment 54 Kyle M Hall 2013-05-07 16:25:55 UTC Comment hidden (obsolete)
Comment 55 Kyle M Hall 2013-05-07 16:29:47 UTC Comment hidden (obsolete)
Comment 56 Kyle M Hall 2013-05-07 16:29:56 UTC Comment hidden (obsolete)
Comment 57 Kyle M Hall 2013-05-07 16:30:03 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2013-05-13 09:24:44 UTC
Kyle,

Your first patch introduces some css. Since you don't use a table element, I think it is useless.

Marked as Failed QA.
Comment 59 Kyle M Hall 2013-06-18 15:10:00 UTC Comment hidden (obsolete)
Comment 60 Kyle M Hall 2013-06-18 15:10:16 UTC Comment hidden (obsolete)
Comment 61 Kyle M Hall 2013-06-18 15:10:31 UTC Comment hidden (obsolete)
Comment 62 Kyle M Hall 2013-06-18 15:11:09 UTC Comment hidden (obsolete)
Comment 63 Jonathan Druart 2013-06-19 07:42:11 UTC
Created attachment 19146 [details]
Odd sorting

Hi Kyle,
Could you explain how these 4 notices are sorted?
I think there is an issue.
On the left, sorting desc ; on the right, sorting asc.
Comment 64 Kyle M Hall 2013-06-19 10:13:03 UTC
I think I understand your confusion. Sorting by the "items" column does not sort the items within the table cell itself. It sorts the rows based on the highest/lowest callnumber within the the table cell. I believe the callnumbers are displayed in the same order the were created in the database.

Example:

Sorted by callnumber ascending:
---
A
E
Z
---
B
C
G
---
D
F
V
---

Sorted by callnumber descending:
---
A
E
Z
---
D
F
V
---
B
C
G
---

Note the order of the callnumbers within a cell does not change. Also note the A,E,Z set is always on top for sorting by callnumber is it contains both the highest and lowest callnumbers!


(In reply to comment #63)
> Created attachment 19146 [details]
> Odd sorting
> 
> Hi Kyle,
> Could you explain how these 4 notices are sorted?
> I think there is an issue.
> On the left, sorting desc ; on the right, sorting asc.
Comment 65 Jonathan Druart 2013-06-19 10:30:44 UTC
(In reply to comment #64)
> Note the order of the callnumbers within a cell does not change. Also note
> the A,E,Z set is always on top for sorting by callnumber is it contains both
> the highest and lowest callnumbers!

Yes, It is what I understood :)
But with my example:
1 = the notice with QC*, callnumber, callNUM, iop
2 = AAAA, ZZZZ, SSSS
3 = CALLNUMBER
4 = without callnumber

I am expecting, for asc:
2, 1, 3, 4 (or 4, 2, 1, 3 if "without callnumber" is first)
and for desc:
2, 1, 3, 4 too
Comment 66 Kyle M Hall 2013-06-19 13:05:43 UTC
I believe this is because the location is taken into account as part of the callnumber for sorting purposes.

(In reply to comment #65)
> (In reply to comment #64)
> > Note the order of the callnumbers within a cell does not change. Also note
> > the A,E,Z set is always on top for sorting by callnumber is it contains both
> > the highest and lowest callnumbers!
> 
> Yes, It is what I understood :)
> But with my example:
> 1 = the notice with QC*, callnumber, callNUM, iop
> 2 = AAAA, ZZZZ, SSSS
> 3 = CALLNUMBER
> 4 = without callnumber
> 
> I am expecting, for asc:
> 2, 1, 3, 4 (or 4, 2, 1, 3 if "without callnumber" is first)
> and for desc:
> 2, 1, 3, 4 too
Comment 67 Jonathan Druart 2013-06-19 13:44:10 UTC
(In reply to comment #66)
> I believe this is because the location is taken into account as part of the
> callnumber for sorting purposes.

Do you think it is simple to add a span or something else in order to sort only callnumbers?
Comment 68 Katrin Fischer 2013-07-19 22:26:24 UTC
Hi Kyle - there are some conflicts, can you please rebase and answer Jonathan's question?

I wonder if we should offer an option to show only the items of one selected branch in the list of items? But couldn't take a look at how this effects the display of items.
Comment 69 Kyle M Hall 2013-07-22 11:43:56 UTC Comment hidden (obsolete)
Comment 70 Kyle M Hall 2013-07-22 11:44:13 UTC Comment hidden (obsolete)
Comment 71 Kyle M Hall 2013-07-22 11:44:27 UTC Comment hidden (obsolete)
Comment 72 Kyle M Hall 2013-07-22 11:44:39 UTC Comment hidden (obsolete)
Comment 73 Kyle M Hall 2013-07-22 11:45:03 UTC Comment hidden (obsolete)
Comment 74 Kyle M Hall 2013-07-22 11:47:29 UTC Comment hidden (obsolete)
Comment 75 Kyle M Hall 2013-07-22 11:54:53 UTC
(In reply to Katrin Fischer from comment #68)
> Hi Kyle - there are some conflicts, can you please rebase and answer
> Jonathan's question?

Rebased and fixed! This latest followup parses the sorted strings down to callnumbers and only callnumbers.

> I wonder if we should offer an option to show only the items of one selected
> branch in the list of items? But couldn't take a look at how this effects
> the display of items.

That is an interesting idea, but I think it would belong in it's own separate bug.
Comment 76 Jonathan Druart 2013-08-12 13:03:26 UTC
I get a really weird js error when I click on the Items header:
  TypeError: y[(("string" + "-") + m[Q][1])] is not a function

Marked as Failed QA.
Comment 77 Kyle M Hall 2013-08-14 15:02:19 UTC Comment hidden (obsolete)
Comment 78 Kyle M Hall 2013-08-14 15:02:59 UTC
Please check again to see if this error still occurs.

(In reply to Jonathan Druart from comment #76)
> I get a really weird js error when I click on the Items header:
>   TypeError: y[(("string" + "-") + m[Q][1])] is not a function
> 
> Marked as Failed QA.
Comment 79 Owen Leonard 2013-08-14 15:31:22 UTC
I just tested this and got an error at first because I didn't clear my browser cache. After a shift-reload it worked without errors.
Comment 80 Jonathan Druart 2013-08-16 07:31:57 UTC
Last followup fixes the js error, thanks Kyle.
All work great now, marked as Passed QA.
Comment 81 Jonathan Druart 2013-08-16 07:32:50 UTC
Created attachment 20385 [details] [review]
Bug 9456 - Add callnumber column to the cart

This patch adds call-number sorting to the staff client cart. Sortining is enabled
on the 'Items' column which uses a custom sorting routine to sort the callnumbers.
If the sort is ascending, each column will be sorted by the highest ordered
callnumber for that row. Likewise, if the sort is descending the table will be
sorted by the lowest valued callnumber for that row.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 82 Jonathan Druart 2013-08-16 07:32:56 UTC
Created attachment 20386 [details] [review]
Bug 9456 - Add callnumber column to the cart - add tooltip

Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 83 Jonathan Druart 2013-08-16 07:33:02 UTC
Created attachment 20387 [details] [review]
Bug 9456 - Add callnumber column to the cart - Followup - Move sort filters to datatables.js

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 84 Jonathan Druart 2013-08-16 07:33:08 UTC
Created attachment 20388 [details] [review]
Bug 9456 - Add callnumber column to the cart - QA Followup - Remove useless CSS

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 85 Jonathan Druart 2013-08-16 07:33:13 UTC
Created attachment 20389 [details] [review]
Bug 9456 - Add callnumber column to the cart - QA Followup 2 - Sort on callnumber only

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 86 Jonathan Druart 2013-08-16 07:36:53 UTC
Created attachment 20390 [details] [review]
Bug 9456: Remove whitespaces and tab

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 87 Galen Charlton 2013-10-31 05:55:23 UTC
Pushed to master.  Thanks, Kyle!