From 3acb984032d5980fb17c697759112b278a11f621 Mon Sep 17 00:00:00 2001 From: blake Date: Mon, 13 Feb 2017 15:19:45 -0600 Subject: [PATCH] LP1629108 metarecord_constituent_result_reroute This patch will route the metasearch logic through the "standard" search logic in order to leverage the heavy use of filters and other features. A column is introduced to unapi.mmr_mr to include the constituent bibs in the return. A tweak was required in the template toolkit code to take advantage of the new payload. This enables TT to decide which icons should be displayed when search results are filtered. Signed-off-by: blake Signed-off-by: Kathy Lussier --- .../Storage/Driver/Pg/QueryParser.pm | 7 + .../Application/Storage/Publisher/metabib.pm | 1 + .../lib/OpenILS/WWW/EGCatLoader/Search.pm | 33 +++-- Open-ILS/src/sql/Pg/990.schema.unapi.sql | 6 +- ...8_metarecord_constituent_result_reroute.pg | 131 ++++++++++++++++++ ...t_page_should_use_standard_search_code.sql | 104 ++++++++++++++ .../src/templates/opac/parts/misc_util.tt2 | 42 ++++-- .../src/templates/opac/parts/result/table.tt2 | 7 +- 8 files changed, 307 insertions(+), 24 deletions(-) create mode 100755 Open-ILS/src/sql/Pg/t/regress/lp1629108_metarecord_constituent_result_reroute.pg create mode 100755 Open-ILS/src/sql/Pg/upgrade/XXXX.metarecord_constituents_search_result_page_should_use_standard_search_code.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 4c386e5e34..0f7c1f0aef 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 @@ -674,6 +674,7 @@ __PACKAGE__->add_search_filter( 'skip_check' ); __PACKAGE__->add_search_filter( 'superpage' ); __PACKAGE__->add_search_filter( 'superpage_size' ); __PACKAGE__->add_search_filter( 'estimation_strategy' ); +__PACKAGE__->add_search_filter( 'from_metarecord' ); __PACKAGE__->add_search_modifier( 'available' ); __PACKAGE__->add_search_modifier( 'staff' ); __PACKAGE__->add_search_modifier( 'deleted' ); @@ -1359,6 +1360,12 @@ sub flatten { . join(',', map { $self->QueryParser->quote_value($_) } @{ $filter->args }) . "), false)"; } + } elsif ($filter->name eq 'from_metarecord') { + if (@{$filter->args} > 0) { + my $key = 'm.metarecord'; + $where .= $joiner if $where ne ''; + $where .= "$key ${NOT}IN (" . join(',', map { $self->QueryParser->quote_value($_) } @{$filter->args}) . ')'; + } } } } diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm index df5d03b41e..92ba04c546 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm @@ -3281,6 +3281,7 @@ sub query_parser_fts_wrapper { $query = "superpage($args{superpage}) $query" if ($args{superpage}); $query = "offset($args{offset}) $query" if ($args{offset}); $query = "#metarecord $query" if ($self->api_name =~ /metabib/); + $query = "from_metarecord($args{from_metarecord}) $query" if ($args{from_metarecord}); $query = "#available $query" if ($args{available}); $query = "#descending $query" if ($args{sort_dir} && $args{sort_dir} =~ /^d/i); $query = "#staff $query" if ($self->api_name =~ /staff/); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Search.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Search.pm index c5a5307879..2d7a8d85bb 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Search.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Search.pm @@ -408,8 +408,11 @@ sub load_rresults { $ctx->{saved_searches} = $list; } } + # Limit and offset will stay here. Everything else should be part of + # the query string, not special args. + my $args = {'limit' => $limit, 'offset' => $offset}; - if ($metarecord) { + if (0) { #($metarecord) { my $bre_ids = $self->recs_from_metarecord( $metarecord, $ctx->{search_ou}, $depth); @@ -421,14 +424,11 @@ sub load_rresults { return Apache2::Const::OK unless $query; - # Limit and offset will stay here. Everything else should be part of - # the query string, not special args. - my $args = {'limit' => $limit, 'offset' => $offset}; - if ($tag_circs) { $args->{tag_circulated_records} = 1; $args->{authtoken} = $self->editor->authtoken; } + $args->{from_metarecord} = $metarecord if $metarecord; # Stuff these into the TT context so that templates can use them in redrawing forms $ctx->{user_query} = $user_query; @@ -520,10 +520,25 @@ sub load_rresults { push(@{$ctx->{records}}, $rec); if ($is_meta) { - # collect filtered, constituent records count for each MR - my $bre_ids = $self->recs_from_metarecord( - $rec_id, $ctx->{search_ou}, $depth); - $rec->{mr_constituent_count} = scalar(@$bre_ids); + my $meta_results; + try { + my $method = 'open-ils.search.biblio.multiclass.query'; + $method .= '.staff' if $ctx->{is_staff}; + my $ses = OpenSRF::AppSession->create('open-ils.search'); + $self->timelog("Firing off the multiclass query"); + $args->{from_metarecord} = $rec_id; + my $req = $ses->request($method, $args, $query, 0); + $meta_results = $req->gather(1); + $self->timelog("Returned from the multiclass query"); + + } catch Error with { + my $err = shift; + $logger->error("multiclass search error: $err"); + $meta_results = {count => 0, ids => []}; + }; + my $meta_rec_ids = [map { $_->[0] } @{$meta_results->{ids}}]; + $rec->{mr_constituent_count} = $meta_results->{count}; + $rec->{mr_constituent_ids} = $meta_rec_ids; } } diff --git a/Open-ILS/src/sql/Pg/990.schema.unapi.sql b/Open-ILS/src/sql/Pg/990.schema.unapi.sql index c7014cef6a..154c284250 100644 --- a/Open-ILS/src/sql/Pg/990.schema.unapi.sql +++ b/Open-ILS/src/sql/Pg/990.schema.unapi.sql @@ -1387,12 +1387,13 @@ CREATE OR REPLACE FUNCTION unapi.mmr_mra ( rad.composite, rad.multi, rad.filter, - rad.sorter + rad.sorter, + cmra.source_list ), cmra.value ) FROM ( - SELECT DISTINCT aid, attr, value + SELECT DISTINCT aid, attr, value, STRING_AGG(x.id::TEXT, ',') AS source_list FROM ( SELECT v.source AS id, c.id AS aid, @@ -1402,6 +1403,7 @@ CREATE OR REPLACE FUNCTION unapi.mmr_mra ( JOIN config.coded_value_map c ON ( c.id = ANY( v.vlist ) ) ) AS x JOIN sourcelist ON (x.id = sourcelist.source) + GROUP BY 1, 2, 3 ) AS cmra JOIN config.record_attr_definition rad ON (cmra.attr = rad.name) UNION ALL diff --git a/Open-ILS/src/sql/Pg/t/regress/lp1629108_metarecord_constituent_result_reroute.pg b/Open-ILS/src/sql/Pg/t/regress/lp1629108_metarecord_constituent_result_reroute.pg new file mode 100755 index 0000000000..2a868753ca --- /dev/null +++ b/Open-ILS/src/sql/Pg/t/regress/lp1629108_metarecord_constituent_result_reroute.pg @@ -0,0 +1,131 @@ +-- Format the output for nice TAP. +\pset format unaligned +\pset tuples_only true +\pset pager + +-- Revert all changes on failure. +\set ON_ERROR_ROLLBACK 1 +\set ON_ERROR_STOP true +\set QUIET 1 + +-- Load the TAP functions. +BEGIN; + +-- Plan the tests. +SELECT plan(2); + +-- Replace the function +CREATE OR REPLACE FUNCTION unapi.mmr_mra ( + obj_id BIGINT, + format TEXT, + ename TEXT, + includes TEXT[], + org TEXT, + depth INT DEFAULT NULL, + slimit HSTORE DEFAULT NULL, + soffset HSTORE DEFAULT NULL, + include_xmlns BOOL DEFAULT TRUE, + pref_lib INT DEFAULT NULL +) RETURNS XML AS $F$ + SELECT XMLELEMENT( + name attributes, + XMLATTRIBUTES( + CASE WHEN $9 THEN 'http://open-ils.org/spec/indexing/v1' ELSE NULL END AS xmlns, + 'tag:open-ils.org:U2@mmr/' || $1 AS metarecord + ), + (SELECT XMLAGG(foo.y) + FROM ( + WITH sourcelist AS ( + WITH aou AS (SELECT COALESCE(id, (evergreen.org_top()).id) AS id + FROM actor.org_unit WHERE shortname = $5 LIMIT 1) + SELECT source + FROM metabib.metarecord_source_map, aou + WHERE metarecord = $1 AND ( + EXISTS ( + SELECT 1 FROM asset.opac_visible_copies + WHERE record = source AND circ_lib IN ( + SELECT id FROM actor.org_unit_descendants(aou.id, $6)) + LIMIT 1 + ) + OR EXISTS (SELECT 1 FROM located_uris(source, aou.id, $10) LIMIT 1) + ) + ) + SELECT cmra.aid, + XMLELEMENT( + name field, + XMLATTRIBUTES( + cmra.attr AS name, + cmra.value AS "coded-value", + cmra.aid AS "cvmid", + rad.composite, + rad.multi, + rad.filter, + rad.sorter, + cmra.source_list + ), + cmra.value + ) + FROM ( + SELECT DISTINCT aid, attr, value, STRING_AGG(x.id::TEXT, ',') AS source_list + FROM ( + SELECT v.source AS id, + c.id AS aid, + c.ctype AS attr, + c.code AS value + FROM metabib.record_attr_vector_list v + JOIN config.coded_value_map c ON ( c.id = ANY( v.vlist ) ) + ) AS x + JOIN sourcelist ON (x.id = sourcelist.source) + GROUP BY 1, 2, 3 + ) AS cmra + JOIN config.record_attr_definition rad ON (cmra.attr = rad.name) + UNION ALL + SELECT umra.aid, + XMLELEMENT( + name field, + XMLATTRIBUTES( + umra.attr AS name, + rad.composite, + rad.multi, + rad.filter, + rad.sorter + ), + umra.value + ) + FROM ( + SELECT DISTINCT aid, attr, value + FROM ( + SELECT v.source AS id, + m.id AS aid, + m.attr AS attr, + m.value AS value + FROM metabib.record_attr_vector_list v + JOIN metabib.uncontrolled_record_attr_value m ON ( m.id = ANY( v.vlist ) ) + ) AS x + JOIN sourcelist ON (x.id = sourcelist.source) + ) AS umra + JOIN config.record_attr_definition rad ON (umra.attr = rad.name) + ORDER BY 1 + + )foo(id,y) + ) + ) +$F$ LANGUAGE SQL STABLE; + +-- Now make sure that the a query against it doesn't break +PREPARE thrower AS select mmr_mra::varchar from unapi.mmr_mra +(15,'','',null::text[],'CONS',0,null::HSTORE,null::HSTORE,true,1); + +SELECT performs_ok( 'thrower',250,'Generic check for unapi.mmr_mra breakage' ); + +-- Make sure that the function returns the new XML property source_list +SELECT is( +( +select mmr_mra::varchar ~ 'source_list="15,16,17"' from unapi.mmr_mra +(15,'','',null::text[],'CONS',0,null::HSTORE,null::HSTORE,true,1) +), true, 'unapi.mmr_mra results have source_list="15,16,17sfaf"' ); + + +-- Finish the tests and clean up. +SELECT * FROM finish(); +ROLLBACK; \ No newline at end of file diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.metarecord_constituents_search_result_page_should_use_standard_search_code.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.metarecord_constituents_search_result_page_should_use_standard_search_code.sql new file mode 100755 index 0000000000..3bdb1514bc --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.metarecord_constituents_search_result_page_should_use_standard_search_code.sql @@ -0,0 +1,104 @@ + +BEGIN; + +SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version); + +CREATE OR REPLACE FUNCTION unapi.mmr_mra ( + obj_id BIGINT, + format TEXT, + ename TEXT, + includes TEXT[], + org TEXT, + depth INT DEFAULT NULL, + slimit HSTORE DEFAULT NULL, + soffset HSTORE DEFAULT NULL, + include_xmlns BOOL DEFAULT TRUE, + pref_lib INT DEFAULT NULL +) RETURNS XML AS $F$ + SELECT XMLELEMENT( + name attributes, + XMLATTRIBUTES( + CASE WHEN $9 THEN 'http://open-ils.org/spec/indexing/v1' ELSE NULL END AS xmlns, + 'tag:open-ils.org:U2@mmr/' || $1 AS metarecord + ), + (SELECT XMLAGG(foo.y) + FROM ( + WITH sourcelist AS ( + WITH aou AS (SELECT COALESCE(id, (evergreen.org_top()).id) AS id + FROM actor.org_unit WHERE shortname = $5 LIMIT 1) + SELECT source + FROM metabib.metarecord_source_map, aou + WHERE metarecord = $1 AND ( + EXISTS ( + SELECT 1 FROM asset.opac_visible_copies + WHERE record = source AND circ_lib IN ( + SELECT id FROM actor.org_unit_descendants(aou.id, $6)) + LIMIT 1 + ) + OR EXISTS (SELECT 1 FROM located_uris(source, aou.id, $10) LIMIT 1) + ) + ) + SELECT cmra.aid, + XMLELEMENT( + name field, + XMLATTRIBUTES( + cmra.attr AS name, + cmra.value AS "coded-value", + cmra.aid AS "cvmid", + rad.composite, + rad.multi, + rad.filter, + rad.sorter, + cmra.source_list + ), + cmra.value + ) + FROM ( + SELECT DISTINCT aid, attr, value, STRING_AGG(x.id::TEXT, ',') AS source_list + FROM ( + SELECT v.source AS id, + c.id AS aid, + c.ctype AS attr, + c.code AS value + FROM metabib.record_attr_vector_list v + JOIN config.coded_value_map c ON ( c.id = ANY( v.vlist ) ) + ) AS x + JOIN sourcelist ON (x.id = sourcelist.source) + GROUP BY 1, 2, 3 + ) AS cmra + JOIN config.record_attr_definition rad ON (cmra.attr = rad.name) + UNION ALL + SELECT umra.aid, + XMLELEMENT( + name field, + XMLATTRIBUTES( + umra.attr AS name, + rad.composite, + rad.multi, + rad.filter, + rad.sorter + ), + umra.value + ) + FROM ( + SELECT DISTINCT aid, attr, value + FROM ( + SELECT v.source AS id, + m.id AS aid, + m.attr AS attr, + m.value AS value + FROM metabib.record_attr_vector_list v + JOIN metabib.uncontrolled_record_attr_value m ON ( m.id = ANY( v.vlist ) ) + ) AS x + JOIN sourcelist ON (x.id = sourcelist.source) + ) AS umra + JOIN config.record_attr_definition rad ON (umra.attr = rad.name) + ORDER BY 1 + + )foo(id,y) + ) + ) +$F$ LANGUAGE SQL STABLE; + +COMMIT; + diff --git a/Open-ILS/src/templates/opac/parts/misc_util.tt2 b/Open-ILS/src/templates/opac/parts/misc_util.tt2 index 50611fc3e6..a01ac503b9 100644 --- a/Open-ILS/src/templates/opac/parts/misc_util.tt2 +++ b/Open-ILS/src/templates/opac/parts/misc_util.tt2 @@ -602,18 +602,36 @@ NEXT IF ccvm.opac_visible == 'f'; format = {}; - type = ccvm.code.remove('-'); # blu-ray to bluray - format.label = ccvm.search_label || ccvm.value; - format.icon = PROCESS get_ccvm_icon ccvm=ccvm; - format.itemtype = schema_typemap.$type || 'CreativeWork'; - - args.all_formats.push(format); # metarecords want all formats - - IF !args.format_label; - # use the first format as the default - args.format_label = format.label; - args.schema.itemtype = format.itemtype; - args.format_icon = format.icon; + this_icon_source = node.getAttribute('source_list'); + including = 'F'; + # Just display everything if we don't have the data + IF NOT args.mr_constituent_ids OR NOT this_icon_source; + including = 'T'; + # We have an array of search-included bib IDs and we have the bib ID that this icon belongs to + ELSE; + FOR mr_constituent_id IN args.mr_constituent_ids; + IF this_icon_source.split(',').grep('^' _ mr_constituent_id _ '$' ).size; + # This bib appears to be in the array of filtered bibs + including = 'T'; + END; + END; + END; + IF including == 'T'; + type = ccvm.code.remove('-'); # blu-ray to bluray + format.label = ccvm.search_label || ccvm.value; + format.icon = PROCESS get_ccvm_icon ccvm=ccvm; + format.itemtype = schema_typemap.$type || 'CreativeWork'; + format.search_format = ccvm.code; + format.source_bibs = this_icon_source.split(','); + + args.all_formats.push(format); # metarecords want all formats + + IF !args.format_label; + # use the first format as the default + args.format_label = format.label; + args.schema.itemtype = format.itemtype; + args.format_icon = format.icon; + END; END; END; END; diff --git a/Open-ILS/src/templates/opac/parts/result/table.tt2 b/Open-ILS/src/templates/opac/parts/result/table.tt2 index f4ff94a52f..8f5902270d 100644 --- a/Open-ILS/src/templates/opac/parts/result/table.tt2 +++ b/Open-ILS/src/templates/opac/parts/result/table.tt2 @@ -66,7 +66,12 @@ [% FOR rec IN ctx.records; - attrs = {marc_xml => rec.marc_xml}; + attrs = {}; + attrs.marc_xml = rec.marc_xml; + attrs.mr_constituent_ids = []; + FOREACH mr_constituent_id IN rec.mr_constituent_ids; + attrs.mr_constituent_ids.push(mr_constituent_id); + END; PROCESS get_marc_attrs args=attrs; IF show_detail_view; attrs.title = attrs.title_extended; -- 2.43.2