Course settings refactoring

conf/permissions.dist.yml

@@ -163,9 +163,11 @@ db_permissions:
       allowed_roles: ['course_admin', 'instructor']
   Settings:
     getDefaultCourseSettings:
-      allowed_roles: ['*']
+      allowed_roles: ['course_admin', 'instructor','student']
     getCourseSettings:
-      allowed_roles: ['*']
+      allowed_roles: ['course_admin', 'instructor','student']
+    getCourseSetting:
+      allowed_roles: ['course_admin', 'instructor','student']
     updateCourseSetting:
       allowed_roles: ['course_admin', 'instructor']
     deleteCourseSetting:

lib/DB/Schema/Result/CourseSetting.pm

@@ -33,7 +33,7 @@ C<value>: the value of the setting as a JSON so different types of data can be s
 
 __PACKAGE__->table('course_setting');
 
-__PACKAGE__->load_components('InflateColumn::Serializer', 'Core');
+__PACKAGE__->load_components(qw/InflateColumn::Serializer InflateColumn::JSONValue Core/);
 
 __PACKAGE__->add_columns(
 	course_setting_id => {
@@ -53,8 +53,8 @@ __PACKAGE__->add_columns(
 	value => {
 		data_type          => 'text',
 		default_value      => '{}',
-		serializer_class   => 'JSON',
-		serializer_options => { utf8 => 1 }
+		retrieve_on_insert => 1,
+		inflate_value      => 1,
 	},
 );
 

lib/DB/Schema/Result/GlobalSetting.pm

@@ -53,7 +53,7 @@ C<subcategory>: the subcategory of the setting (may be null)
 
 __PACKAGE__->table('global_setting');
 
-__PACKAGE__->load_components('InflateColumn::Serializer', 'Core');
+__PACKAGE__->load_components(qw/InflateColumn::Serializer InflateColumn::JSONValue Core/);
 
 __PACKAGE__->add_columns(
 	setting_id => {
@@ -69,8 +69,7 @@ __PACKAGE__->add_columns(
 		data_type          => 'text',
 		default_value      => '{}',
 		retrieve_on_insert => 1,
-		serializer_class   => 'JSON',
-		serializer_options => { utf8 => 1 }
+		inflate_value      => 1,
 	},
 	description => {
 		data_type     => 'text',

lib/DB/Schema/ResultSet/Course.pm

@@ -283,10 +283,6 @@ sub getGlobalSettings ($self, %args) {
 	my @settings = map {
 		{ $_->get_inflated_columns };
 	} @global_settings;
-	for my $setting (@settings) {
-		# The default_value is stored as a JSON and needs to be parsed.
-		$setting->{default_value} = $setting->{default_value}->{value};
-	}
 	return \@settings;
 }
 
@@ -323,7 +319,6 @@ sub getGlobalSetting ($self, %args) {
 		unless $global_setting;
 	return $global_setting if $args{as_result_set};
 	my $setting_to_return = { $global_setting->get_inflated_columns };
-	$setting_to_return->{default_value} = $setting_to_return->{default_value}->{value};
 	return $setting_to_return;
 }
 
@@ -356,20 +351,11 @@ sub getCourseSettings ($self, %args) {
 	my @settings_from_db = $course->course_settings;
 
 	return \@settings_from_db if $args{as_result_set};
-	my @settings_to_return = ($args{merged})
-		? map {
-			{ $_->get_inflated_columns, $_->global_setting->get_inflated_columns };
-		} @settings_from_db
-		: map {
-			{ $_->get_inflated_columns };
-		} @settings_from_db;
-
-	for my $setting (@settings_to_return) {
-		# value and default_value are decoded from JSON as a hash.  Return to a value.
-		for my $key (qw/default_value value/) {
-			$setting->{$key} = $setting->{$key}->{value} if defined($setting->{$key});
-		}
-	}
+	my @settings_to_return = map {
+		$args{merged}
+			? { $_->get_inflated_columns, $_->global_setting->get_inflated_columns }
+			: { $_->get_inflated_columns };
+	} @settings_from_db;
 	return \@settings_to_return;
 }
 
@@ -400,25 +386,31 @@ A single course setting as either a hashref or a C<DBIx::Class::ResultSet::Cours
 =cut
 
 sub getCourseSetting ($self, %args) {
-
 	my $global_setting = $self->getGlobalSetting(info => $args{info}, as_result_set => 1);
 	DB::Exception::SettingNotFound->throw(
-		message => "The setting with name: '" . $args{info}->{setting_name} . "' is not a defined info.")
+		message => "The global setting with name: '" . $args{info}->{setting_name} . "' is not a defined info.")
 		unless defined($global_setting);
 
 	my $course  = $self->getCourse(info => getCourseInfo($args{info}), as_result_set => 1);
 	my $setting = $course->course_settings->find({ setting_id => $global_setting->setting_id });
 
+	DB::Exception::SettingNotFound->throw(
+		message => 'The course setting with '
+			. (
+				$args{info}->{setting_name} ? " name: '$args{info}->{setting_name}'"
+				: "setting_id of $args{info}->{setting_id} is not a found in the course "
+			)
+			. (
+				$args{info}->{course_name} ? ("with name '" . $args{info}->{course_name} . "'")
+				: "with course_id of $args{info}->{course_id}"
+			)
+	) unless defined($setting);
+
 	return $setting if $args{as_result_set};
 	my $setting_to_return =
 		$args{merged}
 		? { $setting->get_inflated_columns, $setting->global_setting->get_inflated_columns }
 		: { $setting->get_inflated_columns };
-
-	# value and default_value are decoded from JSON as a hash.  Return to a value.
-	for my $key (qw/default_value value/) {
-		$setting_to_return->{$key} = $setting_to_return->{$key}->{value} if defined($setting_to_return->{$key});
-	}
 	return $setting_to_return;
 }
 
@@ -451,44 +443,30 @@ A single course setting as either a hashref or a C<DBIx::Class::ResultSet::Cours
 =cut
 
 sub updateCourseSetting ($self, %args) {
-	my $course = $self->getCourse(info => getCourseInfo($args{info}), as_result_set => 1);
-
+	my $course         = $self->getCourse(info => getCourseInfo($args{info}), as_result_set => 1);
 	my $global_setting = $self->getGlobalSetting(info => getSettingInfo($args{info}));
 
 	my $course_setting = $course->course_settings->find({
 		setting_id => $global_setting->{setting_id}
 	});
 
-	# Check that the setting is valid.
-
 	my $params = {
 		course_id  => $course->course_id,
 		setting_id => $global_setting->{setting_id},
-		value      => { value => $args{params}{value} }
+		value      => $args{params}{value}
 	};
 
-	# remove the following fields before checking for valid settings:
-	delete $global_setting->{$_} for (qw/setting_id course_id/);
+	isValidSetting($global_setting, $params->{value});
 
-	isValidSetting($global_setting, $params->{value}{value});
-
-	# The course_id must be deleted to ensure it is written to the database correctly.
-	delete $params->{course_id} if defined($params->{course_id});
-
-	my $updated_course_setting =
+	my $up_setting =
 		defined($course_setting) ? $course_setting->update($params) : $course->add_to_course_settings($params);
 
-	return $updated_course_setting if $args{as_result_set};
+	return $up_setting if $args{as_result_set};
 	my $setting_to_return =
 		($args{merged})
-		? { $updated_course_setting->get_inflated_columns,
-			$updated_course_setting->global_setting->get_inflated_columns }
-		: { $updated_course_setting->get_inflated_columns };
+		? { $up_setting->get_inflated_columns, $up_setting->global_setting->get_inflated_columns }
+		: { $up_setting->get_inflated_columns };
 
-	# value and default_value are decoded from JSON as a hash.  Return to a value.
-	for my $key (qw/default_value value/) {
-		$setting_to_return->{$key} = $setting_to_return->{$key}->{value} if defined($setting_to_return->{$key});
-	}
 	return $setting_to_return;
 }
 
@@ -516,21 +494,8 @@ A single course setting as either a hashref or a C<DBIx::Class::ResultSet::Cours
 =cut
 
 sub deleteCourseSetting ($self, %args) {
-	my $setting         = $self->getCourseSetting(info => $args{info}, as_result_set => 1);
-	my $deleted_setting = $setting->delete;
-
-	return $deleted_setting if $args{as_result_set};
-
-	my $setting_to_return =
-		($args{merged})
-		? { $deleted_setting->get_inflated_columns, $deleted_setting->global_setting->get_inflated_columns }
-		: { $deleted_setting->get_inflated_columns };
-
-	# value and default_value are decoded from JSON as a hash.  Return to a value.
-	for my $key (qw/default_value value/) {
-		$setting_to_return->{$key} = $setting_to_return->{$key}->{value} if defined($setting_to_return->{$key});
-	}
-	return $setting_to_return;
+	$self->getCourseSetting(info => $args{info}, as_result_set => 1)->delete;
+	return;
 }
 
 1;

lib/DBIx/Class/InflateColumn/JSONValue.pm

@@ -0,0 +1,45 @@
+package DBIx::Class::InflateColumn::JSONValue;
+
+=pod
+
+DBIx::Class::InflateColumn::JSONValue - encodes any value (string, number, arrray, ...)
+in the form value => {}, then JSON encoded.
+
+=cut
+
+use strict;
+use warnings;
+
+use base qw/DBIx::Class/;
+use Mojo::JSON qw/encode_json decode_json/;
+use Try::Tiny;
+
+# This is a simplified version of DBIx::Class::InflateColumn::DateTime
+
+sub register_column {
+	my ($self, $column, $info, @rest) = @_;
+
+	$self->next::method($column, $info, @rest);
+	return unless $info->{inflate_value};
+
+	$self->inflate_column(
+		$column => {
+			inflate => sub {
+				my $str = shift;
+				# This is a bit of a hack.  It appears that sometimes the deflate isn't called on the values
+				# of type string and number so they don't need to be decoded.
+				try {
+					my $hash = decode_json($str);
+					return $hash->{value};
+				} catch {
+					# If the value in $str is a number, return the numerical value.
+					return $str =~ /^-?(0|([1-9][0-9]*))(\.[0-9]+)?([eE][-+]?[0-9]+)?$/ ? 0 + $str : $str;
+				};
+			},
+			deflate => sub { return encode_json({ value => shift }); }
+		}
+	);
+	return;
+}
+
+1;

lib/WeBWorK3.pm

@@ -222,6 +222,7 @@ sub settingsRoutes ($app, $course_routes) {
 	$global_settings->get('/:setting_id')->to('Settings#getGlobalSetting');
 	$global_settings->post('/check-timezone')->to('Settings#checkTimeZone');
 	$course_routes->get('/settings')->to('Settings#getCourseSettings');
+	$course_routes->get('/settings/:setting_id')->to('Settings#getCourseSetting');
 	$course_routes->put('/settings/:setting_id')->to('Settings#updateCourseSetting');
 	$course_routes->delete('/settings/:setting_id')->to('Settings#deleteCourseSetting');
 	return;

lib/WeBWorK3/Controller/Settings.pm

@@ -42,6 +42,17 @@ sub getCourseSettings ($c) {
 	return;
 }
 
+sub getCourseSetting ($c) {
+	my $course_setting = $c->schema->resultset('Course')->getCourseSetting(
+		info => {
+			course_id  => int($c->param('course_id')),
+			setting_id => int($c->param('setting_id'))
+		}
+	);
+	$c->render(json => $course_setting);
+	return;
+}
+
 sub updateCourseSetting ($c) {
 	my $course_setting = $c->schema->resultset('Course')->updateCourseSetting(
 		info => {

lib/WeBWorK3/Utils/Settings.pm

@@ -13,7 +13,7 @@ our @EXPORT_OK = qw/isValidSetting mergeCourseSettings isInteger isTimeString is
 
 use Exception::Class qw/
 	DB::Exception::UndefinedCourseField
-	DB::Exception::InvalidCourseField
+	DB::Exception::RequiredCourseField
 	DB::Exception::InvalidCourseFieldType
 	/;
 
@@ -45,8 +45,6 @@ This checks if the setting given the type, value and list of options (if needed)
 
 =over
 
-=item Ensure that all fields passed in are valid
-
 =item Ensure that all require fields are present
 
 =item Checks that the default value is appropriate for the type
@@ -61,18 +59,11 @@ sub isValidSetting ($setting, $value = undef) {
 
 	# If $value is not passed in, use the default_value for the setting
 	my $val = $value // $setting->{default_value};
-	# Check that each of the setting fields is allowed.
-	for my $field (keys %$setting) {
-		my @fields = grep { $_ eq $field } @allowed_fields;
-		DB::Exception::InvalidCourseField->throw(
-			message => "The field: $field is not an allowed field of the setting $setting->{setting_name}")
-			if scalar(@fields) == 0;
-	}
 
 	# Check that each of the required fields is present in the setting.
 	for my $field (@required_fields) {
 		my @fields = grep { $_ eq $field } (keys %$setting);
-		DB::Exception::InvalidCourseField->throw(
+		DB::Exception::RequiredCourseField->throw(
 			message => "The field: $field is a required field for the setting $setting->{setting_name}")
 			if scalar(@fields) == 0;
 	}

src/stores/settings.ts

@@ -98,18 +98,16 @@ export const useSettingsStore = defineStore('settings', {
 		/**
 		 * Deletes the course setting from both the store and sends a delete request to the database.
 		 */
-		async deleteCourseSetting(course_setting: CourseSetting): Promise<CourseSetting> {
+		async deleteCourseSetting(course_setting: CourseSetting): Promise<void> {
 			const session = useSessionStore();
 			const course_id = session.course.course_id;
 
 			const i = this.db_course_settings.findIndex(setting => setting.setting_id == course_setting.setting_id);
 			if (i < 0) {
 				throw `The setting with name: '${course_setting.setting_name}' has not been defined for this course.`;
 			}
-			const response = await api.delete(`/courses/${course_id}/settings/${course_setting.setting_id}`);
+			await api.delete(`/courses/${course_id}/settings/${course_setting.setting_id}`);
 			this.db_course_settings.splice(i, 1);
-			const deleted_setting = new DBCourseSetting(response.data as ParseableDBCourseSetting);
-			return new CourseSetting(Object.assign(deleted_setting.toObject(), course_setting.toObject()));
 		},
 		/**
 		 * Used to clear out all of the settings.  Useful when logging out.