LP1629108 metarecord_constituent_result_reroute
authorblake <blake@mobiusconsortium.org>
Mon, 13 Feb 2017 21:19:45 +0000 (15:19 -0600)
committerKathy Lussier <klussier@masslnc.org>
Wed, 15 Feb 2017 04:31:03 +0000 (23:31 -0500)
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 <blake@mobiusconsortium.org>
Signed-off-by: Kathy Lussier <klussier@masslnc.org>
Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm
Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Search.pm
Open-ILS/src/sql/Pg/990.schema.unapi.sql
Open-ILS/src/sql/Pg/t/regress/lp1629108_metarecord_constituent_result_reroute.pg [new file with mode: 0755]
Open-ILS/src/sql/Pg/upgrade/XXXX.metarecord_constituents_search_result_page_should_use_standard_search_code.sql [new file with mode: 0755]
Open-ILS/src/templates/opac/parts/misc_util.tt2
Open-ILS/src/templates/opac/parts/result/table.tt2

index 4c386e5..0f7c1f0 100644 (file)
@@ -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}) . ')';
+                }
             }
         }
     }
index df5d03b..92ba04c 100644 (file)
@@ -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/);
index c5a5307..2d7a8d8 100644 (file)
@@ -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;
         }
     }
 
index c7014ce..154c284 100644 (file)
@@ -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 (executable)
index 0000000..2a86875
--- /dev/null
@@ -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 (executable)
index 0000000..3bdb151
--- /dev/null
@@ -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;
+
index 50611fc..a01ac50 100644 (file)
                 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;
index f4ff94a..8f59022 100644 (file)
                     </thead>
                     <tbody id="result_table">
                     [%  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;