(sitka) [RT16512] Fixed Sitka level search scope bug
authorLiam Whalen <liam.whalen@bc.libraries.coop>
Wed, 18 Dec 2013 06:15:01 +0000 (22:15 -0800)
committerLiam Whalen <liam.whalen@bc.libraries.coop>
Wed, 18 Dec 2013 06:15:01 +0000 (22:15 -0800)
When a search is scoped to the Sitka level currently, it will
return non-visible items in OPAC searches as well as staff client
searches.  This is due to a faulty IF statement.

This code fixes that problem and cleans up the comments and
variable names to bring them inline with the contribution to
the community.

Open-ILS/src/sql/Pg/300.schema.staged_search.sql

index a3e73ac..0b8a9a5 100644 (file)
@@ -50,29 +50,29 @@ CREATE OR REPLACE FUNCTION search.query_parser_fts (
 ) RETURNS SETOF search.search_result AS $func$
 DECLARE
 
-    current_res         search.search_result%ROWTYPE;
-    search_org_list     INT[];
-    luri_org_list       INT[];
-    tmp_int_list        INT[];
-    search_ou_parents   INT[];
-
-    check_limit         INT;
-    core_limit          INT;
-    core_offset         INT;
-    tmp_int             INT;
-
-    core_result         RECORD;
-    core_cursor         REFCURSOR;
-    core_rel_query      TEXT;
-
-    total_count         INT := 0;
-    check_count         INT := 0;
-    deleted_count       INT := 0;
-    visible_count       INT := 0;
-    excluded_count      INT := 0;
-
-    is_top_level_ou     BOOL := FALSE;
-    is_federation_ou    BOOL := FALSE;
+    current_res           search.search_result%ROWTYPE;
+    search_org_list       INT[];
+    luri_org_list         INT[];
+    tmp_int_list          INT[];
+    search_ou_parents     INT[];
+  
+    check_limit           INT;
+    core_limit            INT;
+    core_offset           INT;
+    tmp_int               INT;
+  
+    core_result           RECORD;
+    core_cursor           REFCURSOR;
+    core_rel_query        TEXT;
+  
+    total_count           INT := 0;
+    check_count           INT := 0;
+    deleted_count         INT := 0;
+    visible_count         INT := 0;
+    excluded_count        INT := 0;
+  
+    is_top_level_ou       BOOL := FALSE;
+    is_second_level_ou    BOOL := FALSE;
 BEGIN
 
     check_limit := COALESCE( param_check, 1000 );
@@ -87,7 +87,7 @@ BEGIN
     IF array_length(search_ou_parents, 1) = 1 THEN
         is_top_level_ou := TRUE;
     ELSIF array_length(search_ou_parents, 1) = 2 THEN
-        is_federation_ou := TRUE;
+        is_second_level_ou := TRUE;
     END IF;
 
     IF param_search_ou > 0 THEN
@@ -169,7 +169,12 @@ BEGIN
 
                 CONTINUE;
             END IF;
-            
+
+            -- This checks for availability.  If an item is not available and the limit
+            -- to available option is chosen, then it is excluded from the list.
+            -- by moving this section here (before the ebook check) it removes all ebooks
+            -- because the do not have a circ_lib.  This allows patrons and staff to filter ebooks
+            -- from results by limiting to avaialable.
             IF param_statuses IS NOT NULL AND array_upper(param_statuses, 1) > 0 THEN
 
                 PERFORM 1
@@ -193,15 +198,35 @@ BEGIN
                       LIMIT 1;
 
                     IF NOT FOUND THEN
-                    -- RAISE NOTICE ' % and multi-home linked records were all status-excluded ... ', core_result.records;
+                        -- RAISE NOTICE ' % and multi-home linked records were all status-excluded ... ', core_result.records;
                         excluded_count := excluded_count + 1;
                         CONTINUE;
                     END IF;
                 END IF;
-
             END IF;
 
-            IF is_federation_ou THEN
+            IF is_top_level_ou AND staff IS NOT NULL AND staff THEN
+                -- Keep all records if this is a search scoped to the top level ou.
+                -- In the case of searching the top level OU, we want to return all ebooks
+                -- so we add them all to the result set buy not adding cn.owning_lib
+                -- in the WHERE clause.
+                PERFORM 1
+                  FROM  asset.call_number cn
+                        JOIN asset.uri_call_number_map map ON (map.call_number = cn.id)
+                        JOIN asset.uri uri ON (map.uri = uri.id)
+                  WHERE NOT cn.deleted
+                        AND cn.label = '##URI##'
+                        AND uri.active
+                        AND ( param_locations IS NULL OR array_upper(param_locations, 1) IS NULL )
+                        AND cn.record IN ( SELECT * FROM unnest( core_result.records ) )
+                  LIMIT 1;
+            ELSIF is_second_level_ou AND staff IS NOT NULL AND staff THEN
+                -- If the search is scoped to a second level OU, then search from that OU down to find
+                -- ebooks.  In Sitka, this allows ebooks to show up in results of searches performed
+                -- at the Federation level (is_secod_levl_ou).  In the case of Sitka, staff may want to find 
+                -- all ebooks available at the Federation's libraries.  This may not be the case in other EG libraries.  
+                -- Either remove this ELSIF and run only the code in the IF and ELSE or hard code is_second_level_ou
+                -- to a false value.
                 PERFORM 1
                   FROM  asset.call_number cn
                         JOIN asset.uri_call_number_map map ON (map.call_number = cn.id)
@@ -214,6 +239,9 @@ BEGIN
                         AND cn.owning_lib IN ( SELECT * FROM unnest( search_org_list ) )
                   LIMIT 1;
             ELSE
+                -- In the case of all other OUs and non-staff searches, return all items with
+                -- a cn.owning_lib in luri_org_list, which is all the ancestors of the current
+                -- search OU.
                 PERFORM 1
                   FROM  asset.call_number cn
                         JOIN asset.uri_call_number_map map ON (map.call_number = cn.id)
@@ -227,8 +255,7 @@ BEGIN
                   LIMIT 1;
             END IF;
 
-            -- keep the records if this is a search scoped to Sitka or a Federation
-            IF FOUND OR is_top_level_ou THEN
+            IF FOUND THEN
                 -- RAISE NOTICE ' % have at least one URI ... ', core_result.records;
                 visible_count := visible_count + 1;
 
@@ -249,7 +276,6 @@ BEGIN
                 RETURN NEXT current_res;
 
                 CONTINUE;
-
             ELSIF staff IS NOT NULL AND staff THEN
                 PERFORM 1
                   FROM  asset.call_number cn
@@ -260,13 +286,17 @@ BEGIN
                   AND uri.active
                   AND ( param_locations IS NULL OR array_upper(param_locations, 1) IS NULL )
                   AND cn.record IN ( SELECT * FROM unnest( core_result.records ) )
-                  -- the staff searches are pulling in all URI items regardless of scope
-                  -- so MB OverDrive books are being returned with BC OverDrive books.
+                  -- The staff searches are pulling in all URI items regardless of scope,
+                  -- so e-books books are being returned in searches scoped to OUs that do
+                  -- do not have access to the ebooks.
                   -- By removing the check for cn.owning_lib here we get rid of all the
                   -- URI resources that were returned by the staff search, but are not
                   -- in that staffs' library's scope.  Those items that are in scope
-                  -- are caught above IF FOUND THEN section with the line
-                  -- cn_owning_lib IN (SELECT * FROM unnest( luri_org_list ) )
+                  -- are caught in the previous IF FOUND THEN sections with the lines
+                  -- cn.owning_lib IN ( SELECT * FROM unnest( search_org_list ) ),
+                  -- cn_owning_lib IN ( SELECT * FROM unnest( luri_org_list ) ),
+                  -- and in the case of is_top_level_ou all results are returned
+                  -- so there is nothing left to catch in this part.
                   LIMIT 1;
               
                 IF FOUND THEN