From fc831adcb200d4b6a20c0b2b71ea7fd3c6ce1dca Mon Sep 17 00:00:00 2001
From: Dan Wells <dbw2@calvin.edu>
Date: Fri, 13 Sep 2013 12:59:59 -0400
Subject: [PATCH] Tweaks to 'my lists' paging code

1) Don't set and refetch the 'my lists' settings if nothing changed.
(minor nit)

2) Get the count through the same code where we get the bookbag data.

For #2, I am concerned with the current duplication of logic. At best
we are doing things twice, and at worst we end up counting a set which
is slightly different than our actual bookbag search results.

In the updated code, I think the count accuracy is subject to
search/superpage limit settings, so lists are effectively limited by
the search settings. If that is a valid concern, we can beef up the
settings for this query.

Signed-off-by: Dan Wells <dbw2@calvin.edu>
Signed-off-by: Mike Rylander <mrylander@gmail.com>
---
 .../lib/OpenILS/WWW/EGCatLoader/Account.pm    | 92 ++++++-------------
 1 file changed, 27 insertions(+), 65 deletions(-)

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 c9d53ae1fb..61b1028a84 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
@@ -488,15 +488,19 @@ sub load_myopac_prefs_my_lists {
         $settings{$key}= $val unless $$set_map{$key} eq $val;
     }
 
-    # Send the modified settings off to be saved
-    $U->simplereq(
-        'open-ils.actor',
-        'open-ils.actor.patron.settings.update',
-        $self->editor->authtoken, undef, \%settings);
+    if (keys %settings) { # we found a different setting value
+        # Send the modified settings off to be saved
+        $U->simplereq(
+            'open-ils.actor',
+            'open-ils.actor.patron.settings.update',
+            $self->editor->authtoken, undef, \%settings);
 
-    # re-fetch user prefs
-    $self->ctx->{updated_user_settings} = \%settings;
-    return $self->_load_user_with_prefs || Apache2::Const::OK;
+        # re-fetch user prefs
+        $self->ctx->{updated_user_settings} = \%settings;
+        $stat = $self->_load_user_with_prefs;
+    }
+
+    return $stat || Apache2::Const::OK;
 }
 
 sub fetch_user_holds {
@@ -1830,61 +1834,23 @@ sub load_myopac_bookbags {
             grep { $_->id eq $self->cgi->param("bbid") } @{$ctx->{bookbags}};
 
         if ($bookbag) {
+            my $query = $self->_prepare_bookbag_container_query(
+                $bookbag->id, $sorter, $modifier
+            );
+
             # Calculate total count of the items in selected bookbag.
             # This total includes record entries that have no assets available.
-            my $iq = {
-                'select' => { 'acn' => [ { 'column' => 'record', 'distinct' => 'true', 'transform' => 'count', 'aggregate' => 'true', 'alias' => 'count' } ] },
-                'from' => {'cbrebi' =>
-                    { 'bre' =>
-                        { 'join' =>
-                            { 'acn' =>
-                                { 'join' =>
-                                    { 'acp' =>
-                                        { 'join' =>
-                                            { 'ccs' => {}
-                                            }
-                                        }
-                                    }
-                                }
-                            }
-                        }
-                    }
-                },
-                'where' => {
-                        '+cbrebi' => { 'bucket' => $bookbag->id },
-                        '+acn' => { 'deleted' => 'f' },
-                        '+ccs' => { 'opac_visible' => 't' },
-                        '+acp' => {
-                                'deleted' => 'f',
-                                'opac_visible' => 't'
-                            }
-                    }
-            };
-            my $ir = $e->json_query($iq);
-            $ctx->{bb_item_count} = $ir->[0]->{'count'};
-            #now add ebooks
-            my $ebook_q = {
-                'select' => { 'cbrebi' => [ { 'column' => 'target_biblio_record_entry', 'distinct' => 'true', 'transform' => 'count', 'aggregate' => 'true', 'alias' => 'count' } ] },
-                'from' => {'cbrebi' =>
-                    { 'bre' =>
-                        { 'join' =>
-                            {
-                                'cbs' => {}
-                            }
-                        }
-                    }
-                },
-                'where' => {
-                        '+cbrebi' => { 'bucket' => $bookbag->id },
-                        '+bre' => {
-                                'deleted' => 'f',
-                                'active' => 't'
-                            },
-                        '+cbs' => { 'transcendant' => 't' }
-                    }
-            };
-            my $ebook_r = $e->json_query($ebook_q);
-            $ctx->{bb_item_count} = $ctx->{bb_item_count} + $ebook_r->[0]->{'count'};
+            my $bb_search_results = $U->simplereq(
+                "open-ils.search", "open-ils.search.biblio.multiclass.query",
+                {"limit" => 1, "offset" => 0}, $query
+            ); # we only need the count, so do the actual search with limit=1
+
+            if ($bb_search_results) {
+                $ctx->{bb_item_count} = $bb_search_results->{count};
+            } else {
+                $logger->warn("search failed in load_myopac_bookbags()");
+                $ctx->{bb_item_count} = 0; # fallback value
+            }
 
             #calculate page count
             $ctx->{bb_page_count} = int ((($ctx->{bb_item_count} - 1) / $item_limit) + 1);
@@ -1914,10 +1880,6 @@ sub load_myopac_bookbags {
             $e->rollback;
 
 
-            my $query = $self->_prepare_bookbag_container_query(
-                $bookbag->id, $sorter, $modifier
-            );
-
             # For list items pagination
             my $args = {
                 "limit" => $item_limit,
-- 
2.43.2