[open-ils-commits] [GIT] Evergreen ILS branch master updated. a52131145f41fe3f19f5c4ae5ccf4cad392c1cf0

Evergreen Git git at git.evergreen-ils.org
Mon Apr 2 10:15:08 EDT 2012


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Evergreen ILS".

The branch, master has been updated
       via  a52131145f41fe3f19f5c4ae5ccf4cad392c1cf0 (commit)
       via  8d2f7b1c7540aba7156c08e493198331558c8ae4 (commit)
       via  05e7270afeace61544059e05facbf51b710e99ce (commit)
       via  0f3291546b37cd3722e3b64e78203c95282fc369 (commit)
       via  5b69128dc054e4b8b79606f76fe614aca211151a (commit)
       via  d944a3780a9106ed881738472e3259b8d77f7d46 (commit)
       via  d43eb99ba936155f01fe95f0bf66ff03aefc8fdb (commit)
       via  5b109f49e3a94c26c8efb7bb55965f20b2f7850b (commit)
      from  7cef76169d6c6f6f4ae3e6ea165d672783b34139 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit a52131145f41fe3f19f5c4ae5ccf4cad392c1cf0
Author: Mike Rylander <mrylander at gmail.com>
Date:   Mon Apr 2 10:21:32 2012 -0400

    Stamping upgrade script for serials holdings display improvements
    
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql
index 4c4379a..58d8db7 100644
--- a/Open-ILS/src/sql/Pg/002.schema.config.sql
+++ b/Open-ILS/src/sql/Pg/002.schema.config.sql
@@ -87,7 +87,7 @@ CREATE TRIGGER no_overlapping_deps
     BEFORE INSERT OR UPDATE ON config.db_patch_dependencies
     FOR EACH ROW EXECUTE PROCEDURE evergreen.array_overlap_check ('deprecates');
 
-INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0699', :eg_version); -- phasefx/miker
+INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0700', :eg_version); -- senator/dbwells/miker
 
 CREATE TABLE config.bib_source (
 	id		SERIAL	PRIMARY KEY,
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql b/Open-ILS/src/sql/Pg/upgrade/0700.schema.serial-holding-groups.sql
similarity index 98%
rename from Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
rename to Open-ILS/src/sql/Pg/upgrade/0700.schema.serial-holding-groups.sql
index 27246a1..077a64f 100644
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
+++ b/Open-ILS/src/sql/Pg/upgrade/0700.schema.serial-holding-groups.sql
@@ -1,6 +1,6 @@
 BEGIN;
 
-SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+SELECT evergreen.upgrade_deps_block_check('0700', :eg_version);
 
 INSERT INTO config.internal_flag (name, value, enabled) VALUES (
     'serial.rematerialize_on_same_holding_code', NULL, FALSE
diff --git a/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql b/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql
index 2236318..aa3f94d 100644
--- a/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql
+++ b/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql
@@ -15099,4 +15099,156 @@ INSERT INTO config.org_unit_setting_type ( name, label, description, datatype, g
     );
 
 
+SELECT evergreen.upgrade_deps_block_check('0700', :eg_version);
+
+INSERT INTO config.internal_flag (name, value, enabled) VALUES (
+    'serial.rematerialize_on_same_holding_code', NULL, FALSE
+);
+
+INSERT INTO config.org_unit_setting_type (
+    name, label, grp, description, datatype
+) VALUES (
+    'serial.default_display_grouping',
+    'Default display grouping for serials distributions presented in the OPAC.',
+    'serial',
+    'Default display grouping for serials distributions presented in the OPAC. This can be "enum" or "chron".',
+    'string'
+);
+
+ALTER TABLE serial.distribution
+    ADD COLUMN display_grouping TEXT NOT NULL DEFAULT 'chron'
+        CHECK (display_grouping IN ('enum', 'chron'));
+
+-- why didn't we just make one summary table in the first place?
+CREATE VIEW serial.any_summary AS
+    SELECT
+        'basic' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.basic_summary
+    UNION
+    SELECT
+        'index' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.index_summary
+    UNION
+    SELECT
+        'supplement' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.supplement_summary ;
+
+
+-- Given the IDs of two rows in actor.org_unit, *the second being an ancestor
+-- of the first*, return in array form the path from the ancestor to the
+-- descendant, with each point in the path being an org_unit ID.  This is
+-- useful for sorting org_units by their position in a depth-first (display
+-- order) representation of the tree.
+--
+-- This breaks with the precedent set by actor.org_unit_full_path() and others,
+-- and gets the parameters "backwards," but otherwise this function would
+-- not be very usable within json_query.
+CREATE OR REPLACE FUNCTION actor.org_unit_simple_path(INT, INT)
+RETURNS INT[] AS $$
+    WITH RECURSIVE descendant_depth(id, path) AS (
+        SELECT  aou.id,
+                ARRAY[aou.id]
+          FROM  actor.org_unit aou
+                JOIN actor.org_unit_type aout ON (aout.id = aou.ou_type)
+          WHERE aou.id = $2
+            UNION ALL
+        SELECT  aou.id,
+                dd.path || ARRAY[aou.id]
+          FROM  actor.org_unit aou
+                JOIN actor.org_unit_type aout ON (aout.id = aou.ou_type)
+                JOIN descendant_depth dd ON (dd.id = aou.parent_ou)
+    ) SELECT dd.path
+        FROM actor.org_unit aou
+        JOIN descendant_depth dd USING (id)
+        WHERE aou.id = $1 ORDER BY dd.path;
+$$ LANGUAGE SQL STABLE;
+
+CREATE TABLE serial.materialized_holding_code (
+    id BIGSERIAL PRIMARY KEY,
+    issuance INTEGER NOT NULL REFERENCES serial.issuance (id) ON DELETE CASCADE,
+    subfield CHAR,
+    value TEXT
+);
+
+CREATE OR REPLACE FUNCTION serial.materialize_holding_code() RETURNS TRIGGER
+AS $func$ 
+use strict;
+
+use MARC::Field;
+use JSON::XS;
+
+# Do nothing if holding_code has not changed...
+
+if ($_TD->{new}{holding_code} eq $_TD->{old}{holding_code}) {
+    # ... unless the following internal flag is set.
+
+    my $flag_rv = spi_exec_query(q{
+        SELECT * FROM config.internal_flag
+        WHERE name = 'serial.rematerialize_on_same_holding_code' AND enabled
+    }, 1);
+    return unless $flag_rv->{processed};
+}
+
+
+my $holding_code = (new JSON::XS)->decode($_TD->{new}{holding_code});
+
+my $field = new MARC::Field('999', @$holding_code); # tag doesnt matter
+
+my $dstmt = spi_prepare(
+    'DELETE FROM serial.materialized_holding_code WHERE issuance = $1',
+    'INT'
+);
+spi_exec_prepared($dstmt, $_TD->{new}{id});
+
+my $istmt = spi_prepare(
+    q{
+        INSERT INTO serial.materialized_holding_code (
+            issuance, subfield, value
+        ) VALUES ($1, $2, $3)
+    }, qw{INT CHAR TEXT}
+);
+
+foreach ($field->subfields) {
+    spi_exec_prepared(
+        $istmt,
+        $_TD->{new}{id},
+        $_->[0],
+        $_->[1]
+    );
+}
+
+return;
+
+$func$ LANGUAGE 'plperlu';
+
+CREATE INDEX assist_holdings_display
+    ON serial.materialized_holding_code (issuance, subfield);
+
+CREATE TRIGGER materialize_holding_code
+    AFTER INSERT OR UPDATE ON serial.issuance
+    FOR EACH ROW EXECUTE PROCEDURE serial.materialize_holding_code() ;
+
+-- starting here, we materialize all existing holding codes.
+
+UPDATE config.internal_flag
+    SET enabled = TRUE
+    WHERE name = 'serial.rematerialize_on_same_holding_code';
+
+UPDATE serial.issuance SET holding_code = holding_code;
+
+UPDATE config.internal_flag
+    SET enabled = FALSE
+    WHERE name = 'serial.rematerialize_on_same_holding_code';
+
+-- finish holding code materialization process
+
+-- fix up missing holding_code fields from serial.issuance
+UPDATE serial.issuance siss
+    SET holding_type = scap.type
+    FROM serial.caption_and_pattern scap
+    WHERE scap.id = siss.caption_and_pattern AND siss.holding_type IS NULL;
+
 COMMIT;

commit 8d2f7b1c7540aba7156c08e493198331558c8ae4
Author: Dan Wells <dbw2 at calvin.edu>
Date:   Fri Mar 30 16:17:47 2012 -0400

    holding_type on serial.issuance not quite dead
    
    The holding_type field was removed from the serial control editor
    prematurely, and this missing data was a source of mild friction.
    We'll put it back for now.
    
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
index 695ce81..27246a1 100644
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
@@ -146,4 +146,10 @@ UPDATE config.internal_flag
 
 -- finish holding code materialization process
 
+-- fix up missing holding_code fields from serial.issuance
+UPDATE serial.issuance siss
+    SET holding_type = scap.type
+    FROM serial.caption_and_pattern scap
+    WHERE scap.id = siss.caption_and_pattern AND siss.holding_type IS NULL;
+
 COMMIT;
diff --git a/Open-ILS/xul/staff_client/server/serial/siss_editor.js b/Open-ILS/xul/staff_client/server/serial/siss_editor.js
index 666c761..bb04131 100644
--- a/Open-ILS/xul/staff_client/server/serial/siss_editor.js
+++ b/Open-ILS/xul/staff_client/server/serial/siss_editor.js
@@ -119,14 +119,15 @@ serial.siss_editor.prototype = {
 
         'siss_editor_middle_pane' :
         [
-/*rjs7 don't think we need these anymore            [
-                'Holding Type',
+            [
+                'holding_type',
                 {
-                    render: 'fm.holding_type();',
-                    input: 'c = function(v){ obj.apply("holding_type",v); if (typeof post_c == "function") post_c(v); }; x = util.widgets.make_menulist( [ ["basic", "basic"], ["index", "index"], ["supplement", "supplement"] ] ); x.addEventListener("apply",function(f){ return function(ev) { f(ev.target.value); } }(c), false);',
+                    input: 'c = function(v){ obj.apply("holding_type",v); if (typeof post_c == "function") post_c(v); }; x = util.widgets.make_menulist( [ ["basic", "basic"], ["index", "index"], ["supplement", "supplement"] ] ); x.setAttribute("value",obj.editor_values.holding_type); x.addEventListener("apply",function(f){ return function(ev) { f(ev.target.value); } }(c), false);',
+                    value_key: 'holding_type',
+                    required: true
                 }
             ],
-            [
+/* deprecated            [
                 'Holding Link ID',
                 {
                     render: 'fm.holding_link_id();',

commit 05e7270afeace61544059e05facbf51b710e99ce
Author: Dan Wells <dbw2 at calvin.edu>
Date:   Fri Mar 30 15:55:19 2012 -0400

    Simplify serial.materialized_holding_code
    
    Attempting to materialize holding_type and ind1/ind2 for less than
    perfect data causes problems.  Since we aren't actually using this
    data, let's get rid of these fields.
    
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
index d395430..695ce81 100644
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
@@ -70,9 +70,6 @@ $$ LANGUAGE SQL STABLE;
 CREATE TABLE serial.materialized_holding_code (
     id BIGSERIAL PRIMARY KEY,
     issuance INTEGER NOT NULL REFERENCES serial.issuance (id) ON DELETE CASCADE,
-    holding_type TEXT NOT NULL,
-    ind1 TEXT,
-    ind2 TEXT,
     subfield CHAR,
     value TEXT
 );
@@ -110,18 +107,15 @@ spi_exec_prepared($dstmt, $_TD->{new}{id});
 my $istmt = spi_prepare(
     q{
         INSERT INTO serial.materialized_holding_code (
-            issuance, holding_type, ind1, ind2, subfield, value
-        ) VALUES ($1, $2, $3, $4, $5, $6)
-    }, qw{INT TEXT TEXT TEXT CHAR TEXT}
+            issuance, subfield, value
+        ) VALUES ($1, $2, $3)
+    }, qw{INT CHAR TEXT}
 );
 
 foreach ($field->subfields) {
     spi_exec_prepared(
         $istmt,
         $_TD->{new}{id},
-        $_TD->{new}{holding_type},
-        $field->indicator(1),
-        $field->indicator(2),
         $_->[0],
         $_->[1]
     );

commit 0f3291546b37cd3722e3b64e78203c95282fc369
Author: Dan Wells <dbw2 at calvin.edu>
Date:   Thu Mar 29 17:07:23 2012 -0400

    Only load "new" serials display if we have data
    
    If we try to load the "new" serials display template, but have no
    data (for instance, if "Use fully compressed serial holdings" is
    false), we get a server error.
    
    Rather than wrap the whole template in an 'if', let's just not
    load it.
    
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/templates/opac/parts/record/issues.tt2 b/Open-ILS/src/templates/opac/parts/record/issues.tt2
index 585cee6..998e2d6 100644
--- a/Open-ILS/src/templates/opac/parts/record/issues.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/issues.tt2
@@ -1,4 +1,8 @@
 <div class='rdetail_extras_div'>
-    [% INCLUDE 'opac/parts/record/issues-db.tt2' # "new" serials holdings %]
+    [%
+        IF ctx.holding_summary_tree;
+            INCLUDE 'opac/parts/record/issues-db.tt2'; # "new" serials holdings
+        END;
+    %]
     [% INCLUDE 'opac/parts/record/issues-mfhd.tt2' # mfhd-based "classic" serials %]
 </div>

commit 5b69128dc054e4b8b79606f76fe614aca211151a
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Mon Mar 19 11:17:38 2012 -0400

    TPAC: ROWS 1 not appropriate here.
    
    I must have thrown this in as a misguided afterthought without testing
    it.
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/sql/Pg/020.schema.functions.sql b/Open-ILS/src/sql/Pg/020.schema.functions.sql
index de70132..81dcdfc 100644
--- a/Open-ILS/src/sql/Pg/020.schema.functions.sql
+++ b/Open-ILS/src/sql/Pg/020.schema.functions.sql
@@ -248,7 +248,7 @@ RETURNS INT[] AS $$
         FROM actor.org_unit aou
         JOIN descendant_depth dd USING (id)
         WHERE aou.id = $1 ORDER BY dd.path;
-$$ LANGUAGE SQL STABLE ROWS 1;
+$$ LANGUAGE SQL STABLE;
 
 CREATE OR REPLACE FUNCTION actor.org_unit_proximity ( INT, INT ) RETURNS INT AS $$
 	SELECT COUNT(id)::INT FROM (
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
index f27fc79..d395430 100644
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
@@ -65,7 +65,7 @@ RETURNS INT[] AS $$
         FROM actor.org_unit aou
         JOIN descendant_depth dd USING (id)
         WHERE aou.id = $1 ORDER BY dd.path;
-$$ LANGUAGE SQL STABLE ROWS 1;
+$$ LANGUAGE SQL STABLE;
 
 CREATE TABLE serial.materialized_holding_code (
     id BIGSERIAL PRIMARY KEY,

commit d944a3780a9106ed881738472e3259b8d77f7d46
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Fri Mar 9 18:07:19 2012 -0500

    TPAC: follow-up to serials holdings paging fix
    
    Some how I didn't notice it before, but my last commit broke things.
    This unbreaks them.
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
index 11bc0a2..6422edc 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
@@ -536,7 +536,7 @@ sub grouped_holdings_for_summary {
 
     # Will we be trying magic auto-expansion of the first top-level grouping?
     if ($auto_expand_first and @$tree and not @$expand_path) {
-        $expand_path = [$tree->[0]->{value}];
+        $expand_path = [$tree->[1]->{value}];
         $offsets = [0];
     }
 

commit d43eb99ba936155f01fe95f0bf66ff03aefc8fdb
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Thu Mar 8 14:14:01 2012 -0500

    TPAC: serials display paging bugfix
    
    In the previous commit, paging at the deepest level of an expanded tree
    was broken.  This fixes that, and improves pager labeling a little bit.
    
    This also makes the default page size for holdings 12 instead of 10,
    since monthlies are a pretty common case, and why not show them all on
    one page.
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
index 65f19bb..11bc0a2 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
@@ -375,8 +375,8 @@ sub grouped_holdings_for_summary {
     ($summary_type .= "") =~ s/[^\w]//g;
     $summary_id = int($summary_id);
     $expand_path ||= [];
-    $limit ||= 10;
-    $limit = 10 if $limit < 1;
+    $limit ||= 12;
+    $limit = 12 if $limit < 1;
     $offsets ||= [0];
 
     foreach ($expand_path, $offsets) {
@@ -519,13 +519,16 @@ sub grouped_holdings_for_summary {
 
     # Make the tree we have so far.
     my $tree = [
+        { display_grouping => $display_grouping,
+            caption => $pattern_field->subfield($subfield) },
         map(
             _make_grouped_holding_node(
                 $_, $subfield, $deepest_level, $pattern_field,
                 $unit_data, $mfhd_cache
             ),
             @$top
-        ), ($top_more ? undef : ())
+        ),
+        ($top_more ? undef : ())
     ];
 
     # We'll need a parent reference at each level as we descend.
@@ -582,13 +585,16 @@ sub grouped_holdings_for_summary {
 
         # Set parent for the next iteration.
         $parent = $point->{children} = [
+            { display_grouping => $display_grouping,
+                caption => $pattern_field->subfield($subfield) },
             map(
                 _make_grouped_holding_node(
                     $_, $subfield, $deepest_level, $pattern_field,
                     $unit_data, $mfhd_cache
                 ),
                 @$level
-            ), ($level_more ? undef : ())
+            ),
+            ($level_more ? undef : ())
         ];
 
         last if $subfield eq $deepest_level;
@@ -610,7 +616,7 @@ __PACKAGE__->register_method(
             { name => "summary_id", type => "number" },
             { name => "expand_path", type => "array",
                 desc => "In root-to-leaf order, the values of the nodes along the axis you want to expand" },
-            { name => "limit (default 10)", type => "number" },
+            { name => "limit (default 12)", type => "number" },
             { name => "offsets", type => "array", desc =>
                 "This must be exactly one element longer than expand_path" },
             { name => "auto_expand_first", type => "boolean", desc =>
diff --git a/Open-ILS/src/templates/opac/parts/record/issues-db.tt2 b/Open-ILS/src/templates/opac/parts/record/issues-db.tt2
index 6e9179c..2864161 100644
--- a/Open-ILS/src/templates/opac/parts/record/issues-db.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/issues-db.tt2
@@ -15,6 +15,10 @@ ght_depth = 0;
 
 VIEW grouped_holding_tree;
     BLOCK list;
+        level_description = item.shift;
+        level_description.caption =
+            level_description.caption.replace('[\(\)]', '');
+
         '<div class="rdetail-holding-group">';
         prev_seoffset_list = seoffset_list.slice(0, ght_depth);
         next_seoffset_list = seoffset_list.slice(0, ght_depth);
@@ -24,11 +28,18 @@ VIEW grouped_holding_tree;
             prev_seoffset_list.$ght_depth = 0;
         END;
 
+        has_more = 0;
+        at_deepest_level = 0;
+
         next_seoffset_list.$ght_depth = next_seoffset_list.$ght_depth + selimit;
-        IF item.0.units.size;
+        IF item.0.units;
             INCLUDE "opac/parts/record/copy_table.tt2" serial_holdings=item;
             "<hr />";
-            "</div>";
+
+            at_deepest_level = 1;
+            IF NOT item.last.label;
+                has_more = 1;
+            END;
         ELSE;
             FOREACH node IN item;
                 IF NOT node.label;
@@ -80,31 +91,58 @@ VIEW grouped_holding_tree;
                     waste = ght_sepath.pop;
                 ELSE;
                     "<div class='rdetail-holding-group'>"; node.label; "</div>";
-                    # XXX Hold placement link here? Maybe not if no units.
+                    at_deepest_level = 1;
                 END;
             END;
+        END;
 
-            to_clear = 0;
-            new_sepath_end = ght_depth - 1;
-            IF new_sepath_end < 0;
-                to_clear = ['sepath'];
-                new_sepath = [];
-            ELSE;
-                new_sepath = expand_path.slice(0, ght_depth - 1);
-            END;
+        to_clear = 0;
+        new_sepath_end = ght_depth - 1;
+        IF new_sepath_end < 0;
+            to_clear = ['sepath'];
+            new_sepath = [];
+        ELSE;
+            new_sepath = expand_path.slice(0, ght_depth - 1);
+        END;
 
-            IF has_more;
-                '<a class="paging" href="';
-                    mkurl('',{seoffset => next_seoffset_list, sepath => new_sepath},to_clear,'issues');
-                '">&laquo; '; l('Earlier holdings'); '</a>';
-            END;
-            IF seoffset_list.$ght_depth > 0;
-                '<a class="paging" href="';
-                    mkurl('',{seoffset => prev_seoffset_list, sepath => new_sepath},to_clear,'issues');
-                '">'; l('Later holdings'); ' &raquo;</a>&nbsp; ';
+        # So the "holdings" level of the tree is sorted ascending, while all
+        # the higher levels are sorted descending.  This seems weird until you
+        # look at it.  I dunno. I think it feels right.  It could be changed I
+        # guess.  Anyway, this means we have to be careful about which
+        # paging link we label "earlier" and which one we label "later."
+
+        next_link = ''; prev_link = '';
+        IF has_more;
+            next_link = '<a class="paging" href="' _
+                mkurl('',{seoffset => next_seoffset_list, sepath => new_sepath},to_clear,'issues') _ '">LABEL_HERE</a>&nbsp; ';
+        END;
+        IF seoffset_list.$ght_depth > 0;
+            prev_link = '<a class="paging" href="' _
+                mkurl('',{seoffset => prev_seoffset_list, sepath => new_sepath},to_clear,'issues') _ '">LABEL_HERE</a>&nbsp; ';
+        END;
+
+        IF at_deepest_level;
+            prev_link.replace('LABEL_HERE', '&laquo; ' _ l('Earlier issues'));
+            next_link.replace('LABEL_HERE', l('Later issues') _ ' &raquo;');
+        ELSE;
+            # XXX this is really bad for i18n (notice the sloppy pluralization),
+            # but then the middle layer for serials only knows English names
+            # for things like "month". There's a bigger problem to solve
+            # here...
+            caption = level_description.caption;
+            IF level_description.display_grouping == 'chron';
+                caption = caption _ 's';
             END;
-            '</div>';
+            next_link.replace(
+                'LABEL_HERE',
+                '&laquo; ' _ l('Earlier') _ ' ' _ caption
+            );
+            prev_link.replace(
+                'LABEL_HERE',
+                l('Later') _ ' ' _ caption _ ' &raquo;'
+            );
         END;
+        '</div>';
     END;
 END;
 

commit 5b109f49e3a94c26c8efb7bb55965f20b2f7850b
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Mon Apr 2 07:20:33 2012 -0400

    TPAC: Improvement to serials display (under the "issues held" label)
    
    (All the following text assumes you're using "new" 2.0+ serials and that
    the org unit setting 'opac.fully_compressed_serial_holdings' is true
    in the context where you're browsing.)
    
    Today on the TPAC record detail page under the "issues held" label, you
    get a list of serial summary statements, which you can expand to a list of
    issuances for which items have been received.  You can place issuance-level
    holds on these.
    
    That existing interface just kind of burps up all your holdings within
    scope and doesn't show you what holdings belong to what org unit.
    Furthermore, it doesn't group your holdings into enumeration or
    chronology units, which can matter a lot if you've got 150 years of
    some daily newspaper and you're trying to browse through them in the
    OPAC.
    
    This new interface presents expanded serials holdings organized into a
    tree, with summaries placed under their org units and holdings grouped
    under their summaries under either chronology units (default) or
    enumeration ones, controlled by a new field on serial.distribution.
    There's also a new org unit setting that lets you change the default
    value for this new field in the Alternate Serial Control view to
    enumeration, if you want.
    
    Like the issues-held interface it's replacing, this knows how to deal
    with holdings where you have one unit per received item, or no units per
    received item, but its behavior is not yet defined for one unit per many
    items (the binding case).  The "regular" Serial Control view doesn't
    have a widget to control the new field on serial.distribution yet.
    These are the areas where I'd be interested in helping to close the
    gaps, before or after this is committed.
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    
    Conflicts:
    
    	Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm
    	Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
    	Open-ILS/web/css/skin/default/opac/style.css
    
    Signed-off-by: Dan Wells <dbw2 at calvin.edu>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

diff --git a/Open-ILS/examples/fm_IDL.xml b/Open-ILS/examples/fm_IDL.xml
index f4127c2..fe0af0d 100644
--- a/Open-ILS/examples/fm_IDL.xml
+++ b/Open-ILS/examples/fm_IDL.xml
@@ -4185,6 +4185,7 @@ SELECT  usr,
 			<field reporter:label="Bind Unit Template" name="bind_unit_template" reporter:datatype="link"/>
 			<field reporter:label="Unit Label Prefix" name="unit_label_prefix" reporter:datatype="text"/>
 			<field reporter:label="Unit Label Suffix" name="unit_label_suffix" reporter:datatype="text"/>
+			<field reporter:label="Display Grouping" name="display_grouping" reporter:datatype="text"/>
 			<field reporter:label="Streams" name="streams" oils_persist:virtual="true" reporter:datatype="link"/>
 			<field reporter:label="Notes" name="notes" oils_persist:virtual="true" reporter:datatype="link"/>
 			<field reporter:label="Basic Issue Summary" name="basic_summary" oils_persist:virtual="true" reporter:datatype="link"/>
@@ -4477,7 +4478,32 @@ SELECT  usr,
 		</links>
 		<!-- Not available via PCRUD at this time -->
 	</class>
-
+	<class id="sasum" controller="open-ils.cstore" oils_obj:fieldmapper="serial::any_summary" oils_persist:tablename="serial.any_summary" reporter:label="All Issues' Summaries" oils_persist:readonly="true">
+		<fields>
+			<field name="summary_type" reporter:label="Summary Type" reporter:datatype="text" />
+			<field name="id" reporter:label="Native ID" reporter:datatype="int" /><!-- not datatype="id", because id is not unique in this view -->
+			<field name="distribution" reporter:label="Distribution" reporter:datatype="link" />
+			<field name="generated_coverage" reporter:label="Generated Coverage" reporter:datatype="text" />
+			<field name="show_generated" reporter:label="Show Generated?" reporter:datatype="bool" />
+		</fields>
+		<links>
+			<link field="distribution" reltype="has_a" key="id" map="" class="sdist"/>
+		</links>
+	</class>
+	<class id="smhc" controller="open-ils.cstore" oils_obj:fieldmapper="serial::materialized_holding_code" oils_persist:tablename="serial.materialized_holding_code" reporter:label="Materialized Holding Code" oils_persist:readonly="true">
+		<fields oils_persist:primary="id" oils_persist:sequence="serial.materialized_holding_code_id_seq">
+			<field name="id" reporter:label="ID" reporter:datatype="id" />
+			<field name="issuance" reporter:label="Issuance" reporter:datatype="link" />
+			<field name="holding_type" reporter:label="Holding Type" reporter:datatype="text" />
+			<field name="ind1" reporter:label="First Indicator" reporter:datatype="text" />
+			<field name="ind2" reporter:label="Second Indicator" reporter:datatype="text" />
+			<field name="subfield" reporter:label="Subfield" reporter:datatype="text" />
+			<field name="value" reporter:label="Value" reporter:datatype="text" oils_obj:validate="^\w$" />
+		</fields>
+		<links>
+			<link field="issuance" reltype="has_a" key="id" map="" class="siss"/>
+		</links>
+	</class>
 	<class id="sbsum" controller="open-ils.cstore open-ils.pcrud" oils_obj:fieldmapper="serial::basic_summary" oils_persist:tablename="serial.basic_summary" reporter:label="Basic Issue Summary">
 		<fields oils_persist:primary="id" oils_persist:sequence="serial.basic_summary_id_seq">
 			<field reporter:label="ID" name="id" reporter:datatype="id" />
diff --git a/Open-ILS/src/extras/ils_events.xml b/Open-ILS/src/extras/ils_events.xml
index 9708e9e..2eeffb7 100644
--- a/Open-ILS/src/extras/ils_events.xml
+++ b/Open-ILS/src/extras/ils_events.xml
@@ -1001,6 +1001,11 @@
         <desc xml:lang="en-US">Checkin attempted on item during minimum transit checkin interval.</desc>
     </event>
 
+    <event code='11104' textcode='SERIAL_CORRUPT_PATTERN_CODE'>
+        <desc xml:lang="en-US">A serial pattern code has been configured
+        that fails to conform to MFHD standards for fields 853-855.</desc>
+    </event>
+
 	<!-- ================================================================ -->
 
 </ils_events>
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
index 07bd151..2bc09cf 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
@@ -1470,7 +1470,15 @@ sub get_org_descendants {
 }
 
 sub get_org_ancestors {
-	my($self, $org_id) = @_;
+	my($self, $org_id, $use_cache) = @_;
+
+    my ($cache, $orgs);
+
+    if ($use_cache) {
+        $cache = OpenSRF::Utils::Cache->new("global", 0);
+        $orgs = $cache->get_cache("org.ancestors.$org_id");
+        return $orgs if $orgs;
+    }
 
 	my $org_list = OpenILS::Utils::CStoreEditor->new->json_query({
 		select => {
@@ -1485,9 +1493,10 @@ sub get_org_ancestors {
 		where => {id => $org_id}
 	});
 
-	my @orgs;
-	push(@orgs, $_->{id}) for @$org_list;
-	return \@orgs;
+	$orgs = [ map { $_->{id} } @$org_list ];
+
+    $cache->put_cache("org.ancestors.$org_id", $orgs) if $use_cache;
+	return $orgs;
 }
 
 sub get_org_full_path {
@@ -1975,5 +1984,118 @@ sub log_user_activity {
     return undef;
 }
 
+# I hate to put this here exactly, but this code needs to be shared between
+# the TPAC's mod_perl module and open-ils.serial.
+#
+# There is a reason every part of the query *except* those parts dealing
+# with scope are moved here from the code's origin in TPAC.  The serials
+# use case does *not* want the same scoping logic.
+#
+# Also, note that for the serials uses case, we may filter in OPAC visible
+# status and copy/call_number deletedness, but we don't filter on any
+# particular values for serial.item.status or serial.item.date_received.
+# Since we're only using this *after* winnowing down the set of issuances
+# that copies should be related to, I'm not sure we need any such serial.item
+# filters.
+
+sub basic_opac_copy_query {
+    ######################################################################
+    # Pass a defined value for either $rec_id OR ($iss_id AND $dist_id), #
+    # not both.                                                          #
+    ######################################################################
+    my ($self,$rec_id,$iss_id,$dist_id,$copy_limit,$copy_offset,$staff) = @_;
+
+    return {
+        select => {
+            acp => ['id', 'barcode', 'circ_lib', 'create_date',
+                    'age_protect', 'holdable'],
+            acpl => [
+                {column => 'name', alias => 'copy_location'},
+                {column => 'holdable', alias => 'location_holdable'}
+            ],
+            ccs => [
+                {column => 'name', alias => 'copy_status'},
+                {column => 'holdable', alias => 'status_holdable'}
+            ],
+            acn => [
+                {column => 'label', alias => 'call_number_label'},
+                {column => 'id', alias => 'call_number'}
+            ],
+            circ => ['due_date'],
+            acnp => [
+                {column => 'label', alias => 'call_number_prefix_label'},
+                {column => 'id', alias => 'call_number_prefix'}
+            ],
+            acns => [
+                {column => 'label', alias => 'call_number_suffix_label'},
+                {column => 'id', alias => 'call_number_suffix'}
+            ],
+            bmp => [
+                {column => 'label', alias => 'part_label'},
+            ],
+            ($iss_id ? (sitem => ["issuance"]) : ())
+        },
+
+        from => {
+            acp => {
+                ($iss_id ? (
+                    sitem => {
+                        fkey => 'id',
+                        field => 'unit',
+                        filter => {issuance => $iss_id},
+                        join => {
+                            sstr => { }
+                        }
+                    }
+                ) : ()),
+                acn => {
+                    join => {
+                        acnp => { fkey => 'prefix' },
+                        acns => { fkey => 'suffix' }
+                    },
+                    filter => [
+                        {deleted => 'f'},
+                        ($rec_id ? {record => $rec_id} : ())
+                    ],
+                },
+                circ => { # If the copy is circulating, retrieve the open circ
+                    type => 'left',
+                    filter => {checkin_time => undef}
+                },
+                acpl => {
+                    ($staff ? () : (filter => { opac_visible => 't' }))
+                },
+                ccs => {
+                    ($staff ? () : (filter => { opac_visible => 't' }))
+                },
+                aou => {},
+                acpm => {
+                    type => 'left',
+                    join => {
+                        bmp => { type => 'left' }
+                    }
+                }
+            }
+        },
+
+        where => {
+            '+acp' => {
+                deleted => 'f',
+                ($staff ? () : (opac_visible => 't'))
+            },
+            ($dist_id ? ( '+sstr' => { distribution => $dist_id } ) : ()),
+            ($staff ? () : ( '+aou' => { opac_visible => 't' } ))
+        },
+
+        order_by => [
+            {class => 'aou', field => 'name'},
+            {class => 'acn', field => 'label'}
+        ],
+
+        limit => $copy_limit,
+        offset => $copy_offset
+    };
+}
+
 1;
 
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
index 7acc52d..4c19954 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
@@ -50,6 +50,9 @@ use OpenILS::Utils::Fieldmapper;
 use OpenILS::Utils::MFHD;
 use DateTime::Format::ISO8601;
 use MARC::File::XML (BinaryEncoding => 'utf8');
+
+use OpenILS::Application::Serial::OPAC;
+
 my $U = 'OpenILS::Application::AppUtils';
 my @MFHD_NAMES = ('basic','supplement','index');
 my %MFHD_NAMES_BY_TAG = (  '853' => $MFHD_NAMES[0],
@@ -503,6 +506,8 @@ sub pub_fleshed_serial_issuance_retrieve_batch {
 }
 
 sub received_siss_by_bib {
+    # XXX this is somewhat wrong in implementation and should not be used in
+    # new places - senator
     my $self = shift;
     my $client = shift;
     my $bib = shift;
@@ -614,6 +619,8 @@ q/A hash of optional arguments.  Valid keys and their meanings:
 
 
 sub scoped_bib_holdings_summary {
+    # XXX this is somewhat wrong in implementation and should not be used in
+    # new places - senator
     my $self = shift;
     my $client = shift;
     my $bibid = shift;
@@ -643,7 +650,7 @@ __PACKAGE__->register_method(
     api_level => 1,
     argc      => 1,
     signature => {
-        desc   => 'Receives a Bib ID and other optional params and returns set of holdings statements',
+        desc   => '** DEPRECATED and only used by JSPAC. Somewhat wrong in implementation. *** Receives a Bib ID and other optional params and returns set of holdings statements',
         params => [
             {   name => 'bibid',
                 desc => 'id of the bre to which the issuances belong',
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
new file mode 100644
index 0000000..65f19bb
--- /dev/null
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
@@ -0,0 +1,633 @@
+package OpenILS::Application::Serial::OPAC;
+
+# This package contains methods for open-ils.serial that present data suitable
+# for OPAC display.
+
+use base qw/OpenILS::Application/;
+use strict;
+use warnings;
+
+# All of the packages we might 'use' are already imported in
+# OpenILS::Application::Serial.  Only those that export symbols
+# need to be mentioned explicitly here.
+
+use OpenSRF::Utils::Logger qw/:logger/;
+use OpenILS::Utils::CStoreEditor q/:funcs/;
+
+my $U = "OpenILS::Application::AppUtils";
+
+my %MFHD_SUMMARIZED_SUBFIELDS = (
+   enum => [ split //, "abcdef" ],   # $g and $h intentionally omitted for now
+   chron => [ split //, "ijklm" ]
+);
+
+# This is a helper for scoped_holding_summary_tree_for_bib() a little further down
+
+sub _place_org_node {
+    my ($node, $tree, $org_tree) = @_;
+
+    my @ancestry = reverse @{ $U->get_org_ancestors($node->{org_unit}, 1) };
+    shift @ancestry;    # discard current org_unit
+
+    foreach (@ancestry) {  # in leaf-to-root order
+        my $graft_point = _find_ou_in_holdings_tree($tree, $_);
+
+        if ($graft_point) {
+            push @{$graft_point->{children}}, $node;
+            return;
+        } else {
+            $node = {
+                org_unit => $_,
+                holding_summaries => [],
+                children => [$node]
+            }
+        }
+    }
+
+    # If we reach this point, we got all the way to the top of the org tree
+    # without finding corresponding nodes in $tree (holdings tree), so the
+    # latter must be empty, and we need to make $tree just contain what $node
+    # contains.
+
+    %$tree = %$node;
+}
+
+# This is a helper for scoped_holding_summary_tree_for_bib() a little further down
+
+sub _find_ou_in_holdings_tree {
+    my ($tree, $id) = @_;
+
+    return $tree if $tree->{org_unit} eq $id;
+    if (ref $tree->{children}) {
+        foreach (@{$tree->{children}}) {
+            my $maybe = _find_ou_in_holdings_tree($_, $id);
+            return $maybe if $maybe;
+        }
+    }
+
+    return;
+}
+
+sub scoped_holding_summary_tree_for_bib {
+    my (
+        $self, $client, $bib, $org_unit, $depth, $limit, $offset, $ascending
+    ) = @_;
+
+    my $org_tree = $U->get_org_tree;    # caches
+
+    $org_unit ||= $org_tree->id;
+    $depth ||= 0;
+    $limit ||= 10;
+    $offset ||= 0;
+
+    my $e = new_editor;
+
+    # What we want to know from this query is essentially the set of
+    # holdings related to a given bib and the org units that have said
+    # holdings.
+
+    # For this we would only need sasum, sdist and ssub, but
+    # because we also need to be able to page (and therefore must sort) the
+    # results we get, we need reasonable columns on which to do the sorting.
+    # So for that we join sitem (via sstr) so we can sort on the maximum
+    # date_expected (which is basically the issue pub date) for items that
+    # have been received.  That maximum date_expected is actually the second
+    # sort key, however.  The first is the holding lib's position in a
+    # depth-first representation of the org tree (if you think about it,
+    # paging through holdings held at diverse points in the tree only makes
+    # sense if you do it this way).
+
+    my $rows = $e->json_query({
+        select => {
+            sasum => [qw/summary_type id generated_coverage/],
+            sdist => ["holding_lib"],
+            sitem => [
+                {column => "date_expected", transform => "max", aggregate => 1}
+            ]
+        },
+        from => {
+            sasum => {
+                sdist => {
+                    join => {
+                        ssub => {},
+                        sstr => {
+                            join => {sitem => {}}
+                        },
+                    }
+                }
+            }
+        },
+        where => {
+            "+sdist" => {
+                holding_lib =>
+                    $U->get_org_descendants(int($org_unit), int($depth))
+            },
+            "+ssub" => {record_entry => int($bib)},
+            "+sitem" => {date_received => {"!=" => undef}}
+        },
+        limit => int($limit) + 1, # see comment below on "limit trick"
+        offset => int($offset),
+        order_by => [
+            {
+                class => "sdist",
+                field => "holding_lib",
+                transform => "actor.org_unit_simple_path",
+                params => [$org_tree->id]
+            },
+            {
+                class => "sitem",
+                field => "date_expected",
+                transform => "max", # to match select clause
+                direction => ($ascending ? "ASC" : "DESC")
+            }
+        ],
+    }) or return $e->die_event;
+
+    $e->disconnect;
+
+    # Now we build a tree out of our result set.
+    my $result = {};
+
+    # Use our "limit trick" from above to cheaply determine whether there's
+    # another page of results, for the UI's benefit.  Put $more into the
+    # result hash at the very end.
+    my $more = 0;
+    if (scalar(@$rows) > int($limit)) {
+        $more = 1;
+        pop @$rows;
+    }
+
+    foreach my $row (@$rows) {
+        my $org_node_needs_placed = 0;
+        my $org_node =
+            _find_ou_in_holdings_tree($result, $row->{holding_lib});
+
+        if (not $org_node) {
+            $org_node_needs_placed = 1;
+            $org_node = {
+                org_unit => $row->{holding_lib},
+                holding_summaries => [],
+                children => []
+            };
+        }
+
+        # Make a very simple object for a single holding summary.
+        # generated_coverage is stored as JSON, and here we can unpack it.
+        my $summary = {
+            id => $row->{id},
+            summary_type => $row->{summary_type},
+            generated_coverage =>
+                OpenSRF::Utils::JSON->JSON2perl($row->{generated_coverage})
+        };
+
+        push @{$org_node->{holding_summaries}}, $summary;
+
+        if ($org_node_needs_placed) {
+            _place_org_node($org_node, $result, $org_tree);
+        }
+    }
+
+    $result->{more} = $more;
+    return $result;
+}
+
+__PACKAGE__->register_method(
+    method    => "scoped_holding_summary_tree_for_bib",
+    api_name  => "open-ils.serial.holding_summary_tree.by_bib",
+    api_level => 1,
+    argc      => 6,
+    signature => {
+        desc   => 'Return a set of holding summaries organized into a tree
+        of nodes that look like:
+            {org_unit:<id>, holding_summaries:[], children:[]}
+
+        The root node has an extra key: "more". Its value is 1 if there
+        are more pages (in the limit/offset sense) of results that the caller
+        could potentially fetch.
+
+        All arguments except the first (bibid) are optional.
+        ',
+        params => [
+            {   name => "bibid",
+                desc => "ID of the bre to which holdings belong",
+                type => "number"
+            },
+            { name => "org_unit", type => "number" },
+            { name => "depth (default 0)", type => "number" },
+            { name => "limit (default 10)", type => "number" },
+            { name => "offset (default 0)", type => "number" },
+            { name => "ascending (default false)", type => "boolean" },
+        ]
+    }
+);
+
+# This is a helper for grouped_holdings_for_summary() later.
+sub _label_holding_level {
+    my ($pattern_field, $subfield, $value, $mfhd_cache) = @_;
+
+    # This is naïve, in that a-f are sometimes chron fields and not enum.
+    # OpenILS::Utils::MFHD understands that, but so far I don't think our
+    # interfaces do.
+
+    my $cache_key = $subfield . $value;
+
+    if (not exists $mfhd_cache->{$cache_key}) {
+        my $link_id = (split(/\./, $pattern_field->subfield('8')))[0];
+        my $fake_holding = new MFHD::Holding(
+            1,
+            new MARC::Field('863', '4', '1', '8', "$link_id.1"),
+            new MFHD::Caption($pattern_field->clone)
+        );
+
+        if ($subfield ge 'i') { # chron
+            $mfhd_cache->{$cache_key} = $fake_holding->format_single_chron(
+                {$subfield => $value}, $subfield, 1, 1
+            );
+        } else {                # enum
+            $mfhd_cache->{$cache_key} = $fake_holding->format_single_enum(
+                {$subfield => $value}, $subfield, 1
+            );
+        }
+    }
+
+    return $mfhd_cache->{$cache_key};
+}
+
+# This is a helper for grouped_holdings_for_summary() later.
+sub _get_deepest_holding_level {
+    my ($display_grouping, $pattern_field) = @_;
+
+    my @present = grep { $pattern_field->subfield($_) } @{
+        $MFHD_SUMMARIZED_SUBFIELDS{$display_grouping}
+    };
+
+    return pop @present;
+}
+
+# This is a helper for grouped_holdings_for_summary() later.
+sub _opac_visible_unit_data {
+    my ($issuance_id_list, $dist_id, $staff, $e) = @_;
+
+    return {} unless @$issuance_id_list;
+
+    my $rows = $e->json_query(
+        $U->basic_opac_copy_query(
+            undef, $issuance_id_list, $dist_id,
+            1000, 0,    # XXX no mechanism for users to page at this level yet
+            $staff
+        )
+    ) or return $e->die_event;
+
+    my $results = {};
+
+    # Take the list of rows returned from json_query() and sort results into
+    # several smaller lists stored in a hash keyed by issuance ID.
+    foreach my $row (@$rows) {
+        $results->{$row->{issuance}} = [] unless
+            exists $results->{$row->{issuance}};
+        push @{ $results->{$row->{issuance}} }, $row;
+    }
+
+    return $results;
+}
+
+# This is a helper for grouped_holdings_for_summary() later.
+sub _make_grouped_holding_node {
+    my (
+        $row, $subfield, $deepest_level, $pattern_field,
+        $unit_data, $mfhd_cache
+    ) = @_;
+
+    return {
+        $subfield eq $deepest_level ? (
+            label => $row->{label},
+            holding => $row->{id},
+            ($unit_data ? (units => ($unit_data->{$row->{id}} || [])) : ())
+        ) : (
+            value => $row->{value},
+            label => _label_holding_level(
+                $pattern_field, $subfield, $row->{value}, $mfhd_cache
+            )
+        )
+    };
+}
+
+# This is a helper for grouped_holdings_for_summary() later.
+sub _make_single_level_grouped_holding_query {
+    my (
+        $subfield, $deepest_level, $summary_hint, $summary_id,
+        $subfield_joins, $subfield_where_clauses,
+        $limit, $offsets
+    ) = @_;
+
+    return {
+        select => {
+            sstr => ["distribution"],
+            "smhc_$subfield" => ["value"], (
+                $subfield eq $deepest_level ?
+                    (siss => [qw/id label date_published/]) : ()
+            )
+        },
+        from => {
+            $summary_hint => {
+                sdist => {
+                    join => {
+                        sstr => {
+                            join => {
+                                sitem => {
+                                    join => {
+                                        siss => {
+                                            join => {%$subfield_joins}
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        },
+        where => {
+            "+$summary_hint" => {id => $summary_id},
+            "+sitem" => {date_received => {"!=" => undef}},
+            %$subfield_where_clauses
+        },
+        distinct => 1,  # sic, this goes here in json_query
+        limit => int($limit) + 1,
+        offset => int(shift(@$offsets)),
+        order_by => {
+            "smhc_$subfield" => {
+                "value" => {
+                    direction => ($subfield eq $deepest_level ? "asc" : "desc")
+                }
+            }
+        }
+    };
+}
+
+sub grouped_holdings_for_summary {
+    my (
+        $self, $client, $summary_type, $summary_id,
+        $expand_path, $limit, $offsets, $auto_expand_first, $with_units
+    ) = @_;
+
+    # Validate input or set defaults.
+    ($summary_type .= "") =~ s/[^\w]//g;
+    $summary_id = int($summary_id);
+    $expand_path ||= [];
+    $limit ||= 10;
+    $limit = 10 if $limit < 1;
+    $offsets ||= [0];
+
+    foreach ($expand_path, $offsets) {
+        if (ref $_ ne 'ARRAY') {
+            return new OpenILS::Event(
+                "BAD_PARAMS", note =>
+                    "'expand_path' and 'offsets' arguments must be arrays"
+            );
+        }
+    }
+
+    if (scalar(@$offsets) != scalar(@$expand_path) + 1) {
+        return new OpenILS::Event(
+            "BAD_PARAMS", note =>
+                "'offsets' array must be one element longer than 'expand_path'"
+        );
+    }
+
+    # Get the class hint for whichever type of summary we're expanding.
+    my $fmclass = "Fieldmapper::serial::${summary_type}_summary";
+    my $summary_hint = $Fieldmapper::fieldmap->{$fmclass}{hint} or
+        return new OpenILS::Event("BAD_PARAMS", note => "summary_type");
+
+    my $e = new_editor;
+
+    # First, get display grouping for requested summary (either chron or enum)
+    # and the pattern code. Even though we have to JOIN through sitem to get
+    # pattern_code from scap, we don't actually care about specific items yet.
+    my $row = $e->json_query({
+        select => {sdist => ["display_grouping"], scap => ["pattern_code"]},
+        from => {
+            $summary_hint => {
+                sdist => {
+                    join => {
+                        sstr => {
+                            join => {
+                                sitem => {
+                                    join => {
+                                        siss => {
+                                            join => {scap => {}}
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        },
+        where => {
+            "+$summary_hint" => {id => $summary_id},
+            "+sitem" => {date_received => {"!=" => undef}}
+        },
+        limit => 1
+    }) or return $e->die_event;
+
+    # Summaries without attached holdings constitute bad data, not benign
+    # empty result sets.
+    return new OpenILS::Event(
+        "BAD_PARAMS",
+        note => "Summary #$summary_id not found, or no holdings attached"
+    ) unless @$row;
+
+    # Unless data has been disarranged, all holdings grouped together under
+    # the same summary should have the same pattern code, so we can take any
+    # result from the set we just got.
+    my $pattern_field;
+    eval {
+        $pattern_field = new MARC::Field(
+            "853", # irrelevant for our purposes
+            @{ OpenSRF::Utils::JSON->JSON2perl($row->[0]->{pattern_code}) }
+        );
+    };
+    if ($@) {
+        return new OpenILS::Event("SERIAL_CORRUPT_PATTERN_CODE", note => $@);
+    }
+
+    # And now we know which subfields we will care about from
+    # serial.materialized_holding_code.
+    my $display_grouping = $row->[0]->{display_grouping};
+
+    # This will tell us when to stop grouping and start showing actual
+    # holdings.
+    my $deepest_level =
+        _get_deepest_holding_level($display_grouping, $pattern_field);
+    if (not defined $deepest_level) {
+        # corrupt pattern code
+        my $msg = "couldn't determine deepest holding level for " .
+            "$summary_type summary #$summary_id";
+        $logger->warn($msg);
+        return new OpenILS::Event("SERIAL_CORRUPT_PATTERN_CODE", note => $msg);
+    }
+
+    my @subfields = @{ $MFHD_SUMMARIZED_SUBFIELDS{$display_grouping} };
+
+    # We look for holdings grouped at the top level once no matter what,
+    # then we'll look deeper with additional queries for every element of
+    # $expand_path later.
+    # Below we define parts of the SELECT and JOIN clauses that we'll
+    # potentially reuse if $expand_path has elements.
+
+    my $subfield = shift @subfields;
+    my %subfield_joins = ("smhc_$subfield" => {class => "smhc"});
+    my %subfield_where_clauses = ("+smhc_$subfield" => {subfield => $subfield});
+
+    # Now get the top level of holdings.
+    my $top = $e->json_query(
+        _make_single_level_grouped_holding_query(
+            $subfield, $deepest_level, $summary_hint, $summary_id,
+            \%subfield_joins, \%subfield_where_clauses,
+            $limit, $offsets
+        )
+    ) or return $e->die_event;
+
+    # Deal with the extra row, if present, that tells are there are more pages
+    # of results.
+    my $top_more = 0;
+    if (scalar(@$top) > int($limit)) {
+        $top_more = 1;
+        pop @$top;
+    }
+
+    # Distribution is the same for all rows anyway, but we may need it for a
+    # copy query later.
+    my $dist_id = @$top ? $top->[0]->{distribution} : undef;
+
+    # This will help us avoid certain repetitive calculations. Examine
+    # _label_holding_level() to see what I mean.
+    my $mfhd_cache = {};
+
+    # Prepare related unit data if appropriate.
+    my $unit_data;
+
+    if ($with_units and $subfield eq $deepest_level) {
+        $unit_data = _opac_visible_unit_data(
+            [map { $_->{id} } @$top], $dist_id, $with_units > 1, $e
+        );
+        return $unit_data if defined $U->event_code($unit_data);
+    }
+
+    # Make the tree we have so far.
+    my $tree = [
+        map(
+            _make_grouped_holding_node(
+                $_, $subfield, $deepest_level, $pattern_field,
+                $unit_data, $mfhd_cache
+            ),
+            @$top
+        ), ($top_more ? undef : ())
+    ];
+
+    # We'll need a parent reference at each level as we descend.
+    my $parent = $tree;
+
+    # Will we be trying magic auto-expansion of the first top-level grouping?
+    if ($auto_expand_first and @$tree and not @$expand_path) {
+        $expand_path = [$tree->[0]->{value}];
+        $offsets = [0];
+    }
+
+    # Ok, that got us the top level, with nothing expanded. Now we loop through
+    # the elements of @$expand_path, issuing similar queries to get us deeper
+    # groupings and even actual specific holdings.
+    foreach my $value (@$expand_path) {
+        my $prev_subfield = $subfield;
+        $subfield = shift @subfields;
+
+        # This wad of JOINs is additive over each iteration.
+        $subfield_joins{"smhc_$subfield"} = {class => "smhc"};
+
+        # The WHERE clauses also change and grow each time.
+        $subfield_where_clauses{"+smhc_$prev_subfield"}->{value} = $value;
+        $subfield_where_clauses{"+smhc_$subfield"}->{subfield} = $subfield;
+
+        my $level = $e->json_query(
+            _make_single_level_grouped_holding_query(
+                $subfield, $deepest_level, $summary_hint, $summary_id,
+                \%subfield_joins, \%subfield_where_clauses,
+                $limit, $offsets
+            )
+        ) or return $e->die_event;
+
+        return $tree unless @$level;
+
+        # Deal with the extra row, if present, that tells are there are more
+        # pages of results.
+        my $level_more = 0;
+        if (scalar(@$level) > int($limit)) {
+            $level_more = 1;
+            pop @$level;
+        }
+
+        # Find attachment point for our results.
+        my ($point) = grep { ref $_ and $_->{value} eq $value } @$parent;
+
+        # Prepare related unit data if appropriate.
+        if ($with_units and $subfield eq $deepest_level) {
+            $unit_data = _opac_visible_unit_data(
+                [map { $_->{id} } @$level], $dist_id, $with_units > 1, $e
+            );
+            return $unit_data if defined $U->event_code($unit_data);
+        }
+
+        # Set parent for the next iteration.
+        $parent = $point->{children} = [
+            map(
+                _make_grouped_holding_node(
+                    $_, $subfield, $deepest_level, $pattern_field,
+                    $unit_data, $mfhd_cache
+                ),
+                @$level
+            ), ($level_more ? undef : ())
+        ];
+
+        last if $subfield eq $deepest_level;
+    }
+
+    return $tree;
+}
+
+__PACKAGE__->register_method(
+    method    => "grouped_holdings_for_summary",
+    api_name  => "open-ils.serial.holdings.grouped_by_summary",
+    api_level => 1,
+    argc      => 7,
+    signature => {
+        desc   => q/Return a tree of holdings associated with a given summary
+        grouped by all but the last of either chron or enum units./,
+        params => [
+            { name => "summary_type", type => "string" },
+            { name => "summary_id", type => "number" },
+            { name => "expand_path", type => "array",
+                desc => "In root-to-leaf order, the values of the nodes along the axis you want to expand" },
+            { name => "limit (default 10)", type => "number" },
+            { name => "offsets", type => "array", desc =>
+                "This must be exactly one element longer than expand_path" },
+            { name => "auto_expand_first", type => "boolean", desc =>
+                "Only if expand_path is empty, automatically expand first top-level grouping" },
+            { name => "with_units", type => "number", desc => q/
+                If true at all, for each holding, if there are associated units,
+                add some information about them to the result tree. These units
+                will be filtered by OPAC visibility unless you provide a value
+                greater than 1.
+
+                IOW:
+                    0 = no units,
+                    1 = opac visible units,
+                    2 = all units (i.e. staff view)
+                / }
+        ]
+    }
+);
+
+1;
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm
index 9b673c2..920bfd2 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm
@@ -233,6 +233,72 @@ sub subfields_list {
     }
     return @subfields;
 }
+my %__mfhd_month_labels = (
+    '01' => ['Jan.', 'January'],
+    '02' => ['Feb.', 'February'],
+    '03' => ['Mar.', 'March'],
+    '04' => ['Apr.', 'April'],
+    '05' => ['May ', 'May'],
+    '06' => ['Jun.', 'June'],
+    '07' => ['Jul.', 'July'],
+    '08' => ['Aug.', 'August'],
+    '09' => ['Sep.', 'September'],
+    '10' => ['Oct.', 'October'],
+    '11' => ['Nov.', 'November'],
+    '12' => ['Dec.', 'December'],
+    '21' => 'Spring',
+    '22' => 'Summer',
+    '23' => 'Autumn',
+    '24' => 'Winter'
+);
+
+sub _get_mfhd_month_label {
+    my ($month, $long) = @_;
+    $long ||= 0;
+
+    my $o = $__mfhd_month_labels{$month};
+    return (ref $o) ? $o->[$long] : $o;
+}
+
+# Called by method 'format_chron'
+#
+sub format_single_chron {
+    my $self = shift;
+    my $holdings = shift;
+    my $key = shift;
+    my $skip_sep = shift;
+    my $long = shift;
+    my $capstr;
+    my $chron;
+    my $sep = ':';
+
+    return if !defined $self->caption->capstr($key);
+
+    $capstr = $self->caption->capstr($key);
+    if (substr($capstr, 0, 1) eq '(') {
+        # a caption enclosed in parentheses is not displayed
+        $capstr = '';
+    }
+
+    # If this is the second level of chronology, then it's
+    # likely to be a month or season, so we should use the
+    # string name rather than the number given.
+    if ($key eq 'b' or $key eq 'j') {
+        # account for possible combined issue chronology
+        my @chron_parts = split('/', $holdings->{$key});
+        for (my $i = 0; $i < @chron_parts; $i++) {
+            my $month_label =  _get_mfhd_month_label($chron_parts[$i], $long);
+            $chron_parts[$i] = $month_label if defined $month_label;
+        }
+        $chron = join('/', @chron_parts);
+    } else {
+        $chron = $holdings->{$key};
+    }
+
+    $skip_sep ||= ($key eq 'a' || $key eq 'i');
+
+    return ($skip_sep ? '' : $sep) . $capstr . $chron;
+}
 
 #
 # Called by method 'format_part' for formatting the chronology portion of
@@ -241,63 +307,49 @@ sub subfields_list {
 sub format_chron {
     my $self     = shift;
     my $holdings = shift;
-    my $caption  = $self->caption;
     my @keys     = @_;
     my $str      = '';
-    my %month    = (
-        '01' => 'Jan.',
-        '02' => 'Feb.',
-        '03' => 'Mar.',
-        '04' => 'Apr.',
-        '05' => 'May ',
-        '06' => 'Jun.',
-        '07' => 'Jul.',
-        '08' => 'Aug.',
-        '09' => 'Sep.',
-        '10' => 'Oct.',
-        '11' => 'Nov.',
-        '12' => 'Dec.',
-        '21' => 'Spring',
-        '22' => 'Summer',
-        '23' => 'Autumn',
-        '24' => 'Winter'
-    );
-
-    foreach my $i (0.. at keys) {
-        my $key = $keys[$i];
-        my $capstr;
-        my $chron;
-        my $sep;
-
-        last if !defined $caption->capstr($key);
-
-        $capstr = $caption->capstr($key);
-        if (substr($capstr, 0, 1) eq '(') {
-            # a caption enclosed in parentheses is not displayed
-            $capstr = '';
-        }
-
-        # If this is the second level of chronology, then it's
-        # likely to be a month or season, so we should use the
-        # string name rather than the number given.
-        if (($i == 1)) {
-            # account for possible combined issue chronology
-            my @chron_parts = split('/', $holdings->{$key});
-            for (my $i = 0; $i < @chron_parts; $i++) {
-                $chron_parts[$i] = $month{$chron_parts[$i]} if exists $month{$chron_parts[$i]};
-            }
-            $chron = join('/', @chron_parts);
-        } else {
-            $chron = $holdings->{$key};
-        }
 
-        $str .= (($i == 0 || $str =~ /[. ]$/) ? '' : ':') . $capstr . $chron;
+    foreach my $key (@keys) {
+        my $skip_sep = ($str =~ /[. ]$/);
+        my $new_part = $self->format_single_chron($holdings, $key, $skip_sep);
+        last unless defined $new_part;
+        $str .= $new_part;
     }
 
     return $str;
 }
 
 #
+# Called by method 'format_part' for each enum subfield
+#
+sub format_single_enum {
+    my $self = shift;
+    my $holding_values = shift;
+    my $key = shift;
+    my $skip_sep = shift;
+    my $capstr;
+    my $chron;
+    my $sep = ':';
+
+    return if !defined $self->caption->capstr($key);
+
+    $capstr = $self->caption->capstr($key);
+    if (substr($capstr, 0, 1) eq '(') {
+        # a caption enclosed in parentheses is not displayed
+        $capstr = '';
+    } elsif ($skip_sep) {
+        # We'll let a $skip_sep parameter of true mean what it means down by
+        # the return statement AND to pad the caption itself here.
+        $capstr .= ' ';
+    }
+
+
+    $skip_sep ||= ($key eq 'a');
+    return ($skip_sep ? '' : $sep) . $capstr . $holding_values->{$key};
+}
+
+#
 # Called by method 'format' for each member of a possibly compressed holding
 #
 sub format_part {
@@ -321,19 +373,9 @@ sub format_part {
 
         # Enumerations
         foreach my $key ('a'..'f') {
-            my $capstr;
-            my $chron;
-            my $sep;
-
-            last if !defined $caption->capstr($key);
-
-            $capstr = $caption->capstr($key);
-            if (substr($capstr, 0, 1) eq '(') {
-                # a caption enclosed in parentheses is not displayed
-                $capstr = '';
-            }
-            $str .=
-              ($key eq 'a' ? '' : ':') . $capstr . $holding_values->{$key};
+            my $new_part = $self->format_single_enum($holding_values, $key);
+            last unless defined $new_part;
+            $str .= $new_part;
         }
 
         # Chronology
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm
index bc74bd1..abb8265 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm
@@ -70,13 +70,9 @@ sub load_record {
         $ctx->{get_org_setting}->
             ($org, "opac.fully_compressed_serial_holdings")
     ) {
-        $ctx->{holding_summaries} =
-            $self->get_holding_summaries($rec_id, $org, $copy_depth);
-
-        $ctx->{have_holdings_to_show} =
-            scalar(@{$ctx->{holding_summaries}->{basic}}) ||
-            scalar(@{$ctx->{holding_summaries}->{index}}) ||
-            scalar(@{$ctx->{holding_summaries}->{supplement}});
+        # We're loading this data here? Are we therefore assuming that we
+        # *are* going to display something in the "issues" expandy?
+        $self->load_serial_holding_summaries($rec_id, $org, $copy_depth);
     } else {
         $ctx->{mfhd_summaries} =
             $self->get_mfhd_summaries($rec_id, $org, $copy_depth);
@@ -92,9 +88,8 @@ sub load_record {
             $ctx->{marchtml} = $self->mk_marc_html($rec_id);
         },
         issues => sub {
-            $ctx->{expanded_holdings} =
-                $self->get_expanded_holdings($rec_id, $org, $copy_depth)
-                if $ctx->{have_holdings_to_show};
+            return;
+            # XXX this needed?
         },
         cnbrowse => sub {
             $self->prepare_browse_call_numbers();
@@ -152,78 +147,9 @@ sub mk_copy_query {
     my $copy_offset = shift;
     my $pref_ou = shift;
 
-    my $query = {
-        select => {
-            acp => ['id', 'barcode', 'circ_lib', 'create_date', 'age_protect', 'holdable'],
-            acpl => [
-                {column => 'name', alias => 'copy_location'},
-                {column => 'holdable', alias => 'location_holdable'}
-            ],
-            ccs => [
-                {column => 'name', alias => 'copy_status'},
-                {column => 'holdable', alias => 'status_holdable'}
-            ],
-            acn => [
-                {column => 'label', alias => 'call_number_label'},
-                {column => 'id', alias => 'call_number'}
-            ],
-            circ => ['due_date'],
-            acnp => [
-                {column => 'label', alias => 'call_number_prefix_label'},
-                {column => 'id', alias => 'call_number_prefix'}
-            ],
-            acns => [
-                {column => 'label', alias => 'call_number_suffix_label'},
-                {column => 'id', alias => 'call_number_suffix'}
-            ],
-            bmp => [
-                {column => 'label', alias => 'part_label'},
-            ]
-        },
-
-        from => {
-            acp => {
-                acn => { 
-                    join => { 
-                        acnp => { fkey => 'prefix' },
-                        acns => { fkey => 'suffix' }
-                    },
-                    filter => [{deleted => 'f'}, {record => $rec_id}],
-                },
-                circ => { # If the copy is circulating, retrieve the open circ
-                    type => 'left',
-                    filter => {checkin_time => undef}
-                },
-                acpl => {},
-                ccs => {},
-                aou => {},
-                acpm => {
-                    type => 'left',
-                    join => {
-                        bmp => { type => 'left' }
-                    }
-                }
-            }
-        },
-
-        where => {
-            '+acp' => {deleted => 'f' }
-        },
-
-        order_by => [
-            { class => "aou", field => 'id', 
-              transform => 'evergreen.rank_ou', params => [$org, $pref_ou]
-            },
-            {class => 'aou', field => 'name'}, 
-            {class => 'acn', field => 'label'},
-            { class => "acp", field => 'status',
-              transform => 'evergreen.rank_cp_status'
-            }
-        ],
-
-        limit => $copy_limit,
-        offset => $copy_offset
-    };
+    my $query = $U->basic_opac_copy_query(
+        $rec_id, undef, undef, $copy_limit, $copy_offset, $self->ctx->{is_staff}
+    );
 
     if($org != $self->ctx->{aou_tree}->()->id) { 
         # no need to add the org join filter if we're not actually filtering
@@ -247,13 +173,17 @@ sub mk_copy_query {
         };
     };
 
-    # Filter hidden items if this is the public catalog
-    unless($self->ctx->{is_staff}) { 
-        $query->{where}->{'+acp'}->{opac_visible} = 't';
-        $query->{from}->{'acp'}->{'acpl'}->{filter} = {opac_visible => 't'};
-        $query->{from}->{'acp'}->{'ccs'}->{filter} = {opac_visible => 't'};
-        $query->{where}->{'+aou'}->{opac_visible} = 't';
-    }
+    # Unsure if we want these in the shared function, leaving here for now
+    unshift(@{$query->{order_by}},
+        { class => "aou", field => 'id',
+          transform => 'evergreen.rank_ou', params => [$org, $pref_ou]
+        }
+    );
+    push(@{$query->{order_by}},
+        { class => "acp", field => 'status',
+          transform => 'evergreen.rank_cp_status'
+        }
+    );
 
     return $query;
 }
@@ -267,47 +197,101 @@ sub mk_marc_html {
         'open-ils.search.biblio.record.html', $rec_id, 1);
 }
 
-sub get_holding_summaries {
+sub load_serial_holding_summaries {
     my ($self, $rec_id, $org, $depth) = @_;
 
+    my $limit = $self->cgi->param("slimit") || 10;
+    my $offset = $self->cgi->param("soffset") || 0;
+
     my $serial = create OpenSRF::AppSession("open-ils.serial");
-    my $result = $serial->request(
-        "open-ils.serial.bib.summary_statements",
-        $rec_id, {"org_id" => $org, "depth" => $depth}
+
+    # First, get the tree of /summaries/ of holdings.
+    my $tree = $serial->request(
+        "open-ils.serial.holding_summary_tree.by_bib",
+        $rec_id, $org, $depth, $limit, $offset
     )->gather(1);
 
+    return if $self->apache_log_if_event(
+        $tree, "getting holding summary tree for record $rec_id"
+    );
+
+    # Next, if requested, get a list of individual holdings under a
+    # particular summary.
+    my $holdings;
+    my $summary_id = int($self->cgi->param("sid") || 0);
+    my $summary_type = $self->cgi->param("stype");
+
+    if ($summary_id and $summary_type) {
+        my $expand_path = [ $self->cgi->param("sepath") ],
+        my $expand_limit = $self->cgi->param("selimit");
+        my $expand_offsets = [ $self->cgi->param("seoffset") ];
+        my $auto_expand_first = 0;
+
+        if (not @$expand_offsets) {
+            $expand_offsets = undef;
+            $auto_expand_first = 1;
+        }
+
+        $holdings = $serial->request(
+            "open-ils.serial.holdings.grouped_by_summary",
+            $summary_type, $summary_id,
+            $expand_path, $expand_limit, $expand_offsets,
+            $auto_expand_first,
+            1 + ($self->ctx->{is_staff} ? 1 : 0)
+        )->gather(1);
+
+        if ($holdings and ref $holdings eq "ARRAY") {
+            $self->place_holdings_with_summary(
+                    $tree, $holdings, $summary_id, $summary_type
+            ) or $self->apache->log->warn(
+                "could not place holdings within summary tree"
+            );
+        } else {
+            $self->apache_log_if_event(
+                $holdings, "getting holdings grouped by summary $summary_id"
+            );
+        }
+    }
+
     $serial->kill_me;
-    return $result;
+
+    # The presence of any keys in the tree hash other than 'more' means that we
+    # must have /something/ we could show.
+    $self->ctx->{have_holdings_to_show} = grep { $_ ne 'more' } (keys %$tree);
+
+    $self->ctx->{holding_summary_tree} = $tree;
 }
 
-sub get_mfhd_summaries {
-    my ($self, $rec_id, $org, $depth) = @_;
+# This helper to load_serial_holding_summaries() recursively searches in
+# $tree for a holding summary matching $sid and $stype, and places $holdings
+# within the node for that summary. IOW, this is about showing expanded
+# holdings under their "parent" summary.
+sub place_holdings_with_summary {
+    my ($self, $tree, $holdings, $sid, $stype) = @_;
+
+    foreach my $sum (@{$tree->{holding_summaries}}) {
+        if ($sum->{id} == $sid and $sum->{summary_type} eq $stype) {
+            $sum->{holdings} = $holdings;
+            return 1;
+        }
+    }
 
-    my $serial = create OpenSRF::AppSession("open-ils.search");
-    my $result = $serial->request(
-        "open-ils.search.serial.record.bib.retrieve",
-        $rec_id, $org, $depth
-    )->gather(1);
+    foreach my $child (@{$tree->{children}}) {
+        return 1 if $self->place_holdings_with_summary(
+            $child, $holdings, $sid, $stype
+        );
+    }
 
-    $serial->kill_me;
-    return $result;
+    return;
 }
 
-sub get_expanded_holdings {
+sub get_mfhd_summaries {
     my ($self, $rec_id, $org, $depth) = @_;
 
-    my $holding_limit = int($self->cgi->param("holding_limit") || 10);
-    my $holding_offset = int($self->cgi->param("holding_offset") || 0);
-    my $type = $self->cgi->param("expand_holding_type");
-
-    my $serial =  create OpenSRF::AppSession("open-ils.serial");
+    my $serial = create OpenSRF::AppSession("open-ils.search");
     my $result = $serial->request(
-        "open-ils.serial.received_siss.retrieve.by_bib.atomic",
-        $rec_id, {
-            "ou" => $org, "depth" => $depth,
-            "limit" => $holding_limit, "offset" => $holding_offset,
-            "type" => $type
-        }
+        "open-ils.search.serial.record.bib.retrieve",
+        $rec_id, $org, $depth
     )->gather(1);
 
     $serial->kill_me;
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
index e9b61f4..f46587f 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
@@ -421,4 +421,27 @@ sub set_file_download_headers {
     return Apache2::Const::OK;
 }
 
+sub apache_log_if_event {
+    my ($self, $event, $prefix_text, $success_ok, $level) = @_;
+
+    $prefix_text ||= "Evergreen returned event";
+    $success_ok ||= 0;
+    $level ||= "warn";
+
+    chomp $prefix_text;
+    $prefix_text .= ": ";
+
+    my $code = $U->event_code($event);
+    if (defined $code and ($code or not $success_ok)) {
+        $self->apache->log->$level(
+            $prefix_text .
+            ($event->{textcode} || "") . " ($code)" .
+            ($event->{note} ? (": " . $event->{note}) : "")
+        );
+        return 1;
+    }
+
+    return;
+}
+
 1;
diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql
index eaf0e9f..4c4379a 100644
--- a/Open-ILS/src/sql/Pg/002.schema.config.sql
+++ b/Open-ILS/src/sql/Pg/002.schema.config.sql
@@ -46,6 +46,7 @@ INSERT INTO config.internal_flag (name) VALUES ('ingest.disable_metabib_full_rec
 INSERT INTO config.internal_flag (name) VALUES ('ingest.disable_metabib_rec_descriptor');
 INSERT INTO config.internal_flag (name) VALUES ('ingest.disable_metabib_field_entry');
 INSERT INTO config.internal_flag (name) VALUES ('ingest.assume_inserts_only');
+INSERT INTO config.internal_flag (name) VALUES ('serial.rematerialize_on_same_holding_code');
 
 CREATE TABLE config.global_flag (
     label   TEXT    NOT NULL
diff --git a/Open-ILS/src/sql/Pg/020.schema.functions.sql b/Open-ILS/src/sql/Pg/020.schema.functions.sql
index ca4306e..de70132 100644
--- a/Open-ILS/src/sql/Pg/020.schema.functions.sql
+++ b/Open-ILS/src/sql/Pg/020.schema.functions.sql
@@ -221,6 +221,35 @@ CREATE OR REPLACE FUNCTION actor.org_unit_common_ancestors ( INT, INT ) RETURNS
 	  FROM	actor.org_unit_ancestors($2);
 $$ LANGUAGE SQL STABLE ROWS 1;
 
+-- Given the IDs of two rows in actor.org_unit, *the second being an ancestor
+-- of the first*, return in array form the path from the ancestor to the
+-- descendant, with each point in the path being an org_unit ID.  This is
+-- useful for sorting org_units by their position in a depth-first (display
+-- order) representation of the tree.
+--
+-- This breaks with the precedent set by actor.org_unit_full_path() and others,
+-- and gets the parameters "backwards," but otherwise this function would
+-- not be very usable within json_query.
+CREATE OR REPLACE FUNCTION actor.org_unit_simple_path(INT, INT)
+RETURNS INT[] AS $$
+    WITH RECURSIVE descendant_depth(id, path) AS (
+        SELECT  aou.id,
+                ARRAY[aou.id]
+          FROM  actor.org_unit aou
+                JOIN actor.org_unit_type aout ON (aout.id = aou.ou_type)
+          WHERE aou.id = $2
+            UNION ALL
+        SELECT  aou.id,
+                dd.path || ARRAY[aou.id]
+          FROM  actor.org_unit aou
+                JOIN actor.org_unit_type aout ON (aout.id = aou.ou_type)
+                JOIN descendant_depth dd ON (dd.id = aou.parent_ou)
+    ) SELECT dd.path
+        FROM actor.org_unit aou
+        JOIN descendant_depth dd USING (id)
+        WHERE aou.id = $1 ORDER BY dd.path;
+$$ LANGUAGE SQL STABLE ROWS 1;
+
 CREATE OR REPLACE FUNCTION actor.org_unit_proximity ( INT, INT ) RETURNS INT AS $$
 	SELECT COUNT(id)::INT FROM (
 		SELECT id FROM actor.org_unit_combined_ancestors($1, $2)
diff --git a/Open-ILS/src/sql/Pg/210.schema.serials.sql b/Open-ILS/src/sql/Pg/210.schema.serials.sql
index 29617cd..8a82141 100644
--- a/Open-ILS/src/sql/Pg/210.schema.serials.sql
+++ b/Open-ILS/src/sql/Pg/210.schema.serials.sql
@@ -105,6 +105,8 @@ CREATE TABLE serial.distribution (
 	                              REFERENCES actor.org_unit (id)
 								  DEFERRABLE INITIALLY DEFERRED,
 	label                 TEXT    NOT NULL,
+	display_grouping      TEXT    NOT NULL DEFAULT 'chron'
+	                              CHECK (display_grouping IN ('enum', 'chron')),
 	receive_call_number   BIGINT  REFERENCES asset.call_number (id)
 	                              DEFERRABLE INITIALLY DEFERRED,
 	receive_unit_template INT     REFERENCES asset.copy_template (id)
@@ -326,5 +328,92 @@ CREATE TABLE serial.index_summary (
 );
 CREATE INDEX serial_index_summary_dist_idx ON serial.index_summary (distribution);
 
+CREATE VIEW serial.any_summary AS
+    SELECT
+        'basic' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.basic_summary
+    UNION
+    SELECT
+        'index' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.index_summary
+    UNION
+    SELECT
+        'supplement' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.supplement_summary ;
+
+
+CREATE TABLE serial.materialized_holding_code (
+    id BIGSERIAL PRIMARY KEY,
+    issuance INTEGER NOT NULL REFERENCES serial.issuance (id) ON DELETE CASCADE,
+    holding_type TEXT NOT NULL,
+    ind1 TEXT,
+    ind2 TEXT,
+    subfield CHAR,
+    value TEXT
+);
+
+CREATE OR REPLACE FUNCTION serial.materialize_holding_code() RETURNS TRIGGER
+AS $func$ 
+use strict;
+
+use MARC::Field;
+use JSON::XS;
+
+# Do nothing if holding_code has not changed...
+
+if ($_TD->{new}{holding_code} eq $_TD->{old}{holding_code}) {
+    # ... unless the following internal flag is set.
+
+    my $flag_rv = spi_exec_query(q{
+        SELECT * FROM config.internal_flag
+        WHERE name = 'serial.rematerialize_on_same_holding_code' AND enabled
+    }, 1);
+    return unless $flag_rv->{processed};
+}
+
+
+my $holding_code = (new JSON::XS)->decode($_TD->{new}{holding_code});
+
+my $field = new MARC::Field('999', @$holding_code); # tag doesnt matter
+
+my $dstmt = spi_prepare(
+    'DELETE FROM serial.materialized_holding_code WHERE issuance = $1',
+    'INT'
+);
+spi_exec_prepared($dstmt, $_TD->{new}{id});
+
+my $istmt = spi_prepare(
+    q{
+        INSERT INTO serial.materialized_holding_code (
+            issuance, holding_type, ind1, ind2, subfield, value
+        ) VALUES ($1, $2, $3, $4, $5, $6)
+    }, qw{INT TEXT TEXT TEXT CHAR TEXT}
+);
+
+foreach ($field->subfields) {
+    spi_exec_prepared(
+        $istmt,
+        $_TD->{new}{id},
+        $_TD->{new}{holding_type},
+        $field->indicator(1),
+        $field->indicator(2),
+        $_->[0],
+        $_->[1]
+    );
+}
+
+return;
+
+$func$ LANGUAGE 'plperlu';
+
+CREATE INDEX assist_holdings_display
+    ON serial.materialized_holding_code (issuance, subfield);
+
+CREATE TRIGGER materialize_holding_code
+    AFTER INSERT OR UPDATE ON serial.issuance
+    FOR EACH ROW EXECUTE PROCEDURE serial.materialize_holding_code() ;
 COMMIT;
 
diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql
index ccacf9f..2771725 100644
--- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql
+++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql
@@ -4664,6 +4664,21 @@ INSERT into config.org_unit_setting_type
         'description'
     ),
     'bool', null)
+,( 'serial.default_display_grouping', 'serial'
+    oils_i18n_gettext(
+        'serial.default_display_grouping',
+        'Default display grouping for serials distributions presented in the OPAC.',
+        'coust',
+        'label'
+    ),
+    oils_i18n_gettext(
+        'serial.default_display_grouping',
+        'Default display grouping for serials distributions presented in the OPAC. This can be "enum" or "chron".',
+        'coust',
+        'description'
+    ),
+    'string', null)
+
 ;
 
 UPDATE config.org_unit_setting_type
@@ -10117,7 +10132,6 @@ INSERT INTO config.usr_setting_type (name,grp,opac_visible,label,description,dat
     ),
     'string'
 );
-
 SELECT setval( 'config.sms_carrier_id_seq', 1000 );
 INSERT INTO config.sms_carrier VALUES
 
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
new file mode 100644
index 0000000..f27fc79
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-groups.sql
@@ -0,0 +1,155 @@
+BEGIN;
+
+SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+
+INSERT INTO config.internal_flag (name, value, enabled) VALUES (
+    'serial.rematerialize_on_same_holding_code', NULL, FALSE
+);
+
+INSERT INTO config.org_unit_setting_type (
+    name, label, grp, description, datatype
+) VALUES (
+    'serial.default_display_grouping',
+    'Default display grouping for serials distributions presented in the OPAC.',
+    'serial',
+    'Default display grouping for serials distributions presented in the OPAC. This can be "enum" or "chron".',
+    'string'
+);
+
+ALTER TABLE serial.distribution
+    ADD COLUMN display_grouping TEXT NOT NULL DEFAULT 'chron'
+        CHECK (display_grouping IN ('enum', 'chron'));
+
+-- why didn't we just make one summary table in the first place?
+CREATE VIEW serial.any_summary AS
+    SELECT
+        'basic' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.basic_summary
+    UNION
+    SELECT
+        'index' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.index_summary
+    UNION
+    SELECT
+        'supplement' AS summary_type, id, distribution,
+        generated_coverage, textual_holdings, show_generated
+    FROM serial.supplement_summary ;
+
+
+-- Given the IDs of two rows in actor.org_unit, *the second being an ancestor
+-- of the first*, return in array form the path from the ancestor to the
+-- descendant, with each point in the path being an org_unit ID.  This is
+-- useful for sorting org_units by their position in a depth-first (display
+-- order) representation of the tree.
+--
+-- This breaks with the precedent set by actor.org_unit_full_path() and others,
+-- and gets the parameters "backwards," but otherwise this function would
+-- not be very usable within json_query.
+CREATE OR REPLACE FUNCTION actor.org_unit_simple_path(INT, INT)
+RETURNS INT[] AS $$
+    WITH RECURSIVE descendant_depth(id, path) AS (
+        SELECT  aou.id,
+                ARRAY[aou.id]
+          FROM  actor.org_unit aou
+                JOIN actor.org_unit_type aout ON (aout.id = aou.ou_type)
+          WHERE aou.id = $2
+            UNION ALL
+        SELECT  aou.id,
+                dd.path || ARRAY[aou.id]
+          FROM  actor.org_unit aou
+                JOIN actor.org_unit_type aout ON (aout.id = aou.ou_type)
+                JOIN descendant_depth dd ON (dd.id = aou.parent_ou)
+    ) SELECT dd.path
+        FROM actor.org_unit aou
+        JOIN descendant_depth dd USING (id)
+        WHERE aou.id = $1 ORDER BY dd.path;
+$$ LANGUAGE SQL STABLE ROWS 1;
+
+CREATE TABLE serial.materialized_holding_code (
+    id BIGSERIAL PRIMARY KEY,
+    issuance INTEGER NOT NULL REFERENCES serial.issuance (id) ON DELETE CASCADE,
+    holding_type TEXT NOT NULL,
+    ind1 TEXT,
+    ind2 TEXT,
+    subfield CHAR,
+    value TEXT
+);
+
+CREATE OR REPLACE FUNCTION serial.materialize_holding_code() RETURNS TRIGGER
+AS $func$ 
+use strict;
+
+use MARC::Field;
+use JSON::XS;
+
+# Do nothing if holding_code has not changed...
+
+if ($_TD->{new}{holding_code} eq $_TD->{old}{holding_code}) {
+    # ... unless the following internal flag is set.
+
+    my $flag_rv = spi_exec_query(q{
+        SELECT * FROM config.internal_flag
+        WHERE name = 'serial.rematerialize_on_same_holding_code' AND enabled
+    }, 1);
+    return unless $flag_rv->{processed};
+}
+
+
+my $holding_code = (new JSON::XS)->decode($_TD->{new}{holding_code});
+
+my $field = new MARC::Field('999', @$holding_code); # tag doesnt matter
+
+my $dstmt = spi_prepare(
+    'DELETE FROM serial.materialized_holding_code WHERE issuance = $1',
+    'INT'
+);
+spi_exec_prepared($dstmt, $_TD->{new}{id});
+
+my $istmt = spi_prepare(
+    q{
+        INSERT INTO serial.materialized_holding_code (
+            issuance, holding_type, ind1, ind2, subfield, value
+        ) VALUES ($1, $2, $3, $4, $5, $6)
+    }, qw{INT TEXT TEXT TEXT CHAR TEXT}
+);
+
+foreach ($field->subfields) {
+    spi_exec_prepared(
+        $istmt,
+        $_TD->{new}{id},
+        $_TD->{new}{holding_type},
+        $field->indicator(1),
+        $field->indicator(2),
+        $_->[0],
+        $_->[1]
+    );
+}
+
+return;
+
+$func$ LANGUAGE 'plperlu';
+
+CREATE INDEX assist_holdings_display
+    ON serial.materialized_holding_code (issuance, subfield);
+
+CREATE TRIGGER materialize_holding_code
+    AFTER INSERT OR UPDATE ON serial.issuance
+    FOR EACH ROW EXECUTE PROCEDURE serial.materialize_holding_code() ;
+
+-- starting here, we materialize all existing holding codes.
+
+UPDATE config.internal_flag
+    SET enabled = TRUE
+    WHERE name = 'serial.rematerialize_on_same_holding_code';
+
+UPDATE serial.issuance SET holding_code = holding_code;
+
+UPDATE config.internal_flag
+    SET enabled = FALSE
+    WHERE name = 'serial.rematerialize_on_same_holding_code';
+
+-- finish holding code materialization process
+
+COMMIT;
diff --git a/Open-ILS/src/templates/opac/parts/header.tt2 b/Open-ILS/src/templates/opac/parts/header.tt2
index d67a1d3..ba75412 100644
--- a/Open-ILS/src/templates/opac/parts/header.tt2
+++ b/Open-ILS/src/templates/opac/parts/header.tt2
@@ -26,7 +26,10 @@
     #
     # Current page, clear 'some_param' from the existing params:
     # mkurl('', {foo => 'bar', boo => 'baz'}, ['some_param']);
-    MACRO mkurl(page, params, clear_params) BLOCK;
+    #
+    # Current page to a named anchor 'copies'
+    # mkurl('', {}, [], 'copies');
+    MACRO mkurl(page, params, clear_params, named_anchor) BLOCK;
 
         # clone the query string to avoid clobberation
         cgi = CGI.new(CGI.query_string);
@@ -46,7 +49,12 @@
         FOR k IN params.keys;
             encoded = [];
             max = params.$k.max;
-            list = (params.$k.0 OR max == -1) ? params.$k : [params.$k];
+
+            # The following commented-out line can be fooled. Its replacement
+            # below is what you really mean.
+            # list = (params.$k.0 OR max == -1) ? params.$k : [params.$k];
+            list = params.$k.list;
+
             IF list.size == 0; NEXT; END;
             # CGI croaks on already-decoded strings.  force-encode to be safe.
             FOR p IN list; encoded.push(ctx.encode_utf8(p)); END;
@@ -64,15 +72,17 @@
             END;
         END;
 
+        final = named_anchor ? '#' _ named_anchor : '';
+
         IF page;
             IF cgi.query_string;
-                page _ '?' _ cgi.query_string;
+                page _ '?' _ cgi.query_string _ final;
             ELSE;
-                page;
+                page _ final;
             END;
         ELSE;
             # staying on the current page
-            cgi.url("-path" => 1, "-query" => 1);
+            cgi.url("-path" => 1, "-query" => 1) _ final;
         END;
     END;
 
diff --git a/Open-ILS/src/templates/opac/parts/record/copy_table.tt2 b/Open-ILS/src/templates/opac/parts/record/copy_table.tt2
index 240f970..b806f5c 100644
--- a/Open-ILS/src/templates/opac/parts/record/copy_table.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/copy_table.tt2
@@ -1,5 +1,19 @@
 [%-
-FOREACH copy_info IN ctx.copies;
+
+# If being used in serial mode, flatten list of units so that they can be
+# used like one long list of copies without changing so much code below.
+IF serial_holdings;
+    copies = [];
+    FOREACH h IN serial_holdings;
+        units = h.units.slice(0); # copy
+        FOREACH unit IN units;
+            unit.holding_label = h.label;
+        END;
+        copies = copies.merge(units);
+    END;
+END;
+
+FOREACH copy_info IN copies;
     IF copy_info.call_number_label != '##URI##';
         has_copies = 'true';
     END;
@@ -10,12 +24,16 @@ FOREACH copy_info IN ctx.copies;
         LAST;
     END;
 END;
-%]
+-%]
 [%- IF has_copies; %]
 <table cellpadding="0" cellspacing="0" border="0" width="100%" id="rdetails_status">
     <thead>
         <tr>
+            [% IF serial_holdings -%]
+            <th id='copy_header_holding_label'>[% l("Issue Label") %]</th>
+            [%- ELSE -%]
             <th id='copy_header_library'>[% l("Location") %]</th>
+            [%- END %]
             <th id='copy_header_callnmber'>[% l("Call Number") %]</th>
             [%- IF has_parts == 'true' %]
             <th id='copy_header_part'>[% l("Part") %]</th>
@@ -25,6 +43,8 @@ END;
             [%- IF ctx.is_staff %]
             <th id='copy_header_age_hold'>[% l("Age Hold Protection") %]</th>
             <th id='copy_header_create_date'>[% l("Create Date") %]</th>
+            [%- END %]
+            [%- IF ctx.is_staff OR serial_holdings %]
             <th id='copy_header_holdable'>[% l("Holdable?") %]</th>
             [%- END %]
             <th id='copy_header_status'>[% l("Status") %]</th>
@@ -33,7 +53,7 @@ END;
     </thead>
     <tbody class="copy_details_table">
         [%- last_cn = 0;
-        FOR copy_info IN ctx.copies;
+        FOR copy_info IN copies;
             callnum = copy_info.call_number_label;
             NEXT IF callnum == '##URI##';
 
@@ -48,13 +68,16 @@ END;
             END;
         -%]
         <tr>
-            <td header='copy_header_library'>
+            [%- IF serial_holdings %]<td header='copy_header_holding_label' class='rdetail-issue-issue'>
+                [%- copy_info.holding_label | html; -%]
+            </td>
+            [%- ELSE %]<td header='copy_header_library'>
             [%-
                 org_name = ctx.get_aou(copy_info.circ_lib).name;
                 org_name | html
             -%]
-            </td>
-            <td header='copy_header_callnumber'>[% callnum | html %] [% IF ctx.get_org_setting(ctx.search_ou, 'sms.enable') == 1 %](<a href="[% mkurl(ctx.opac_root _ '/sms_cn', {copy_id => copy_info.id}) %]">Text</a>)[% END %]</td>
+            </td>[% END %]
+            <td header='copy_header_callnumber'>[% callnum | html %] [% IF ctx.get_org_setting(CGI.param('loc') OR ctx.aou_tree.id, 'sms.enable') == 1 %](<a href="[% mkurl(ctx.opac_root _ '/sms_cn', {copy_id => copy_info.id}) %]">Text</a>)[% END %]</td>
             [%- IF has_parts == 'true' %]
             <td header='copy_header_part'>[% copy_info.part_label | html %]</td>
             [%- END %]
@@ -73,13 +96,18 @@ END;
                 ctx.parse_datetime(copy_info.create_date),
                 DATE_FORMAT
             ) %]</td>
+            [% END # is_staff %]
+            [% IF ctx.is_staff OR serial_holdings %]
             <td header='copy_header_holdable'>[%  # Show copy/volume hold links to staff (without
                     # checking whether they have permissions to do those).
-                    overall_holdable = (copy_info.holdable == 't' AND
+                    overall_holdable = (
+                        copy_info.holdable == 't' AND
                         copy_info.location_holdable == 't' AND
                         copy_info.status_holdable == 't');
                     IF overall_holdable;
-                        l("Place on"); %]
+                        l("Place on");
+                        IF ctx.is_staff;
+                    %]
                 <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
                     {hold_target => copy_info.id, hold_type => 'C', hold_source_page => mkurl()}) %]">[% l("copy") %]</a>
                 [%-      IF copy_info.call_number != last_cn;
@@ -88,6 +116,18 @@ END;
                 <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
                     {hold_target => copy_info.call_number, hold_type => 'V', hold_source_page => mkurl()}) %]">[% l("volume") %]</a>
                 [%-      END;
+                         IF serial_holdings;
+                            l(" / ");
+                         END;
+                        END;
+                        IF serial_holdings;
+                %]
+                <a class="rdetail-issue-place-hold"
+                    href="[% mkurl(ctx.opac_root _ '/place_hold', {
+                    hold_target => copy_info.issuance, hold_type => 'I',
+                    hold_source_page => mkurl()
+                }) %]">[% l("issue") %]</a>[%-
+                        END;
                     ELSE;
                         l("No");
                     END %]</td>
@@ -105,7 +145,7 @@ END;
         </tr>
         [%- END %]
         <tr>
-        [%- IF ctx.copy_offset > 0;
+        [%- IF ctx.copy_offset > 0 AND NOT serial_holdings;
             new_offset = ctx.copy_offset - ctx.copy_limit;
             IF new_offset < 0; new_offset = 0; END %]
             <td>
@@ -113,17 +153,18 @@ END;
                     l('Previous [_1]', ctx.copy_offset - new_offset) %]</a>
             </td>
         [%- END %]
-        [%- IF ctx.copies.size >= ctx.copy_limit %]
+        [%- IF copies.size >= ctx.copy_limit AND NOT serial_holdings %]
             <td>
                 <a href="[% mkurl('', {copy_offset => ctx.copy_offset + ctx.copy_limit, copy_limit => ctx.copy_limit}) %]">[%
                     l('Next [_1]', ctx.copy_limit) %] &raquo;</a>
             </td>
         [%- END %]
         </tr>
+        [% IF NOT serial_holdings -%]
         <tr>
             <td>
                 [%- more_copies_limit = 50 %] [%# TODO: config %]
-                [%- IF  ctx.copy_limit != more_copies_limit AND ctx.copies.size >= ctx.copy_limit %]
+                [%- IF  ctx.copy_limit != more_copies_limit AND copies.size >= ctx.copy_limit %]
                     <div class="rdetail_show_copies">
                         <img src="[% ctx.media_prefix %]/images/plus_sign.png" />
                         <a href="[% mkurl('', {copy_limit => more_copies_limit, copy_offset => 0}) %]">[% l('Show more copies') %]</a>
@@ -136,6 +177,7 @@ END;
                 [%- END %]
             </td>
         </tr>
+        [%- END %]
     </tbody>
 </table>
 [% END; %]
diff --git a/Open-ILS/src/templates/opac/parts/record/issues-db.tt2 b/Open-ILS/src/templates/opac/parts/record/issues-db.tt2
new file mode 100644
index 0000000..6e9179c
--- /dev/null
+++ b/Open-ILS/src/templates/opac/parts/record/issues-db.tt2
@@ -0,0 +1,161 @@
+[%-
+expand_path = CGI.param('sepath') || [];
+expand_path = expand_path.list; # sic
+
+seoffset_list = CGI.param('seoffset') || [];
+seoffset_list = seoffset_list.list; # sic
+
+IF expand_path.size == 0 AND seoffset_list.size == 0;
+    seoffset_list = [0,0]; # compensate for $auto_expand_first; see ML
+END;
+
+selimit = CGI.param('selimit') || 10;
+ght_sepath = [];
+ght_depth = 0;
+
+VIEW grouped_holding_tree;
+    BLOCK list;
+        '<div class="rdetail-holding-group">';
+        prev_seoffset_list = seoffset_list.slice(0, ght_depth);
+        next_seoffset_list = seoffset_list.slice(0, ght_depth);
+
+        prev_seoffset_list.$ght_depth = prev_seoffset_list.$ght_depth - selimit;
+        IF prev_seoffset_list.$ght_depth < 0;
+            prev_seoffset_list.$ght_depth = 0;
+        END;
+
+        next_seoffset_list.$ght_depth = next_seoffset_list.$ght_depth + selimit;
+        IF item.0.units.size;
+            INCLUDE "opac/parts/record/copy_table.tt2" serial_holdings=item;
+            "<hr />";
+            "</div>";
+        ELSE;
+            FOREACH node IN item;
+                IF NOT node.label;
+                    has_more = 1;
+                    LAST;
+                END;
+
+                IF node.value;
+                    ght_sepath.push(node.value);
+                    new_seoffsets = seoffset_list.slice(0, ght_depth);
+                    new_seoffsets.push(0);
+
+                    expand_link = mkurl(
+                        '', {'sepath' => ght_sepath, 'seoffset' => new_seoffsets},
+                        0, 'issues'
+                    );
+
+                    collapse_sepath = ght_sepath.slice(0, -2);
+                    IF collapse_sepath.size == 0;
+                        collapse_clear_params = ['sepath'];
+                    ELSE;
+                        collapse_clear_params = 0;
+                    END;
+
+                    collapse_link = mkurl(
+                        '', {
+                            'sepath' => collapse_sepath,
+                            'seoffset' => new_seoffsets.slice(0, -2)
+                        }, collapse_clear_params, 'issues'
+                    );
+
+                    "<div class='rdetail-holding-group'>";
+                    IF node.children.size;
+                        # TODO: make images or figure out a CSS trick or
+                        # something. I doubt we can count on all OPAC clients
+                        # having adequate fonts to cover these Unicode glyphs.
+                        "&#x25bc; <a href='"; collapse_link;
+                    ELSE;
+                        "&#x25ba; <a href='"; expand_link;
+                    END;
+                    "'>"; node.label; "</a></div>";
+
+                    IF node.children.size;
+                        ght_depth = ght_depth + 1;
+                        view.print(node.children);
+                        ght_depth = ght_depth - 1;
+                    END;
+
+                    waste = ght_sepath.pop;
+                ELSE;
+                    "<div class='rdetail-holding-group'>"; node.label; "</div>";
+                    # XXX Hold placement link here? Maybe not if no units.
+                END;
+            END;
+
+            to_clear = 0;
+            new_sepath_end = ght_depth - 1;
+            IF new_sepath_end < 0;
+                to_clear = ['sepath'];
+                new_sepath = [];
+            ELSE;
+                new_sepath = expand_path.slice(0, ght_depth - 1);
+            END;
+
+            IF has_more;
+                '<a class="paging" href="';
+                    mkurl('',{seoffset => next_seoffset_list, sepath => new_sepath},to_clear,'issues');
+                '">&laquo; '; l('Earlier holdings'); '</a>';
+            END;
+            IF seoffset_list.$ght_depth > 0;
+                '<a class="paging" href="';
+                    mkurl('',{seoffset => prev_seoffset_list, sepath => new_sepath},to_clear,'issues');
+                '">'; l('Later holdings'); ' &raquo;</a>&nbsp; ';
+            END;
+            '</div>';
+        END;
+    END;
+END;
+
+VIEW holding_summary_tree;
+    BLOCK hash;
+        '<div class="rdetail-holding-group">';
+        ctx.get_aou(item.org_unit).name; "<br />";
+        FOREACH summary IN item.holding_summaries;
+            IF summary.holdings;
+                twisty = '&#x25bc; ';
+                link = mkurl(
+                    '', {},
+                    ['sid','stype','selimit','sepath','seoffset'], 'issues'
+                );
+                link_title = l('Collapse');
+            ELSE;
+                twisty = '&#x25ba; ';
+                link = mkurl(
+                    '', {sid => summary.id, stype => summary.summary_type},
+                    ['selimit','sepath','seoffset'], 'issues'
+                );
+                link_title = l('Expand');
+            END;
+            '<span>'; twisty;
+            '<a href="' _ link _ '" title="' _ link_title _ '">';
+            summary.generated_coverage.join(", ");
+            '</a></span><br />';
+            IF summary.holdings;
+                grouped_holding_tree.print(summary.holdings);
+            END;
+        END;
+        FOREACH child IN item.children;
+            view.print(child);
+        END;
+        '</div>';
+    END;
+END %]
+    <div class="holding-summary-tree">
+    [% holding_summary_tree.print(ctx.holding_summary_tree) %]
+    </div>
+    <div class="holding-summary-tree-pager">
+        [%  slimit = CGI.param('slimit') || 10;
+            soffset = CGI.param('soffset') || 0;
+            soffset_prev = soffset - slimit;
+            IF soffset_prev < 0; soffset_prev = 0; END;
+            soffset_next = soffset + slimit;
+        %]
+        [% IF soffset > 0 %]
+        <a href="[% mkurl('', {soffset => soffset_prev}, ['sid','stype','sepath','selimit','seoffset'], 'issues') %]>[% l('Previous') %]</a>
+        [% END %]
+        [% IF ctx.holding_summary_tree.more %]
+        <a href="[% mkurl('', {soffset => soffset_next}, ['sid','stype','sepath','selimit','seoffset'], 'issues') %]">[% l('Next') %]</a>
+        [% END %]
+    </div>
diff --git a/Open-ILS/src/templates/opac/parts/record/issues-mfhd.tt2 b/Open-ILS/src/templates/opac/parts/record/issues-mfhd.tt2
new file mode 100644
index 0000000..15b9ab9
--- /dev/null
+++ b/Open-ILS/src/templates/opac/parts/record/issues-mfhd.tt2
@@ -0,0 +1,40 @@
+[% IF ctx.mfhd_summaries.size; %]
+    <div class="rdetail-mfhd-holdings">
+        <table><tbody>
+[%
+        mfhd = {
+            basic_holdings = l('Volumes'),
+            basic_holdings_add = l('Additional Volume Information'),
+            supplement_holdings = l('Supplements'),
+            supplement_holdings_add = l('Additional Supplement Information'),
+            index_holdings = l('Indexes'),
+            index_holdings_add = l('Additional Index Information'),
+            online = l('Online'),
+            missing = l('Missing'),
+            incomplete = l('Incomplete'),
+        };
+
+        FOREACH serial IN ctx.mfhd_summaries;
+%]
+            <tr>
+                <td class="rdetail-mfhd-head" colspan="2">[% l('Holdings summary ([_1])', serial.location) %]</td>
+            </tr>
+[%
+            FOREACH type IN mfhd.keys;
+                NEXT UNLESS serial.$type.size;
+%]
+            <tr>
+                <td class="rdetail-mfhd-type">[% mfhd.$type %]</td>
+                <td class="rdetail-mfhd-contents">[%
+                    FOR thing IN serial.$type;
+                        thing.join(", ");
+                    END %]</td>
+            </tr>
+        [% END %]
+            <tr>
+                <td class="rdetail-mfhd-foot" colspan="2"> </td>
+            </tr>
+    [% END %]
+        </tbody></table>
+    </div>
+[% END %]
diff --git a/Open-ILS/src/templates/opac/parts/record/issues.tt2 b/Open-ILS/src/templates/opac/parts/record/issues.tt2
index f346587..585cee6 100644
--- a/Open-ILS/src/templates/opac/parts/record/issues.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/issues.tt2
@@ -1,67 +1,4 @@
 <div class='rdetail_extras_div'>
-[%
-base_expando = ctx.full_path _ "?expand=issues";
-FOREACH type IN ctx.holding_summaries.keys;
-    NEXT UNLESS ctx.holding_summaries.$type.size;
-    expanded = CGI.param('expand_holding_type') == type; %]
-    <div class="rdetail-issue-type">
-        <a href="[% base_expando; expanded ? '' : '&amp;expand_holding_type=' _ type; %]#issues">[[% expanded ? '-' : '+' %]]</a>
-        [% ctx.holding_summaries.$type.join(", ") %]
-        [% IF expanded %]
-        <table>
-            [% FOR blob IN ctx.expanded_holdings %]
-            <tr>
-                <td class="rdetail-issue-issue">[% blob.issuance.label | html %]</td>
-                [% IF blob.has_units %]
-                <td class="rdetail-issue-place-hold">
-                    <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
-                        {hold_target => blob.issuance.id, hold_type => 'I', hold_source_page => mkurl()}) %]">[% l("Place Hold") %]</a>
-                </td>
-                [% END %]
-            </tr>
-            [% END %]
-        </table>
-        [% END %]
-    </div>
-[% END %]
-[% IF ctx.mfhd_summaries.size; %]
-    <div class="rdetail-mfhd-holdings">
-        <table><tbody>
-[%
-        mfhd = {
-            basic_holdings = l('Volumes'),
-            basic_holdings_add = l('Additional Volume Information'),
-            supplement_holdings = l('Supplements'),
-            supplement_holdings_add = l('Additional Supplement Information'),
-            index_holdings = l('Indexes'),
-            index_holdings_add = l('Additional Index Information'),
-            online = l('Online'),
-            missing = l('Missing'),
-            incomplete = l('Incomplete'),
-        };
-
-        FOREACH serial IN ctx.mfhd_summaries;
-%]
-            <tr>
-                <td class="rdetail-mfhd-head" colspan="2">[% l('Holdings summary ([_1])', serial.location) %]</td>
-            </tr>
-[%
-            FOREACH type IN mfhd.keys;
-                NEXT UNLESS serial.$type.size;
-%]
-            <tr>
-                <td class="rdetail-mfhd-type">[% mfhd.$type %]</td>
-                <td class="rdetail-mfhd-contents">[%
-                    FOR thing IN serial.$type;
-                        thing.join(", ");
-                    END %]</td>
-            </tr>
-        [% END %]
-            <tr>
-                <td class="rdetail-mfhd-foot" colspan="2"> </td>
-            </tr>
-    [% END %]
-        </tbody></table>
-    </div>
-[% END %]
+    [% INCLUDE 'opac/parts/record/issues-db.tt2' # "new" serials holdings %]
+    [% INCLUDE 'opac/parts/record/issues-mfhd.tt2' # mfhd-based "classic" serials %]
 </div>
diff --git a/Open-ILS/src/templates/opac/parts/record/summary.tt2 b/Open-ILS/src/templates/opac/parts/record/summary.tt2
index 69a9f38..062bf2f 100644
--- a/Open-ILS/src/templates/opac/parts/record/summary.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/summary.tt2
@@ -106,7 +106,7 @@ IF num_uris > 0;
                 ctx.record_hold_count, ctx.copy_summary.0.count) %]
         </p>
     </span>
-[%- INCLUDE "opac/parts/record/copy_table.tt2" %]
+[%- INCLUDE "opac/parts/record/copy_table.tt2" copies=ctx.copies %]
 </div>
 [%- END %]
 
diff --git a/Open-ILS/src/templates/serial/subscription.tt2 b/Open-ILS/src/templates/serial/subscription.tt2
index 47eb714..d4bc789 100644
--- a/Open-ILS/src/templates/serial/subscription.tt2
+++ b/Open-ILS/src/templates/serial/subscription.tt2
@@ -75,6 +75,40 @@
                     new dijit.form.TextBox({
                         "disabled": true, "value": sub_id
                     });
+
+                var _display_grouping_store = new dojo.data.ItemFileReadStore({
+                    "data": {
+                        "identifier": "display_grouping",
+                        "label": "label",
+                        "items": [
+                            {"display_grouping": "chron",
+                                "label": "Chronology"},
+                            {"display_grouping": "enum",
+                                "label": "Enumeration"}
+                        ]
+                    }
+                });
+
+                var settings = fieldmapper.aou.fetchOrgSettingBatch(
+                    openils.User.user.ws_ou(),
+                    ["serial.default_display_grouping"]
+                );
+                var default_display_grouping = "chron";
+
+                if (settings && settings["serial.default_display_grouping"]) {
+                    default_display_grouping =
+                        settings["serial.default_display_grouping"].value;
+                }
+
+                dist_grid.overrideEditWidgets.display_grouping =
+                    new dijit.form.FilteringSelect({
+                        "store": _display_grouping_store,
+                        "searchAttr": "label",
+                        "name": "display_grouping"
+                    });
+                    dist_grid.overrideEditWidgets.display_grouping.shove = {
+                        "create": default_display_grouping
+                    };
             }
         </script>
         [% INCLUDE "serial/subscription/distribution.tt2" %]
diff --git a/Open-ILS/web/css/skin/default/opac/style.css b/Open-ILS/web/css/skin/default/opac/style.css
index f36511c..62b677f 100644
--- a/Open-ILS/web/css/skin/default/opac/style.css
+++ b/Open-ILS/web/css/skin/default/opac/style.css
@@ -1422,3 +1422,6 @@ a.preflib_change {
   line-height: normal;
   text-decoration: none;
 }
+.rdetail-holding-group { margin-left: 1.5em; }
+.rdetail-holding-group span { margin-left: 1.5em; }
+.rdetail-holding-group .paging { margin-left: 1.5em; }

-----------------------------------------------------------------------

Summary of changes:
 Open-ILS/examples/fm_IDL.xml                       |   28 +-
 Open-ILS/src/extras/ils_events.xml                 |    5 +
 .../perlmods/lib/OpenILS/Application/AppUtils.pm   |  130 ++++-
 .../src/perlmods/lib/OpenILS/Application/Serial.pm |    9 +-
 .../lib/OpenILS/Application/Serial/OPAC.pm         |  639 ++++++++++++++++++++
 .../src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm |  164 ++++--
 .../perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm |  214 +++----
 .../perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm   |   23 +
 Open-ILS/src/sql/Pg/002.schema.config.sql          |    3 +-
 Open-ILS/src/sql/Pg/020.schema.functions.sql       |   29 +
 Open-ILS/src/sql/Pg/210.schema.serials.sql         |   89 +++
 Open-ILS/src/sql/Pg/950.data.seed-values.sql       |   16 +-
 .../upgrade/0700.schema.serial-holding-groups.sql  |  155 +++++
 .../sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql  |  152 +++++
 Open-ILS/src/templates/opac/parts/header.tt2       |   20 +-
 .../src/templates/opac/parts/record/copy_table.tt2 |   64 ++-
 .../src/templates/opac/parts/record/issues-db.tt2  |  199 ++++++
 .../templates/opac/parts/record/issues-mfhd.tt2    |   40 ++
 .../src/templates/opac/parts/record/issues.tt2     |   71 +--
 .../src/templates/opac/parts/record/summary.tt2    |    2 +-
 Open-ILS/src/templates/serial/subscription.tt2     |   34 +
 Open-ILS/web/css/skin/default/opac/style.css       |    3 +
 .../xul/staff_client/server/serial/siss_editor.js  |   11 +-
 23 files changed, 1829 insertions(+), 271 deletions(-)
 create mode 100644 Open-ILS/src/perlmods/lib/OpenILS/Application/Serial/OPAC.pm
 create mode 100644 Open-ILS/src/sql/Pg/upgrade/0700.schema.serial-holding-groups.sql
 create mode 100644 Open-ILS/src/templates/opac/parts/record/issues-db.tt2
 create mode 100644 Open-ILS/src/templates/opac/parts/record/issues-mfhd.tt2


hooks/post-receive
-- 
Evergreen ILS


More information about the open-ils-commits mailing list