Bug 7349 - Sorting patron search result by cardnumber doesn't work
Summary: Sorting patron search result by cardnumber doesn't work
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Adrien SAURAT
QA Contact: Ian Walls
URL:
Keywords:
: 7066 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-12-13 10:19 UTC by Adrien SAURAT
Modified: 2015-11-13 09:14 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
proposed patch (1.09 KB, patch)
2011-12-13 15:10 UTC, Adrien SAURAT
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7349: corrects sort order for Patron search (1.47 KB, patch)
2011-12-15 11:29 UTC, Koustubha Kale
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Adrien SAURAT 2011-12-13 10:19:47 UTC
[Mantis ref. MT2062]
In Home › Patrons › Search Results, the results are always sorted by name.

There is a listbox "order by" allowing 2 choices:
1) Surname.
When searching with this choice, the results are indeed sorted by surname.

2) Cardnumber.
When searching with this choice, the results seem to be sorted by first name.


=====

What is exactly the problem then?

A) Should we be able to sort by first name/surname and then Cardnumber must be renamed to First name?

B) Should we be able to sort by surname/cardnumber and then the sorting must be repaired for the cardnumber option?

C) Should we be able to sort by first name/surname/cardnumber and then we add a new choice and repair cardnumber?
Comment 1 Owen Leonard 2011-12-13 14:03:55 UTC
It looks to me like the server-side sort of the whole result set is working fine but the client-side javascript sorts by name by default. I don't think having a client-side sort makes much sense for sorted paged results, so perhaps the answer (pending a conversion to datatables) is to remove the table sorter.
Comment 2 Adrien SAURAT 2011-12-13 14:55:35 UTC
Indeed!

Didn't think I would find a client-side sort here!! I don't see the point, I'll thus delete the following part:

<script type="text/javascript" src="[% themelang %]/lib/jquery/plugins/jquery.tablesorter.min.js"></script>
<script type="text/javascript">//<![CDATA[
$(document).ready(function() {
	$("#memberresultst").tablesorter({
		sortList: [[1,0]],
		widgets:	['zebra']
	});
});
//]]>
</script>



(it seems to work ok without it)
Comment 3 Adrien SAURAT 2011-12-13 15:10:12 UTC Comment hidden (obsolete)
Comment 4 Adrien SAURAT 2011-12-13 16:02:02 UTC
We lose something with this patch, the widget ['zebra'] is needed to give a different color to uneven lines.

I'll make another patch deleting only the "sortList: [[1,0]]," line.
Comment 5 Owen Leonard 2011-12-13 16:16:15 UTC
Why not move the table row coloring to the markup instead?

[% IF ( loop.even ) %]<tr class="highlight">[% ELSE %]<tr>[% END %]
Comment 6 Adrien SAURAT 2011-12-14 08:37:13 UTC
Actually, there is already something like that in the template, the highlights look ok.
No problem then, the patch is fine.
Comment 7 Koustubha Kale 2011-12-15 11:29:50 UTC
Created attachment 6809 [details] [review]
[SIGNED-OFF] Bug 7349: corrects sort order for Patron search

Sorting by cardnumber now works (removed a client-side
sort, the server-side sort is enough).

Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>

1) Verified that before applying this patch, when searching patrons with cardnumber as order by preference, patrons are sorted ny name.
2) Verified that after applying this patch, when searching with cardnumber as order by preference, patrons are correctly sorted by cardnumber.
Comment 8 Ian Walls 2011-12-15 12:32:17 UTC
Template only change, removes JS block only.  Marking as Passed QA
Comment 9 Paul Poulain 2011-12-16 15:14:56 UTC
QA comment: this patch removes code only, that can't be wrong ;-)

patch pushed, please test
Comment 10 Owen Leonard 2012-08-22 19:40:03 UTC
*** Bug 7066 has been marked as a duplicate of this bug. ***