WIP: use non-neg ints in db for time_durations and human readability on UI.

Author: pstaabp

lib/DB/Utils.pm

@@ -8,10 +8,10 @@ no warnings qw/experimental::signatures/;
 require Exporter;
 use base qw(Exporter);
 our @EXPORT_OK = qw/getCourseInfo getUserInfo getSetInfo updateAllFields updatePermissions
-	getPoolInfo getProblemInfo getPoolProblemInfo getSettingInfo removeLoginParams/;
+	getPoolInfo getProblemInfo getPoolProblemInfo getSettingInfo removeLoginParams
+	convertTimeDuration/;
 
 use Clone qw/clone/;
-use List::Util qw/first/;
 use Scalar::Util qw/reftype/;
 use YAML::XS qw/LoadFile/;
 
@@ -193,4 +193,27 @@ sub updatePermissions ($ww3_conf, $role_perm_file) {
 	return;
 }
 
+=pod
+=head2 convertTimeDuration
+
+This subroutine converts time durations stored as a string in human-readable format
+to a number of seconds.
+
+=cut
+
+sub convertTimeDuration ($time_duration) {
+	if ($time_duration =~ /^(\d+)\s(sec)s?$/) {
+		return $1;
+	} elsif ($time_duration =~ /^(\d+)\s(min(ute)?)s?$/) {
+		return $1*60;
+	} elsif ($time_duration =~ /^(\d+)\s(h(ou)?r)s?$/) {
+		return $1*60*60;
+	} elsif ($time_duration =~ /^(\d+)\s(day)s?$/) {
+		return $1*60*60*24;
+	} elsif ($time_duration =~ /^(\d+)\s(week)s?$/) {
+		return $1*60*60*24*7;
+	}
+}
+
+
 1;

lib/WeBWorK3/Utils/Settings.pm

@@ -103,7 +103,7 @@ sub isValidSetting ($setting, $value = undef) {
 	} 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/)
-			unless isTimeDuration($val);
+			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);

src/common/models/parsers.ts

@@ -99,7 +99,8 @@ export const mail_re = /^[\w.]+@([a-zA-Z_.]+)+\.[a-zA-Z]{2,9}$/;
 export const username_re = /^[_a-zA-Z]([a-zA-Z._0-9])+$/;
 export const time_re = /^([01][0-9]|2[0-3]):[0-5]\d$/;
 // Update this for localization
-export const time_duration_re = /^(\d+)\s(sec|second|min|minute|day|week|hr|hour)s?$/i;
+// This a regexp for time durations separated by commas.
+export const time_duration_re = /^(((\d+)\s(sec|second|min|minute|day|week|hr|hour)s?),?\s?)+$/i;
 
 // Checking functions
 
@@ -110,7 +111,7 @@ export const isValidEmail = (v: string) => mail_re.test(v);
 export const isTimeDuration = (v: string) => time_duration_re.test(v);
 export const isTime = (v: string) => time_re.test(v);
 
-// Parsing functionis
+// Parsing functions
 
 export function parseNonNegInt(val: string | number) {
 	if (isNonNegInt(val)) return parseInt(`${val}`);
@@ -162,3 +163,53 @@ export function parseString(_value: string | number | boolean) {
 		return _value;
 	}
 }
+
+/**
+ * Converts a time_duration type setting to a human-readable one.
+ * TODO: use localization for this.
+ * @params td - time duration in seconds.
+ */
+
+export const humanReadableTimeDuration = (td: number): string => {
+	const times = {
+		week: Math.floor(td / 604800),
+		day: Math.floor(td % 604800 / 86400),
+		hour: Math.floor(td % 86400 / 3600),
+		min: Math.floor(td % 3600 / 60),
+		sec: td % 60
+	};
+
+	return Object.entries(times).reduce((prev: string, [key, value]) => prev +
+		// if the time value is non zero, and there is already something in prev, add a comma
+		(prev != '' && value > 0 ? ', ' : '') +
+		// pluralize.
+		(value > 0 ? `${value} ${key}${value === 1 ? '' : 's'}` : ''), '');
+};
+
+/**
+ * Convert a time_duration as a string (possibility separated by commas) to a number of seconds.
+ */
+
+export const convertTimeDuration = (dur: string): number => {
+	const times = dur.split(/,\s/);
+	let time_duration = 0;
+	times.forEach(t => {
+		const match_sec = /^(\d+)\s(sec(ond)?)s?$/.exec(t);
+		const match_min = /^(\d+)\s(min(ute)?)s?$/.exec(t);
+		const match_hr = /^(\d+)\s(h(ou)?r)s?$/.exec(t);
+		const match_day = /^(\d+)\s(day)s?$/.exec(t);
+		const match_week = /^(\d+)\s(week)s?$/.exec(t);
+		if (match_sec) {
+			time_duration += parseInt(match_sec[0]);
+		} else if (match_min) {
+			time_duration += parseInt(match_min[0]) * 60;
+		} else if (match_hr) {
+			time_duration += parseInt(match_hr[0]) * 3600;
+		} else if (match_day) {
+			time_duration += parseInt(match_day[0]) * 86400;
+		} else if (match_week) {
+			time_duration += parseInt(match_week[0]) * 604800;
+		}
+	});
+	return time_duration;
+};

src/common/models/settings.ts

@@ -2,7 +2,7 @@
 /* These are related to Course Settings */
 
 import { Model } from '.';
-import { isTime, isTimeDuration } from './parsers';
+import { isNonNegInt, isTime } from './parsers';
 
 export enum SettingType {
 	int = 'int',
@@ -132,7 +132,7 @@ const validSettingValue = (setting: GlobalSetting | CourseSetting, v: SettingVal
 	case SettingType.text: return typeof(v) === 'string';
 	case SettingType.boolean: return typeof(v) === 'boolean';
 	case SettingType.time: return typeof(v) === 'string' && isTime(v);
-	case SettingType.time_duration: return typeof(v) === 'string' && isTimeDuration(v);
+	case SettingType.time_duration: return typeof(v) === 'number' && isNonNegInt(v);
 	case SettingType.timezone: return typeof(v) === 'string';
 	default: return false;
 

src/components/instructor/SingleSetting.vue

@@ -28,7 +28,7 @@
 			<q-select v-if="setting.type === 'multilist'" multiple v-model="multilist_value" :options="options" />
 			<input-with-blur
 				v-if="setting.type === 'time_duration'"
-				v-model="setting_value"
+				v-model="time_duration_value"
 				lazy-rules
 				:rules="[checkTimeDuration]"
 			/>
@@ -48,7 +48,7 @@ import InputWithBlur from 'src/components/common/InputWithBlur.vue';
 import { logger } from 'src/boot/logger';
 
 import { useSettingsStore } from 'src/stores/settings';
-import { isTimeDuration } from 'src/common/models/parsers';
+import { convertTimeDuration, humanReadableTimeDuration, isTimeDuration } from 'src/common/models/parsers';
 import { api } from 'src/boot/axios';
 
 const props = defineProps<{
@@ -69,6 +69,9 @@ const option_value = ref<OptionType>({ value: '', label: '' });
 const multilist_value = ref<OptionType[]>([]);
 const options = ref<OptionType[]>([]);
 
+// Needed for time_duration
+const time_duration_value = ref('');
+
 // Determine if the help in settings.doc is shown.
 const show_help = ref(false);
 
@@ -77,6 +80,11 @@ const checkTimeDuration = (val: string) => isTimeDuration(val) || 'This must be
 
 const valid_timezone = ref(true);
 
+// Convert the time_duration to human readable format:
+if (course_setting.value.type === 'time_duration') {
+	time_duration_value.value = humanReadableTimeDuration(course_setting.value.value as number);
+}
+
 // These are for type list/multilist
 if (course_setting.value.options) {
 	options.value = course_setting.value.options.map((opt: string | OptionType) =>
@@ -137,6 +145,13 @@ watch(() => multilist_value.value, async () => {
 		await updateCourseSetting();
 	}
 });
+
+watch(() => time_duration_value.value, async () => {
+	if (time_duration_value.value) {
+		course_setting.value.value = convertTimeDuration(time_duration_value.value);
+		await updateCourseSetting();
+	}
+});
 </script>
 
 <style lang="scss" scoped>

t/db/002_course_settings.t

@@ -24,6 +24,7 @@ use DB::Schema;
 use WeBWorK3::Utils::Settings qw/isInteger isTimeString isTimeDuration isDecimal mergeCourseSettings
 	isValidSetting/;
 
+use DB::Utils qw/convertTimeDuration/;
 use TestUtils qw/removeIDs loadCSV/;
 
 # Load the database
@@ -77,6 +78,26 @@ ok(isDecimal('00.33'),  'check type: decimal');
 ok(!isDecimal("0-.33"), 'check type: not a decimal');
 ok(!isDecimal('abc'),   'check type: not a decimal');
 
+# Check that time duration conversion works as intended.
+is(convertTimeDuration('15 sec'), 15, 'convertTimeDuration: 15 sec');
+is(convertTimeDuration('15 secs'), 15, 'convertTimeDuration: 15 secs');
+
+is(convertTimeDuration('15 min'), 900, 'convertTimeDuration: 15 min');
+is(convertTimeDuration('15 mins'), 900, 'convertTimeDuration: 15 mins');
+is(convertTimeDuration('15 minute'), 900, 'convertTimeDuration: 15 minute');
+is(convertTimeDuration('15 minutes'), 900, 'convertTimeDuration: 15 minutes');
+
+is(convertTimeDuration('6 hour'), 21600, 'convertTimeDuration: 6 hour');
+is(convertTimeDuration('6 hours'), 21600, 'convertTimeDuration: 6 hours');
+is(convertTimeDuration('6 hr'), 21600, 'convertTimeDuration: 6 hr');
+is(convertTimeDuration('6 hrs'), 21600, 'convertTimeDuration: 6 hrs');
+
+is(convertTimeDuration('3 day'), 259200, 'convertTimeDuration: 3 day');
+is(convertTimeDuration('3 days'), 259200, 'convertTimeDuration: 3 days');
+
+is(convertTimeDuration('2 week'), 1209600, 'convertTimeDuration: 2 week');
+is(convertTimeDuration('2 weeks'), 1209600, 'convertTimeDuration: 2 weeks');
+
 # Check that each of the given course_setting types are both valid and invalid.
 my $valid_setting = {
 	setting_name  => 'my_setting',

t/db/build_db.pl

@@ -22,7 +22,7 @@ BEGIN
 use Mojo::JSON qw/true false/;
 
 use DB::Schema;
-use DB::Utils qw/updatePermissions/;
+use DB::Utils qw/updatePermissions convertTimeDuration/;
 use TestUtils qw/loadCSV/;
 
 my $verbose = 1;
@@ -82,17 +82,24 @@ sub addCourses {
 	}
 	return;
 }
+use Data::Dumper;
+
 
 sub addSettings {
 	say 'adding default settings' if $verbose;
 
 	my $settings_file = "$main::ww3_dir/conf/course_settings.yml";
 	$settings_file = "$main::ww3_dir/conf/course_settings.dist.yml" unless -r $settings_file;
-	say $settings_file;
 	die "The default settings file: '$settings_file' does not exist or is not readable"
 		unless -r $settings_file;
 	my $course_settings = LoadFile($settings_file);
 	for my $setting (@$course_settings) {
+
+		# If the setting is a time_duration, store it as a number of seconds in the db.
+		if ($setting->{type} eq 'time_duration') {
+			$setting->{default_value} = convertTimeDuration($setting->{default_value});
+		}
+
 		# encode default_value as a JSON object.
 		$setting->{default_value} = { value => $setting->{default_value} };
 		$global_setting_rs->create($setting);
@@ -106,6 +113,12 @@ sub addSettings {
 		my $global_setting = $global_setting_rs->find({ setting_name => $setting->{setting_name} });
 		die "the setting: '$setting->{setting_name}' does not exist in the db" unless $global_setting;
 
+		# If the setting is a time_duration, store it as a number of seconds in the db.
+		if ($global_setting->type eq 'time_duration') {
+			$setting->{setting_value} = convertTimeDuration($setting->{setting_value});
+		}
+
+
 		$course->add_to_course_settings({
 			course_id  => $course->course_id,
 			setting_id => $global_setting->setting_id,

t/db/sample_data/course_settings.csv

@@ -1,5 +1,6 @@
 course_name,setting_name,setting_value
 Precalculus,institution,"Springfield CC"
+Precalculus,session_key_timeout,"20 mins"
 "Abstract Algebra",institution,"Springfield University"
 Topology,institution,"Springfield University"
 Arithmetic,institution,"Springfield CC"

t/mojolicious/015_course_settings.t

@@ -40,7 +40,9 @@ $t->post_ok('/webwork3/api/login' => json => { username => 'lisa', password => '
 	->json_is('/user/username' => 'lisa')->json_is('/user/is_admin' => false);
 
 # Load the global settings from the file
-my $global_settings_from_file = LoadFile("$main::ww3_dir/conf/course_settings.yml");
+my $settings_file = "$main::ww3_dir/conf/course_settings.yml";
+$settings_file = "$main::ww3_dir/conf/course_settings.dist.yml" unless -r $settings_file;
+my $global_settings_from_file = LoadFile($settings_file);
 
 # Get the global/default settings
 

tests/unit-tests/settings.spec.ts

@@ -2,6 +2,7 @@
 
 import { CourseSetting, DBCourseSetting, GlobalSetting, SettingType
 } from 'src/common/models/settings';
+import { convertTimeDuration, humanReadableTimeDuration } from 'src/common/models/parsers';
 
 describe('Testing Course Settings', () => {
 	const global_setting = {
@@ -804,7 +805,7 @@ describe('Testing Course Settings', () => {
 
 			course_setting.set({
 				setting_name: 'time_duration',
-				default_value: '10 days',
+				default_value: 1234,
 				description: 'I am an time interval',
 				doc: 'Extended help',
 				type: 'time_duration',
@@ -816,6 +817,9 @@ describe('Testing Course Settings', () => {
 			course_setting.set({ value: 3.14 });
 			expect(course_setting.isValid()).toBe(false);
 
+			course_setting.set({ value: '3 days' });
+			expect(course_setting.isValid()).toBe(false);
+
 			course_setting.set({ value: 'hi' });
 			expect(course_setting.isValid()).toBe(false);
 
@@ -824,4 +828,71 @@ describe('Testing Course Settings', () => {
 		});
 
 	});
+
+	describe('Test converting of human readable time duration to number of seconds', () => {
+		test('Test time duration of seconds', () => {
+			expect(convertTimeDuration('1 sec')).toBe(1);
+			expect(convertTimeDuration('15 secs')).toBe(15);
+		});
+
+		test('Test time duration of mins', () => {
+			expect(convertTimeDuration('1 min')).toBe(60);
+			expect(convertTimeDuration('5 mins')).toBe(5 * 60);
+			expect(convertTimeDuration('5 mins, 30 secs')).toBe(5*60+30);
+		});
+
+		test('Test time duration of hours', () => {
+			expect(convertTimeDuration('1 hour')).toBe(1 * 60 * 60);
+			expect(convertTimeDuration('1 hr')).toBe(1 * 60 * 60);
+			expect(convertTimeDuration('5 hours')).toBe(5 * 60 * 60);
+			expect(convertTimeDuration('3 hrs')).toBe(3 * 60 * 60);
+			expect(convertTimeDuration('3 hrs, 10 mins, 15 seconds')).toBe(3*60*60+10*60+15);
+		});
+
+		test('Test time duration of days', () => {
+			expect(convertTimeDuration('1 day')).toBe(1 * 24 * 60 * 60);
+			expect(convertTimeDuration('3 days')).toBe(3 * 24 * 60 * 60);
+			expect(convertTimeDuration('3 days, 12 hours')).toBe(3 * 24 * 60 * 60+ 12*60*60);
+		});
+
+		test('Test time duration of weeks', () => {
+			expect(convertTimeDuration('1 week')).toBe(1 * 7 * 24 * 60 * 60);
+			expect(convertTimeDuration('2 weeks')).toBe(2 * 7 * 24 * 60 * 60);
+			expect(convertTimeDuration('2 weeks, 5 days')).toBe(2 * 7 * 24 * 60 * 60 + 5 *24 *60 *60);
+		});
+
+	});
+
+	describe('Test conversion of num. seconds to human readable time durations', () => {
+		test('Test time duration of secs', () => {
+			expect(humanReadableTimeDuration(0)).toBe('');
+			expect(humanReadableTimeDuration(1)).toBe('1 sec');
+			expect(humanReadableTimeDuration(15)).toBe('15 secs');
+		});
+
+		test('Test time duration of mins', () => {
+			expect(humanReadableTimeDuration(60)).toBe('1 min');
+			expect(humanReadableTimeDuration(5 * 60)).toBe('5 mins');
+			expect(humanReadableTimeDuration(5 * 60 + 30)).toBe('5 mins, 30 secs');
+		});
+
+		test('Test time duration of hours', () => {
+			expect(humanReadableTimeDuration(3600)).toBe('1 hour');
+			expect(humanReadableTimeDuration(5 * 3600)).toBe('5 hours');
+			expect(humanReadableTimeDuration(5 * 3600 + 30 * 60)).toBe('5 hours, 30 mins');
+		});
+
+		test('Test time duration of days', () => {
+			expect(humanReadableTimeDuration(3600 * 24)).toBe('1 day');
+			expect(humanReadableTimeDuration(3 * 3600 * 24)).toBe('3 days');
+			expect(humanReadableTimeDuration(3 * 3600 * 24 + 6 * 3600)).toBe('3 days, 6 hours');
+			expect(humanReadableTimeDuration(3 * 3600 * 24 + 6 * 3600 + 30 * 60)).toBe('3 days, 6 hours, 30 mins');
+		});
+
+		test('Test time duration of weeks', () => {
+			expect(humanReadableTimeDuration(3600 * 24 * 7)).toBe('1 week');
+			expect(humanReadableTimeDuration(3600 * 24 * 7 * 2)).toBe('2 weeks');
+			expect(humanReadableTimeDuration(3600 * 24 * 7 * 2 + 3 * 3600 * 24)).toBe('2 weeks, 3 days');
+		});
+	});
 });