From c8493cabf211ebfd14bfd179923f8bc6622334d9 Mon Sep 17 00:00:00 2001 From: Dan Pearl Date: Wed, 14 Nov 2012 12:14:11 -0500 Subject: [PATCH] LP# 1086934 - TPAC: Complete column sorting in some screens (specifically holds, circs, and circs_history) An earlier LP issue #1010277 concerned the halfway implementation of the column sort facility, and was addressed at the time by ripping out any hint of column sort capability, among other cleanup issues. The sorting capability has now been implemented with the following functionality: * Clicking on the appropriate column heads now sorts the contents from "ascending" to "descending" to "no sort". (The "no sort" will restore the original list as presented to the patron.) * The sort indicator (an up or down arrow) is placed to the right of the column head, as appropriate. * The combined "Title/Author" column in the circ screens is now separated into two independently sortable columns (Title and Author). * Title sorting is done with the 'filing' characters (leading "the", "a", "an", and other langugage equivalents) removed. To clarify the behavior for the patron, the leading articles are rendered in a smaller font, so as to keep the main entry prominent. In addition to the filing characters removed for the sort, leading non-alphanumeric characters are ignored in the sort. This commit only affects those screens and columns that suggested column sorting capability. Signed-off-by: Dan Pearl Signed-off-by: Kathy Lussier Signed-off-by: Ben Shum Conflicts: Open-ILS/src/templates/opac/parts/topnav.tt2 Conflicts: Open-ILS/src/templates/opac/css/style.css.tt2 Signed-off-by: Kathy Lussier --- .../lib/OpenILS/WWW/EGCatLoader/Account.pm | 29 ++++- Open-ILS/src/templates/opac/css/style.css.tt2 | 6 + .../templates/opac/myopac/circ_history.tt2 | 116 ++++++++++++++---- Open-ILS/src/templates/opac/myopac/circs.tt2 | 102 +++++++++++---- Open-ILS/src/templates/opac/myopac/holds.tt2 | 92 +++++++++++--- .../src/templates/opac/parts/misc_util.tt2 | 18 +++ .../src/templates/opac/parts/myopac/base.tt2 | 2 +- .../opac/parts/myopac/column_sort_support.tt2 | 38 ++++++ Open-ILS/src/templates/opac/parts/topnav.tt2 | 12 +- docs/RELEASE_NOTES_NEXT/OPAC/column_sort.txt | 21 ++++ 10 files changed, 365 insertions(+), 71 deletions(-) create mode 100644 Open-ILS/src/templates/opac/parts/myopac/column_sort_support.tt2 create mode 100644 docs/RELEASE_NOTES_NEXT/OPAC/column_sort.txt diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm index 3dae3bff52..30e07561c0 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm @@ -892,7 +892,14 @@ sub load_myopac_holds { my $hold_handle_result; $hold_handle_result = $self->handle_hold_update($action) if $action; - my $holds_object = $self->fetch_user_holds($hold_id ? [$hold_id] : undef, 0, 1, $available, $limit, $offset); + my $holds_object; + if ($self->cgi->param('sort') ne "") { + $holds_object = $self->fetch_user_holds($hold_id ? [$hold_id] : undef, 0, 1, $available); + } + else { + $holds_object = $self->fetch_user_holds($hold_id ? [$hold_id] : undef, 0, 1, $available, $limit, $offset); + } + if($holds_object->{holds}) { $ctx->{holds} = $holds_object->{holds}; } @@ -1519,7 +1526,22 @@ sub load_myopac_circ_history { $ctx->{circ_history_limit} = $limit; $ctx->{circ_history_offset} = $offset; - my $circ_ids = $e->json_query({ + my $circ_ids; + if ($self->cgi->param('sort') ne "") { # Defer limitation to circ_history.tt2 + $circ_ids = $e->json_query({ + select => { + au => [{ + column => 'id', + transform => 'action.usr_visible_circs', + result_field => 'id' + }] + }, + from => 'au', + where => {id => $e->requestor->id} + }); + + } else { + $circ_ids = $e->json_query({ select => { au => [{ column => 'id', @@ -1531,7 +1553,8 @@ sub load_myopac_circ_history { where => {id => $e->requestor->id}, limit => $limit, offset => $offset - }); + }); + } $ctx->{circs} = $self->fetch_user_circs(1, [map { $_->{id} } @$circ_ids]); return Apache2::Const::OK; diff --git a/Open-ILS/src/templates/opac/css/style.css.tt2 b/Open-ILS/src/templates/opac/css/style.css.tt2 index cafccb64dc..ff8c6ad592 100644 --- a/Open-ILS/src/templates/opac/css/style.css.tt2 +++ b/Open-ILS/src/templates/opac/css/style.css.tt2 @@ -2108,3 +2108,9 @@ label[for*=expert_] { font-weight: bold; } +} + +.sort_deemphasize { + font-weight: lighter; + font-size: 70%; +} diff --git a/Open-ILS/src/templates/opac/myopac/circ_history.tt2 b/Open-ILS/src/templates/opac/myopac/circ_history.tt2 index 4775b934e1..5339b25551 100644 --- a/Open-ILS/src/templates/opac/myopac/circ_history.tt2 +++ b/Open-ILS/src/templates/opac/myopac/circ_history.tt2 @@ -1,5 +1,6 @@ [% PROCESS "opac/parts/header.tt2"; PROCESS "opac/parts/misc_util.tt2"; + PROCESS "opac/parts/myopac/column_sort_support.tt2"; WRAPPER "opac/parts/myopac/base.tt2"; myopac_page = "circs" limit = ctx.circ_history_limit; @@ -11,13 +12,25 @@ + [% + # In the sorting case, the size is the size of ALL the circ items. In the non-sorting case, + # the size is simply the size of the chunk passed in. See the TODO below for the still-lingering + # bug. + sort_field = CGI.param('sort'); + IF (sort_field); + no_next = ctx.circs.size - offset <= limit; + ELSE; + no_next = ctx.circs.size < limit; + END; + %] +
[% l('Previously Checked Out Items') %] @@ -25,7 +38,7 @@ [% IF offset == 0 %] class='invisible' [% END %]>◄[% l('Previous') %] [%# TODO: get total to prevent paging off then end of the list.. %] limit, offset => (offset + limit)}) %]' - [% IF ctx.circs.size < limit %] class='invisible' [% END %] >[% l('Next') %]► + [% IF no_next %] class='invisible' [% END %] >[% l('Next') %]►
@@ -50,29 +63,91 @@ title="[% l('History of Items Checked Out') %]"> - [% l('Title / Author') %] - [% l('Checkout Date') %] - [% l('Due Date') %] - [% l('Date Returned') %] - [% l('Barcode') %] - [% l('Call Number') %] + [% sort_head("sort_title", l("Title")) %] + [% sort_head("author", l("Author")) %] + [% sort_head("checkout", l("Checkout Date")) %] + [% sort_head("due", l("Due Date")) %] + [% sort_head("returned", l("Date Returned")) %] + [% sort_head("barcode", l("Barcode")) %] + [% sort_head("callnum", l("Call Number")) %] - [% FOR circ IN ctx.circs; - attrs = {marc_xml => circ.marc_xml}; - PROCESS get_marc_attrs args=attrs; %] + [%# Copy the ctx.circs into a local array, then add a SORT field + that contains the value to sort on. Since we need the item attrs, + invoke it and save the result in ATTRS. + %] + [% + circ_items = ctx.circs; # Array assignment + + FOR circ IN circ_items; + circ.ATTRS = {marc_xml => circ.marc_xml}; + PROCESS get_marc_attrs args=circ.ATTRS; + + SWITCH sort_field; + + CASE "sort_title"; + circ.SORTING = circ.ATTRS.sort_title; + + CASE "author"; + circ.SORTING = circ.ATTRS.author; + + CASE "checkout"; + circ.SORTING = circ.circ.xact_start; + + CASE "due"; + circ.SORTING = circ.circ.due_date; + + CASE "returned"; + circ.SORTING = circ.circ.checkin_time; + + CASE "barcode"; + circ.SORTING = circ.circ.target_copy.barcode; + + CASE "callnum"; + circ.SORTING = circ.circ.target_copy.call_number.label; + + CASE; + sort_field = ""; + END; # SWITCH + END; #FOR circ + + IF (sort_field != "sort_title"); + deemphasize_class = ""; + ELSE; + deemphasize_class = " class=\"sort_deemphasize\""; + END; + + # Apply sorting to circ_items + IF (sort_field); + circ_items = circ_items.sort("SORTING"); + IF (CGI.param("sort_type") == "desc"); + circ_items = circ_items.reverse; + END; + + # Shorten the circ_items list per offset/limit/cout + hi = offset + limit - 1; + hi = hi > circ_items.max ? circ_items.max : hi; + + circ_items = circ_items.slice(offset, hi); + END; + + # circ_items list is now sorted. Traverse and dump the information. + + FOR circ IN circ_items; %] - - [% attrs.title | html %] - - [% IF attrs.author %] / + + [%- circ.ATTRS.title.substr(0,circ.ATTRS.nonfiling_characters) | html %] + [%- circ.ATTRS.title.substr(circ.ATTRS.nonfiling_characters) | html %] + + [% attrs.author | html %] - [% END %] + {qtype => 'author', query => circ.ATTRS.author.replace('[,\.:;]', '')}, + 1 + ) %]">[% circ.ATTRS.author | html %] [% date.format(ctx.parse_datetime(circ.circ.xact_start),DATE_FORMAT); %] @@ -81,8 +156,7 @@ [% date.format(ctx.parse_datetime(circ.circ.due_date),DATE_FORMAT); %] - [% - IF circ.circ.checkin_time; + [% IF circ.circ.checkin_time; date.format(ctx.parse_datetime(circ.circ.checkin_time),DATE_FORMAT); ELSE; %] * diff --git a/Open-ILS/src/templates/opac/myopac/circs.tt2 b/Open-ILS/src/templates/opac/myopac/circs.tt2 index 6ca68a1fa0..c78c153632 100644 --- a/Open-ILS/src/templates/opac/myopac/circs.tt2 +++ b/Open-ILS/src/templates/opac/myopac/circs.tt2 @@ -1,5 +1,6 @@ [% PROCESS "opac/parts/header.tt2"; PROCESS "opac/parts/misc_util.tt2"; + PROCESS "opac/parts/myopac/column_sort_support.tt2"; WRAPPER "opac/parts/myopac/base.tt2"; myopac_page = "circs" %]

[% l('Current Items Checked Out') %]

@@ -10,7 +11,7 @@ [% l("Current Items Checked Out") %]
- [% l("Check Out History") %] + [% l("Check Out History") %]
@@ -65,17 +66,70 @@ onclick="var inputs=document.getElementsByTagName('input'); for (i = 0; i < inputs.length; i++) { if (inputs[i].name == 'circ' && !inputs[i].disabled) inputs[i].checked = this.checked;}" /> - [% l('Title / Author') %] - [% l('Renewals Left') %] - [% l('Due Date') %] - [% l('Barcode') %] - [% l('Call number') %] + [% sort_head("sort_title", l("Title")) %] + [% sort_head("author", l("Author")) %] + [% sort_head("renews", l("Renewals Left")) %] + [% sort_head("due", l("Due Date")) %] + [% sort_head("barcode", l("Barcode")) %] + [% sort_head("callnum", l("Call number")) %] - - [% FOR circ IN ctx.circs; - attrs = {marc_xml => circ.marc_xml}; - PROCESS get_marc_attrs args=attrs; %] + + [%# Copy the ctx.circs into a local array, then add a SORT field + that contains the value to sort on. Since we need the item attrs, + invoke it and save the result in ATTRS. + %] + [% + circ_items = ctx.circs; # Array assignment + + sort_field = CGI.param('sort'); # unless changed below... + + FOR circ IN circ_items; + circ.ATTRS = {marc_xml => circ.marc_xml}; + PROCESS get_marc_attrs args=circ.ATTRS; + + SWITCH sort_field; + + CASE "sort_title"; + circ.SORTING = circ.ATTRS.sort_title; + + CASE "author"; + circ.SORTING = circ.ATTRS.author; + + CASE "renews"; + circ.SORTING = circ.circ.renewal_remaining; + + CASE "due"; + circ.SORTING = circ.circ.due_date; + + CASE "barcode"; + circ.SORTING = circ.circ.target_copy.barcode; + + CASE "callnum"; + circ.SORTING = circ.circ.target_copy.call_number.label; + + CASE; + sort_field = ""; + END; # SWITCH + END; #FOR circ + + IF (sort_field != "sort_title"); + deemphasize_class = ""; + ELSE; + deemphasize_class = " class=\"sort_deemphasize\""; + END; + + # Apply sorting to circ_items + IF (sort_field); + circ_items = circ_items.sort("SORTING"); + IF (CGI.param("sort_type") == "desc"); + circ_items = circ_items.reverse; + END; + END; + + # circ_items list is now sorted. Traverse and dump the information. + + FOR circ IN circ_items; %] - + [% IF circ.circ.target_copy.call_number.id == -1 %] [% circ.circ.target_copy.dummy_title | html %] - [% ELSIF attrs.title %] - [% attrs.title | html %] + [% ELSIF circ.ATTRS.title %] + + [%- circ.ATTRS.title.substr(0,circ.ATTRS.nonfiling_characters) | html %] + [%- circ.ATTRS.title.substr(circ.ATTRS.nonfiling_characters) | html %] [% END %] - [% IF circ.circ.target_copy.call_number.id == -1 %] / + + + [% IF circ.circ.target_copy.call_number.id == -1 %] [% circ.circ.target_copy.dummy_author | html %] - [% ELSIF attrs.author %] / - [% attrs.author | html %] + [% ELSIF circ.ATTRS.author %] + [% circ.ATTRS.author | html %] [% END %] @@ -137,7 +196,8 @@ - [% END; + [% END; # FOR + END %] diff --git a/Open-ILS/src/templates/opac/myopac/holds.tt2 b/Open-ILS/src/templates/opac/myopac/holds.tt2 index 6dbd4a269d..68f9b4edc4 100644 --- a/Open-ILS/src/templates/opac/myopac/holds.tt2 +++ b/Open-ILS/src/templates/opac/myopac/holds.tt2 @@ -1,6 +1,7 @@ [% PROCESS "opac/parts/header.tt2"; PROCESS "opac/parts/misc_util.tt2"; PROCESS "opac/parts/hold_status.tt2"; + PROCESS "opac/parts/myopac/column_sort_support.tt2"; WRAPPER "opac/parts/myopac/base.tt2"; myopac_page = "holds"; limit = (ctx.holds_limit.defined) ? ctx.holds_limit : 0; @@ -15,7 +16,7 @@ [% l("Items on Hold") %]
- [% l("Holds History") %] + [% l("Holds History") %]
@@ -47,9 +48,6 @@ [% IF count <= limit + offset %] class='invisible' [% END %] >[% l('Next') %]► - - [% l('Export List') %] -
@@ -107,9 +105,9 @@ - [% l('Title') %] - [% l('Author') %] - [% l('Format') %] + [% sort_head("sort_title", l('Title')) %] + [% sort_head("author", l('Author')) %] + [% sort_head("format", l('Format')) %] [% l('Pickup Location') %] [% l('Activate') %] [% l('Cancel if not filled by') %] @@ -119,9 +117,59 @@ - [% FOR hold IN ctx.holds; - attrs = {marc_xml => hold.marc_xml}; - PROCESS get_marc_attrs args=attrs; + + [%# Copy the ctx.holds into a local array, then add a SORT field + that contains the value to sort on. Since we need the item attrs, + invoke it and save the result in ATTRS. + %] + [% + hold_items = ctx.holds; + + sort_field = CGI.param('sort'); + + FOR hold IN hold_items; + hold.ATTRS = {marc_xml => hold.marc_xml}; + PROCESS get_marc_attrs args=hold.ATTRS; + + SWITCH sort_field; + + CASE "sort_title"; + hold.SORTING = hold.ATTRS.sort_title; + + CASE "author"; + hold.SORTING = hold.ATTRS.author; + + CASE "format"; + hold.SORTING = hold.ATTRS.format_label; + + CASE; + sort_field = ""; + END; # SWITCH + END; #FOR hold + + IF (sort_field != "sort_title"); + deemphasize_class = ""; + ELSE; + deemphasize_class = " class=\"sort_deemphasize\""; + END; + + # Apply sorting to hold_items + IF (sort_field != ""); + hold_items = hold_items.sort("SORTING"); + IF (CGI.param("sort_type") == "desc"); + hold_items = hold_items.reverse; + END; + + # Shorten the hold_items list per offset/limit/count + hi = offset + limit - 1; + hi = hi > hold_items.max ? hold_items.max : hi; + + hold_items = hold_items.slice(offset, hi); + END; + + # hold_items list is now sorted. Traverse and dump the information. + + FOR hold IN hold_items; ahr = hold.hold.hold %] @@ -131,26 +179,30 @@
- [% - title = attrs.title; - IF ahr.hold_type == 'P'; - title = l('[_1] ([_2])', title, hold.hold.part.label); - END; - %] - [% title | html %] + [% title = hold.ATTRS.title; + IF ahr.hold_type == 'P'; + title = l('[_1] ([_2])', title, hold.hold.part.label); + END; %] + + + [%- title.substr(0,hold.ATTRS.nonfiling_characters) | html %] + [%- title.substr(hold.ATTRS.nonfiling_characters) | html %]
[% attrs.author | html %] + {qtype => 'author', query => hold.ATTRS.author.replace('[,\.:;]', '')}, + 1 + ) %]">[% hold.ATTRS.author | html %]
[% - formats = attrs.all_formats; + formats = hold.ATTRS.all_formats; IF ahr.hold_type == 'M'; # only show selected formats for metarecords formats = []; diff --git a/Open-ILS/src/templates/opac/parts/misc_util.tt2 b/Open-ILS/src/templates/opac/parts/misc_util.tt2 index 8edeab5f12..6c2cb47910 100644 --- a/Open-ILS/src/templates/opac/parts/misc_util.tt2 +++ b/Open-ILS/src/templates/opac/parts/misc_util.tt2 @@ -207,6 +207,7 @@ titresults = xml.findnodes('//*[@tag="245"]/*[@code="a" or @code="b" or @code="n" or @code="p"]'); titresults_content = []; FOR sub IN titresults; titresults_content.push(sub.textContent); END; + args.title = titresults_content.join(" "); # Avoid ugly trailing syntax on brief titles args.title = args.title | replace('[:;/]$', ''); @@ -220,6 +221,23 @@ END; args.title_extended = (args.titles.size) ? args.titles.0 : ''; + # Create a version of the title designed for sorted displays. + args.sort_title = args.title | upper; + + # If the title has a "non-filing chaaracters" + # (to logically remove leading "The " for example) + # chop the title. Otherwise, chop until the first alphanumeric. + # BTW: Template Toolkit folds 1-element arrays to scalars! + title_node = xml.findnodes('//*[@tag="245"]'); + + args.nonfiling_characters = title_node.findvalue('@ind2'); + + IF (args.nonfiling_characters > 0); + args.sort_title = args.sort_title.substr(args.nonfiling_characters); + ELSE; + args.sort_title = args.sort_title.replace('^[^A-Z0-9]*',''); + END; + args.pubplaces = []; pubplace_hunt = xml.findnodes('//*[@tag="260"]/*[@code="a"]') || xml.findnodes('//*[@tag="264" and @ind2="1"]/*[@code="a"]'); diff --git a/Open-ILS/src/templates/opac/parts/myopac/base.tt2 b/Open-ILS/src/templates/opac/parts/myopac/base.tt2 index 74426df03c..828337b09d 100644 --- a/Open-ILS/src/templates/opac/parts/myopac/base.tt2 +++ b/Open-ILS/src/templates/opac/parts/myopac/base.tt2 @@ -37,7 +37,7 @@ ELSE; cls_which = "acct-tab-off"; END -%] - [% page.name; %] [% END %]
diff --git a/Open-ILS/src/templates/opac/parts/myopac/column_sort_support.tt2 b/Open-ILS/src/templates/opac/parts/myopac/column_sort_support.tt2 new file mode 100644 index 0000000000..eb2bc497d0 --- /dev/null +++ b/Open-ILS/src/templates/opac/parts/myopac/column_sort_support.tt2 @@ -0,0 +1,38 @@ +[%# Produce a URL for a given field that cycles for sorting from + "nothing" to "ascending" to "descending" then back to "nothing". +%] +[% MACRO sort_url(field) + IF (CGI.param('sort') == field); + SWITCH CGI.param('sort_type'); + CASE "asc"; + mkurl('',{sort=>field, sort_type=>'desc'},1); + CASE "desc"; + mkurl('',{},1); + CASE; + mkurl('',{sort=>field, sort_type=>'asc'}, 1); + END; + ELSE; + mkurl('',{sort=>field, sort_type=>'asc'}, 1); + END; +%] +[%# SET click_sort = l("click to sort") %] +[%# SET click_sort = "title=\"$click_sort\"" %] + +[%# Produce arrows to indicate the sorting status of the column %] +[% MACRO sort_indicator(field) + IF (CGI.param('sort') == field); + SWITCH CGI.param('sort_type'); + CASE "asc"; +" ▲"; + CASE "desc"; +" ▼"; + END; + END; +%] + +[%# Column headers for sortable columns %] +[% MACRO sort_head(field, field_label) + BLOCK %] +[% l(field_label) %][%- sort_indicator(field) %] +[% END +%] diff --git a/Open-ILS/src/templates/opac/parts/topnav.tt2 b/Open-ILS/src/templates/opac/parts/topnav.tt2 index aca62a82b1..719ffa1390 100644 --- a/Open-ILS/src/templates/opac/parts/topnav.tt2 +++ b/Open-ILS/src/templates/opac/parts/topnav.tt2 @@ -34,7 +34,7 @@
[% END %] - [% l('My Account') %] [% l('My Lists') %] @@ -44,25 +44,27 @@
- [% ctx.user_stats.checkouts.total_out %] [% l("Checked Out") %] | - [% ctx.user_stats.holds.total %] [% l("On Hold") %] | [% + {available => 1}, ['single', 'message_id', 'sort','sort_type']) %]">[% ctx.user_stats.holds.ready %] [% l("Ready for Pickup") %] | - [% money(ctx.user_stats.fines.balance_owed) %] [% l("Fines") %] diff --git a/docs/RELEASE_NOTES_NEXT/OPAC/column_sort.txt b/docs/RELEASE_NOTES_NEXT/OPAC/column_sort.txt new file mode 100644 index 0000000000..52fe317206 --- /dev/null +++ b/docs/RELEASE_NOTES_NEXT/OPAC/column_sort.txt @@ -0,0 +1,21 @@ +Column sorting in circulation screens +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Sorting of selected columns is now available in the *Items Checked Out*, *Check Out History*, +and *Holds* screen. + +* Clicking on the appropriate column heads now sorts the contents from +``ascending'' to ``descending'' to ``no sort''. (The ``no sort'' restores the +original list as presented in the screen.) + +* The sort indicator (an up or down arrow) is placed to the right +of the column head, as appropriate. + +* The combined *Title/Author* column in the *Items Checked Out* screen is now separated into two +independently sortable columns (Title and Author). + +* Title sorting is done with the so-called `filing' characters (leading ``the'', ``a'', +``an'', and other langugage equivalents) removed. The leading articles are rendered in +a smaller font, so as to keep the main entry prominent. In +addition to the filing characters removed for the sort, leading +non-alphanumeric characters are ignored in the sort. -- 2.43.2