LP#1743650: Bib vis testing needs different handling

For bib-level visibility testing, we can only use the source helper for
positive inclusion, and have to deal with LURIs on a case-by-case basis.
This is because, unlike the copy visibility cache, the LURI cache is pre-
composed in a single value for all LURIs on a record, not separate ones
for each.  This is fine, as we just need to find the effectively visible
org units and test for that subset of the relevant orgs. This commit provides
a sub to test for that, and handles LURI test composition at the perl
level rather than in the database.

Signed-off-by: Mike Rylander <mrylander@gmail.com>
Signed-off-by: Kathy Lussier <klussier@masslnc.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
index 1a3d40a..6a1d872 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
@@ -1041,20 +1041,21 @@ sub toSQL {
         $b_vis_test = join("||'&'||",@{$$flat_plan{vis_filter}{b_attr}});
     }
 
+    # bib vis tests are handled a little bit differently, as they're simpler but need special handling
     if ($b_vis_test or $pb_vis_test) {
         my $vis_test = '';
 
-        if ($b_vis_test and $pb_vis_test) {
-            $vis_test = $pb_vis_test . ",". $b_vis_test;
-        } elsif ($pb_vis_test) {
-            $vis_test = $pb_vis_test;
+        if ($b_vis_test and $pb_vis_test) { # $pb_vis_test supplies a query_int operator at its end
+            $vis_test = $pb_vis_test . '||'. $b_vis_test;
+        } elsif ($pb_vis_test) { # here we want to remove it
+            $vis_test = "RTRIM($pb_vis_test,'|&')";
         } else {
             $vis_test = $b_vis_test;
         }
 
         # WITH-clause just generates vis test
         $$flat_plan{with} .= "\n," if $$flat_plan{with};
-        $$flat_plan{with} .= "b_attr AS (SELECT (ARRAY_TO_STRING(ARRAY[$vis_test],'|'))::query_int AS vis_test FROM asset.patron_default_visibility_mask() x)";
+        $$flat_plan{with} .= "b_attr AS (SELECT ($vis_test)::query_int AS vis_test FROM asset.patron_default_visibility_mask() x)";
 
         # These are magic numbers... see: search.calculate_visibility_attribute() UDF
         $final_b_attr_test = '(b_attr.vis_test IS NULL OR bre.vis_attr_vector @@ b_attr.vis_test)';
@@ -1129,6 +1130,20 @@ SQL
 
 }
 
+sub is_org_visible {
+    my $org = shift;
+    return 0 if (!$U->is_true($org->opac_visible));
+
+    my $non_inherited_vis_gf = shift || $U->get_global_flag('opac.org_unit.non_inherited_visibility');
+    return 1 if ($U->is_true($non_inherited_vis_gf->enabled));
+
+    my $ot = $U->get_org_tree;
+    while ($org = $U->find_org($ot,$org->parent_ou)) {
+        return 0 if (!$U->is_true($org->opac_visible));
+    }
+    return 1;
+}
+
 sub flatten {
     my $self = shift;
 
@@ -1337,6 +1352,12 @@ sub flatten {
     my $dorgs = $U->get_org_descendants($site_org->id, $depth_filter);
     my $aorgs = $U->get_org_ancestors($site_org->id);
 
+    if (!$self->find_modifier('staff')) {
+        my $non_inherited_vis_gf = $U->get_global_flag('opac.org_unit.non_inherited_visibility');
+        $dorgs = [ grep { is_org_visible($U->find_org($ot,$_), $non_inherited_vis_gf) } @$dorgs ];
+        $aorgs = [ grep { is_org_visible($U->find_org($ot,$_), $non_inherited_vis_gf) } @$aorgs ];
+    }
+
     push @{$vis_filter{'c_attr'}},
         "search.calculate_visibility_attribute_test('circ_lib','{".join(',', @$dorgs)."}',$negate)";
 
@@ -1578,7 +1599,7 @@ sub flatten {
                 if (@{$filter->args} > 0) {
                     $uses_bre = 1;
                     my $negate = $filter->negate ? 'TRUE' : 'FALSE';
-                    push @{$vis_filter{'c_attr'}},
+                    push @{$vis_filter{'b_attr'}},
                         "search.calculate_visibility_attribute_test('source','{".join(',', @{$filter->args})."}',$negate)";
                 }
             } elsif ($filter->name eq 'from_metarecord') {

diff --git a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql
index 5157aaf..51be63d 100644 (file)
--- a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql
+++ b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql
@@ -795,6 +795,8 @@ DECLARE
 
     luri_org        TEXT; -- "b" attr
     bib_sources     TEXT; -- "b" attr
+
+    bib_tests       TEXT := '';
 BEGIN
     copy_flags      := asset.all_visible_flags(); -- Will always have at least one
 
@@ -805,14 +807,20 @@ BEGIN
     location        := NULLIF(asset.location_default(),'!()');
     location_group  := NULLIF(asset.location_group_default(),'!()');
 
-    luri_org        := NULLIF(asset.luri_org_default(),'!()');
+    -- LURIs will be handled at the perl layer directly
+    -- luri_org        := NULLIF(asset.luri_org_default(),'!()');
     bib_sources     := NULLIF(asset.bib_source_default(),'()');
 
-    RETURN QUERY SELECT
-        '('||ARRAY_TO_STRING(
-            ARRAY[luri_org,bib_sources],
-            '|'
-        )||')',
+
+    IF luri_org IS NOT NULL AND bib_sources IS NOT NULL THEN
+        bib_tests := '('||ARRAY_TO_STRING( ARRAY[luri_org,bib_sources], '|')||')&('||luri_org||')&';
+    ELSIF luri_org IS NOT NULL THEN
+        bib_tests := luri_org || '&';
+    ELSIF bib_sources IS NOT NULL THEN
+        bib_tests := bib_sources || '|';
+    END IF;
+
+    RETURN QUERY SELECT bib_tests,
         '('||ARRAY_TO_STRING(
             ARRAY[copy_flags,owning_lib,circ_lib,status,location,location_group]::TEXT[],
             '&'

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.function.special-bib-vis-handling.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.special-bib-vis-handling.sql
new file mode 100644 (file)
index 0000000..11c5a2d
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.special-bib-vis-handling.sql
@@ -0,0 +1,49 @@
+BEGIN;
+
+CREATE OR REPLACE FUNCTION asset.patron_default_visibility_mask () RETURNS TABLE (b_attrs TEXT, c_attrs TEXT)  AS $f$
+DECLARE
+    copy_flags      TEXT; -- "c" attr
+
+    owning_lib      TEXT; -- "c" attr
+    circ_lib        TEXT; -- "c" attr
+    status          TEXT; -- "c" attr
+    location        TEXT; -- "c" attr
+    location_group  TEXT; -- "c" attr
+
+    luri_org        TEXT; -- "b" attr
+    bib_sources     TEXT; -- "b" attr
+
+    bib_tests       TEXT := '';
+BEGIN
+    copy_flags      := asset.all_visible_flags(); -- Will always have at least one
+
+    owning_lib      := NULLIF(asset.owning_lib_default(),'!()');
+
+    circ_lib        := NULLIF(asset.circ_lib_default(),'!()');
+    status          := NULLIF(asset.status_default(),'!()');
+    location        := NULLIF(asset.location_default(),'!()');
+    location_group  := NULLIF(asset.location_group_default(),'!()');
+
+    -- LURIs will be handled at the perl layer directly
+    -- luri_org        := NULLIF(asset.luri_org_default(),'!()');
+    bib_sources     := NULLIF(asset.bib_source_default(),'()');
+
+
+    IF luri_org IS NOT NULL AND bib_sources IS NOT NULL THEN
+        bib_tests := '('||ARRAY_TO_STRING( ARRAY[luri_org,bib_sources], '|')||')&('||luri_org||')&';
+    ELSIF luri_org IS NOT NULL THEN
+        bib_tests := luri_org || '&';
+    ELSIF bib_sources IS NOT NULL THEN
+        bib_tests := bib_sources || '|';
+    END IF;
+
+    RETURN QUERY SELECT bib_tests,
+        '('||ARRAY_TO_STRING(
+            ARRAY[copy_flags,owning_lib,circ_lib,status,location,location_group]::TEXT[],
+            '&'
+        )||')';
+END;
+$f$ LANGUAGE PLPGSQL STABLE ROWS 1;
+
+COMMIT;
+