From 19bd5bf1e2376d652440946b2f6b2d5f2daa2f24 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Fri, 14 Oct 2011 17:53:49 -0400 Subject: [PATCH] TPac: bookbags - stop trying to show everything at once Go back to displaying one bookbag at a time (not counting the temporary list, when that is populated) This hopefully constitutes a usability improvement. While it doesn't provide paging of bookbags themselves or of the items therein, it should make that easier to do. Showing all bookbags and all their items made it nonobvious how paging would even be expected to behave. This also provides the same sorting for the items in the temporary list as in real bookbags. Signed-off-by: Lebbeous Fogle-Weekley Signed-off-by: Dan Scott --- .../perlmods/lib/OpenILS/Application/AppUtils.pm | 22 ++ .../lib/OpenILS/WWW/EGCatLoader/Account.pm | 38 ++- .../lib/OpenILS/WWW/EGCatLoader/Container.pm | 9 + Open-ILS/src/templates/opac/myopac/lists.tt2 | 274 +++++++++++---------- Open-ILS/src/templates/opac/parts/anon_list.tt2 | 53 ++-- Open-ILS/src/templates/opac/parts/filtersort.tt2 | 2 +- Open-ILS/web/css/skin/default/opac/style.css | 12 + 7 files changed, 239 insertions(+), 171 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm index c0dbf7b2d9..0ecf415419 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm @@ -1854,6 +1854,28 @@ sub get_bre_attrs { return $attrs; } +# Shorter version of bib_container_items_via_search() below, only using +# the queryparser record_list filter instead of the container filter. +sub bib_record_list_via_search { + my ($class, $search_query, $search_args) = @_; + + # First, Use search API to get container items sorted in any way that crad + # sorters support. + my $search_result = $class->simplereq( + "open-ils.search", "open-ils.search.biblio.multiclass.query", + $search_args, $search_query + ); + + unless ($search_result) { + # empty result sets won't cause this, but actual errors should. + $logger->warn("bib_record_list_via_search() got nothing from search"); + return; + } + + # Throw away other junk from search, keeping only bib IDs. + return [ map { pop @$_ } @{$search_result->{ids}} ]; +} + sub bib_container_items_via_search { my ($class, $container_id, $search_query, $search_args) = @_; 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 9952c8cf68..a1c2de47d7 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm @@ -245,13 +245,13 @@ sub _load_user_with_prefs { } sub _get_bookbag_sort_params { - my ($self) = @_; + my ($self, $param_name) = @_; # The interface that feeds this cgi parameter will provide a single # argument for a QP sort filter, and potentially a modifier after a period. # In practice this means the "sort" parameter will be something like # "titlesort" or "authorsort.descending". - my $sorter = $self->cgi->param("sort") || ""; + my $sorter = $self->cgi->param($param_name) || ""; my $modifier; if ($sorter) { $sorter =~ s/^(.*?)\.(.*)/$1/; @@ -272,6 +272,18 @@ sub _prepare_bookbag_container_query { ); } +sub _prepare_anonlist_sorting_query { + my ($self, $list, $sorter, $modifier) = @_; + + return sprintf( + "record_list(%s)%s%s", + join(",", @$list), + ($sorter ? " sort($sorter)" : ""), + ($modifier ? "#$modifier" : "") + ); +} + + sub load_myopac_prefs_settings { my $self = shift; @@ -1323,7 +1335,7 @@ sub load_myopac_bookbags { my $e = $self->editor; my $ctx = $self->ctx; - my ($sorter, $modifier) = $self->_get_bookbag_sort_params; + my ($sorter, $modifier) = $self->_get_bookbag_sort_params("sort"); $e->xact_begin; # replication... my $rv = $self->load_mylist; @@ -1348,13 +1360,18 @@ sub load_myopac_bookbags { return Apache2::Const::HTTP_INTERNAL_SERVER_ERROR; } - # Here is the loop that uses search to find the bib records in each - # bookbag. XXX This should be parallelized. Should this be done - # with OpenSRF::MultiSession, or is it enough to use OpenSRF::AppSession - # and call ->request() without calling ->gather() on any of those objects - # until all the requests have been issued? + # If the user wants a specific bookbag's items, load them. + # XXX add bookbag item paging support + + if ($self->cgi->param("id")) { + my ($bookbag) = + grep { $_->id eq $self->cgi->param("id") } @{$ctx->{bookbags}}; + + if (!$bookbag) { + $e->rollback; + return Apache2::Const::HTTP_INTERNAL_SERVER_ERROR; + } - foreach my $bookbag (@{$ctx->{bookbags}}) { my $query = $self->_prepare_bookbag_container_query( $bookbag->id, $sorter, $modifier ); @@ -1491,6 +1508,7 @@ sub load_myopac_bookbag_update { } } elsif ($action eq 'save_notes') { $success = $self->update_bookbag_item_notes; + $url .= "&id=" . uri_escape($cgi->param("id")) if $cgi->param("id"); } return $self->generic_redirect($url) if $success; @@ -1591,7 +1609,7 @@ sub load_myopac_bookbag_print { my $id = int($self->cgi->param("list")); - my ($sorter, $modifier) = $self->_get_bookbag_sort_params; + my ($sorter, $modifier) = $self->_get_bookbag_sort_params("sort"); my $item_search = $self->_prepare_bookbag_container_query($id, $sorter, $modifier); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Container.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Container.pm index b1ebf4a807..6cc67966cf 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Container.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Container.pm @@ -33,6 +33,15 @@ sub fetch_mylist { $marc_xml = $self->fetch_marc_xml_by_id($list); } + # Leverage QueryParser to sort the items by values of config.metabib_fields + # from the items' marc records. + if (@$list) { + my ($sorter, $modifier) = $self->_get_bookbag_sort_params("anonsort"); + my $query = $self->_prepare_anonlist_sorting_query($list, $sorter, $modifier); + $list = $U->bib_record_list_via_search($query) or + return Apache2::Const::HTTP_INTERNAL_SERVER_ERROR; + } + return ($cache_key, $list, $marc_xml); } diff --git a/Open-ILS/src/templates/opac/myopac/lists.tt2 b/Open-ILS/src/templates/opac/myopac/lists.tt2 index 5615fa9373..a49b6540ce 100644 --- a/Open-ILS/src/templates/opac/myopac/lists.tt2 +++ b/Open-ILS/src/templates/opac/myopac/lists.tt2 @@ -22,7 +22,7 @@
-

[% l('Create new list') %]

+

[% l('Create new list') %]

@@ -64,155 +64,157 @@
-

[% l("Your existing lists") %]

-

-

- - [% INCLUDE "opac/parts/filtersort.tt2" - value=CGI.param('sort') %] - -
-

- +

[% l("Your existing lists") %]

[% INCLUDE "opac/parts/anon_list.tt2" %] [% IF ctx.bookbags.size %]
[% FOR bbag IN ctx.bookbags %] -
-
-
-
- - [% IF bbag.pub != 't' %] - - - [% ELSE %] - - - [% END %] -
-
-
-
- - - -
-
-
-
- - - -
-
-
- - [% IF bbag.pub == 't' %] - [% url = 'http://' _ ctx.hostname _ '/opac/extras/feed/bookbag/html-full/' _ bbag.id %] - [% bbag.name | html %] +
+
+ [% baseurl = ctx.opac_root _ '/myopac/lists'; + IF bbag.id != CGI.param("id"); + url = mkurl(baseurl, {id => bbag.id}, ['edit_notes']); + ltitle = l("Show items in list"); + ELSE; + url = mkurl(baseurl, {}, ['id', 'edit_notes']); + ltitle = l("Hide items in list"); + END %] + [% bbag.name | html %] + [% IF bbag.description %]
[% bbag.description | html %][% END %] +
+
+
+ + [% IF bbag.pub != 't' %] + + [% ELSE %] - [% bbag.name | html %] + + [% END %] - - [% IF bbag.description %]
[% bbag.description | html %][% END %]
+
+
+
+ + + +
+
+
- [% IF bbag.pub == 't'; %] - [% l('RSS Feed') %] - [% END %] + + +
-
+
+
+ [% IF bbag.pub == 't'; %] + [% l('RSS Feed') %] + [% END %]
-
- - - - - - + [% END %] + [% IF CGI.param("edit_notes") == bbag.id %] + + + + + [% END %] + +
- +
+ + [% IF CGI.param("id") == bbag.id %] +
+ + + [% INCLUDE "opac/parts/filtersort.tt2" + value=CGI.param('sort') %] + + + +
+
+ + + + + + - - - - - - - - [% UNLESS bbag.items.size %] - - [% END %] - [% FOR item IN bbag.items; - rec_id = item.target_biblio_record_entry.id; - attrs = {marc_xml => ctx.bookbags_marc_xml.$rec_id}; - PROCESS get_marc_attrs args=attrs %] - - - - - [% ELSE %] - + + + + - [% END %] + + + + + + [% UNLESS bbag.items.size %] + + [% END %] + [% FOR item IN bbag.items; + rec_id = item.target_biblio_record_entry.id; + attrs = {marc_xml => ctx.bookbags_marc_xml.$rec_id}; + PROCESS get_marc_attrs args=attrs %] + + + + - - - + + [% ELSE %] + [% END %] - -
+ - - [% l('Title') %] - - [% l('Author(s)') %] - - [% l('Notes') %] - [% IF CGI.param("edit_notes") != bbag.id %] - | [% l('Edit') %] - [% END %] - - - -
- [% l("This list contains no items.") %] -
- - - [% attrs.title | html %] - - [% attrs.author | html %] - [% IF CGI.param("edit_notes") == bbag.id %] - - [% FOR note IN item.notes %] - - [% END %] - - - [% FOR note IN item.notes %] -
[% note.note | html %]
- [% END %] -
+ [% l('Title') %] + + [% l('Author(s)') %] + + [% l('Notes') %] + [% IF CGI.param("edit_notes") != bbag.id %] + | [% l('Edit') %] [% END %] -
+ + +
+ [% l("This list contains no items.") %] +
+ + + [% attrs.title | html %] + + [% attrs.author | html %] [% IF CGI.param("edit_notes") == bbag.id %] -
- -
+ [% FOR note IN item.notes %] + + [% END %] + + + [% FOR note IN item.notes %] +
[% note.note | html %]
+ [% END %] +
-
-

- +
+ + +
+ + [% END %] [% END %]
[% END %] diff --git a/Open-ILS/src/templates/opac/parts/anon_list.tt2 b/Open-ILS/src/templates/opac/parts/anon_list.tt2 index 5101117bcc..3c32f993f2 100644 --- a/Open-ILS/src/templates/opac/parts/anon_list.tt2 +++ b/Open-ILS/src/templates/opac/parts/anon_list.tt2 @@ -1,22 +1,20 @@ [% IF ctx.mylist.size %] +
+
+ + [% INCLUDE "opac/parts/filtersort.tt2" + id="anonsort" name="anonsort" value=CGI.param("anonsort") %] + + + +
+
+
-
- - - - - -
- [% l('Temporary List') %] - - -
-
-
-
+

[% l('Temporary List') %]

@@ -26,13 +24,13 @@ for (i = 0; i < inputs.length; i++) { if (inputs[i].name == 'record' && !inputs[i].disabled) inputs[i].checked = this.checked;}"/> - - + + - - + + [% END %] diff --git a/Open-ILS/src/templates/opac/parts/filtersort.tt2 b/Open-ILS/src/templates/opac/parts/filtersort.tt2 index 74d9c8a6f4..a1b1fd4ba7 100644 --- a/Open-ILS/src/templates/opac/parts/filtersort.tt2 +++ b/Open-ILS/src/templates/opac/parts/filtersort.tt2 @@ -1,4 +1,4 @@ - diff --git a/Open-ILS/web/css/skin/default/opac/style.css b/Open-ILS/web/css/skin/default/opac/style.css index 449a4b2b8b..d7481a6c96 100644 --- a/Open-ILS/web/css/skin/default/opac/style.css +++ b/Open-ILS/web/css/skin/default/opac/style.css @@ -1149,3 +1149,15 @@ a.opac-button { margin-left: 1em; padding-left: 1em; } +.bookbag-controls-holder { width: 100%; } +.bookbag-controls-holder:nth-child(odd) { background-color: #d7d7d7; } +.bookbag-controls-holder:nth-child(even) { background-color: #e3e3e3; } +.bookbag-controls-holder .most { padding-left: 0; width: 55%; } +.bookbag-share .fixed { min-width: 4em; } +.bookbag-specific { margin-left: 1em; } +table.bookbag-specific { + border-right: 1px solid #999; + border-bottom: 1px solid #666; + margin-bottom: 2ex; +} +.save-notes { padding-bottom: 1.5ex; } -- 2.11.0
[% l('Title') %][% l('Author(s)') %][% l('Title') %][% l('Author(s)') %] - + + + [% IF ctx.user AND ctx.bookbags.size %] [% FOR bbag IN ctx.bookbags %]] @@ -53,8 +51,15 @@ [% attrs.title | html %][% attrs.author | html %][% attrs.title | html %][% attrs.author | html %]