Add sorting so that paging makes sense (but..)

It's wrong.  I can't /just/ sort by last received pub date, but must
also sort in an orgly fashion so that paging when there are lots of
holdings and diverse points in the org tree makes sense.

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
index 3c422c0..a6ce788 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
@@ -691,7 +691,8 @@ sub _place_org_node {
 
     # If we reach this point, we got all the way to the top of the org tree
     # without finding corresponding nodes in $tree (holdings tree), so the
-    # latter must be empty, and we need to make $tree just point to $node.
+    # latter must be empty, and we need to make $tree just contain what $node
+    # contains.
 
     %$tree = %$node;
 }
@@ -715,24 +716,47 @@ sub _find_ou_in_holdings_tree {
 # XXX deprecate/replace scoped_bib_holdings_summary
 
 sub scoped_holding_summary_tree_for_bib {
-    my ($self, $client, $bib, $org_unit, $depth, $limit, $offset) = @_;
+    my (
+        $self, $client, $bib, $org_unit, $depth, $limit, $offset, $ascending
+    ) = @_;
 
+    my $org_tree = $U->get_org_tree;    # caches
+
+    $org_unit ||= $org_tree->id;
     $depth ||= 0;
     $limit ||= 10;
     $offset ||= 0;
 
-    my $org_tree = $U->get_org_tree;    # caches
-
     my $e = new_editor;
+
+    # What we want to know from this query is essentially the set of
+    # holdings related to a given bib and the org units that have said
+    # holdings.
+
+    # For this we would only need sasum, sdist and ssub, but
+    # because we also need to be able to page (and therefore must sort) the
+    # results we get, we need a reasonable column on which to do the sorting,
+    # so for that we join sitem (via sstr) so we can sort on the maximum
+    # date_expected (which is basically the issue pub date) for items that
+    # have been received.
+
     my $rows = $e->json_query({
         select => {
             sasum => [qw/summary_type id generated_coverage/],
-            sdist => [qw/holding_lib/],
+            sdist => ["holding_lib"],
+            sitem => [
+                {column => "date_expected", transform => "max", aggregate => 1}
+            ]
         },
         from => {
             sasum => {
                 sdist => {
-                    join => { ssub => { } }
+                    join => {
+                        ssub => {},
+                        sstr => {
+                            join => {sitem => {}}
+                        },
+                    }
                 }
             }
         },
@@ -741,14 +765,24 @@ sub scoped_holding_summary_tree_for_bib {
                 holding_lib =>
                     $U->get_org_descendants(int($org_unit), int($depth))
             },
-            "+ssub" => {record_entry => int($bib)}
+            "+ssub" => {record_entry => int($bib)},
+            "+sitem" => {date_received => {"!=" => undef}}
         },
         limit => int($limit),
-        offset => int($offset)
+        offset => int($offset),
+        order_by => [
+            {
+                class => "sitem",
+                field => "date_expected",
+                transform => "max", # to match select clause
+                direction => ($ascending ? "ASC" : "DESC")
+            }
+        ],
     }) or return $e->die_event;
 
     $e->disconnect;
 
+    # Now we build a tree out of our result set.
     my $result = {};
 
     foreach my $row (@$rows) {
@@ -765,13 +799,17 @@ sub scoped_holding_summary_tree_for_bib {
             };
         }
 
-        push @{$org_node->{holding_summaries}}, {
+        # Make a very simple object for a single holding summary.
+        # generated_coverage is stored as JSON, and here we can unpack it.
+        my $summary = {
             id => $row->{id},
             summary_type => $row->{summary_type},
             generated_coverage =>
                 OpenSRF::Utils::JSON->JSON2perl($row->{generated_coverage})
         };
 
+        push @{$org_node->{holding_summaries}}, $summary;
+
         if ($org_node_needs_placed) {
             _place_org_node($org_node, $result, $org_tree);
         }
@@ -798,7 +836,8 @@ __PACKAGE__->register_method(
             { name => "org_unit", type => "number" },
             { name => "depth", type => "number" },
             { name => "limit", type => "number" },
-            { name => "offset", type => "number" }
+            { name => "offset", type => "number" },
+            { name => "ascending", type => "boolean" },
         ]
     }
 );