From 1bc4f497b59ec8688d1a81c9c59fcd8fa7dc8f1a Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Wed, 24 Dec 2025 14:58:01 -0700 Subject: [PATCH] Update permissions for viewing info on ProblemSet page. Rework when a user is able to see set information on the ProblemSet page. This includes being able to see the set description and information header before a set is open. The goal is to make the ProblemSet page friendlier when a student accesses it from an LMS before the open date or if the set is not currently visible. In addition the set headers can be used to provide information to students before the set opens. This is done by adding a new permission `view_unoppend_set_info` which defaults to `guest` (this allows instructors to change this to `ta` to get the old behavior). Users with this permission will be shown a ProblemSet page that includes when the set opens along with the set information header and a button to email the instructor before the open date. In addition users with this permission will be shown a warning alert instead of a danger alert if they try to access a hidden set (the set header will not be included in this case). Note, that the permission `view_unoppend_sets` description in the course configuration is just being able to view problems on sets which are not open, so this new permission is consistent with the previous description, it just separates seeing set info from seeing set problems. Care is taken to ensure that students can view (thus access) any existing test versions for tests that have ip restrictions, or if the template open date was changed. Last, only show the right info panel div when the panel is not empty and doesn't include only whitespace. And translations were added to messages about IP restrictions. --- conf/defaults.config | 1 + lib/WeBWorK/Authz.pm | 60 ++++++++++--------- lib/WeBWorK/ConfigValues.pm | 9 +++ lib/WeBWorK/ContentGenerator/ProblemSet.pm | 13 ++-- lib/WeBWorK/ContentGenerator/ProblemSets.pm | 3 +- templates/ContentGenerator/ProblemSet.html.ep | 50 ++++++++++------ .../ProblemSet/auxiliary_tools.html.ep | 5 ++ .../ProblemSet/version_list.html.ep | 9 +-- templates/layouts/system.html.ep | 12 ++-- 9 files changed, 98 insertions(+), 64 deletions(-) diff --git a/conf/defaults.config b/conf/defaults.config index eb631e9790..78f044e42e 100644 --- a/conf/defaults.config +++ b/conf/defaults.config @@ -751,6 +751,7 @@ $authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption']; view_proctored_tests => "student", view_hidden_work => "ta", + view_unopened_set_info => "guest", view_multiple_sets => "ta", view_unopened_sets => "ta", view_hidden_sets => "ta", diff --git a/lib/WeBWorK/Authz.pm b/lib/WeBWorK/Authz.pm index a238f3fb67..628854ed7a 100644 --- a/lib/WeBWorK/Authz.pm +++ b/lib/WeBWorK/Authz.pm @@ -415,25 +415,27 @@ 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); - } - # Check to make sure that the set is visible, and that the user is allowed to view hidden sets. + # If the user can view unopened set information, they will be given warning messages vs error message. my $visible = $set && $set->visible ne '0' && $set->visible ne '1' ? 1 : $set->visible; if (!$visible && !$self->hasPermissions($userName, "view_hidden_sets")) { - return $c->maketext("Requested set '[_1]' is not available yet.", $setName); + $c->{viewSetCheck} = 'hidden' + if $node_name eq 'problem_list' && $self->hasPermissions($userName, 'view_unopened_set_info'); + 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')) { + # Show problem set info if user has permissions, or there exists any test versions. + $c->{viewSetCheck} = 'unopened' + if $node_name eq 'problem_list' + && ( + $self->hasPermissions($userName, 'view_unopened_set_info') + || (defined $set->assignment_type + && $set->assignment_type =~ /gateway/ + && $db->countSetVersions($effectiveUserName, $set->set_id)) + ); + return $c->maketext("Requested set '[_1]' is not yet open.", $setName); } # Check to see if conditional release conditions have been met. @@ -474,7 +476,11 @@ sub checkSet { # Check for ip restrictions. my $badIP = $self->invalidIPAddress($set); - return $badIP if $badIP; + if ($badIP) { + # Allow viewing of set info of restricted sets. + $c->{viewSetCheck} = 'restricted' if $node_name eq 'problem_list'; + return $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. @@ -530,7 +536,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 +560,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/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 27ccd65cbb..f4a24ec40f 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -590,6 +590,15 @@ sub getConfigValues ($ce) { doc2 => x('These users and higher get the "Show Past Answers" button on the problem page.'), type => 'permission' }, + { + var => 'permissionLevels{view_unopened_set_info}', + doc => x('Allowed to view set information for sets which are not open yet'), + doc2 => x( + 'This includes being able to see the set description attached to the links on the assignments ' + . 'page, and seeing the set header on the set information page.' + ), + type => 'permission' + }, { var => 'permissionLevels{view_unopened_sets}', doc => x('Allowed to view problems in sets which are not open yet'), diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm index ba77939ec5..97f69c5111 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSet.pm @@ -23,13 +23,11 @@ async sub initialize ($c) { 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'); @@ -161,8 +159,7 @@ sub siblings ($c) { return $c->include('ContentGenerator/ProblemSet/siblings', setIDs => \@setIDs); } -sub info { - my ($c) = @_; +sub info ($c) { return '' unless $c->{pg}; return $c->include('ContentGenerator/ProblemSet/info'); } diff --git a/lib/WeBWorK/ContentGenerator/ProblemSets.pm b/lib/WeBWorK/ContentGenerator/ProblemSets.pm index 3eeb4fb4c3..57be273d63 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSets.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSets.pm @@ -118,7 +118,7 @@ sub getSetStatus ($c, $set) { 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 $canViewUnopened = $authz->hasPermissions($c->param('user'), 'view_unopened_set_info'); my @restricted = $ce->{options}{enableConditionalRelease} ? is_restricted($db, $set, $effectiveUser) : (); @@ -134,6 +134,7 @@ sub getSetStatus ($c, $set) { $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; + # Test versions are only available on the ProblemSet page, so allow access if there are any test versions. $link_is_active = 0 unless $canViewUnopened || ($set->assignment_type =~ /gateway/ && $db->countSetVersions($effectiveUser, $set->set_id)); diff --git a/templates/ContentGenerator/ProblemSet.html.ep b/templates/ContentGenerator/ProblemSet.html.ep index 62eb24f808..2631b0f41d 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 =%> % +% # Shows warning about restricted IP settings. +% 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..1df39d4603 100644 --- a/templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep +++ b/templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep @@ -11,6 +11,11 @@ showSolutions => '', ) =%> + % # If viewing a restricted set, don't show anymore buttons. + % if ($c->{viewSetCheck}) { + + % last; + % } % if ($ce->{achievementsEnabled} && $ce->{achievementItemsEnabled}) { % my $achievementItems = $c->{achievementItems}; % if ($achievementItems && @$achievementItems) { diff --git a/templates/ContentGenerator/ProblemSet/version_list.html.ep b/templates/ContentGenerator/ProblemSet/version_list.html.ep index 0d332c749e..0e7f4c5694 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 ($timeLimit > 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/layouts/system.html.ep b/templates/layouts/system.html.ep index 282890b782..8750b57046 100644 --- a/templates/layouts/system.html.ep +++ b/templates/layouts/system.html.ep @@ -136,11 +136,15 @@ <%= content =%> % if ($c->can('info')) { -
-
- <%= $c->info =%> + % # Only show info div if the content is not empty. + % my $info = $c->info; + % if ($info !~ /^\s*$/) { +
+
+ <%= $info =%> +
-
+ % } % }
%