Allow viewing ProblemSet page for any valid set.

Rework the ProblemSet page so users can always see the set header
and additional set information for any set assigned to them.
Instead of giving an error message when a user tries to access
a set that is not open, lti restricted, conditional release,
or ip restricted, checkSet will set a flag which is used to
determine what information a user can see.

If a set not visible and a user cannot see hidden sets, only a
warning message is shown. In all other cases the set header
and any date information, such as when the set opens, closes,
etc, is shown. If the set is restricted due to a conditional
release, ip restrictions, or lti restrictions, a warning message
is shown informing the user of the restriction.

When a set is restricted, never show the set problems and always
show any previous taken set versions. This way users can still
access set versions they have taken for a restricted set from
a non restricted location.

Note, this is consistent with the permission `view_unoppend_sets`
description, which states it only configures if a set problems
can be seen. So this permission is no longer used to see if a
user can see set information such as open date and set header.

This also makes it so the right info panel div on the ProblemSet
page is only shown if the set header exists, and is not empty.

Translations were added to messages about IP restrictions.

Author: somiaj

lib/WeBWorK/Authz.pm

@@ -46,7 +46,7 @@ use warnings;
 use Carp qw/croak/;
 
 use WeBWorK::Utils::DateTime qw(before);
-use WeBWorK::Utils::Sets     qw(is_restricted);
+use WeBWorK::Utils::Sets     qw(restricted_set_message);
 use WeBWorK::Authen::Proctor;
 use Net::IP;
 use Scalar::Util qw(weaken);
@@ -415,33 +415,26 @@ sub checkSet {
 	$self->{merged_set} = $set;
 
 	# Now we know that the set is assigned to the appropriate user.
-	# Check to see if the user is trying to access a set that is not open.
-	if (
-		before($set->open_date)
-		&& !$self->hasPermissions($userName, "view_unopened_sets")
-		&& !(
-			defined $set->assignment_type
-			&& $set->assignment_type =~ /gateway/
-			&& $node_name eq 'problem_list'
-			&& $db->countSetVersions($effectiveUserName, $set->set_id)
-		)
-		)
-	{
-		return $c->maketext("Requested set '[_1]' is not yet open.", $setName);
-	}
+	# $c->{viewSetCheck} is used to configure what is shown on ProblemSet page.
 
 	# Check to make sure that the set is visible, and that the user is allowed to view hidden sets.
 	my $visible = $set && $set->visible ne '0' && $set->visible ne '1' ? 1 : $set->visible;
 	if (!$visible && !$self->hasPermissions($userName, "view_hidden_sets")) {
+		$c->{viewSetCheck} = 'hidden';
+		return $c->maketext("Requested set '[_1]' is not available.", $setName);
+	}
+
+	# Check to see if the user is trying to access a set that is not open.
+	if (before($set->open_date) && !$self->hasPermissions($userName, 'view_unopened_sets')) {
+		$c->{viewSetCheck} = 'not-open';
 		return $c->maketext("Requested set '[_1]' is not available yet.", $setName);
 	}
 
 	# Check to see if conditional release conditions have been met.
-	if ($ce->{options}{enableConditionalRelease}
-		&& is_restricted($db, $set, $effectiveUserName)
-		&& !$self->hasPermissions($userName, "view_unopened_sets"))
-	{
-		return $c->maketext("The prerequisite conditions have not been met for set '[_1]'.", $setName);
+	my $conditional_msg = restricted_set_message($c, $set, 'conditional');
+	if ($conditional_msg) {
+		$c->{viewSetCheck} = 'restricted';
+		return $conditional_msg;
 	}
 
 	# Check to be sure that gateways are being sent to the correct content generator.
@@ -474,23 +467,16 @@ sub checkSet {
 
 	# Check for ip restrictions.
 	my $badIP = $self->invalidIPAddress($set);
-	return $badIP if $badIP;
-
-	# If LTI grade passback is enabled and set to 'homework' mode then we need to make sure that there is a sourcedid
-	# for this set before students access it.
-	my $LTIGradeMode = $ce->{LTIGradeMode} // '';
-
-	if ($LTIGradeMode eq 'homework' && !$self->hasPermissions($userName, "view_unopened_sets")) {
-		my $LMS =
-			$ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
-			? $c->link_to($ce->{LTI}{ $ce->{LTIVersion} }{LMS_name} => $ce->{LTI}{ $ce->{LTIVersion} }{LMS_url})
-			: $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name};
-		return $c->b($c->maketext(
-			'You must use your Learning Management System ([_1]) to access this set.  '
-				. 'Try logging in to the Learning Management System and visiting the set from there.',
-			$LMS
-		))
-			unless $set->lis_source_did || ($ce->{LTIVersion} eq 'v1p3' && $ce->{LTI}{v1p3}{ignoreMissingSourcedID});
+	if ($badIP) {
+		$c->{viewSetCheck} = 'restricted';
+		return $badIP;
+	}
+
+	# Check for lis_source_did if LTI grade passback is 'homework'.
+	my $lti_msg = restricted_set_message($c, $set, 'lti');
+	if ($lti_msg) {
+		$c->{viewSetCheck} = 'restricted';
+		return $lti_msg;
 	}
 
 	return 0;
@@ -530,7 +516,9 @@ sub invalidIPAddress {
 	# if there are no addresses in the locations, return an error that
 	#    says this
 	return $c->maketext(
-		"Client ip address [_1] is not allowed to work this assignment, because the assignment has ip address restrictions and there are no allowed locations associated with the restriction.  Contact your professor to have this problem resolved.",
+		'Client ip address [_1] is not allowed to work this assignment, because the assignment has ip address '
+			. 'restrictions and there are no allowed locations associated with the restriction.  Contact your '
+			. 'professor to have this problem resolved.',
 		$clientIP->ip()
 	) if (!@restrictAddresses);
 
@@ -552,17 +540,13 @@ sub invalidIPAddress {
 	# this is slightly complicated by having to check relax_restrict_ip
 	my $badIP = '';
 	if ($restrictType eq 'RestrictTo' && !$inRestrict) {
-		$badIP =
-			"Client ip address "
-			. $clientIP->ip()
-			. " is not in the list of addresses from "
-			. "which this assignment may be worked.";
+		$badIP = $c->maketext(
+			'Client ip address [_1] is not in the list of addresses from which this assignment may be worked.',
+			$clientIP->ip());
 	} elsif ($restrictType eq 'DenyFrom' && $inRestrict) {
-		$badIP =
-			"Client ip address "
-			. $clientIP->ip()
-			. " is in the list of addresses from "
-			. "which this assignment may not be worked.";
+		$badIP = $c->maketext(
+			'Client ip address [_1] is in the list of addresses from which this assignment may not be worked.',
+			$clientIP->ip());
 	} else {
 		return 0;
 	}

lib/WeBWorK/ContentGenerator.pm

@@ -110,8 +110,6 @@ async sub go ($c) {
 
 	my $tx = $c->render_later->tx;
 
-	$c->stash->{footerWidthClass} = $c->can('info') ? 'col-md-8' : 'col-12';
-
 	if ($c->can('pre_header_initialize')) {
 		my $pre_header_initialize = $c->pre_header_initialize;
 		await $pre_header_initialize
@@ -133,6 +131,8 @@ async sub go ($c) {
 		await $initialize if ref $initialize eq 'Future' || ref $initialize eq 'Mojo::Promise';
 	}
 
+	$c->stash->{footerWidthClass} //= $c->can('info') ? 'col-md-8' : 'col-12';
+
 	$c->content;
 
 	# All content generator modules must have rendered at this point unless there was an error in which case an error

lib/WeBWorK/ContentGenerator/ProblemSet.pm

@@ -19,18 +19,23 @@ use WeBWorK::Localize;
 use WeBWorK::AchievementItems;
 use WeBWorK::HTML::StudentNav qw(studentNav);
 
+sub can ($c, $arg) {
+	if ($arg eq 'info') {
+		return $c->{pg} ? 1 : 0;
+	}
+	return $c->SUPER::can($arg);
+}
+
 async sub initialize ($c) {
 	my $db    = $c->db;
 	my $ce    = $c->ce;
 	my $authz = $c->authz;
 
-	# $c->{invalidSet} is set in checkSet which is called by ContentGenerator.pm
-	return
-		if $c->{invalidSet}
-		&& ($c->{invalidSet} !~ /^Client ip address .* is not in the list of addresses/
-			|| $authz->{merged_set}->assignment_type !~ /gateway/);
+	# $c->{invalidSet} is set in checkSet which is called by ContentGenerator.pm.
+	# If $c->{viewSetCheck} is also set, we want to view some information unless the set is hidden.
+	return if $c->{invalidSet} && (!$c->{viewSetCheck} || $c->{viewSetCheck} eq 'hidden');
 
-	# This will all be valid if checkSet did not set $c->{invalidSet}.
+	# This will all be valid if the above check passes.
 	my $userID  = $c->param('user');
 	my $eUserID = $c->param('effectiveUser');
 
@@ -106,6 +111,7 @@ async sub initialize ($c) {
 
 	$c->{pg} =
 		await renderPG($c, $effectiveUser, $c->{set}, $problem, $c->{set}->psvn, {}, { displayMode => $displayMode });
+	$c->{pg} = '' unless $c->{pg}{body_text} =~ /\S/;
 
 	return;
 }
@@ -169,10 +175,8 @@ sub siblings ($c) {
 	return $c->include('ContentGenerator/ProblemSet/siblings', setIDs => \@setIDs);
 }
 
-sub info {
-	my ($c) = @_;
-	return '' unless $c->{pg};
-	return $c->include('ContentGenerator/ProblemSet/info');
+sub info ($c) {
+	return $c->{pg} ? $c->include('ContentGenerator/ProblemSet/info') : '';
 }
 
 # This is called by the ContentGenerator/ProblemSet/body template for a regular homework set.

lib/WeBWorK/ContentGenerator/ProblemSets.pm

@@ -11,7 +11,7 @@ use WeBWorK::Debug;
 use WeBWorK::Utils           qw(sortByName);
 use WeBWorK::Utils::DateTime qw(after);
 use WeBWorK::Utils::Files    qw(readFile path_is_subdir);
-use WeBWorK::Utils::Sets     qw(is_restricted format_set_name_display);
+use WeBWorK::Utils::Sets     qw(restricted_set_message);
 use WeBWorK::Localize;
 
 # The "default" data in the course_info.txt file.
@@ -114,15 +114,11 @@ sub info ($c) {
 }
 
 sub getSetStatus ($c, $set) {
-	my $ce              = $c->ce;
-	my $db              = $c->db;
-	my $authz           = $c->authz;
-	my $effectiveUser   = $c->param('effectiveUser') || $c->param('user');
-	my $canViewUnopened = $authz->hasPermissions($c->param('user'), 'view_unopened_sets');
-
-	my @restricted = $ce->{options}{enableConditionalRelease} ? is_restricted($db, $set, $effectiveUser) : ();
-
-	my $link_is_active = 1;
+	my $ce             = $c->ce;
+	my $db             = $c->db;
+	my $authz          = $c->authz;
+	my $effectiveUser  = $c->param('effectiveUser')                      || $c->param('user');
+	my $restricted_msg = restricted_set_message($c, $set, 'conditional') || restricted_set_message($c, $set, 'lti');
 
 	# Determine set status.
 	my $status_msg;
@@ -132,11 +128,7 @@ sub getSetStatus ($c, $set) {
 		$status = 'not-open';
 		$status_msg =
 			$c->maketext('Will open on [_1].', $c->formatDateTime($set->open_date, $ce->{studentDateDisplayFormat}));
-		push(@$other_messages, $c->restricted_progression_msg(1, $set->restricted_status * 100, @restricted))
-			if @restricted;
-		$link_is_active = 0
-			unless $canViewUnopened
-			|| ($set->assignment_type =~ /gateway/ && $db->countSetVersions($effectiveUser, $set->set_id));
+		push(@$other_messages, $restricted_msg) if $restricted_msg;
 	} elsif ($c->submitTime < $set->due_date) {
 		$status = 'open';
 
@@ -172,31 +164,8 @@ sub getSetStatus ($c, $set) {
 				$c->maketext('Open. Due [_1].', $c->formatDateTime($set->due_date, $ce->{studentDateDisplayFormat}));
 		}
 
-		if (@restricted) {
-			$link_is_active = 0 unless $canViewUnopened;
-			push(@$other_messages, $c->restricted_progression_msg(0, $set->restricted_status * 100, @restricted));
-		} elsif (!$canViewUnopened
-			&& $ce->{LTIVersion}
-			&& ($ce->{LTIVersion} ne 'v1p3' || !$ce->{LTI}{v1p3}{ignoreMissingSourcedID})
-			&& defined $ce->{LTIGradeMode}
-			&& $ce->{LTIGradeMode} eq 'homework'
-			&& !$set->lis_source_did)
-		{
-			# The set shouldn't be shown if LTI grade mode is set to homework and a
-			# sourced_id is not available to use to send back grades
-			# (unless we are using LTI 1.3 and $LTI{v1p3}{ignoreMissingSourcedID} is set)
-			push(
-				@$other_messages,
-				$c->maketext(
-					'You must log into this set via your Learning Management System ([_1]).',
-					$ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
-					? $c->link_to(
-						$ce->{LTI}{ $ce->{LTIVersion} }{LMS_name} => $ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
-						)
-					: $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}
-				)
-			);
-			$link_is_active = 0;
+		if ($restricted_msg) {
+			push(@$other_messages, $restricted_msg);
 		}
 	} elsif ($c->submitTime < $set->answer_date) {
 		$status_msg = $c->maketext('Answers available for review on [_1].',
@@ -209,8 +178,7 @@ sub getSetStatus ($c, $set) {
 		status         => $status,
 		status_msg     => $status_msg,
 		other_messages => $other_messages,
-		link_is_active => $link_is_active,
-		is_restricted  => scalar(@restricted)
+		is_restricted  => $restricted_msg ? 1 : 0
 	);
 }
 
@@ -232,20 +200,4 @@ sub byUrgency {
 	return $a->set_id cmp $b->set_id;
 }
 
-sub restricted_progression_msg ($c, $open, $restriction, @restricted) {
-	if (@restricted == 1) {
-		return $c->maketext(
-			'To access this set you must score at least [_1]% on set [_2].',
-			sprintf('%.0f', $restriction),
-			$c->tag('span', dir => 'ltr', format_set_name_display($restricted[0]))
-		);
-	} else {
-		return $c->maketext(
-			'To access this set you must score at least [_1]% on the following sets: [_2].',
-			sprintf('%.0f', $restriction),
-			join(', ', map { $c->tag('span', dir => 'ltr', format_set_name_display($_)) } @restricted)
-		);
-	}
-}
-
 1;

lib/WeBWorK/Utils/Sets.pm

@@ -17,6 +17,7 @@ our @EXPORT_OK = qw(
 	is_restricted
 	get_test_problem_position
 	list_set_versions
+	restricted_set_message
 );
 
 sub format_set_name_internal ($set_name) {
@@ -255,6 +256,50 @@ sub list_set_versions ($db, $studentName, $setName, $setIsVersioned = 0) {
 	return (\@allSetNames, $notAssignedSet);
 }
 
+sub restricted_set_message($c, $set, $status) {
+	my $ce    = $c->ce;
+	my $db    = $c->db;
+	my $authz = $c->authz;
+
+	return '' if $authz->hasPermissions($c->param('user'), 'view_unopened_sets');
+
+	if ($status eq 'conditional') {
+		my $user       = $c->param('effectiveUser') || $c->param('user');
+		my @restricted = $ce->{options}{enableConditionalRelease} ? is_restricted($db, $set, $user) : ();
+		return '' unless @restricted;
+
+		if (@restricted == 1) {
+			return $c->maketext(
+				'To access this set you must score at least [_1]% on set [_2].',
+				sprintf('%.0f', $set->restricted_status * 100),
+				$c->tag('span', dir => 'ltr', format_set_name_display($restricted[0]))
+			);
+		} else {
+			return $c->maketext(
+				'To access this set you must score at least [_1]% on the following sets: [_2].',
+				sprintf('%.0f', $set->restricted_status * 100),
+				join(', ', map { $c->tag('span', dir => 'ltr', format_set_name_display($_)) } @restricted)
+			);
+		}
+	}
+
+	if ($status eq 'lti') {
+		if ($ce->{LTIVersion}
+			&& ($ce->{LTIVersion} ne 'v1p3' || !$ce->{LTI}{v1p3}{ignoreMissingSourcedID})
+			&& $ce->{LTIGradeMode} eq 'homework'
+			&& !$set->lis_source_did)
+		{
+			return $c->maketext(
+				'You must access this set from your Learning Management System ([_1]) before starting.',
+				$ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
+				? $c->link_to($ce->{LTI}{ $ce->{LTIVersion} }{LMS_name} => $ce->{LTI}{ $ce->{LTIVersion} }{LMS_url})
+				: $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}
+			);
+		}
+		return '';
+	}
+}
+
 1;
 
 =head1 NAME
@@ -354,4 +399,12 @@ assigned to the set.  The list of names will be a list of set versions if the
 set is versioned (i.e., if C<setIsVersioned> is true), and a list containing
 only the original set id otherwise.
 
+=head2 restricted_set_message
+
+Usage: C<restricted_set_message($c, $set, $status)>
+
+Checks for and returns any restricted messages for the given set and status.
+C<$status> can be either 'conditional' for conditional release, or 'lti' to
+check for lis_source_did when using homework grade passback.
+
 =cut