Add tests for user_sets and user_set versioning

.yamllint.yml

@@ -0,0 +1,10 @@
+---
+
+yaml-files:
+  - '*.yaml'
+  - '*.yml'
+  - '.yamllint'
+
+rules:
+  line-length:
+    max: 120

conf/permissions.dist.yml

@@ -102,6 +102,9 @@ db_permissions:
     getUserSets:
       allow_self_access: true
       allowed_roles: ['course_admin', 'instructor']
+    getUserSet:
+      allow_self_access: true
+      allowed_roles: ['course_admin', 'instructor']
     addUserSet:
       allowed_roles: ['course_admin', 'instructor']
     updateUserSet:

lib/DB/Schema/ResultSet/Course.pm

@@ -169,18 +169,13 @@ C<as_result_set>, a boolean if the return is to be a result_set
 
 =head3 output
 
-The deleted course either as a C< DBIx::Class::ResultSet::Course> object or a hashref
-of the fields. See above.
+Nothing (undef) is returned.
 
 =cut
 
 sub deleteCourse ($self, %args) {
-	my $course_to_delete = $self->getCourse(info => getCourseInfo($args{info}), as_result_set => 1);
-
-	my $deleted_course = $course_to_delete->delete;
-	return $deleted_course if $args{as_result_set};
-
-	return { $course_to_delete->get_inflated_columns };
+	$self->getCourse(info => getCourseInfo($args{info}), as_result_set => 1)->delete;
+	return;
 }
 
 =head1 updateCourse

lib/DB/Schema/ResultSet/ProblemPool.pm

@@ -88,12 +88,12 @@ sub getProblemPool ($self, %args) {
 
 	my $pool = $self->find($search_info, { join => [qw/courses/] });
 
-	unless ($pool) {
-		my $course_name = $course->course_name;
+	DB::Exception::PoolNotInCourse->throw(message => 'The pool with '
+			. ($args{info}{pool_name} ? ' name ' . $args{info}{pool_name} : 'id ' . $args{info}{problem_pool_id})
+			. 'is not in the course '
+			. $course->course_name)
+		unless $pool;
 
-		DB::Exception::PoolNotInCourse->throw(
-			message => "The pool with name \{$args{info}->{pool_name}} is not in the course $course_name");
-	}
 	return $pool if $args{as_result_set};
 	return { $pool->get_columns };
 }
@@ -102,6 +102,27 @@ sub getProblemPool ($self, %args) {
 
 Add a problem pool for a given course
 
+=head3 arguments
+
+=over
+
+=item * C<params> a hashref specifying information on the added problem pool
+One must include either the C<course_id> or C<course_name> and the C<pool_name>.
+If there is not enough info to get the course and pool, a C<ParametersNeeded> exception is thrown.
+
+
+=item * C<as_result_set>: boolean
+
+If C<as_result_set> is true, then the user sets are returned as a C<ResultSet>.
+See C<DBIx::Class::ResultSet> for more information
+
+=back
+
+=head3 output
+
+Either a hashref of the added problem pool or a C<DBIx::Class::ResultSet::ProblemPool>
+if C<as_result_set> is true.
+
 =cut
 
 sub addProblemPool ($self, %args) {
@@ -147,13 +168,32 @@ sub addProblemPool ($self, %args) {
 
 updates the parameters of an existing problem pool
 
+=head3 arguments
+
+=over
+
+=item * C<info> a hashref specifying information on the problem pool
+One must include either the C<course_id> or C<course_name> and either the C<pool_name>
+or C<problem_pool_id>.  If there is not enough info to get the course and pool, a
+C<ParametersNeeded> exception is thrown.
+
+=item * C<params>: a hashref containing the information to be updated.
+
+=item * C<as_result_set>: boolean
+
+If C<as_result_set> is true, then the user sets are returned as a C<ResultSet>.
+See C<DBIx::Class::ResultSet> for more information
+
+=back
+
+
 =cut
 
 sub updateProblemPool ($self, %args) {
 
 	my $pool = $self->getProblemPool(info => $args{info}, as_result_set => 1);
 
-	DB::Excpetion::PoolNotInCourse->throw(
+	DB::Exception::PoolNotInCourse->throw(
 		message => 'The problem pool '
 			. (
 				$args{info}->{pool_name}
@@ -173,21 +213,36 @@ sub updateProblemPool ($self, %args) {
 	return { $updated_pool->get_columns };
 }
 
-=head2 updateProblemPool
+=head2 deleteProblemPool
 
-updates the parameters of an existing problem pool
+delete a Problem Pool
 
-=cut
+=head3 arguments
 
-sub deleteProblemPool ($self, %args) {
-	my $pool = $self->getProblemPool(info => $args{info}, as_result_set => 1);
+=over
 
-	DB::Exception::PoolNotInCourse->throws(error => 'The problem pool does not exist')
-		unless defined($pool);
+=item * C<info> a hashref specifying information on the problem pool
+One must include either the C<course_id> or C<course_name> and either the C<pool_name>
+or C<problem_pool_id>.  If there is not enough info to get the course and pool, a
+C<ParametersNeeded> exception is thrown.
+
+
+=item * C<as_result_set>: boolean
+
+If C<as_result_set> is true, then the user sets are returned as a C<ResultSet>.
+See C<DBIx::Class::ResultSet> for more information
+
+=back
+
+=head3 output
 
-	my $deleted_pool = $pool->delete();
+Nothing (undef) is returned.
 
-	return $args{as_result_set} ? $deleted_pool : { $deleted_pool->get_columns };
+=cut
+
+sub deleteProblemPool ($self, %args) {
+	$self->getProblemPool(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 #####
@@ -257,7 +312,6 @@ This gets a single problem out of a ProblemPool.
 =cut
 
 sub getPoolProblem ($self, %args) {
-
 	my $problem_pool = $self->getProblemPool(info => $args{info}, as_result_set => 1);
 
 	my $pool_problem_info = {};
@@ -273,6 +327,13 @@ sub getPoolProblem ($self, %args) {
 
 	if (scalar(@pool_problems) == 1) {
 		return $args{as_result_set} ? $pool_problems[0] : { $pool_problems[0]->get_inflated_columns };
+	} elsif (scalar(@pool_problems) == 0) {
+		DB::Exception::PoolProblemNotInPool->throw(message => 'The problem with id '
+				. $pool_problem_info->{pool_problem_id}
+				. ' is not in the pool named \''
+				. $problem_pool->pool_name
+				. "'");
+		return;
 	} else {
 		# Pick a random problem.
 		my $prob = $pool_problems[ rand @pool_problems ];
@@ -374,12 +435,8 @@ remove a Problem out of a ProblemPool in a course
 =cut
 
 sub removePoolProblem ($self, %args) {
-	my $prob = $self->getPoolProblem(info => $args{info}, as_result_set => 1);
-	DB::Exception::PoolProblemNotInPool->throw(info => $args{info}) unless defined($prob);
-
-	my $prob2 = $prob->delete;
-	return $prob2 if $args{as_result_set};
-	return { $prob2->get_inflated_columns };
+	$self->getPoolProblem(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 # just a small subroutine to shorten access to the db.

lib/DB/Schema/ResultSet/ProblemSet.pm

@@ -509,19 +509,13 @@ if false, a hashrefs of ProblemSet.
 
 =head3 output
 
-An hashref of the deleted problem set or an object of type C<DBIx::Class::ResultSet::ProblemSet>
+Nothing (undef) is returned.
 
 =cut
 
 sub deleteProblemSet ($self, %args) {
-
-	my $set_to_delete = $self->getProblemSet(info => $args{info}, as_result_set => 1);
-	$set_to_delete->delete;
-
-	return $set_to_delete if $args{as_result_set};
-	my $set = { $set_to_delete->get_inflated_columns, set_type => $set_to_delete->set_type };
-	delete $set->{type};
-	return $set;
+	$self->getProblemSet(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 # The following are private methods used in this module.

lib/DB/Schema/ResultSet/SetProblem.pm

@@ -157,7 +157,7 @@ if either the course or set doesn't exist, an exception will be thrown.
 
 =head3 output
 
-An array of Problems (as hashrefs) or an array of C<DBIx::Class::ResultSet::Problem>
+A problem (as a hashref) or an array of C<DBIx::Class::ResultSet::Problem>
 
 =cut
 
@@ -350,26 +350,13 @@ if either the course or set doesn't exist, an exception will be thrown.
 
 =head3 output
 
-A problem (as hashrefs) or an object of class C<DBIx::Class::ResultSet::Problem>
+Nothing (undef) is returned.
 
 =cut
 
 sub deleteSetProblem ($self, %args) {
-	my $set_problem = $self->getSetProblem(info => $args{info}, as_result_set => 1);
-	my $problem_set = $self->rs('ProblemSet')->getProblemSet(
-		info => {
-			course_id => $set_problem->problem_set->course_id,
-			set_id    => $set_problem->set_id
-		},
-		as_result_set => 1
-	);
-
-	my $problem = $problem_set->search_related('problems', getProblemInfo($args{info}))->single;
-
-	my $deleted_problem = $problem->delete;
-
-	return $deleted_problem if $args{as_result_set};
-	return { $deleted_problem->get_inflated_columns };
+	$self->getSetProblem(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 # just a small subroutine to shorten access to the db.

lib/DB/Schema/ResultSet/User.pm

@@ -149,11 +149,8 @@ The deleted user as a C<DBIx::Class::ResultSet::User> object.
 # TODO: Delete everything related to the user from all tables.
 
 sub deleteGlobalUser ($self, %args) {
-	my $user_to_delete = $self->getGlobalUser(info => $args{info}, as_result_set => 1);
-
-	my $deleted_user = $user_to_delete->delete;
-	return $deleted_user if $args{as_result_set};
-	return removeLoginParams({ $deleted_user->get_inflated_columns });
+	$self->getGlobalUser(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 =head1 updateGlobalUser
@@ -326,9 +323,17 @@ An hashref of the user or merged user or a C<DBIx::Class::ResultSet>
 sub getCourseUser ($self, %args) {
 	my $course_user;
 
-	if (defined($args{info}->{course_user_id})) {
+	if (defined($args{info}{course_user_id})) {
 		$course_user = $self->rs('CourseUser')
 			->find({ course_user_id => $args{info}->{course_user_id} }, { prefetch => [qw/role/] });
+		DB::Exception::UserNotInCourse->throw(
+			message => "The user with id '$args{info}->{course_user_id}' is not enrolled in the course "
+				. (
+					$args{info}->{course_name}
+					? " with name '$args{info}->{course_name}'"
+					: " with course_id '$args{info}->{course_id}'."
+				)
+		) unless defined $course_user || $args{skip_throw};
 	} else {
 		my $course = $self->rs('Course')->getCourse(info => getCourseInfo($args{info}), as_result_set => 1);
 		my $user   = $self->getGlobalUser(info => getUserInfo($args{info}), as_result_set => 1);
@@ -533,21 +538,18 @@ from the global user table)
 
 =over
 =item - If either information about the user or the course is missing, an exception will be thrown
-=item - If the user is already in the course, an exception will be thrown.
+=item - If the user is not in the course, an exception will be thrown.
 =back
 
 =head3 output
 
-An hashref of the deleted user or merged user or a C<DB::Schema::ResultSet::CourseUser>
+Nothing (undef) is returned.
 
 =cut
 
 sub deleteCourseUser ($self, %args) {
-	my $course_user_to_delete = $self->getCourseUser(info => $args{info}, as_result_set => 1)->delete;
-
-	return $course_user_to_delete if $args{as_result_set};
-	return $args{merged} ? _getMergedUser($course_user_to_delete) : _getCourseUser($course_user_to_delete);
-
+	$self->getCourseUser(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 # This is a small subroutine to shorten access to the db.

lib/DB/Schema/ResultSet/UserProblem.pm

@@ -490,27 +490,8 @@ or a C<DBIx::Class::ResultSet::UserProblem>
 =cut
 
 sub deleteUserProblem ($self, %args) {
-	my $user_problem =
-		$args{info}->{user_problem_id}
-		? $self->find({ user_problem_id => $args{info}->{user_problem_id} })
-		: $self->getUserProblem(
-			info          => $args{info},
-			skip_throw    => 1,
-			as_result_set => 1
-		);
-
-	DB::Exception::UserProblemNotFound->throw(message => 'The user '
-			. getUserInfo($args{info})->{username} // getUserInfo($args{info})->{user_id}
-			. ' already has problem number '
-			. getProblemInfo($args{info})->{problem_number}
-			// ("(set_problem_id): " . getProblemInfo($args{info})->{set_problem_id})
-			. ' in set with name'
-			. getSetInfo($args{info})->{set_name} // ("(set_id): " . getSetInfo($args{info})->{set_id}))
-		unless $user_problem;
-
-	my $user_problem_to_delete = $user_problem->delete;
-	return $user_problem_to_delete if $args{as_result_set};
-	return $args{merged} ? _mergeUserProblem($user_problem_to_delete) : _getUserProblem($user_problem_to_delete);
+	$self->getUserProblem(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 =head2 getUserProblemVersions

lib/DB/Schema/ResultSet/UserSet.pm

@@ -374,10 +374,10 @@ sub updateUserSet ($self, %args) {
 	my $user_set = $self->getUserSet(info => $args{info}, as_result_set => 1, skip_throw => 1);
 
 	DB::Exception::UserSetNotInCourse->throw(message => 'The user '
-			. $args{info}{username}
+			. ($args{info}{username} ? "with username '$args{info}{username}'" : "with user_id '$args{info}{user_id}'")
 			. ' is not assigned to the set '
-			. $args{info}{set_name}
-			. ' with set version '
+			. ($args{info}{set_name} ? "with set name '$args{info}{set_name}'" : "with set_id '$args{info}{set_id}'")
+			. ' and set version '
 			. ($args{info}{set_version} // 0))
 		unless defined($user_set);
 
@@ -408,18 +408,8 @@ delete a single UserSet for a given course, user, and ProblemSet
 =cut
 
 sub deleteUserSet ($self, %args) {
-	my $user_set = $self->getUserSet(info => $args{info}, as_result_set => 1);
-
-	DB::Exception::UserSetNotInCourse->throw(
-		set_name    => $args{info}->{set_name},
-		course_name => $args{info}->{course_name},
-		username    => $args{info}->{username}
-	) unless defined($user_set);
-
-	$user_set->delete;
-	# why are we returning anything from this call? success/failure instead?
-	return $user_set if $args{as_result_set};
-	return $args{merged} ? _mergeUserSet($user_set) : _getUserSet($user_set);
+	my $user_set = $self->getUserSet(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 =head2 getUserSetVersions