Improvement to the general models

lib/DB/Schema/Result/ProblemSet/HWSet.pm

@@ -27,6 +27,10 @@ sub valid_dates ($=) {
 	return [ 'open', 'reduced_scoring', 'due', 'answer' ];
 }
 
+sub optional_fields_in_dates ($=) {
+	return { enable_reduced_scoring => 'bool' };
+}
+
 =head2 C<required_dates>
 
 subroutine that returns the array for the required dates: C<['open', 'due' ,'answer']>
@@ -79,11 +83,10 @@ This is a description of the homework set.
 
 sub valid_params ($=) {
 	return {
-		enable_reduced_scoring => 'bool',
-		hide_hint              => 'bool',
-		hardcopy_header        => q{.*},
-		set_header             => q{.*},
-		description            => q{.*}
+		hide_hint       => 'bool',
+		hardcopy_header => q{.*},
+		set_header      => q{.*},
+		description     => q{.*}
 	};
 }
 

lib/DB/Schema/Result/ProblemSet/Quiz.pm

@@ -27,6 +27,8 @@ sub valid_dates ($=) {
 	return [ 'open', 'due', 'answer' ];
 }
 
+sub optional_fields_in_dates ($=) { return {}; }
+
 =head2 C<required_dates>
 
 subroutine that returns the array for the required dates: C<['open', 'due' ,'answer']>

lib/DB/Schema/Result/ProblemSet/ReviewSet.pm

@@ -26,6 +26,8 @@ sub valid_dates ($=) {
 	return [ 'open', 'closed' ];
 }
 
+sub optional_fields_in_dates ($=) { return {}; }
+
 =head2 C<required_dates>
 
 subroutine that returns the array for the required dates: C<['open', 'closed']>

lib/DB/Schema/Result/UserSet.pm

@@ -155,13 +155,6 @@ __PACKAGE__->typecast_map(
 	}
 );
 
-my $set_type = {
-	1 => 'DB::Schema::Result::UserSet::HWSet',
-	2 => 'DB::Schema::Result::UserSet::Quiz',
-	3 => 'DB::Schema::Result::UserSet::JITAR',
-	4 => 'DB::Schema::Result::UserSet::ReviewSet'
-};
-
 sub set_type ($) {
 	my %set_type_rev = reverse %{$DB::Schema::ResultSet::ProblemSet::SET_TYPES};
 	return $set_type_rev{ shift->type };

lib/DB/Schema/Result/UserSet/HWSet.pm

@@ -16,6 +16,10 @@ sub required_dates ($=) {
 	return DB::Schema::Result::ProblemSet::HWSet::required_dates();
 }
 
+sub optional_fields_in_dates ($=) {
+	return DB::Schema::Result::ProblemSet::HWSet::optional_fields_in_dates();
+}
+
 sub valid_params ($=) {
 	return DB::Schema::Result::ProblemSet::HWSet::valid_params();
 }

lib/DB/Schema/Result/UserSet/Quiz.pm

@@ -16,6 +16,10 @@ sub required_dates ($=) {
 	return DB::Schema::Result::ProblemSet::Quiz::required_dates();
 }
 
+sub optional_fields_in_dates ($=) {
+	return DB::Schema::Result::ProblemSet::Quiz::optional_fields_in_dates();
+}
+
 sub valid_params ($=) {
 	return DB::Schema::Result::ProblemSet::Quiz::valid_params();
 }

lib/DB/Schema/Result/UserSet/ReviewSet.pm

@@ -16,6 +16,10 @@ sub required_dates ($=) {
 	return DB::Schema::Result::ProblemSet::ReviewSet::required_dates();
 }
 
+sub optional_fields_in_dates ($=) {
+	return DB::Schema::Result::ProblemSet::ReviewSet::optional_fields_in_dates();
+}
+
 sub valid_params ($=) {
 	return DB::Schema::Result::ProblemSet::ReviewSet::valid_params();
 }

lib/DB/Schema/ResultSet/Course.pm

@@ -12,7 +12,7 @@ use DB::Utils qw/getCourseInfo getUserInfo/;
 use DB::Exception;
 use Exception::Class ('DB::Exception::CourseNotFound', 'DB::Exception::CourseExists');
 
-#use DB::TestUtils qw/removeIDs/;
+#use TestUtils qw/removeIDs/;
 use WeBWorK3::Utils::Settings qw/getDefaultCourseSettings mergeCourseSettings
 	getDefaultCourseValues validateCourseSettings/;
 

lib/DB/Schema/ResultSet/UserSet.pm

@@ -392,10 +392,26 @@ sub addUserSet ($self, %args) {
 	$params->{set_version}    = 1  unless defined($params->{set_version});
 	my $original_dates = $params->{set_dates} // {};
 
-	# If any of the date are 0, then remove them.
+	# If any of the dates are 0, then remove them.
+	# Only check if the date_fields are 0.  There is the option to store non-dates in the
+	# set_dates field.
+	my $date_fields;
+
+	# Note: although this works okay, if another subtype of a UserSet is created, this needs to be updated.
+	if ($params->{type} == 1) {
+		$date_fields = \&DB::Schema::Result::UserSet::HWSet::valid_dates;
+	} elsif ($params->{type} == 2) {
+		$date_fields = \&DB::Schema::Result::UserSet::Quiz::valid_dates;
+	} elsif ($params->{type} == 4) {
+		$date_fields = \&DB::Schema::Result::UserSet::ReviewSet::valid_dates;
+	} else {
+		die "The type $params->{type} is not valid.";
+	}
+
 	if ($args{params}->{set_dates}) {
-		for my $key (keys %{ $args{params}->{set_dates} }) {
-			delete $args{params}->{set_dates}->{$key} if $args{params}->{set_dates}->{$key} == 0;
+		for my $key (@{ $date_fields->() }) {
+			delete $args{params}->{set_dates}->{$key}
+				if defined($args{params}->{set_dates}->{$key}) && $args{params}->{set_dates}->{$key} == 0;
 		}
 	}
 

lib/DB/WithDates.pm

@@ -10,25 +10,30 @@ use DB::Schema::Result::ProblemSet::HWSet;
 
 use DB::Exception;
 
-our $valid_dates;       # Arrayref of allowed/valid dates
-our $required_dates;    # Arrayref of required dates
+my $valid_dates;                 # Arrayref of allowed/valid dates
+my $required_dates;              # Arrayref of required dates
+my $optional_fields_in_dates;    # hashref of other non-date fields in the hash and the type.
 
 sub validDates ($self, $field_name) {
-	$valid_dates    = ref($self)->valid_dates;
-	$required_dates = ref($self)->required_dates;
+	$valid_dates              = ref($self)->valid_dates;
+	$required_dates           = ref($self)->required_dates;
+	$optional_fields_in_dates = ref($self)->optional_fields_in_dates;
 
 	$self->validDateFields($field_name);
 	$self->hasRequiredDateFields($field_name);
 	$self->validDateFormat($field_name);
 	$self->checkDates($field_name);
+	$self->validateOptionalFields($field_name);
 	return 1;
 }
 
 sub validDateFields ($self, $field_name) {
-	my @fields = keys %{ $self->get_inflated_column($field_name) };
+	my @fields     = keys %{ $self->get_inflated_column($field_name) };
+	my @all_fields = (@$valid_dates, keys %$optional_fields_in_dates);
+
 	# If this is not empty, there are illegal fields.
-	my @bad_fields = array_minus(@fields, @$valid_dates);
-	DB::Exception::InvalidDateField->throw(field_names => join(', ', @bad_fields))
+	my @bad_fields = array_minus(@fields, @all_fields);
+	DB::Exception::InvalidDateField->throw(field_names => join(", ", @bad_fields))
 		if (scalar(@bad_fields) != 0);
 
 	return 1;
@@ -70,4 +75,21 @@ sub checkDates ($self, $field_name) {
 	return 1;
 }
 
+# This checks the options fields that aren't dates
+sub validateOptionalFields ($self, $field_name) {
+	my $params_hash = $self->get_inflated_column($field_name);
+	# if it doesn't exist, it is valid
+	return 1 unless defined $params_hash;
+
+	for my $key (keys %$optional_fields_in_dates) {
+		next unless defined $params_hash->{$key};
+		my $re    = $params_hash->{$key};
+		my $valid = $re eq 'bool' ? JSON::PP::is_bool($params_hash->{$key}) : $params_hash->{$key} =~ qr/^$re$/x;
+		DB::Exception::InvalidParameter->throw(
+			message => "The parameter named $key is not valid. It has value $params_hash->{$key}")
+			unless $valid;
+	}
+	return 1;
+}
+
 1;

lib/WeBWorK3.pm

@@ -104,29 +104,40 @@ sub userRoutes ($self) {
 	my $user_routes = $self->routes->any('/webwork3/api/users')->requires(authenticated => 1)->to(controller => 'User');
 	$user_routes->get('/')->to(action => 'getGlobalUsers');
 	$user_routes->post('/')->to(action => 'addGlobalUser');
-	$user_routes->get('/:user_id')->to(action => 'getGlobalUser');
+	$user_routes->get('/:user')->to(action => 'getGlobalUser');
 	$user_routes->put('/:user_id')->to(action => 'updateGlobalUser');
 	$user_routes->delete('/:user_id')->to(action => 'deleteGlobalUser');
 	$user_routes->get('/:user_id/courses')->to(action => 'getUserCourses');
 	# Get global users for a course.
 	$self->routes->get('/webwork3/api/courses/:course_id/global-users')->to('User#getGlobalCourseUsers');
-	# This is needed to get global users as instructor permission.  Need to have
-	# the parameter course_id.
-	$self->routes->any('/webwork3/api/courses/:course_id/users/:user/exists')->requires(authenticated => 1)
-		->to('User#getGlobalUser');
+
 	return;
 }
 
 sub courseUserRoutes ($self) {
-	my $course_user_routes = $self->routes->any('/webwork3/api/courses/:course_id/users')->requires(authenticated => 1)
-		->to(controller => 'User');
-	$course_user_routes->get('/')->to(action => 'getCourseUsers');
-	$course_user_routes->post('/')->to(action => 'addCourseUser');
-	$course_user_routes->get('/:user_id')->to(action => 'getCourseUser');
-	$course_user_routes->put('/:user_id')->to(action => 'updateCourseUser');
-	$course_user_routes->delete('/:user_id')->to(action => 'deleteCourseUser');
-	$self->routes->any('/webwork3/api/courses/:course_id/courseusers')->requires(authenticated => 1)
-		->to('User#getMergedCourseUsers');
+	my $course_routes =
+		$self->routes->any('/webwork3/api/courses/:course_id')->requires(authenticated => 1)->to(controller => 'User');
+	$course_routes->get('/users')->to(action => 'getCourseUsers');
+	$course_routes->get('/users')->to(action => 'getCourseUsers');
+	$course_routes->post('/users')->to(action => 'addCourseUser');
+	$course_routes->get('/users/:user_id')->to(action => 'getCourseUser');
+	$course_routes->put('/users/:user_id')->to(action => 'updateCourseUser');
+	$course_routes->delete('/users/:user_id')->to(action => 'deleteCourseUser');
+
+	# There are some routes needed for global user crud, but the permssions require that the
+	# user has permissions within a course.
+
+	$course_routes->get('/global-courseusers')->to('User#getGlobalCourseUsers');
+	$course_routes->post('/global-users')->to('User#addGlobalUserFromCourse');
+	$course_routes->get('/global-users/:user')->to('User#getGlobalUserFromCourse');
+	$course_routes->put('/global-users/:user_id')->to('User#updateGlobalUserFromCourse');
+	$course_routes->delete('/global-users/:user_id')->to('User#deleteGlobalUserFromCourse');
+
+	$course_routes->get('/courseusers')->to('User#getMergedCourseUsers');
+
+	# This is used to check if a user with given username exists.
+	$course_routes->get('/users/:username/exists')->to('User#checkGlobalUser');
+
 	return;
 }
 

lib/WeBWorK3/Controller/User.pm

@@ -1,17 +1,32 @@
 package WeBWorK3::Controller::User;
 use Mojo::Base 'Mojolicious::Controller', -signatures;
 
+use Try::Tiny;
+
 sub getGlobalUsers ($self) {
 	my @global_users = $self->schema->resultset('User')->getAllGlobalUsers;
 	$self->render(json => \@global_users);
 	return;
 }
 
+# Passing the username into the getGlobalUser results in a problem with permssions.  This route
+# should be used to pass in the username.
+sub checkGlobalUser ($self) {
+	try {
+		my $user = $self->schema->resultset('User')->getGlobalUser(info => { username => $self->param('username') });
+		$self->render(json => $user);
+		return;
+	} catch {
+		$self->render(json => {}) if ref($_) eq 'DB::Exception::UserNotFound';
+	};
+	return;
+}
+
 sub getGlobalUser ($self) {
 	my $user =
-		$self->param('user_id') =~ /^\d+$/
-		? $self->schema->resultset('User')->getGlobalUser(info => { user_id  => int($self->param('user_id')) })
-		: $self->schema->resultset('User')->getGlobalUser(info => { username => $self->param('user_id') });
+		$self->param('user') =~ /^\d+$/
+		? $self->schema->resultset('User')->getGlobalUser(info => { user_id  => int($self->param('user')) })
+		: $self->schema->resultset('User')->getGlobalUser(info => { username => $self->param('user') });
 	$self->render(json => $user);
 	return;
 }
@@ -48,6 +63,14 @@ sub getGlobalCourseUsers ($self) {
 	return;
 }
 
+# The following are needed for handling global users from instructors in a course
+
+sub getGlobalUsersFromCourse   ($self) { $self->getGlobalUsers;   return }
+sub getGlobalUserFromCourse    ($self) { $self->getGlobalUser;    return }
+sub addGlobalUserFromCourse    ($self) { $self->addGlobalUser;    return }
+sub updateGlobalUserFromCourse ($self) { $self->updateGlobalUser; return }
+sub deleteGlobalUserFromCourse ($self) { $self->deleteGlobalUser; return }
+
 # The following subs are related to users within a given course.
 
 sub getMergedCourseUsers ($self) {

src/common/api-requests/errors.ts

@@ -0,0 +1,15 @@
+// General Error coming from the API service
+
+import { logger } from 'boot/logger';
+import { Model } from '../models';
+
+export interface ResponseError {
+	exception: string;
+	message: string;
+}
+
+export const invalidError = (model: Model, msg: string) => {
+	logger.error(msg);
+	logger.error(JSON.stringify(model.toObject()));
+	return Promise.reject(new Error(msg));
+};

src/common/api-requests/user.ts

@@ -1,24 +1,32 @@
 import { api } from 'boot/axios';
 
-import { ParseableCourseUser, ParseableUser } from 'src/common/models/users';
-import { ResponseError } from 'src/common/api-requests/interfaces';
+import { ParseableCourseUser, ParseableUser, User } from 'src/common/models/users';
+import { ResponseError } from 'src/common/api-requests/errors';
 
-export async function checkIfUserExists(course_id: number, username: string) {
-	const response = await api.get(`courses/${course_id}/users/${username}/exists`);
-	if (response.status === 250) {
-		throw response.data as ResponseError;
+/**
+ * Checks if a global user exists. Both the course_id and username need to be passed in
+ * in order to check permissions.
+ *
+ * @returns either an existing user or an empty object.
+ */
+
+export async function checkIfUserExists(course_id: number, username: string): Promise<ParseableCourseUser> {
+	try {
+		const response = await api.get(`courses/${course_id}/users/${username}/exists`);
+		return response.data as ParseableCourseUser;
+	} catch (_err) {
+		return {};
 	}
-	return response.data as ParseableCourseUser;
 }
 
 /**
  * Gets the global user in the database given by username. This returns a user or throws a
  * ResponseError if the user is not found.
  */
-export async function getUser(username: string): Promise<ParseableUser> {
+export async function getUser(username: string): Promise<User> {
 	const response = await api.get(`users/${username}`);
 	if (response.status === 200) {
-		return response.data as ParseableUser;
+		return new User(response.data as ParseableUser);
 	} else {
 		throw response.data as ResponseError;
 	}