Merge pull request #2869 from somiaj/unopened-set-preview

Allow viewing ProblemSet page for any valid set.

Author: pstaabp

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);
@@ -414,34 +414,35 @@ sub checkSet {
 	# Cache the set for future use as needed.  This should probably be more sophisticated than this.
 	$self->{merged_set} = $set;
 
+	# Save restricted set messages to show to instructors if they exist.
+	my $canViewUnopened = $self->hasPermissions($userName, "view_unopened_sets");
+	my @restrictedSetMessages;
+
 	# 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) && !$canViewUnopened) {
+		$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) {
+		if ($canViewUnopened) {
+			push(@restrictedSetMessages, $conditional_msg);
+		} else {
+			$c->{viewSetCheck} = 'restricted';
+			return $conditional_msg;
+		}
 	}
 
 	# Check to be sure that gateways are being sent to the correct content generator.
@@ -474,25 +475,27 @@ 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) {
+		if ($self->hasPermissions($userName, 'view_ip_restricted_sets')) {
+			push(@restrictedSetMessages, $badIP);
+		} else {
+			$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) {
+		if ($canViewUnopened) {
+			push(@restrictedSetMessages, $lti_msg);
+		} else {
+			$c->{viewSetCheck} = 'restricted';
+			return $lti_msg;
+		}
 	}
 
+	$c->{restrictedSetMessages} = \@restrictedSetMessages if @restrictedSetMessages;
 	return 0;
 }
 
@@ -514,8 +517,7 @@ sub invalidIPAddress {
 	return 0
 		if (!defined($set->restrict_ip)
 			|| $set->restrict_ip eq ''
-			|| $set->restrict_ip eq 'No'
-			|| $self->hasPermissions($userName, 'view_ip_restricted_sets'));
+			|| $set->restrict_ip eq 'No');
 
 	my $clientIP = new Net::IP($c->tx->remote_address);
 
@@ -530,7 +532,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 +556,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;