From: erickson <erickson@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Date: Fri, 13 Oct 2006 21:17:07 +0000 (+0000)
Subject: fixed some permission checking errors in copy/volume update.  did some refactoring... 
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=b3d8cbe6424ee6dca125d0dc750c31a39b37f327;p=evergreen%2Fpines.git

fixed some permission checking errors in copy/volume update.  did some refactoring of the record merging code to fix logic errors causing some volumes to not get merged over


git-svn-id: svn://svn.open-ils.org/ILS/trunk@6470 dcc99617-32d9-48b4-a31d-7c20da2025e4
---

diff --git a/Open-ILS/src/perlmods/OpenILS/Application/Cat.pm b/Open-ILS/src/perlmods/OpenILS/Application/Cat.pm
index 4404f37e1b..a85a5418ad 100644
--- a/Open-ILS/src/perlmods/OpenILS/Application/Cat.pm
+++ b/Open-ILS/src/perlmods/OpenILS/Application/Cat.pm
@@ -845,8 +845,12 @@ sub fleshed_copy_update {
 	my $editor = new_editor(requestor => $reqr, xact => 1);
 	my $override = $self->api_name =~ /override/;
 	$evt = update_fleshed_copies($editor, $override, undef, $copies, $delete_stats);
-	return $evt if $evt;
-	$editor->finish;
+	if( $evt ) { 
+		$logger->info("fleshed copy update failed with event: ".JSON->perl2JSON($evt));
+		$editor->rollback; 
+		return $evt; 
+	}
+	$editor->commit;
 	$logger->info("fleshed copy update successfully updated ".scalar(@$copies)." copies");
 	return 1;
 }
@@ -993,6 +997,9 @@ sub update_fleshed_copies {
 			return $editor->event unless $vol;
 		}
 
+		return $editor->event unless 
+			$editor->allowed('UPDATE_COPY', $vol->owning_lib);
+
 		$copy->editor($editor->requestor->id);
 		$copy->edit_date('now');
 
@@ -1053,10 +1060,8 @@ sub update_copy {
 		if ref $copy->age_protect;
 
 	fix_copy_price($copy);
-	return $editor->event unless
-		$editor->update_asset_copy( 
-			$copy, {checkperm=>1, permorg=>$vol->owning_lib});
 
+	return $editor->event unless $editor->update_asset_copy($copy);
 	return remove_empty_objects($editor, $override, $orig_vol);
 }
 
@@ -1065,15 +1070,16 @@ sub remove_empty_objects {
 	my( $editor, $override, $vol ) = @_; 
 	if( title_is_empty($editor, $vol->record) ) {
 
-		if( $override ) {
+		# disable the TITLE_LAST_COPY event for now
+		# if( $override ) {
+		if( 1 ) {
 
 			# delete this volume if it's not already marked as deleted
 			unless( $U->is_true($vol->deleted) || $vol->isdeleted ) {
 				$vol->deleted('t');
 				$vol->editor($editor->requestor->id);
 				$vol->edit_date('now');
-				$editor->update_asset_call_number($vol, {checkperm=>0})
-					or return $editor->event;
+				$editor->update_asset_call_number($vol) or return $editor->event;
 			}
 
 			# then delete the record this volume points to
@@ -1083,8 +1089,7 @@ sub remove_empty_objects {
 			unless( $U->is_true($rec->deleted) ) {
 				$rec->deleted('t');
 				$rec->active('f');
-				$editor->update_biblio_record_entry($rec, {checkperm=>0})
-					or return $editor->event;
+				$editor->update_biblio_record_entry($rec) or return $editor->event;
 			}
 
 		} else {
@@ -1104,9 +1109,7 @@ sub delete_copy {
 
 	$copy->editor($editor->requestor->id);
 	$copy->edit_date('now');
-	$editor->update_asset_copy(
-		$copy, {checkperm=>1, permorg=>$vol->owning_lib})
-		or return $editor->event;
+	$editor->update_asset_copy($copy) or return $editor->event;
 
 	# Delete any open transits for this copy
 	my $transits = $editor->search_action_transit_copy(
@@ -1134,10 +1137,7 @@ sub create_copy {
 	$copy->create_date('now');
 	fix_copy_price($copy);
 
-	$editor->create_asset_copy(
-		$copy, {checkperm=>1, permorg=>$vol->owning_lib})
-		or return $editor->event;
-
+	$editor->create_asset_copy($copy) or return $editor->event;
 	return undef;
 }
 
@@ -1272,7 +1272,7 @@ sub batch_volume_transfer {
 
 	my $e = new_editor(authtoken => $auth, xact =>1);
 	return $e->event unless $e->checkauth;
-	return $e->event unless $e->allowed('VOLUME_UPDATE');
+	return $e->event unless $e->allowed('VOLUME_UPDATE', $o_lib);
 
 	my $dorg = $e->retrieve_actor_org_unit($o_lib)
 		or return $e->event;
diff --git a/Open-ILS/src/perlmods/OpenILS/Application/Cat/Merge.pm b/Open-ILS/src/perlmods/OpenILS/Application/Cat/Merge.pm
index fec432cd92..9804fb2962 100644
--- a/Open-ILS/src/perlmods/OpenILS/Application/Cat/Merge.pm
+++ b/Open-ILS/src/perlmods/OpenILS/Application/Cat/Merge.pm
@@ -45,7 +45,7 @@ sub merge_records {
 	my @recs = keys %r;
 
 	my $reqr = $editor->requestor;
-	$logger->activity("merge: user ".$reqr->id." merging bib records: @recs");
+	$logger->activity("merge: user ".$reqr->id." merging bib records: @recs with master = $master");
 
 	# -----------------------------------------------------------
 	# collect all of the volumes, merge any with duplicate 
@@ -53,7 +53,7 @@ sub merge_records {
 	# -----------------------------------------------------------
 	my @volumes;
 	for (@recs) {
-		my $vs = $editor->search_asset_call_number({record => $_});
+		my $vs = $editor->search_asset_call_number({record => $_, deleted=>'f'});
 		push( @volumes, @$vs );
 	}
 
@@ -61,22 +61,46 @@ sub merge_records {
 
 	my @trimmed;
 	# de-duplicate any volumes with the same label and owning_lib
+
+	my %seen_vols;
+
 	for my $v (@volumes) {
 		my $l = $v->label;
 		my $o = $v->owning_lib;
-		my @dups = rgrep( 
-			sub { $_->label eq $l and $_->owning_lib == $o }, \@volumes );
+
+		if($seen_vols{$v->id}) {
+			$logger->debug("merge: skipping ".$v->id." since it's already been merged");
+			next;
+		}
+
+		$seen_vols{$v->id} = 1;
+
+		$logger->debug("merge: [".$v->id."] looking for dupes with label $l and owning_lib $o");
+
+		my @dups;
+		for my $vv (@volumes) {
+			if( $vv->label eq $v->label and $vv->owning_lib == $v->owning_lib ) {
+				$logger->debug("merge: pushing dupe volume ".$vv->id) if @dups;
+				push( @dups, $vv );
+				$seen_vols{$vv->id} = 1;
+			} 
+		}
 
 		if( @dups == 1 ) {
+			$logger->debug("merge: pushing unique volume into trimmed volume set: ".$v->id);
 			push( @trimmed, @dups );
 
 		} else {
 			my($vol, $e) = merge_volumes($editor, \@dups);
 			return $e if $e;
+			$logger->debug("merge: pushing vol-merged volume into trimmed volume set: ".$vol->id);
 			push(@trimmed, $vol);
 		}
 	}
 
+	my $s = 'merge: trimmed volume set contains the following vols: ';
+	$s .= 'id = '.$_->id .' : record = '.$_->record.' | ' for @trimmed;
+	$logger->debug($s);
 
 	# make all the volumes point to the master record
 	my $stat;
@@ -183,8 +207,7 @@ sub merge_volumes {
 			$copy->call_number($bigcn);
 			$copy->editor($editor->requestor->id);
 			$copy->edit_date('now');
-			$editor->update_asset_copy($copy, {checkperm=>1})
-				or return (undef, $editor->event);
+			$editor->update_asset_copy($copy) or return (undef, $editor->event);
 		}
 	}
 
@@ -194,8 +217,8 @@ sub merge_volumes {
 		$_->deleted('t');
 		$_->editor($editor->requestor->id);
 		$_->edit_date('now');
-		$editor->update_asset_call_number($_,{checkperm=>1}) 
-			or return (undef, $editor->event);
+		return (undef,$editor->event) unless $editor->allowed('VOLUME_UPDATE', $_->owning_lib);
+		$editor->update_asset_call_number($_) or return (undef, $editor->event);
 	}
 
 	my ($mvol) = grep { $_->id == $bigcn } @$volumes;