WIP: fix up tests and general cleanup.

WIP: fix up tests and general cleanup.


WIP: fix up tests and general cleanup.

Author: pstaabp

lib/DB/Schema/Result/CourseSetting.pm

@@ -43,19 +43,16 @@ __PACKAGE__->add_columns(
 		is_auto_increment => 1,
 	},
 	course_id => {
-		data_type   => 'integer',
-		size        => 16,
-		is_nullable => 0,
+		data_type => 'integer',
+		size      => 16,
 	},
 	setting_id => {
-		data_type   => 'integer',
-		size        => 16,
-		is_nullable => 0,
+		data_type => 'integer',
+		size      => 16,
 	},
 	value => {
 		data_type          => 'text',
-		is_nullable        => 0,
-		default_value      => '\'\'',
+		default_value      => '{}',
 		serializer_class   => 'JSON',
 		serializer_options => { utf8 => 1 }
 	},

lib/DB/Schema/Result/CourseSettings.pm

@@ -37,7 +37,7 @@ C<type>: a string representation of the type of setting (boolean, text, list, ..
 
 =item *
 
-C<options>: a JSON object that stores options if the setting is an list or multilist
+C<options>: a JSON array that stores options if the setting is an list or multilist
 
 =item *
 
@@ -59,24 +59,21 @@ __PACKAGE__->add_columns(
 	setting_id => {
 		data_type         => 'integer',
 		size              => 16,
-		is_nullable       => 0,
 		is_auto_increment => 1,
 	},
 	setting_name => {
-		data_type   => 'varchar',
-		size        => 256,
-		is_nullable => 0,
+		data_type => 'varchar',
+		size      => 256,
 	},
 	default_value => {
 		data_type          => 'text',
-		is_nullable        => 0,
-		default_value      => '\'\'',
+		default_value      => '{}',
+		retrieve_on_insert => 1,
 		serializer_class   => 'JSON',
 		serializer_options => { utf8 => 1 }
 	},
 	description => {
 		data_type     => 'text',
-		is_nullable   => 0,
 		default_value => '',
 	},
 	doc => {
@@ -85,20 +82,19 @@ __PACKAGE__->add_columns(
 	},
 	type => {
 		data_type     => 'varchar',
-		size          => 16,
-		is_nullable   => 0,
+		size          => 64,
 		default_value => '',
 	},
 	options => {
 		data_type          => 'text',
 		is_nullable        => 1,
+		retrieve_on_insert => 1,
 		serializer_class   => 'JSON',
 		serializer_options => { utf8 => 1 }
 	},
 	category => {
 		data_type     => 'varchar',
 		size          => 64,
-		is_nullable   => 0,
 		default_value => ''
 	},
 	subcategory => {
@@ -112,6 +108,4 @@ __PACKAGE__->set_primary_key('setting_id');
 
 __PACKAGE__->has_many(course_settings => 'DB::Schema::Result::CourseSetting', 'setting_id');
 
-# __PACKAGE__->belongs_to(course => 'DB::Schema::Result::Course', 'course_id');
-
 1;

lib/DB/Schema/Result/GlobalSetting.pm

@@ -10,11 +10,11 @@ use base 'DBIx::Class::ResultSet';
 use Clone qw/clone/;
 use DB::Utils qw/getCourseInfo getUserInfo getSettingInfo/;
 
-use Exception::Class qw(
+use Exception::Class qw/
 	DB::Exception::CourseNotFound
 	DB::Exception::CourseExists
 	DB::Exception::SettingNotFound
-);
+	/;
 
 use WeBWorK3::Utils::Settings qw/ mergeCourseSettings isValidSetting/;
 

lib/DB/Schema/ResultSet/Course.pm

@@ -6,10 +6,10 @@ use feature 'signatures';
 no warnings qw/experimental::signatures/;
 
 require Exporter;
-use base qw(Exporter);
+use base qw/Exporter/;
 our @EXPORT_OK = qw/getCourseInfo getUserInfo getSetInfo updateAllFields updatePermissions
 	getPoolInfo getProblemInfo getPoolProblemInfo getSettingInfo removeLoginParams
-	convertTimeDuration/;
+	convertTimeDuration humanReadableTimeDuration/;
 
 use Clone qw/clone/;
 use Scalar::Util qw/reftype/;
@@ -205,15 +205,43 @@ sub convertTimeDuration ($time_duration) {
 	if ($time_duration =~ /^(\d+)\s(sec)s?$/) {
 		return $1;
 	} elsif ($time_duration =~ /^(\d+)\s(min(ute)?)s?$/) {
-		return $1*60;
+		return $1 * 60;
 	} elsif ($time_duration =~ /^(\d+)\s(h(ou)?r)s?$/) {
-		return $1*60*60;
+		return $1 * 60 * 60;
 	} elsif ($time_duration =~ /^(\d+)\s(day)s?$/) {
-		return $1*60*60*24;
+		return $1 * 60 * 60 * 24;
 	} elsif ($time_duration =~ /^(\d+)\s(week)s?$/) {
-		return $1*60*60*24*7;
+		return $1 * 60 * 60 * 24 * 7;
+	} else {
+		return 0;
 	}
 }
 
+=pod
+=head2
+
+This coverts a number of seconds to a human-readable format
+
+=cut
+
+sub humanReadableTimeDuration ($td) {
+	my $times = {
+		week => int($td / 604800),
+		day  => int($td % 604800 / 86400),
+		hour => int($td % 86400 / 3600),
+		min  => int($td % 3600 / 60),
+		sec  => $td % 60
+	};
+
+	my $time_duration = '';
+	# Order is important so the keys are defined.
+	for (qw/week day hour min sec/) {
+		my $val = $times->{$_};
+		$time_duration .= ($time_duration ne '' && $val ? ', ' : '')
+			# pluralize for more than 1.
+			. ($val > 0 ? "$val $_" . ($val == 1 ? '' : 's') : '');
+	}
+	return $time_duration;
+}
 
 1;

lib/DB/Utils.pm

@@ -216,8 +216,8 @@ sub problemRoutes ($app, $course_routes) {
 	return;
 }
 
-sub settingsRoutes ($self, $course_routes) {
-	my $global_settings = $self->routes->any('/webwork3/api/global-settings')->requires(authenticated => 1);
+sub settingsRoutes ($app, $course_routes) {
+	my $global_settings = $app->routes->any('/webwork3/api/global-settings')->requires(authenticated => 1);
 	$global_settings->get('/')->to('Settings#getGlobalSettings');
 	$global_settings->get('/:setting_id')->to('Settings#getGlobalSetting');
 	$global_settings->post('/check-timezone')->to('Settings#checkTimeZone');

lib/WeBWorK3.pm

@@ -65,6 +65,9 @@ sub deleteCourseSetting ($c) {
 	return;
 }
 
+# This is useful for checking if the string passed in is a valid timezone, instead of
+# having the UI download all possible timezones (bloated).
+
 sub checkTimeZone ($c) {
 	try {
 		DateTime::TimeZone->new(name => $c->req->json->{timezone});

lib/WeBWorK3/Controller/Settings.pm

@@ -8,14 +8,14 @@ no warnings qw/experimental::signatures/;
 use YAML::XS qw/LoadFile/;
 
 require Exporter;
-use base qw(Exporter);
+use base qw/Exporter/;
 our @EXPORT_OK = qw/isValidSetting mergeCourseSettings isInteger isTimeString isTimeDuration isDecimal/;
 
-use Exception::Class qw(
+use Exception::Class qw/
 	DB::Exception::UndefinedCourseField
 	DB::Exception::InvalidCourseField
 	DB::Exception::InvalidCourseFieldType
-);
+	/;
 
 use DateTime::TimeZone;
 use JSON::PP;
@@ -80,35 +80,34 @@ sub isValidSetting ($setting, $value = undef) {
 	if ($setting->{type} eq 'text') {
 		# any val is valid.
 	} elsif ($setting->{type} eq 'boolean') {
-		my $is_bool = JSON::PP::is_bool($val);
 		DB::Exception::InvalidCourseFieldType->throw(
-			message => qq/The variable $setting->{setting_name} has value $val and must be a boolean./)
-			unless $is_bool;
+			message => "The variable $setting->{setting_name} has value $val and must be a boolean.")
+			unless JSON::PP::is_bool($val);
 	} elsif ($setting->{type} eq 'list') {
 		validateList($setting, $val);
 	} elsif ($setting->{type} eq 'multilist') {
 		validateMultilist($setting, $val);
 	} elsif ($setting->{type} eq 'time') {
-		DB::Exception::InvalidCourseFieldType->throw(message =>
-				qq/The variable $setting->{setting_name} has value $val and must be a time in the form XX:XX/)
+		DB::Exception::InvalidCourseFieldType->throw(
+			message => "The variable $setting->{setting_name} has value $val and must be a time in the form XX:XX")
 			unless isTimeString($val);
 	} elsif ($setting->{type} eq 'int') {
 		DB::Exception::InvalidCourseFieldType->throw(
-			message => qq/The variable $setting->{setting_name} has value $val and must be an integer./)
+			message => "The variable $setting->{setting_name} has value $val and must be an integer.")
 			unless isInteger($val);
 	} elsif ($setting->{type} eq 'decimal') {
 		DB::Exception::InvalidCourseFieldType->throw(
-			message => qq/The variable $setting->{setting_name} has value $val and must be a decimal/)
+			message => "The variable $setting->{setting_name} has value $val and must be a decimal.")
 			unless isDecimal($val);
 	} elsif ($setting->{type} eq 'time_duration') {
 		DB::Exception::InvalidCourseFieldType->throw(
-			message => qq/The variable $setting->{setting_name} has value $val and must be a time duration/)
+			message => "The variable $setting->{setting_name} has value $val and must be a time duration.")
 			unless $val =~ /^\d+$/;
 	} elsif ($setting->{type} eq 'timezone') {
 		# try to make a new timeZone.  If the name isn't valid an 'Invalid offset:' will be thrown.
 		DateTime::TimeZone->new(name => $val);
 	} else {
-		DB::Exception::InvalidCourseFieldType->throw(message => qq/The setting type $setting->{type} is not valid/);
+		DB::Exception::InvalidCourseFieldType->throw(message => "The setting type $setting->{type} is not valid.");
 	}
 	return 1;
 }

lib/WeBWorK3/Utils/Settings.pm

@@ -166,10 +166,9 @@ export function parseString(_value: string | number | boolean) {
 
 /**
  * Converts a time_duration type setting to a human-readable one.
- * TODO: use localization for this.
  * @params td - time duration in seconds.
  */
-
+// TODO: use localization for this.
 export const humanReadableTimeDuration = (td: number): string => {
 	const times = {
 		week: Math.floor(td / 604800),