[open-ils-commits] r15971 - trunk/Open-ILS/src/perlmods/OpenILS/Application/Circ (erickson)

[svn at svn.open-ils.org]

Author: erickson
Date: 2010-03-25 09:19:57 -0400 (Thu, 25 Mar 2010)
New Revision: 15971

Modified:
   trunk/Open-ILS/src/perlmods/OpenILS/Application/Circ/Holds.pm
Log:
Minor refactors for clarity:

~ avoid name confusion between $holds and $holds[0] (@holds)
~ more consistent style (return X if X)
~ factor out common case from all conditional blocks

Modified: trunk/Open-ILS/src/perlmods/OpenILS/Application/Circ/Holds.pm
===================================================================
--- trunk/Open-ILS/src/perlmods/OpenILS/Application/Circ/Holds.pm	2010-03-25 13:19:56 UTC (rev 15970)
+++ trunk/Open-ILS/src/perlmods/OpenILS/Application/Circ/Holds.pm	2010-03-25 13:19:57 UTC (rev 15971)
@@ -171,16 +171,12 @@
 	my( $user, $evt ) = $apputils->checkses($login_session);
 	return $evt if $evt;
 
-	my $holds;
-	if(ref($holds[0]) eq 'ARRAY') {
-		$holds = $holds[0];
-	} else { $holds = [ @holds ]; }
+	my $holdsref = (ref($holds[0]) eq 'ARRAY') ? $holds[0] : [ @holds ];
 
-	$logger->debug("Iterating over holds requests...");
+	$logger->debug("Iterating over " . scalar(@$holdsref) . " holds requests...");
 
-	for my $hold (@$holds) {
-
-		if(!$hold){next};
+	for my $hold (@$holdsref) {
+        $hold or next;
 		my $type = $hold->hold_type;
 
 		$logger->activity("User " . $user->id . 
@@ -190,22 +186,18 @@
 		if($user->id ne $hold->usr) {
 			( $recipient, $evt ) = $apputils->fetch_user($hold->usr);
 			return $evt if $evt;
-
 		} else {
 			$recipient = $user;
 		}
 
-
-		my $perm = undef;
-
 		# am I allowed to place holds for this user?
 		if($hold->requestor ne $hold->usr) {
-			$perm = _check_request_holds_perm($user->id, $user->home_ou);
-			if($perm) { return $perm; }
+			my $perm = _check_request_holds_perm($user->id, $user->home_ou);
+            return $perm if $perm;
 		}
 
 		# is this user allowed to have holds of this type?
-		$perm = _check_holds_perm($type, $hold->requestor, $recipient->home_ou);
+		my $perm = _check_holds_perm($type, $hold->requestor, $recipient->home_ou);
         return $perm if $perm;
 
 		#enforce the fact that the login is the one requesting the hold
@@ -230,31 +222,17 @@
 	my($type, $user_id, $org_id) = @_;
 
 	my $evt;
-	if($type eq "M") {
-		if($evt = $apputils->check_perms(
-			$user_id, $org_id, "MR_HOLDS")) {
-			return $evt;
-		} 
-
+	if ($type eq "M") {
+		$evt = $apputils->check_perms($user_id, $org_id, "MR_HOLDS"    );
 	} elsif ($type eq "T") {
-		if($evt = $apputils->check_perms(
-			$user_id, $org_id, "TITLE_HOLDS")) {
-			return $evt;
-		}
-
+		$evt = $apputils->check_perms($user_id, $org_id, "TITLE_HOLDS" );
 	} elsif($type eq "V") {
-		if($evt = $apputils->check_perms(
-			$user_id, $org_id, "VOLUME_HOLDS")) {
-			return $evt;
-		}
-
+		$evt = $apputils->check_perms($user_id, $org_id, "VOLUME_HOLDS");
 	} elsif($type eq "C") {
-		if($evt = $apputils->check_perms(
-			$user_id, $org_id, "COPY_HOLDS")) {
-			return $evt;
-		}
+		$evt = $apputils->check_perms($user_id, $org_id, "COPY_HOLDS"  );
 	}
 
+    return $evt if $evt;
 	return undef;
 }
 
@@ -748,7 +726,7 @@
 
 sub transit_hold {
     my($e, $orig_hold, $hold, $copy) = @_;
-    my $src = $orig_hold->pickup_lib;
+    my $src  = $orig_hold->pickup_lib;
     my $dest = $hold->pickup_lib;
 
     $logger->info("putting hold into transit on pickup_lib update");
@@ -1281,7 +1259,7 @@
 	return $e->event unless $e->checkauth;
 
 	$type ||= "T";
-	$org ||= $e->requestor->ws_ou;
+	$org  ||= $e->requestor->ws_ou;
 
 #	return $e->search_action_hold_request(
 #		{ target => $id, hold_type => $type, fulfillment_time => undef }, {idlist=>1});
@@ -1475,28 +1453,22 @@
             return {success => 1, depth => $depth, local_avail => $status[1]} if $status[0];
             $depth--;
         }
-        return {success => 0};
-
     } elsif(defined $hard_boundary and $$params{depth} < $hard_boundary) {
         # there is no soft boundary, enforce the hard boundary if it exists
         $logger->info("performing hold possibility check with hard boundary $hard_boundary");
         my @status = do_possibility_checks($e, $patron, $request_lib, $hard_boundary, %params);
         if($status[0]) {
             return {success => 1, depth => $hard_boundary, local_avail => $status[1]}
-        } else {
-            return {success => 0};
         }
-
     } else {
         # no boundaries defined, fall back to user specifed boundary or no boundary
         $logger->info("performing hold possibility check with no boundary");
         my @status = do_possibility_checks($e, $patron, $request_lib, $params{depth}, %params);
         if($status[0]) {
             return {success => 1, depth => $hard_boundary, local_avail => $status[1]};
-        } else {
-            return {success => 0};
         }
     }
+    return {success => 0};
 }
 
 sub do_possibility_checks {
@@ -2072,10 +2044,10 @@
     # Find holds on the shelf that have been there too long
     my $hold_ids = $e->search_action_hold_request(
         {   shelf_expire_time => {'<' => 'now'},
-            pickup_lib => $org_id,
-            cancel_time => undef,
-            fulfillment_time => undef,
-            shelf_time => {'!=' => undef}
+            pickup_lib        => $org_id,
+            cancel_time       => undef,
+            fulfillment_time  => undef,
+            shelf_time        => {'!=' => undef}
         },
         { idlist => 1 }
     );
@@ -2170,7 +2142,7 @@
         {  
             usr =>  $user_id , 
             fulfillment_time => undef,
-            cancel_time => undef,
+            cancel_time      => undef,
         }
     );