Merge pull request openwebwork#1846 from taniwallach/group-problems-adds

Make adding all/multiple users to a given set more efficient

lib/WeBWorK/ContentGenerator/Instructor/AddUsers.pm

@@ -95,7 +95,7 @@ sub initialize ($c) {
 		}
 		if (defined $c->param('assignSets')) {
 			my @setIDs = $c->param('assignSets');
-			assignSetsToUsers($db, \@setIDs, \@userIDs);
+			assignSetsToUsers($db, $ce, \@setIDs, \@userIDs);
 		}
 	}
 

lib/WeBWorK/ContentGenerator/Instructor/Assigner.pm

@@ -45,7 +45,7 @@ sub pre_header_initialize ($c) {
 		if (@selected_users && @selected_sets) {
 			my @results;    # This is not used?
 			if (defined $c->param('assign')) {
-				assignSetsToUsers($db, \@selected_sets, \@selected_users);
+				assignSetsToUsers($db, $ce, \@selected_sets, \@selected_users);
 				$c->addgoodmessage($c->maketext('All assignments were made successfully.'));
 			}
 			if (defined $c->param('unassign')) {

lib/WeBWorK/ContentGenerator/Instructor/UsersAssignedToSet.pm

@@ -25,7 +25,7 @@ users to which sets are assigned.
 
 use WeBWorK::Debug;
 use WeBWorK::Utils qw(format_set_name_display);
-use WeBWorK::Utils::Instructor qw(assignSetToUser assignSetToAllUsers);
+use WeBWorK::Utils::Instructor qw(assignSetToAllUsers assignSetToGivenUsers);
 
 sub initialize ($c) {
 	my $authz = $c->authz;
@@ -70,18 +70,23 @@ sub initialize ($c) {
 		die "Unable to get global set record for $setID " unless $setRecord;
 
 		my %setUsers = map { $_ => 1 } $db->listSetUsers($setID);
+		my @usersToAdd;
 		for my $selectedUser (map { $_->user_id } @{ $c->{user_records} }) {
 			if (exists $selectedUsers{$selectedUser}) {
 				unless ($setUsers{$selectedUser}) {    # skip users already in the set
-					debug("assignSetToUser($selectedUser, ...)");
-					assignSetToUser($db, $selectedUser, $setRecord);
-					debug("done assignSetToUser($selectedUser, ...)");
+					debug("saving $selectedUser to be added to set later");
+					push(@usersToAdd, $selectedUser);
 				}
 			} else {
 				next unless $setUsers{$selectedUser};    # skip users not in the set
 				$db->deleteUserSet($selectedUser, $setID);
 			}
 		}
+		if (@usersToAdd) {
+			debug("assignSetToGivenUsers(...)");
+			assignSetToGivenUsers($db, $c->ce, $setID, 1, $db->getUsers(@usersToAdd));
+			debug("done assignSetToGivenUsers(...)");
+		}
 	}
 
 	return;

lib/WeBWorK/DB.pm

@@ -464,6 +464,48 @@ sub restore_tables {
 	return 1;
 }
 
+################################################################################
+# transaction support
+################################################################################
+
+# Any course will have the user table, so that allows getting the database
+# handle.
+
+sub start_transaction {
+	my $self = shift;
+	eval { $self->{user}->dbh->begin_work; };
+	if ($@) {
+		my $msg = "Error in start_transaction: $@";
+		if ($msg =~ /Already in a transaction/) {
+			warn "Aborting active transaction.";
+			$self->{user}->dbh->rollback;
+		}
+		croak $msg;
+	}
+}
+
+sub end_transaction {
+	my $self = shift;
+	eval { $self->{user}->dbh->commit; };
+	if ($@) {
+		my $msg = "Error in end_transaction: $@";
+		$self->abort_transaction;
+		croak $msg;
+	}
+}
+
+sub abort_transaction {
+	my $self = shift;
+	eval {
+		$self->{user}->dbh->{AutoCommit} = 0;
+		$self->{user}->dbh->rollback;
+	};
+	if ($@) {
+		my $msg = "Error in abort_transaction: $@";
+		croak $msg;
+	}
+}
+
 ################################################################################
 # user functions
 ################################################################################
@@ -1537,6 +1579,41 @@ sub addUserSet {
 	}
 }
 
+# Used to add a list of UserSet records to the DB in less DB calls.
+sub addMultipleUserSets {
+	my ($self, @userSetList) = @_;
+
+	# Do a single checkArgs call, all records in the list were made in the same manner
+	$self->checkArgs([ $userSetList[0] ], qw/REC:set_user/);
+
+	my @badUserIDs;
+	my %badSetIDs;
+	my @toInsert;
+	for my $userSet (@userSetList) {
+		if ($self->{user}->exists($userSet->user_id)) {
+			if ($self->{set}->exists($userSet->set_id)) {
+				# This record is OK
+				push(@toInsert, $userSet);
+			} else {
+				$badSetIDs{ $userSet->set_id } = 1;
+			}
+		} else {
+			push(@badUserIDs, join("", 'user ', $userSet->user_id, " not found, cannot add set ", $userSet->set_id));
+		}
+	}
+	my $badSetIDerrorMsg =
+		keys(%badSetIDs) ? join(" ", 'bad set IDs seen:', keys(%badSetIDs)) : "";
+
+	eval { return $self->{set_user}->insert_records([@toInsert]); };
+	if ($@) {
+		croak join("\n", "addMultipleUserSets errors:\nDB call error: $@", $badSetIDerrorMsg, @badUserIDs);
+	}
+
+	if (@badUserIDs || keys(@badUserIDs)) {
+		croak join("\n", "addMultipleUserSets errors:\n", $badSetIDerrorMsg, @badUserIDs);
+	}
+}
+
 # the code from putUserSet() is duplicated in large part in the following
 # putVersionedUserSet; c.f. that routine
 sub putUserSet {
@@ -2053,6 +2130,44 @@ sub addUserProblem {
 	}
 }
 
+# Used to add multiple problems to a SINGLE set with less DB calls.
+# Not for use by versioned sets
+# This expects @problemLists to be a 2 dimensional array, each
+# entry being the problemList array for one user.
+sub addUserMultipleProblems {
+	my ($self, @problemLists) = @_;
+
+	croak "Error in request to addUserMultipleProblems" if ref($problemLists[0]) ne 'ARRAY';
+
+	# Do a single checkArgs call, all records in the list were made in the same manner
+	$self->checkArgs([ $problemLists[0][0] ], qw/REC:problem_user/);
+
+	my $set_id = $problemLists[0][0]->set_id;
+
+	# Array in which all records to add are collected
+	my @collectedProblemList;
+
+	for my $problemListRef (@problemLists) {
+		my $user_id = $problemListRef->[0]->user_id;
+		croak "addMultipleUserProblems: user set $set_id for user $user_id not found"
+			unless $self->{set_user}->exists($user_id, $set_id);
+
+		# Test whether there are already problem records for this user in this set, as in that case
+		# inserting multiple records at once would trigger an error.
+		my $where = [ user_id_eq_set_id_eq => $user_id, $set_id ];
+
+		croak "addMultipleUserProblems cannot be run as set $set_id already contains some problems for user $user_id"
+			if $self->{problem_user}->count_where($where);
+		# If we got here, we can add this list of problems to those to insert into the database
+		push(@collectedProblemList, @{$problemListRef});
+	}
+
+	eval { $self->{problem_user}->insert_records([@collectedProblemList]); };
+	if ($@) {
+		croak "addUserMultipleProblems: $@";
+	}
+}
+
 # versioned_ok is an optional argument which lets us slip versioned setIDs through checkArgs.
 sub putUserProblem {
 	my $V = $_[2] ? "V" : "";