LP#937789: various improvements to logical deletion of parts

This patch ensures that when a monograph part is marked as
deleted, that mappings between it and its copies are also
removed, similar to the previous behavior of the cascade
on delete.

This patch also adds some pgTAP tests and ensures that
unapi.bmp() and unapi.holdings_xml() get updated during upgrade.

To test the fix for this bug:

[1] Apply the patches.
[2] Create a monograph part on a record, then delete it. Verify that
    it no longer shows up in the parts maintenance interface, but that
    a row in the biblio.monograph_parts table still exists with the
    deleted flag set to true.
[3] Create another monograph part on the same record.
[4] Create a new volume/copy and assign the part created in step
    3 to it. Verify that the part shown in step 2 is *not*
    included in the XUL part selector.
[5] Place a part-level hold. Verify that the hold request
    is displayed correct in both the public catalog and the staff
    interface.
[6] Delete the part created in step 3.
[7] Verify that viewing the list of the patron's holds in the XUL staff
    client doesn't result in any 'network errors'.  Also verify that
    the hold request is still displayed in patron's view in the public
    catalog.
[8] Using SQL or some other means, set the expire_time and
    prev_check_time of the test hold request to a couple days in the
    past, then run the hold targeter.  Verify that the hold gets
    cancelled for lack of a suitable copy to fill it.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Remington Steed <rjs7@calvin.edu>
Signed-off-by: Ben Shum <bshum@biblio.org>

diff --git a/Open-ILS/src/sql/Pg/800.fkeys.sql b/Open-ILS/src/sql/Pg/800.fkeys.sql
index a760483..ed2e79f 100644 (file)
--- a/Open-ILS/src/sql/Pg/800.fkeys.sql
+++ b/Open-ILS/src/sql/Pg/800.fkeys.sql
@@ -39,7 +39,9 @@ CREATE RULE protect_mono_part_delete AS
     ON DELETE TO biblio.monograph_part DO INSTEAD (
         UPDATE biblio.monograph_part
             SET deleted = TRUE
-            WHERE OLD.id = biblio.monograph_part.id
+            WHERE OLD.id = biblio.monograph_part.id;
+        DELETE FROM asset.copy_part_map
+        WHERE part = OLD.id
     );
 
 ALTER TABLE actor.usr ADD CONSTRAINT actor_usr_mailing_address_fkey FOREIGN KEY (mailing_address) REFERENCES actor.usr_address (id) DEFERRABLE INITIALLY DEFERRED;

diff --git a/Open-ILS/src/sql/Pg/t/regress/lp937789_fake_bmp_delete.sql b/Open-ILS/src/sql/Pg/t/regress/lp937789_fake_bmp_delete.sql
new file mode 100644 (file)
index 0000000..882ab01
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/t/regress/lp937789_fake_bmp_delete.sql
@@ -0,0 +1,73 @@
+BEGIN;
+
+SELECT plan(6);
+
+INSERT INTO biblio.record_entry (id, last_xact_id, marc)
+VALUES (999999998, 'pgtap', '<record    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"    xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"    xmlns="http://www.loc.gov/MARC21/slim">
+  <leader>00531nam a2200157 a 4500</leader>
+  <controlfield tag="005">20080729170300.0</controlfield>
+  <controlfield tag="008">      t19981999enka              0 eng  </controlfield>
+  <datafield tag="245" ind1="1" ind2="4">
+    <subfield code="a">test-value</subfield>
+  </datafield>
+</record>');
+
+INSERT INTO biblio.monograph_part(record, label) VALUES (999999998, 'Part 1');
+
+SELECT is(
+    label,
+    'Part 1',
+    'LP#937789: new monograph parts start out active'
+)
+FROM biblio.monograph_part
+WHERE record = 999999998
+AND NOT deleted;
+
+SELECT is(
+    (XPATH(
+        '//ns:monograph_parts/ns:monograph_part/@label',
+        unapi.holdings_xml(999999998, 1, 'CONS', 0, '{bmp}'),
+        '{{ns,http://open-ils.org/spec/holdings/v1}}'
+    ))[1]::TEXT,
+    'Part 1',
+    'LP#937789: unapi.holdings_xml returns monograph part'
+);
+
+SELECT is(
+    (XPATH(
+        '/ns:monograph_part/@label',
+        unapi.bmp(CURRVAL('biblio.monograph_part_id_seq'), '', '', '{}', 'CONS'),
+        '{{ns,http://open-ils.org/spec/holdings/v1}}'
+    ))[1]::TEXT,
+    'Part 1',
+    'LP#937789: unapi.bmp returns monograph part'
+);
+
+DELETE FROM biblio.monograph_part WHERE record = 999999998;
+
+SELECT is(
+    deleted,
+    TRUE,
+    'LP#937789: deleting monograph part sets deleted flag'
+)
+FROM biblio.monograph_part
+WHERE record = 999999998
+AND label = 'Part 1';
+
+SELECT is(
+    (XPATH(
+        '//ns:monograph_parts/ns:monograph_part/@label',
+        unapi.holdings_xml(999999998, 1, 'CONS', 0, '{bmp}'),
+        '{{ns,http://open-ils.org/spec/holdings/v1}}'
+    ))[1]::TEXT,
+    NULL,
+    'LP#937789: unapi.holdings_xml does not return deleted monograph part'
+);
+
+SELECT is(
+    unapi.bmp(CURRVAL('biblio.monograph_part_id_seq'), '', '', '{}', 'CONS')::TEXT,
+    NULL,
+    'LP#937789: unapi.bmp does not return deleted monograph part'
+);
+
+ROLLBACK;

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.fake-delete-parts.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.fake-delete-parts.sql
index ada71d2..554e876 100644 (file)
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.fake-delete-parts.sql
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.fake-delete-parts.sql
@@ -1,7 +1,137 @@
 BEGIN;
 
 ALTER TABLE biblio.monograph_part ADD COLUMN deleted BOOL NOT NULL DEFAULT FALSE;
-CREATE RULE protect_mono_part_delete AS ON DELETE TO biblio.monograph_part DO INSTEAD (UPDATE biblio.monograph_part SET deleted = TRUE WHERE OLD.id = biblio.monograph_part.id);
+CREATE RULE protect_mono_part_delete AS ON DELETE TO biblio.monograph_part DO INSTEAD (
+    UPDATE biblio.monograph_part SET deleted = TRUE WHERE OLD.id = biblio.monograph_part.id;
+    DELETE FROM asset.copy_part_map WHERE part = OLD.id
+);
+
+CREATE OR REPLACE FUNCTION unapi.holdings_xml (
+    bid BIGINT,
+    ouid INT,
+    org TEXT,
+    depth INT DEFAULT NULL,
+    includes TEXT[] DEFAULT NULL::TEXT[],
+    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 holdings,
+                 XMLATTRIBUTES(
+                    CASE WHEN $8 THEN 'http://open-ils.org/spec/holdings/v1' ELSE NULL END AS xmlns,
+                    CASE WHEN ('bre' = ANY ($5)) THEN 'tag:open-ils.org:U2@bre/' || $1 || '/' || $3 ELSE NULL END AS id,
+                    (SELECT record_has_holdable_copy FROM asset.record_has_holdable_copy($1)) AS has_holdable
+                 ),
+                 XMLELEMENT(
+                     name counts,
+                     (SELECT  XMLAGG(XMLELEMENT::XML) FROM (
+                         SELECT  XMLELEMENT(
+                                     name count,
+                                     XMLATTRIBUTES('public' as type, depth, org_unit, coalesce(transcendant,0) as transcendant, available, visible as count, unshadow)
+                                 )::text
+                           FROM  asset.opac_ou_record_copy_count($2,  $1)
+                                     UNION
+                         SELECT  XMLELEMENT(
+                                     name count,
+                                     XMLATTRIBUTES('staff' as type, depth, org_unit, coalesce(transcendant,0) as transcendant, available, visible as count, unshadow)
+                                 )::text
+                           FROM  asset.staff_ou_record_copy_count($2, $1)
+                                     UNION
+                         SELECT  XMLELEMENT(
+                                     name count,
+                                     XMLATTRIBUTES('pref_lib' as type, depth, org_unit, coalesce(transcendant,0) as transcendant, available, visible as count, unshadow)
+                                 )::text
+                           FROM  asset.opac_ou_record_copy_count($9,  $1)
+                                     ORDER BY 1
+                     )x)
+                 ),
+                 CASE 
+                     WHEN ('bmp' = ANY ($5)) THEN
+                        XMLELEMENT(
+                            name monograph_parts,
+                            (SELECT XMLAGG(bmp) FROM (
+                                SELECT  unapi.bmp( id, 'xml', 'monograph_part', evergreen.array_remove_item_by_value( evergreen.array_remove_item_by_value($5,'bre'), 'holdings_xml'), $3, $4, $6, $7, FALSE)
+                                  FROM  biblio.monograph_part
+                                  WHERE NOT deleted AND record = $1
+                            )x)
+                        )
+                     ELSE NULL
+                 END,
+                 XMLELEMENT(
+                     name volumes,
+                     (SELECT XMLAGG(acn ORDER BY rank, name, label_sortkey) FROM (
+                        -- Physical copies
+                        SELECT  unapi.acn(y.id,'xml','volume',evergreen.array_remove_item_by_value( evergreen.array_remove_item_by_value($5,'holdings_xml'),'bre'), $3, $4, $6, $7, FALSE), y.rank, name, label_sortkey
+                        FROM evergreen.ranked_volumes($1, $2, $4, $6, $7, $9, $5) AS y
+                        UNION ALL
+                        -- Located URIs
+                        SELECT unapi.acn(uris.id,'xml','volume',evergreen.array_remove_item_by_value( evergreen.array_remove_item_by_value($5,'holdings_xml'),'bre'), $3, $4, $6, $7, FALSE), uris.rank, name, label_sortkey
+                        FROM evergreen.located_uris($1, $2, $9) AS uris
+                     )x)
+                 ),
+                 CASE WHEN ('ssub' = ANY ($5)) THEN 
+                     XMLELEMENT(
+                         name subscriptions,
+                         (SELECT XMLAGG(ssub) FROM (
+                            SELECT  unapi.ssub(id,'xml','subscription','{}'::TEXT[], $3, $4, $6, $7, FALSE)
+                              FROM  serial.subscription
+                              WHERE record_entry = $1
+                        )x)
+                     )
+                 ELSE NULL END,
+                 CASE WHEN ('acp' = ANY ($5)) THEN 
+                     XMLELEMENT(
+                         name foreign_copies,
+                         (SELECT XMLAGG(acp) FROM (
+                            SELECT  unapi.acp(p.target_copy,'xml','copy',evergreen.array_remove_item_by_value($5,'acp'), $3, $4, $6, $7, FALSE)
+                              FROM  biblio.peer_bib_copy_map p
+                                    JOIN asset.copy c ON (p.target_copy = c.id)
+                              WHERE NOT c.deleted AND p.peer_record = $1
+                            LIMIT ($6 -> 'acp')::INT
+                            OFFSET ($7 -> 'acp')::INT
+                        )x)
+                     )
+                 ELSE NULL END
+             );
+$F$ LANGUAGE SQL STABLE;
+
+CREATE OR REPLACE FUNCTION unapi.bmp ( 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 ) RETURNS XML AS $F$
+        SELECT  XMLELEMENT(
+                    name monograph_part,
+                    XMLATTRIBUTES(
+                        CASE WHEN $9 THEN 'http://open-ils.org/spec/holdings/v1' ELSE NULL END AS xmlns,
+                        'tag:open-ils.org:U2@bmp/' || id AS id,
+                        id AS ident,
+                        label,
+                        label_sortkey,
+                        'tag:open-ils.org:U2@bre/' || record AS record
+                    ),
+                    CASE 
+                        WHEN ('acp' = ANY ($4)) THEN
+                            XMLELEMENT( name copies,
+                                (SELECT XMLAGG(acp) FROM (
+                                    SELECT  unapi.acp( cp.id, 'xml', 'copy', evergreen.array_remove_item_by_value($4,'bmp'), $5, $6, $7, $8, FALSE)
+                                      FROM  asset.copy cp
+                                            JOIN asset.copy_part_map cpm ON (cpm.target_copy = cp.id)
+                                      WHERE cpm.part = $1
+                                          AND cp.deleted IS FALSE
+                                      ORDER BY COALESCE(cp.copy_number,0), cp.barcode
+                                      LIMIT ($7 -> 'acp')::INT
+                                      OFFSET ($8 -> 'acp')::INT
+
+                                )x)
+                            )
+                        ELSE NULL
+                    END,
+                    CASE WHEN ('bre' = ANY ($4)) THEN unapi.bre( record, 'marcxml', 'record', evergreen.array_remove_item_by_value($4,'bmp'), $5, $6, $7, $8, FALSE) ELSE NULL END
+                )
+          FROM  biblio.monograph_part
+          WHERE NOT deleted AND id = $1
+          GROUP BY id, label, label_sortkey, record;
+$F$ LANGUAGE SQL STABLE;
 
 COMMIT;