diff --git a/lib/WeBWorK/Authz.pm b/lib/WeBWorK/Authz.pm index a238f3fb67..94dd806d3b 100644 --- a/lib/WeBWorK/Authz.pm +++ b/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; } diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index 21b106dde9..a8802f13dd 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/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 diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm index 6c15eeca7f..d20438d94d 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm +++ b/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. diff --git a/lib/WeBWorK/ContentGenerator/ProblemSets.pm b/lib/WeBWorK/ContentGenerator/ProblemSets.pm index 3eeb4fb4c3..2c02a51ceb 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSets.pm +++ b/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; diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index a45c8e1567..850b4463c4 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/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 is true), and a list containing only the original set id otherwise. +=head2 restricted_set_message + +Usage: C + +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 diff --git a/templates/ContentGenerator/ProblemSet.html.ep b/templates/ContentGenerator/ProblemSet.html.ep index 62eb24f808..0fdcc4cfa9 100644 --- a/templates/ContentGenerator/ProblemSet.html.ep +++ b/templates/ContentGenerator/ProblemSet.html.ep @@ -1,21 +1,27 @@ % use WeBWorK::Utils::DateTime qw(before between); % -% if ( - % $c->{invalidSet} - % && ($c->{invalidSet} !~ /^Client ip address .* is not in the list of addresses/ - % || $authz->{merged_set}->assignment_type !~ /gateway/) - % ) -% { -
-

- <%= maketext( - 'The selected problem set ([_1]) is not a valid set for [_2].', - stash('setID'), param('effectiveUser') - ) =%> -

-

<%= $c->{invalidSet} %>

-
- % last; +% if ($c->{invalidSet}) { + % # If $c->{viewSetCheck} is set, show some set information. + % if ($c->{viewSetCheck}) { + % # Do nothing unless the set is hidden, then show a warning message instead of an error. + % if ($c->{viewSetCheck} eq 'hidden') { +
+

<%= $c->{invalidSet} %>

+
+ % last; + % } + % } else { +
+

+ <%= maketext( + 'The selected problem set ([_1]) is not a valid set for [_2].', + stash('setID'), param('effectiveUser') + ) =%> +

+

<%= $c->{invalidSet} %>

+
+ % last; + % } % } % % my $set = $c->{set}; @@ -23,6 +29,16 @@ % # Stats message displays the current status of the set and states the next important date. <%= include 'ContentGenerator/Base/set_status', set => $set =%> % +% # Show any restricted messages. +% if ($c->{viewSetCheck} && $c->{viewSetCheck} eq 'restricted') { +
<%== $c->{invalidSet} %>
+% } +% <%= include 'ContentGenerator/ProblemSet/auxiliary_tools' =%> % -<%= $set->assignment_type =~ /gateway/ ? $c->gateway_body : $c->problem_list =%> +% # Always show test versions, but only show problem list if the set has no restrictions. +% if ($set->assignment_type =~ /gateway/) { + <%= $c->gateway_body =%> +% } elsif (!$c->{viewSetCheck}) { + <%= $c->problem_list =%> +% } diff --git a/templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep b/templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep index 0b81b30b71..34a70e4385 100644 --- a/templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep +++ b/templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep @@ -1,3 +1,22 @@ +% # If viewing a restricted set, only show the email instructor button. +% if ($c->{viewSetCheck}) { +
+
+ <%= $c->feedbackMacro( + route => current_route, + set => $c->{set}->set_id, + problem => '', + displayMode => $c->{displayMode}, + showOldAnswers => '', + showCorrectAnswers => '', + showHints => '', + showSolutions => '', + ) =%> +
+
+ % last; +% } +%
<%= $c->feedbackMacro( diff --git a/templates/ContentGenerator/ProblemSet/version_list.html.ep b/templates/ContentGenerator/ProblemSet/version_list.html.ep index f5eaa58597..fb521457dd 100644 --- a/templates/ContentGenerator/ProblemSet/version_list.html.ep +++ b/templates/ContentGenerator/ProblemSet/version_list.html.ep @@ -8,11 +8,8 @@ % % my $routeName = $set->assignment_type =~ /proctored/ ? 'proctored_gateway_quiz' : 'gateway_quiz'; % -% if ($c->{invalidSet}) { - % # If this is an invalidSet it is because the IP address is not allowed to access the set. - % # Display that message here. Note that the set is valid so the set versions can still be displayed, - % # but the "Start New Test" or "Continue Test" buttons should not be. -
<%= $c->{invalidSet} %>
+% if ($c->{viewSetCheck}) { + % # If we are viewing a restricted set, then only show existing set versions and no other information. % } elsif ($continueVersion) { % # Display information about the current test and a continue open test button. % if ($continueVersion->version_time_limit > 0) { @@ -249,6 +246,6 @@ % } else { <%= $version_list->() =%> % } -% } else { +% } elsif (!$c->{viewSetCheck}) {

<%= maketext('No versions of this test have been taken.') %>

% } diff --git a/templates/ContentGenerator/ProblemSets/set_list_row.html.ep b/templates/ContentGenerator/ProblemSets/set_list_row.html.ep index 24cbc8cc3e..1e1d7f13a2 100644 --- a/templates/ContentGenerator/ProblemSets/set_list_row.html.ep +++ b/templates/ContentGenerator/ProblemSets/set_list_row.html.ep @@ -16,18 +16,10 @@
- % if ($link_is_active) { - <%= link_to $display_name => $c->systemLink(url_for('problem_list', setID => $set->set_id)), - class => 'fw-bold set-id-tooltip', - data => { bs_toggle => 'tooltip', bs_placement => 'right', bs_title => $set->description } - =%> - % } else { - - <%= $display_name =%> - - <%= $set->description %> - % } + <%= link_to $display_name => $c->systemLink(url_for('problem_list', setID => $set->set_id)), + class => 'fw-bold set-id-tooltip', + data => { bs_toggle => 'tooltip', bs_placement => 'right', bs_title => $set->description } + =%>
<%= $status_msg %>
% if (!$set->visible && $authz->hasPermissions(param('user'), 'view_unopened_sets')) {