From: Lebbeous Fogle-Weekley Date: Mon, 5 Mar 2012 19:37:04 +0000 (-0500) Subject: racing through UI implementation X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=c53e3e3debacdf94d03ead31853fa2f3bbe9cc50;p=evergreen%2Fequinox.git racing through UI implementation some middle layer changes: both the ML methods in this branch now have ways to tell the caller when they could get at least one more partial "page" of results if they shifted their offset up by limit and repeated their request. auto-expansion of first top level groupin now supported in open-ils.serial.holdings.grouped_by_summary. open-ils.serial.holdings.grouped_by_summary now takes one limit param, not an array of them. still needs an array of offsets, though. Signed-off-by: Lebbeous Fogle-Weekley --- diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm index 5160307b8d..a6ebba6d5b 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm @@ -777,7 +777,7 @@ sub scoped_holding_summary_tree_for_bib { "+ssub" => {record_entry => int($bib)}, "+sitem" => {date_received => {"!=" => undef}} }, - limit => int($limit), + limit => int($limit) + 1, # see comment below on "limit trick" offset => int($offset), order_by => [ { @@ -800,6 +800,15 @@ sub scoped_holding_summary_tree_for_bib { # Now we build a tree out of our result set. my $result = {}; + # Use our "limit trick" from above to cheaply determine whether there's + # another page of results, for the UI's benefit. Put $more into the + # result hash at the very end. + my $more = 0; + if (scalar(@$rows) > int($limit)) { + $more = 1; + pop @$rows; + } + foreach my $row (@$rows) { my $org_node_needs_placed = 0; my $org_node = @@ -830,6 +839,7 @@ sub scoped_holding_summary_tree_for_bib { } } + $result->{more} = $more; return $result; } @@ -839,22 +849,26 @@ __PACKAGE__->register_method( api_level => 1, argc => 6, signature => { - desc => q/Return a set of holding summaries organized into a tree + desc => 'Return a set of holding summaries organized into a tree of nodes that look like: {org_unit:, holding_summaries:[], children:[]} + The root node has an extra key: "more". Its value is 1 if there + are more pages (in the limit/offset sense) of results that the caller + could potentially fetch. + All arguments except the first (bibid) are optional. - /, + ', params => [ { name => "bibid", desc => "ID of the bre to which holdings belong", type => "number" }, { name => "org_unit", type => "number" }, - { name => "depth", type => "number" }, - { name => "limit", type => "number" }, - { name => "offset", type => "number" }, - { name => "ascending", type => "boolean" }, + { name => "depth (default 0)", type => "number" }, + { name => "limit (default 10)", type => "number" }, + { name => "offset (default 0)", type => "number" }, + { name => "ascending (default false)", type => "boolean" }, ] } ); @@ -922,32 +936,30 @@ sub _make_grouped_holding_node { sub grouped_holdings_for_summary { my ( $self, $client, $summary_type, $summary_id, - $expand_path, $limits, $offsets + $expand_path, $limit, $offsets, $auto_expand_first ) = @_; # Validate input or set defaults. ($summary_type .= "") =~ s/[^\w]//g; $summary_id = int($summary_id); $expand_path ||= []; - $limits ||= [10]; + $limit ||= 10; + $limit = 10 if $limit < 1; $offsets ||= [0]; - foreach ($expand_path, $limits, $offsets) { + foreach ($expand_path, $offsets) { if (ref $_ ne 'ARRAY') { return new OpenILS::Event( "BAD_PARAMS", note => - "'expand_path', 'limits' and 'offsets' arguments must " . - "be arrays" + "'expand_path' and 'offsets' arguments must be arrays" ); } } - if (scalar(@$limits) != scalar(@$expand_path) + 1 or - scalar(@$offsets) != scalar(@$expand_path) + 1) { + if (scalar(@$offsets) != scalar(@$expand_path) + 1) { return new OpenILS::Event( "BAD_PARAMS", note => - "'limits' and 'offsets' arrays each must be one element " . - "longer than 'expand_path'" + "'offsets' array must be one element longer than 'expand_path'" ); } @@ -1071,7 +1083,7 @@ sub grouped_holdings_for_summary { %subfield_where_clauses }, distinct => 1, # sic, this goes here in json_query - limit => int(shift(@$limits)), + limit => int($limit) + 1, offset => int(shift(@$offsets)), order_by => { "smhc_$subfield" => { @@ -1082,23 +1094,37 @@ sub grouped_holdings_for_summary { } }) or return $e->die_event; + my $top_more = 0; + if (scalar(@$top) > int($limit)) { + $top_more = 1; + pop @$top; + } + # This will help us avoid certain repetitive calculations. Examine # _label_holding_level() to see what I mean. my $mfhd_cache = {}; # Make the tree we have so far. - my $tree = [ map( - _make_grouped_holding_node( - $_, $subfield, $deepest_level, $pattern_field, $mfhd_cache - ), - @$top - ) ]; + my $tree = [ + map( + _make_grouped_holding_node( + $_, $subfield, $deepest_level, $pattern_field, $mfhd_cache + ), + @$top + ), ($top_more ? undef : ()) + ]; # Ok, that got us the top level, with nothing expanded. Now we loop through # the elements of @$expand_path, issuing similar queries to get us deeper # groupings and even actual specific holdings. my $parent = $tree; + # Will we try magic auto-expansion of the first top-level grouping? + if ($auto_expand_first and (not @$expand_path) and @$tree) { + $expand_path = [$tree->[0]->{value}]; + unshift @$offsets, 0; + } + foreach my $value (@$expand_path) { my $prev_subfield = $subfield; $subfield = shift @subfields; @@ -1142,7 +1168,7 @@ sub grouped_holdings_for_summary { %subfield_where_clauses }, distinct => 1, # sic, this goes here in json_query - limit => int(shift(@$limits)), + limit => int($limit) + 1, offset => int(shift(@$offsets)), order_by => { "smhc_$subfield" => { @@ -1155,8 +1181,14 @@ sub grouped_holdings_for_summary { return $tree unless @$level; + my $level_more = 0; + if (scalar(@$level) > int($limit)) { + $level_more = 1; + pop @$level; + } + # Find attachment point for our results. - my ($point) = grep { $_->{value} eq $value } @$parent; + my ($point) = grep { ref $_ and $_->{value} eq $value } @$parent; # Set parent for the next iteration. $parent = $point->{children} = [ @@ -1165,7 +1197,7 @@ sub grouped_holdings_for_summary { $_, $subfield, $deepest_level, $pattern_field, $mfhd_cache ), @$level - ) + ), ($level_more ? undef : ()) ]; last if $subfield eq $deepest_level; @@ -1178,7 +1210,7 @@ __PACKAGE__->register_method( method => "grouped_holdings_for_summary", api_name => "open-ils.serial.holdings.grouped_by_summary", api_level => 1, - argc => 5, + argc => 6, signature => { desc => q/Return a tree of holdings associated with a given summary grouped by all but the last of either chron or enum units./, @@ -1186,10 +1218,11 @@ __PACKAGE__->register_method( { name => "summary_type", type => "string" }, { name => "summary_id", type => "number" }, { name => "expand_path", type => "array" }, - { name => "limits", type => "array", desc => - "This must be exactly one element longer than expand_path" }, + { name => "limit (default 10)", type => "number" }, { name => "offsets", type => "array", desc => - "This must be exactly one element longer than expand_path" } + "This must be exactly one element longer than expand_path" }, + { name => "auto_expand_first", type => "boolean", desc => + "Only if expand_path is empty, automatically expand first top-level grouping" } ] } ); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm index 3d1d6c819e..4b92fbed08 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm @@ -55,13 +55,9 @@ sub load_record { $ctx->{get_org_setting}-> ($org, "opac.fully_compressed_serial_holdings") ) { - $ctx->{holding_summaries} = - $self->get_holding_summaries($rec_id, $org, $depth); - - $ctx->{have_holdings_to_show} = - scalar(@{$ctx->{holding_summaries}->{basic}}) || - scalar(@{$ctx->{holding_summaries}->{index}}) || - scalar(@{$ctx->{holding_summaries}->{supplement}}); + # We're loading this data here? Are we therefore assuming that we + # *are* going to display something in the "issues" expandy? + $self->load_serial_holding_summaries($rec_id, $org, $depth); } else { $ctx->{mfhd_summaries} = $self->get_mfhd_summaries($rec_id, $org, $depth); @@ -77,9 +73,8 @@ sub load_record { $ctx->{marchtml} = $self->mk_marc_html($rec_id); }, issues => sub { - $ctx->{expanded_holdings} = - $self->get_expanded_holdings($rec_id, $org, $depth) - if $ctx->{have_holdings_to_show}; + return; + # XXX this needed? }, cnbrowse => sub { $self->prepare_browse_call_numbers(); @@ -254,47 +249,101 @@ sub mk_marc_html { 'open-ils.search.biblio.record.html', $rec_id, 1); } -sub get_holding_summaries { +sub load_serial_holding_summaries { my ($self, $rec_id, $org, $depth) = @_; + my $limit = $self->cgi->param("slimit") || 10; + my $offset = $self->cgi->param("soffset") || 0; + my $serial = create OpenSRF::AppSession("open-ils.serial"); - my $result = $serial->request( - "open-ils.serial.bib.summary_statements", - $rec_id, {"org_id" => $org, "depth" => $depth} + + # First, get the tree of /summaries/ of holdings. + my $tree = $serial->request( + "open-ils.serial.holding_summary_tree.by_bib", + $rec_id, $org, $depth, $limit, $offset )->gather(1); + return if $self->apache_log_if_event( + $tree, "getting holding summary tree for record $rec_id" + ); + + # Next, if requested, get a list of individual holdings under a + # particular summary. + my $holdings; + my $summary_id = int($self->cgi->param("sid") || 0); + my $summary_type = $self->cgi->param("stype"); + + if ($summary_id and $summary_type) { + my $expand_path = [ $self->cgi->param("sepath") ], + my $expand_limit = $self->cgi->param("selimit"); + my $expand_offsets = [ $self->cgi->param("seoffset") ]; + my $auto_expand_first = 0; + + if (not @$expand_offsets) { + $expand_offsets = undef; + $auto_expand_first = 1; + } + + $holdings = $serial->request( + "open-ils.serial.holdings.grouped_by_summary", + $summary_type, $summary_id, + $expand_path, $expand_limit, $expand_offsets, + $auto_expand_first + )->gather(1); + + if ($holdings and ref $holdings eq "ARRAY") { + $self->place_holdings_with_summary( + $tree, $holdings, $summary_id, $summary_type + ); +# or $self->apache->log->warn( +# "could not place holdings within summary tree" +# ); + } else { + $self->apache_log_if_event( + $holdings, "getting holdings grouped by summary $summary_id" + ); + } + } + $serial->kill_me; - return $result; + + # The presence of any keys in the tree hash other than 'more' means that we + # must have /something/ we could show. + $self->ctx->{have_holdings_to_show} = grep { $_ ne 'more' } (keys %$tree); + + $self->ctx->{holding_summary_tree} = $tree; } -sub get_mfhd_summaries { - my ($self, $rec_id, $org, $depth) = @_; +# This helper to load_serial_holding_summaries() recursively searches in +# $tree for a holding summary matching $sid and $stype, and places $holdings +# within the node for that summary. IOW, this is about showing expanded +# holdings under their "parent" summary. +sub place_holdings_with_summary { + my ($self, $tree, $holdings, $sid, $stype) = @_; + + foreach my $sum (@{$tree->{holding_summaries}}) { + if ($sum->{id} == $sid and $sum->{summary_type} eq $stype) { + $sum->{holdings} = $holdings; + return 1; + } + } - my $serial = create OpenSRF::AppSession("open-ils.search"); - my $result = $serial->request( - "open-ils.search.serial.record.bib.retrieve", - $rec_id, $org, $depth - )->gather(1); + foreach my $child (@{$tree->{children}}) { + return if $self->place_holdings_with_summary( + $child, $holdings, $sid, $stype + ); + } - $serial->kill_me; - return $result; + return; } -sub get_expanded_holdings { +sub get_mfhd_summaries { my ($self, $rec_id, $org, $depth) = @_; - my $holding_limit = int($self->cgi->param("holding_limit") || 10); - my $holding_offset = int($self->cgi->param("holding_offset") || 0); - my $type = $self->cgi->param("expand_holding_type"); - - my $serial = create OpenSRF::AppSession("open-ils.serial"); + my $serial = create OpenSRF::AppSession("open-ils.search"); my $result = $serial->request( - "open-ils.serial.received_siss.retrieve.by_bib.atomic", - $rec_id, { - "ou" => $org, "depth" => $depth, - "limit" => $holding_limit, "offset" => $holding_offset, - "type" => $type - } + "open-ils.search.serial.record.bib.retrieve", + $rec_id, $org, $depth )->gather(1); $serial->kill_me; diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm index f10ccb796c..6eb28162dd 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm @@ -340,4 +340,27 @@ sub load_eg_cache_hash { } } +sub apache_log_if_event { + my ($self, $event, $prefix_text, $success_ok, $level) = @_; + + $prefix_text ||= "Evergreen returned event"; + $success_ok ||= 0; + $level ||= "warn"; + + chomp $prefix_text; + $prefix_text .= ": "; + + my $code = $U->event_code($event); + if (defined $code and ($code or not $success_ok)) { + $self->apache->log->$level( + $prefix_text . + ($event->{textcode} || "") . " ($code)" . + ($event->{note} ? (": " . $event->{note}) : "") + ); + return 1; + } + + return; +} + 1; diff --git a/Open-ILS/src/templates/opac/parts/record/issues.tt2 b/Open-ILS/src/templates/opac/parts/record/issues.tt2 index f346587a80..a26d6b7828 100644 --- a/Open-ILS/src/templates/opac/parts/record/issues.tt2 +++ b/Open-ILS/src/templates/opac/parts/record/issues.tt2 @@ -1,29 +1,59 @@ -
-[% -base_expando = ctx.full_path _ "?expand=issues"; -FOREACH type IN ctx.holding_summaries.keys; - NEXT UNLESS ctx.holding_summaries.$type.size; - expanded = CGI.param('expand_holding_type') == type; %] -
- [[% expanded ? '-' : '+' %]] - [% ctx.holding_summaries.$type.join(", ") %] - [% IF expanded %] - - [% FOR blob IN ctx.expanded_holdings %] - - - [% IF blob.has_units %] - - [% END %] - - [% END %] -
[% blob.issuance.label | html %] - [% l("Place Hold") %] -
- [% END %] -
-[% END %] +
[% +VIEW grouped_holding_tree; + BLOCK list; + '
    '; + has_more = 0; + FOREACH node IN item; + IF NOT node.label; + has_more = 1; + LAST; + END; + "
  • "; node.label; "
    • "; + IF node.children; + view.print(node.children); + END; + END; + '
'; + END; +END; + +VIEW holding_summary_tree; + BLOCK hash; + '
    '; + ''; + ctx.get_aou(item.org_unit).name; + ''; + FOREACH summary IN item.holding_summaries; + '
  • '; + summary.generated_coverage.join(", "); + '
    • '; + IF summary.holdings; + grouped_holding_tree.print(summary.holdings); + END; + END; + FOREACH child IN item.children; + view.print(child); + END; + '
'; + END; +END %] +
+[% holding_summary_tree.print(ctx.holding_summary_tree) %] +
+
+ [% slimit = CGI.param('slimit') || 10; + soffset = CGI.param('soffset') || 0; + soffset_prev = soffset - slimit; + IF soffset_prev < 0; soffset_prev = 0; END; + soffset_next = soffset + slimit; + %] + [% IF soffset > 0 %] + soffset_next}, ['sid','stype']) %]">[% l('Next') %] + [% END %] +
[% IF ctx.mfhd_summaries.size; %]