LP#1386347: Clear hold-copy-map efficiently
[Evergreen.git] / Open-ILS / src / perlmods / lib / OpenILS / Application / Circ / Holds.pm
index 7b63cfd..1ec6066 100644 (file)
@@ -36,6 +36,7 @@ use DateTime::Format::ISO8601;
 use OpenSRF::Utils qw/:datetime/;
 use Digest::MD5 qw(md5_hex);
 use OpenSRF::Utils::Cache;
+use OpenSRF::Utils::JSON;
 my $apputils = "OpenILS::Application::AppUtils";
 my $U = $apputils;
 
@@ -69,8 +70,12 @@ __PACKAGE__->register_method(
 sub test_and_create_hold_batch {
     my( $self, $conn, $auth, $params, $target_list, $oargs ) = @_;
 
-    my $override = 1 if $self->api_name =~ /override/;
-    $oargs = { all => 1 } unless defined $oargs;
+    my $override = 0;
+    if ($self->api_name =~ /override/) {
+        $override = 1;
+        $oargs = { all => 1 } unless defined $oargs;
+        $$params{oargs} = $oargs; # for is_possible checking.
+    }
 
     my $e = new_editor(authtoken=>$auth);
     return $e->die_event unless $e->checkauth;
@@ -87,8 +92,15 @@ sub test_and_create_hold_batch {
     elsif ($$params{'hold_type'} eq 'P') { $target_field = 'partid'; }
     else { return undef; }
 
+    my $formats_map = delete $$params{holdable_formats_map};
+
     foreach (@$target_list) {
         $$params{$target_field} = $_;
+
+        # copy the requested formats from the target->formats map
+        # into the top-level formats attr for each hold
+        $$params{holdable_formats} = $formats_map->{$_};
+
         my $res;
         ($res) = $self->method_lookup(
             'open-ils.circ.title_hold.is_possible')->run($auth, $params, $override ? $oargs : {});
@@ -96,6 +108,11 @@ sub test_and_create_hold_batch {
 
             $params->{'depth'} = $res->{'depth'} if $res->{'depth'};
 
+            # Remove oargs from params so holds can be created.
+            if ($$params{oargs}) {
+                delete $$params{oargs};
+            }
+
             my $ahr = construct_hold_request_object($params);
             my ($res2) = $self->method_lookup(
                 $override
@@ -230,8 +247,11 @@ sub create_hold {
     my $e = new_editor(authtoken=>$auth, xact=>1);
     return $e->die_event unless $e->checkauth;
 
-    my $override = 1 if $self->api_name =~ /override/;
-    $oargs = { all => 1 } unless defined $oargs;
+    my $override = 0;
+    if ($self->api_name =~ /override/) {
+        $override = 1;
+        $oargs = { all => 1 } unless defined $oargs;
+    }
 
     my @events;
 
@@ -326,6 +346,31 @@ sub create_hold {
         $hold->expire_time(calculate_expire_time($recipient->home_ou));
     }
 
+
+    # if behind-the-desk pickup is supported at the hold pickup lib,
+    # set the value to the patron default, unless a value has already
+    # been applied.  If it's not supported, force the value to false.
+
+    my $bdous = $U->ou_ancestor_setting_value(
+        $hold->pickup_lib, 
+        'circ.holds.behind_desk_pickup_supported', $e);
+
+    if ($bdous) {
+        if (!defined $hold->behind_desk) {
+
+            my $set = $e->search_actor_user_setting({
+                usr => $hold->usr, 
+                name => 'circ.holds_behind_desk'
+            })->[0];
+        
+            $hold->behind_desk('t') if $set and 
+                OpenSRF::Utils::JSON->JSON2perl($set->value);
+        }
+    } else {
+        # behind the desk not supported, force it to false
+        $hold->behind_desk('f');
+    }
+
     $hold->requestor($e->requestor->id);
     $hold->request_lib($e->requestor->ws_ou);
     $hold->selection_ou($hold->pickup_lib) unless $hold->selection_ou;
@@ -558,7 +603,7 @@ sub retrieve_holds {
         if($available) {
             $holds_query->{where}->{shelf_time} = {'!=' => undef};
             # Maybe?
-            $holds_query->{where}->{pickup_lib} = {'=' => 'current_shelf_lib'};
+            $holds_query->{where}->{pickup_lib} = {'=' => {'+ahr' => 'current_shelf_lib'}};
         }
     }
 
@@ -781,8 +826,6 @@ sub cancel_hold {
     $e->update_action_hold_request($hold)
         or return $e->die_event;
 
-    delete_hold_copy_maps($self, $e, $hold->id);
-
     $e->commit;
 
     # re-fetch the hold to pick up the real cancel_time (not "now") for A/T
@@ -799,20 +842,6 @@ sub cancel_hold {
     return 1;
 }
 
-sub delete_hold_copy_maps {
-    my $class  = shift;
-    my $editor = shift;
-    my $holdid = shift;
-
-    my $maps = $editor->search_action_hold_copy_map({hold=>$holdid});
-    for(@$maps) {
-        $editor->delete_action_hold_copy_map($_)
-            or return $editor->event;
-    }
-    return undef;
-}
-
-
 my $update_hold_desc = 'The login session is the requestor. '       .
    'If the requestor is different from the usr field on the hold, ' .
    'the requestor must have UPDATE_HOLDS permissions. '             .
@@ -1065,7 +1094,7 @@ sub set_hold_shelf_expire_time {
 
     $start_time = ($start_time) ?
         DateTime::Format::ISO8601->new->parse_datetime(cleanse_ISO8601($start_time)) :
-        DateTime->now;
+        DateTime->now(time_zone => 'local'); # without time_zone we get UTC ... yuck!
 
     my $seconds = OpenSRF::Utils->interval_to_seconds($shelf_expire);
     my $expire_time = $start_time->add(seconds => $seconds);
@@ -1320,18 +1349,22 @@ sub retrieve_hold_queue_status_impl {
         # fetch cut_in_line and request_time since they're in the order_by
         # and we're asking for distinct values
         select => {ahr => ['id', 'cut_in_line', 'request_time']},
-        from   => {
-            ahr => {
-                'ahcm' => {
-                    join => {
+        from   => 'ahr',
+        where => {
+            id => { in => {
+                select => { ahcm => ['hold'] },
+                from   => {
+                    'ahcm' => {
                         'ahcm2' => {
                             'class' => 'ahcm',
                             'field' => 'target_copy',
                             'fkey'  => 'target_copy'
                         }
                     }
-                }
-            }
+                },
+                where => { '+ahcm2' => { hold => $hold->id } },
+                distinct => 1
+            }}
         },
         order_by => [
             {
@@ -1343,10 +1376,7 @@ sub retrieve_hold_queue_status_impl {
             },
             { "class" => "ahr", "field" => "request_time" }
         ],
-        distinct => 1,
-        where => {
-            '+ahcm2' => { hold => $hold->id }
-        }
+        distinct => 1
     });
 
     if (!@$q_holds) { # none? maybe we don't have a map ...
@@ -1365,7 +1395,13 @@ sub retrieve_hold_queue_status_impl {
             ],
             where    => {
                 hold_type => $hold->hold_type,
-                target    => $hold->target
+                target    => $hold->target,
+                capture_time => undef,
+                cancel_time => undef,
+                '-or' => [
+                    {expire_time => undef },
+                    {expire_time => {'>' => 'now'}}
+                ]
            }
         });
     }
@@ -1503,6 +1539,25 @@ __PACKAGE__->register_method(
     }
 );
 
+__PACKAGE__->register_method(
+    method    => "hold_pull_list",
+    stream => 1,
+    # TODO: tag with api_level 2 once fully supported
+    api_name  => "open-ils.circ.hold_pull_list.fleshed.stream",
+    signature => {
+        desc   => q/Returns a stream of fleshed holds  that need to be 
+                    "pulled" by a given location.  The location is 
+                    determined by the login session.  
+                    This API calls always run in authoritative mode./,
+        params => [
+            { desc => 'Limit (optional)',  type => 'number'},
+            { desc => 'Offset (optional)', type => 'number'},
+        ],
+        return => {
+            desc => 'Stream of holds holds, or event on failure',
+        }
+    }
+);
 
 sub hold_pull_list {
     my( $self, $conn, $authtoken, $limit, $offset ) = @_;
@@ -1525,12 +1580,24 @@ sub hold_pull_list {
         return $count;
 
     } elsif( $self->api_name =~ /id_list/ ) {
-        return $U->storagereq(
+        $U->storagereq(
             'open-ils.storage.direct.action.hold_request.pull_list.id_list.current_copy_circ_lib.status_filtered.atomic',
             $org, $limit, $offset );
 
+    } elsif ($self->api_name =~ /fleshed/) {
+
+        my $ids = $U->storagereq(
+            'open-ils.storage.direct.action.hold_request.pull_list.id_list.current_copy_circ_lib.status_filtered.atomic',
+            $org, $limit, $offset );
+
+        my $e = new_editor(xact => 1, requestor => $reqr);
+        $conn->respond(uber_hold_impl($e, $_, {flesh_acpl => 1})) for @$ids;
+        $e->rollback;
+        $conn->respond_complete;
+        return;
+
     } else {
-        return $U->storagereq(
+        $U->storagereq(
             'open-ils.storage.direct.action.hold_request.pull_list.search.current_copy_circ_lib.status_filtered.atomic',
             $org, $limit, $offset );
     }
@@ -2037,6 +2104,30 @@ __PACKAGE__->register_method(
     /
 );
 
+__PACKAGE__->register_method(
+    method    => 'fetch_captured_holds',
+    api_name  => 
+      'open-ils.circ.captured_holds.expired_on_shelf_or_wrong_shelf.retrieve',
+    stream    => 1,
+    authoritative => 1,
+    signature => q/
+        Returns list of shelf-expired un-fulfilled holds OR wrong shelf holds
+        for a given shelf lib
+    /
+);
+
+__PACKAGE__->register_method(
+    method    => 'fetch_captured_holds',
+    api_name  => 
+      'open-ils.circ.captured_holds.id_list.expired_on_shelf_or_wrong_shelf.retrieve',
+    stream    => 1,
+    authoritative => 1,
+    signature => q/
+        Returns list of shelf-expired un-fulfilled holds OR wrong shelf holds
+        for a given shelf lib
+    /
+);
+
 
 sub fetch_captured_holds {
     my( $self, $conn, $auth, $org, $match_copy ) = @_;
@@ -2061,7 +2152,7 @@ sub fetch_captured_holds {
             }
         },
         where => {
-            '+acp' => { status => OILS_COPY_STATUS_ON_HOLDS_SHELF },
+            '+acp' => { status => OILS_COPY_STATUS_ON_HOLDS_SHELF, deleted => 'f' },
             '+alhr' => {
                 capture_time      => { "!=" => undef },
                 current_copy      => $current_copy,
@@ -2072,12 +2163,23 @@ sub fetch_captured_holds {
     };
     if($self->api_name =~ /expired/) {
         $query->{'where'}->{'+alhr'}->{'-or'} = {
-                shelf_expire_time => { '<' => 'now'},
+                shelf_expire_time => { '<' => 'today'},
                 cancel_time => { '!=' => undef },
         };
     }
     my $hold_ids = $e->json_query( $query );
 
+    if ($self->api_name =~ /wrong_shelf/) {
+        # fetch holds whose current_shelf_lib is $org, but whose pickup 
+        # lib is some other org unit.  Ignore already-retrieved holds.
+        my $wrong_shelf =
+            pickup_lib_changed_on_shelf_holds(
+                $e, $org, [map {$_->{id}} @$hold_ids]);
+        # match the layout of other items in $hold_ids
+        push (@$hold_ids, {id => $_}) for @$wrong_shelf;
+    }
+
+
     for my $hold_id (@$hold_ids) {
         if($self->api_name =~ /id_list/) {
             $conn->respond($hold_id->{id});
@@ -2390,10 +2492,13 @@ sub do_possibility_checks {
 
     } elsif( $hold_type eq OILS_HOLD_TYPE_METARECORD ) {
 
-        my $maps = $e->search_metabib_metarecord_source_map({metarecord=>$mrid});
-        my @recs = map { $_->source } @$maps;
+        # pasing undef as the depth to filtered_records causes the depth
+        # of the selection_ou to be used, which is not what we want here.
+        $depth ||= 0;
+
+        my ($recs) = __PACKAGE__->method_lookup('open-ils.circ.holds.metarecord.filtered_records')->run($mrid, $holdable_formats, $selection_ou, $depth);
         my @status = ();
-        for my $rec (@recs) {
+        for my $rec (@$recs) {
             @status = _check_title_hold_is_possible(
                 $rec, $depth, $request_lib, $patron, $e->requestor, $pickup_lib, $selection_ou, $holdable_formats, $oargs
             );
@@ -2404,6 +2509,27 @@ sub do_possibility_checks {
 #   else { Unrecognized hold_type ! }   # FIXME: return error? or 0?
 }
 
+sub MR_filter_records {
+    my $self = shift;
+    my $client = shift;
+    my $m = shift;
+    my $f = shift;
+    my $o = shift;
+    my $d = shift;
+    my $opac_visible = shift;
+    
+    my $org_at_depth = defined($d) ? $U->org_unit_ancestor_at_depth($o, $d) : $o;
+    return $U->storagereq(
+        'open-ils.storage.metarecord.filtered_records.atomic', 
+        $m, $f, $org_at_depth, $opac_visible
+    );
+}
+__PACKAGE__->register_method(
+    method   => 'MR_filter_records',
+    api_name => 'open-ils.circ.holds.metarecord.filtered_records',
+);
+
+
 my %prox_cache;
 sub create_ranged_org_filter {
     my($e, $selection_ou, $depth) = @_;
@@ -2431,11 +2557,7 @@ sub create_ranged_org_filter {
 
 sub _check_title_hold_is_possible {
     my( $titleid, $depth, $request_lib, $patron, $requestor, $pickup_lib, $selection_ou, $holdable_formats, $oargs ) = @_;
-
-    my ($types, $formats, $lang);
-    if (defined($holdable_formats)) {
-        ($types, $formats, $lang) = split '-', $holdable_formats;
-    }
+    # $holdable_formats is now unused. We pre-filter the MR's records.
 
     my $e = new_editor();
     my %org_filter = create_ranged_org_filter($e, $selection_ou, $depth);
@@ -2449,23 +2571,7 @@ sub _check_title_hold_is_possible {
                     acn => {
                         field  => 'id',
                         fkey   => 'call_number',
-                        'join' => {
-                            bre => {
-                                field  => 'id',
-                                filter => { id => $titleid },
-                                fkey   => 'record'
-                            },
-                            mrd => {
-                                field  => 'record',
-                                fkey   => 'record',
-                                filter => {
-                                    record => $titleid,
-                                    ( $types   ? (item_type => [split '', $types])   : () ),
-                                    ( $formats ? (item_form => [split '', $formats]) : () ),
-                                    ( $lang    ? (item_lang => $lang)                : () )
-                                }
-                            }
-                        }
+                        filter => { record => $titleid }
                     },
                     acpl => { field => 'id', filter => { holdable => 't'}, fkey => 'location' },
                     ccs  => { field => 'id', filter => { holdable => 't'}, fkey => 'status'   },
@@ -2885,7 +2991,7 @@ sub _check_volume_hold_is_possible {
 
 sub verify_copy_for_hold {
     my( $patron, $requestor, $title, $copy, $pickup_lib, $request_lib, $oargs ) = @_;
-    $oargs = {} unless defined $oargs;
+    # $oargs should be undef unless we're overriding.
     $logger->info("checking possibility of copy in hold request for copy ".$copy->id);
     my $permitted = OpenILS::Utils::PermitHold::permit_copy_hold(
         {
@@ -2901,15 +3007,46 @@ sub verify_copy_for_hold {
         }
     );
 
-    # All overridden?
-    my $permit_anyway = 0;
-    foreach my $permit_event (@$permitted) {
-        if (grep { $_ eq $permit_event->{textcode} } @{$oargs->{events}}) {
-            $permit_anyway = 1;
-            last;
+    # Check for override permissions on events.
+    if ($oargs && $permitted && scalar @$permitted) {
+        # Remove the events from permitted that we can override.
+        if ($oargs->{events}) {
+            foreach my $evt (@{$oargs->{events}}) {
+                $permitted = [grep {$_->{textcode} ne $evt} @{$permitted}];
+            }
+        }
+        # Now, we handle the override all case by checking remaining
+        # events against override permissions.
+        if (scalar @$permitted && $oargs->{all}) {
+            # Pre-set events and failed members of oargs to empty
+            # arrays, if they are not set, yet.
+            $oargs->{events} = [] unless ($oargs->{events});
+            $oargs->{failed} = [] unless ($oargs->{failed});
+            # When we're done with these checks, we swap permitted
+            # with a reference to @disallowed.
+            my @disallowed = ();
+            foreach my $evt (@{$permitted}) {
+                # Check if we've already seen the event in this
+                # session and it failed.
+                if (grep {$_ eq $evt->{textcode}} @{$oargs->{failed}}) {
+                    push(@disallowed, $evt);
+                } else {
+                    # We have to check if the requestor has the
+                    # override permission.
+
+                    # AppUtils::check_user_perms returns the perm if
+                    # the user doesn't have it, undef if they do.
+                    if ($apputils->check_user_perms($requestor->id, $requestor->ws_ou, $evt->{textcode} . '.override')) {
+                        push(@disallowed, $evt);
+                        push(@{$oargs->{failed}}, $evt->{textcode});
+                    } else {
+                        push(@{$oargs->{events}}, $evt->{textcode});
+                    }
+                }
+            }
+            $permitted = \@disallowed;
         }
     }
-    $permitted = [] if $permit_anyway;
 
     my $age_protect_only = 0;
     if (@$permitted == 1 && @$permitted[0]->{textcode} eq 'ITEM_AGE_PROTECTED') {
@@ -2957,10 +3094,14 @@ sub find_nearest_permitted_hold {
 
     my $fifo = $U->ou_ancestor_setting_value($user->ws_ou, 'circ.holds_fifo');
 
+    # the nearest_hold API call now needs this
+    $copy->call_number($editor->retrieve_asset_call_number($copy->call_number))
+        unless ref $copy->call_number;
+
     # search for what should be the best holds for this copy to fulfill
     my $best_holds = $U->storagereq(
-        "open-ils.storage.action.hold_request.nearest_hold.atomic",
-        $user->ws_ou, $copy->id, 100, $hold_stall_interval, $fifo );
+        "open-ils.storage.action.hold_request.nearest_hold.atomic", 
+        $user->ws_ou, $copy, 100, $hold_stall_interval, $fifo );
 
     # Add any pre-targeted holds to the list too? Unless they are already there, anyway.
     if ($old_holds) {
@@ -2987,6 +3128,11 @@ sub find_nearest_permitted_hold {
         $logger->info("circulator: checking if hold $holdid is permitted for copy $bc");
 
         my $hold = $editor->retrieve_action_hold_request($holdid) or next;
+        # Force and recall holds bypass all rules
+        if ($hold->hold_type eq 'R' || $hold->hold_type eq 'F') {
+            $best_hold = $hold;
+            last;
+        }
         my $reqr = $reqr_cache{$hold->requestor} || $editor->retrieve_actor_user($hold->requestor);
         my $rlib = $org_cache{$hold->request_lib} || $editor->retrieve_actor_org_unit($hold->request_lib);
 
@@ -3068,7 +3214,16 @@ sub all_rec_holds {
     $args->{fulfillment_time} = undef; #  we don't want to see old fulfilled holds
     $args->{cancel_time} = undef;
 
-    my $resp = { volume_holds => [], copy_holds => [], recall_holds => [], force_holds => [], metarecord_holds => [], part_holds => [], issuance_holds => [] };
+    my $resp = {
+          metarecord_holds => []
+        , title_holds      => []
+        , volume_holds     => []
+        , copy_holds       => []
+        , recall_holds     => []
+        , force_holds      => []
+        , part_holds       => []
+        , issuance_holds   => []
+    };
 
     my $mr_map = $e->search_metabib_metarecord_source_map({source => $title_id})->[0];
     if($mr_map) {
@@ -3109,7 +3264,7 @@ sub all_rec_holds {
             {subscription => $subs}, {idlist=>1}
         );
 
-        if ($issuances) {
+        if (@$issuances) {
             $resp->{issuance_holds} = $e->search_action_hold_request(
                 {
                     hold_type => OILS_HOLD_TYPE_ISSUANCE,
@@ -3208,16 +3363,16 @@ sub uber_hold_impl {
     ) or return $e->event;
 
     if($hold->usr->id ne $e->requestor->id) {
-        # A user is allowed to see his/her own holds
+        # caller is asking for someone else's hold
         $e->allowed('VIEW_HOLD') or return $e->event;
-        $hold->notes( # filter out any non-staff ("private") notes
-            [ grep { !$U->is_true($_->staff) } @{$hold->notes} ] );
+        $hold->notes( # filter out any non-staff ("private") notes (unless marked as public)
+            [ grep { $U->is_true($_->staff) or $U->is_true($_->pub) } @{$hold->notes} ] );
 
     } else {
         # caller is asking for own hold, but may not have permission to view staff notes
         unless($e->allowed('VIEW_HOLD')) {
-            $hold->notes( # filter out any staff notes
-                [ grep { $U->is_true($_->staff) } @{$hold->notes} ] );
+            $hold->notes( # filter out any staff notes (unless marked as public)
+                [ grep { !$U->is_true($_->staff) or $U->is_true($_->pub) } @{$hold->notes} ] );
         }
     }
 
@@ -3244,6 +3399,10 @@ sub uber_hold_impl {
         %$details
     };
 
+    $resp->{copy}->location(
+        $e->retrieve_asset_copy_location($resp->{copy}->location))
+        if $resp->{copy} and $args->{flesh_acpl};
+
     unless($args->{suppress_patron_details}) {
         my $card = $e->retrieve_actor_card($user->card) or return $e->event;
         $resp->{patron_first}   = $user->first_given_name,
@@ -3430,7 +3589,7 @@ __PACKAGE__->register_method(
 sub clear_shelf_process {
     my($self, $client, $auth, $org_id, $match_copy) = @_;
 
-    my $e = new_editor(authtoken=>$auth, xact => 1);
+    my $e = new_editor(authtoken=>$auth);
     $e->checkauth or return $e->die_event;
     my $cache = OpenSRF::Utils::Cache->new('global');
 
@@ -3443,6 +3602,8 @@ sub clear_shelf_process {
         "open-ils.circ.captured_holds.id_list.expired_on_shelf.retrieve"
     )->run($auth, $org_id, $match_copy);
 
+    $e->xact_begin;
+
     my @holds;
     my @canceled_holds; # newly canceled holds
     my $chunk_size = 25; # chunked status updates
@@ -3462,7 +3623,6 @@ sub clear_shelf_process {
             $hold->cancel_time('now');
             $hold->cancel_cause(2); # Hold Shelf expiration
             $e->update_action_hold_request($hold) or return $e->die_event;
-            delete_hold_copy_maps($self, $e, $hold->id) and return $e->die_event;
             push(@canceled_holds, $hold_id);
         }
 
@@ -3485,7 +3645,8 @@ sub clear_shelf_process {
         my %cache_data = (
             hold => [],
             transit => [],
-            shelf => []
+            shelf => [],
+            pl_changed => pickup_lib_changed_on_shelf_holds($e, $org_id, \@hold_ids)
         );
 
         for my $hold (@holds) {
@@ -3534,6 +3695,42 @@ sub clear_shelf_process {
     }
 }
 
+# returns IDs for holds that are on the holds shelf but 
+# have had their pickup_libs change while on the shelf.
+sub pickup_lib_changed_on_shelf_holds {
+    my $e = shift;
+    my $org_id = shift;
+    my $ignore_holds = shift;
+    $ignore_holds = [$ignore_holds] if !ref($ignore_holds);
+
+    my $query = {
+        select => { alhr => ['id'] },
+        from   => {
+            alhr => {
+                acp => {
+                    field => 'id',
+                    fkey  => 'current_copy'
+                },
+            }
+        },
+        where => {
+            '+acp' => { status => OILS_COPY_STATUS_ON_HOLDS_SHELF },
+            '+alhr' => {
+                capture_time     => { "!=" => undef },
+                fulfillment_time => undef,
+                current_shelf_lib => $org_id,
+                pickup_lib => {'!='  => {'+alhr' => 'current_shelf_lib'}}
+            }
+        }
+    };
+
+    $query->{where}->{'+alhr'}->{id} =
+        {'not in' => $ignore_holds} if @$ignore_holds;
+
+    my $hold_ids = $e->json_query($query);
+    return [ map { $_->{id} } @$hold_ids ];
+}
+
 __PACKAGE__->register_method(
     method    => 'usr_hold_summary',
     api_name  => 'open-ils.circ.holds.user_summary',
@@ -3879,6 +4076,11 @@ __PACKAGE__->register_method(
             selected bib record or its associated copies and call_numbers/,
         params => [
             { desc => 'Bib ID', type => 'number' },
+            { desc => q/Optional arguments.  Supported arguments include:
+                "pickup_lib_descendant" -> limit holds to those whose pickup
+                library is equal to or is a child of the provided org unit/,
+                type => 'object'
+            }
         ],
         return => {desc => 'Hold count', type => 'number'}
     }
@@ -3897,15 +4099,15 @@ __PACKAGE__->register_method(
     }
 );
 
-# XXX Need to add type I (and, soon, type P) holds to these counts
+# XXX Need to add type I holds to these counts
 sub rec_hold_count {
-    my($self, $conn, $target_id) = @_;
-
+    my($self, $conn, $target_id, $args) = @_;
+    $args ||= {};
 
     my $mmr_join = {
         mmrsm => {
-            field => 'id',
-            fkey => 'source',
+            field => 'source',
+            fkey => 'id',
             filter => {metarecord => $target_id}
         }
     };
@@ -3963,6 +4165,17 @@ sub rec_hold_count {
                     },
                     {
                         '-and' => {
+                            hold_type => 'P',
+                            target => {
+                                in => {
+                                    select => {bmp => ['id']},
+                                    from => {bmp => $bre_join}
+                                }
+                            }
+                        }
+                    },
+                    {
+                        '-and' => {
                             hold_type => 'T',
                             target => $target_id
                         }
@@ -3973,7 +4186,7 @@ sub rec_hold_count {
     };
 
     if($self->api_name =~ /mmr/) {
-        $query->{where}->{'+ahr'}->{'-or'}->[2] = {
+        $query->{where}->{'+ahr'}->{'-or'}->[3] = {
             '-and' => {
                 hold_type => 'T',
                 target => {
@@ -3985,7 +4198,7 @@ sub rec_hold_count {
             }
         };
 
-        $query->{where}->{'+ahr'}->{'-or'}->[3] = {
+        $query->{where}->{'+ahr'}->{'-or'}->[4] = {
             '-and' => {
                 hold_type => 'M',
                 target => $target_id
@@ -3994,6 +4207,26 @@ sub rec_hold_count {
     }
 
 
+    if (my $pld = $args->{pickup_lib_descendant}) {
+
+        my $top_ou = new_editor()->search_actor_org_unit(
+            {parent_ou => undef}
+        )->[0]; # XXX Assumes single root node. Not alone in this...
+
+        $query->{where}->{'+ahr'}->{pickup_lib} = {
+            in => {
+                select  => {aou => [{ 
+                    column => 'id', 
+                    transform => 'actor.org_unit_descendants', 
+                    result_field => 'id' 
+                }]},
+                from    => 'aou',
+                where   => {id => $pld}
+            }
+        } if ($pld != $top_ou->id);
+    }
+
+
     return new_editor()->json_query($query)->[0]->{count};
 }
 
@@ -4012,4 +4245,143 @@ sub calculate_expire_time
     return undef;
 }
 
+
+__PACKAGE__->register_method(
+    method    => 'mr_hold_filter_attrs',
+    api_name  => 'open-ils.circ.mmr.holds.filters',
+    authoritative => 1,
+    stream => 1,
+    signature => {
+        desc => q/
+            Returns the set of available formats and languages for the
+            constituent records of the provided metarcord.
+            If an array of hold IDs is also provided, information about
+            each is returned as well.  This information includes:
+            1. a slightly easier to read version of holdable_formats
+            2. attributes describing the set of format icons included
+               in the set of desired, constituent records.
+        /,
+        params => [
+            {desc => 'Metarecord ID', type => 'number'},
+            {desc => 'Context Org ID', type => 'number'},
+            {desc => 'Hold ID List', type => 'array'},
+        ],
+        return => {
+            desc => q/
+                Stream of objects.  The first will have a 'metarecord' key
+                containing non-hold-specific metarecord information, subsequent
+                responses will contain a 'hold' key containing hold-specific
+                information
+            /, 
+            type => 'object'
+        }
+    }
+);
+
+sub mr_hold_filter_attrs { 
+    my ($self, $client, $mr_id, $org_id, $hold_ids) = @_;
+    my $e = new_editor();
+
+    # by default, return MR / hold attributes for all constituent
+    # records with holdable copies.  If there is a hard boundary,
+    # though, limit to records with copies within the boundary,
+    # since anything outside the boundary can never be held.
+    my $org_depth = 0;
+    if ($org_id) {
+        $org_depth = $U->ou_ancestor_setting_value(
+            $org_id, OILS_SETTING_HOLD_HARD_BOUNDARY) || 0;
+    }
+
+    # get all org-scoped records w/ holdable copies for this metarecord
+    my ($bre_ids) = $self->method_lookup(
+        'open-ils.circ.holds.metarecord.filtered_records')->run(
+            $mr_id, undef, $org_id, $org_depth);
+
+    my $item_lang_attr = 'item_lang'; # configurable?
+    my $format_attr = $e->retrieve_config_global_flag(
+        'opac.metarecord.holds.format_attr')->value;
+
+    # helper sub for fetching ccvms for a batch of record IDs
+    sub get_batch_ccvms {
+        my ($e, $attr, $bre_ids) = @_;
+        return [] unless $bre_ids and @$bre_ids;
+        my $vals = $e->search_metabib_record_attr_flat({
+            attr => $attr,
+            id => $bre_ids
+        });
+        return [] unless @$vals;
+        return $e->search_config_coded_value_map({
+            ctype => $attr,
+            code => [map {$_->value} @$vals]
+        });
+    }
+
+    my $langs = get_batch_ccvms($e, $item_lang_attr, $bre_ids);
+    my $formats = get_batch_ccvms($e, $format_attr, $bre_ids);
+
+    $client->respond({
+        metarecord => {
+            id => $mr_id,
+            formats => $formats,
+            langs => $langs
+        }
+    });
+
+    return unless $hold_ids;
+    my $icon_attr = $e->retrieve_config_global_flag('opac.icon_attr');
+    $icon_attr = $icon_attr ? $icon_attr->value : '';
+
+    for my $hold_id (@$hold_ids) {
+        my $hold = $e->retrieve_action_hold_request($hold_id) 
+            or return $e->event;
+
+        next unless $hold->hold_type eq 'M';
+
+        my $resp = {
+            hold => {
+                id => $hold_id,
+                formats => [],
+                langs => []
+            }
+        };
+
+        # collect the ccvm's for the selected formats / language
+        # (i.e. the holdable formats) on the MR.
+        # this assumes a two-key structure for format / language,
+        # though no assumption is made about the keys themselves.
+        my $hformats = OpenSRF::Utils::JSON->JSON2perl($hold->holdable_formats);
+        my $lang_vals = [];
+        my $format_vals = [];
+        for my $val (values %$hformats) {
+            # val is either a single ccvm or an array of them
+            $val = [$val] unless ref $val eq 'ARRAY';
+            for my $node (@$val) {
+                push (@$lang_vals, $node->{_val})   
+                    if $node->{_attr} eq $item_lang_attr; 
+                push (@$format_vals, $node->{_val})   
+                    if $node->{_attr} eq $format_attr;
+            }
+        }
+
+        # fetch the ccvm's for consistency with the {metarecord} blob
+        $resp->{hold}{formats} = $e->search_config_coded_value_map({
+            ctype => $format_attr, code => $format_vals});
+        $resp->{hold}{langs} = $e->search_config_coded_value_map({
+            ctype => $item_lang_attr, code => $lang_vals});
+
+        # find all of the bib records within this metarcord whose 
+        # format / language match the holdable formats on the hold
+        my ($bre_ids) = $self->method_lookup(
+            'open-ils.circ.holds.metarecord.filtered_records')->run(
+                $hold->target, $hold->holdable_formats, 
+                $hold->selection_ou, $hold->selection_depth);
+
+        # now find all of the 'icon' attributes for the records
+        $resp->{hold}{icons} = get_batch_ccvms($e, $icon_attr, $bre_ids);
+        $client->respond($resp);
+    }
+
+    return;
+}
+
 1;