FIX: setting_id -> global_setting_id and switching inflator to FilterColumn

Author: pstaabp

lib/DB/Schema/Result/CourseSetting.pm

@@ -21,7 +21,7 @@ C<course_id>: database id of the course for the setting (foreign key)
 
 =item *
 
-C<setting_id>: database id that the given setting is related to (foreign key)
+C<global_setting_id>: database id of the global setting that the given setting is related to (foreign key)
 
 =item *
 
@@ -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(qw/InflateColumn::Serializer InflateColumn::JSONValue Core/);
+__PACKAGE__->load_components(qw/FilterColumn Core/);
 
 __PACKAGE__->add_columns(
 	course_setting_id => {
@@ -46,24 +46,33 @@ __PACKAGE__->add_columns(
 		data_type => 'integer',
 		size      => 16,
 	},
-	setting_id => {
+	global_setting_id => {
 		data_type => 'integer',
 		size      => 16,
 	},
 	value => {
-		data_type          => 'text',
-		default_value      => '{}',
-		retrieve_on_insert => 1,
-		inflate_value      => 1,
+		data_type   => 'text',
+		is_nullable => 1
 	},
 );
 
+__PACKAGE__->filter_column(
+	value => {
+		filter_to_storage => sub {
+			return JSON::MaybeXS->new({ utf8 => 1, allow_nonref => 1 })->encode($_[1] // '');
+		},
+		filter_from_storage => sub {
+			return JSON::MaybeXS->new({ utf8 => 1, allow_nonref => 1 })->decode($_[1] // '');
+		}
+	}
+);
+
 __PACKAGE__->set_primary_key('course_setting_id');
 
-__PACKAGE__->add_unique_constraint([qw/course_id setting_id/]);
+__PACKAGE__->add_unique_constraint([qw/course_id global_setting_id/]);
 
 __PACKAGE__->belongs_to(course => 'DB::Schema::Result::Course', 'course_id');
 
-__PACKAGE__->belongs_to(global_setting => 'DB::Schema::Result::GlobalSetting', 'setting_id');
+__PACKAGE__->belongs_to(global_setting => 'DB::Schema::Result::GlobalSetting', 'global_setting_id');
 
 1;

lib/DB/Schema/Result/GlobalSetting.pm

@@ -13,7 +13,7 @@ This is the database schema for the Global Course Settings.
 
 =item *
 
-C<setting_id>: database id (autoincrement integer)
+C<global_setting_id>: database id (autoincrement integer)
 
 =item *
 
@@ -53,10 +53,10 @@ C<subcategory>: the subcategory of the setting (may be null)
 
 __PACKAGE__->table('global_setting');
 
-__PACKAGE__->load_components(qw/InflateColumn::Serializer InflateColumn::JSONValue Core/);
+__PACKAGE__->load_components(qw/InflateColumn::Serializer FilterColumn Core/);
 
 __PACKAGE__->add_columns(
-	setting_id => {
+	global_setting_id => {
 		data_type         => 'integer',
 		size              => 16,
 		is_auto_increment => 1,
@@ -66,10 +66,8 @@ __PACKAGE__->add_columns(
 		size      => 256,
 	},
 	default_value => {
-		data_type          => 'text',
-		default_value      => '{}',
-		retrieve_on_insert => 1,
-		inflate_value      => 1,
+		data_type     => 'text',
+		default_value => '""'
 	},
 	description => {
 		data_type     => 'text',
@@ -87,7 +85,6 @@ __PACKAGE__->add_columns(
 	options => {
 		data_type          => 'text',
 		is_nullable        => 1,
-		retrieve_on_insert => 1,
 		serializer_class   => 'JSON',
 		serializer_options => { utf8 => 1 }
 	},
@@ -102,9 +99,20 @@ __PACKAGE__->add_columns(
 		is_nullable => 1
 	}
 );
+use Data::Dumper;
+__PACKAGE__->filter_column(
+	default_value => {
+		filter_to_storage => sub {
+			return JSON::MaybeXS->new({ utf8 => 1, allow_nonref => 1 })->encode($_[1] // '');
+		},
+		filter_from_storage => sub {
+			return JSON::MaybeXS->new({ utf8 => 1, allow_nonref => 1 })->decode($_[1] // '');
+		}
+	}
+);
 
-__PACKAGE__->set_primary_key('setting_id');
+__PACKAGE__->set_primary_key('global_setting_id');
 
-__PACKAGE__->has_many(course_settings => 'DB::Schema::Result::CourseSetting', 'setting_id');
+__PACKAGE__->has_many(course_settings => 'DB::Schema::Result::CourseSetting', 'global_setting_id');
 
 1;

lib/DB/Schema/ResultSet/Course.pm

@@ -296,7 +296,7 @@ This gets a single global/default setting.
 
 =over
 
-=item * C<info> which is a hash of either a C<setting_id> or C<setting_name> with information
+=item * C<info> which is a hash of either a C<global_setting_id> or C<setting_name> with information
 on the setting.
 
 =item * C<$as_result_set>, a boolean if the return is to be a result_set
@@ -315,11 +315,10 @@ sub getGlobalSetting ($self, %args) {
 
 	DB::Exception::SettingNotFound->throw(message => $setting_info->{setting_name}
 		? "The setting with name $setting_info->{setting_name} is not found"
-		: "The setting with setting_id $setting_info->{setting_id} is not found")
+		: "The setting with global_setting_id $setting_info->{global_setting_id} is not found")
 		unless $global_setting;
 	return $global_setting if $args{as_result_set};
-	my $setting_to_return = { $global_setting->get_inflated_columns };
-	return $setting_to_return;
+	return { $global_setting->get_inflated_columns };
 }
 
 =head2 getCourseSettings
@@ -369,7 +368,7 @@ This gets a single course setting.
 
 =over
 
-=item * C<info> which is a hash of either a C<setting_id> or C<setting_name> with information
+=item * C<info> which is a hash of either a C<global_setting_id> or C<setting_name> with information
 on the setting.
 
 =item * C<merged>, a boolean on whether the course setting is merged with its corresponding
@@ -388,21 +387,21 @@ A single course setting as either a hashref or a C<DBIx::Class::ResultSet::Cours
 sub getCourseSetting ($self, %args) {
 	my $global_setting = $self->getGlobalSetting(info => $args{info}, as_result_set => 1);
 	DB::Exception::SettingNotFound->throw(
-		message => "The global setting with name: '" . $args{info}->{setting_name} . "' is not a defined info.")
+		message => "The global setting with name: '" . $args{info}{setting_name} . "' is not defined.")
 		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 });
+	my $setting = $course->course_settings->find({ global_setting_id => $global_setting->global_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}{setting_name} ? " name: '$args{info}{setting_name}'"
+				: "global_setting_id of $args{info}{global_setting_id} is not found in the course "
 			)
 			. (
-				$args{info}->{course_name} ? ("with name '" . $args{info}->{course_name} . "'")
-				: "with course_id of $args{info}->{course_id}"
+				$args{info}{course_name} ? ("with name '" . $args{info}{course_name} . "'")
+				: "with course_id of $args{info}{course_id}"
 			)
 	) unless defined($setting);
 
@@ -425,7 +424,7 @@ Update a single course setting.
 =over
 
 =item * C<info> which is a hash containing information about the course (either a
-C<course_id> or C<course_name>) and a setting (either a C<setting_id> or C<setting_name>).
+C<course_id> or C<course_name>) and a setting (either a C<global_setting_id> or C<setting_name>).
 
 =item * C<params> the updated value of the course setting.
 
@@ -446,14 +445,12 @@ sub updateCourseSetting ($self, %args) {
 	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}
-	});
+	my $course_setting = $course->course_settings->find({ global_setting_id => $global_setting->{global_setting_id} });
 
 	my $params = {
-		course_id  => $course->course_id,
-		setting_id => $global_setting->{setting_id},
-		value      => $args{params}{value}
+		course_id         => $course->course_id,
+		global_setting_id => $global_setting->{global_setting_id},
+		value             => $args{params}{value} =~ /^$/ ? undef : $args{params}{value}
 	};
 
 	isValidSetting($global_setting, $params->{value});
@@ -462,12 +459,9 @@ sub updateCourseSetting ($self, %args) {
 		defined($course_setting) ? $course_setting->update($params) : $course->add_to_course_settings($params);
 
 	return $up_setting if $args{as_result_set};
-	my $setting_to_return =
-		($args{merged})
+	return ($args{merged})
 		? { $up_setting->get_inflated_columns, $up_setting->global_setting->get_inflated_columns }
 		: { $up_setting->get_inflated_columns };
-
-	return $setting_to_return;
 }
 
 =pod
@@ -481,7 +475,7 @@ Delete a single course setting.
 =over
 
 =item * C<info> which is a hash containing information about the course (either a
-C<course_id> or C<course_name>) and a setting (either a C<setting_id> or C<setting_name>).
+C<course_id> or C<course_name>) and a setting (either a C<global_setting_id> or C<setting_name>).
 
 =item * C<$as_result_set>, a boolean if the return is to be a result_set
 

lib/DB/Utils.pm

@@ -42,7 +42,7 @@ sub getPoolProblemInfo ($in) {
 }
 
 sub getSettingInfo ($in) {
-	return _get_info($in, qw/setting_name setting_id/);
+	return _get_info($in, qw/setting_name global_setting_id/);
 }
 
 # This is a generic internal subroutine to check that the info passed in contains certain fields.

lib/DBIx/Class/InflateColumn/JSONValue.pm

@@ -219,12 +219,12 @@ sub problemRoutes ($app, $course_routes) {
 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->get('/:global_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');
+	$course_routes->get('/settings/:global_setting_id')->to('Settings#getCourseSetting');
+	$course_routes->put('/settings/:global_setting_id')->to('Settings#updateCourseSetting');
+	$course_routes->delete('/settings/:global_setting_id')->to('Settings#deleteCourseSetting');
 	return;
 }