From d32b247085081f483955f48fb8382977c0a25370 Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Tue, 16 Jan 2018 18:54:47 -0500 Subject: [PATCH] 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 Signed-off-by: Kathy Lussier --- .../Storage/Driver/Pg/QueryParser.pm | 33 ++++++++++--- .../src/sql/Pg/300.schema.staged_search.sql | 20 +++++--- ...XXXX.function.special-bib-vis-handling.sql | 49 +++++++++++++++++++ 3 files changed, 90 insertions(+), 12 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.function.special-bib-vis-handling.sql 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 1a3d40a79f..6a1d872e7c 100644 --- 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 5157aaf3f4..51be63d2ed 100644 --- 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 index 0000000000..11c5a2d098 --- /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; + -- 2.43.2