Skip to content

Commit 98dab46

Browse files
committed
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. Only show the right info panel div on the ProblemSet page when the set header exists and the user has permissions to see it. Translations were added to messages about IP restrictions.
1 parent 00d1921 commit 98dab46

File tree

9 files changed

+99
-63
lines changed

9 files changed

+99
-63
lines changed

conf/defaults.config

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,7 @@ $authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption'];
751751
view_proctored_tests => "student",
752752
view_hidden_work => "ta",
753753

754+
view_unopened_set_info => "guest",
754755
view_multiple_sets => "ta",
755756
view_unopened_sets => "ta",
756757
view_hidden_sets => "ta",

lib/WeBWorK/Authz.pm

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -415,25 +415,27 @@ sub checkSet {
415415
$self->{merged_set} = $set;
416416

417417
# Now we know that the set is assigned to the appropriate user.
418-
# Check to see if the user is trying to access a set that is not open.
419-
if (
420-
before($set->open_date)
421-
&& !$self->hasPermissions($userName, "view_unopened_sets")
422-
&& !(
423-
defined $set->assignment_type
424-
&& $set->assignment_type =~ /gateway/
425-
&& $node_name eq 'problem_list'
426-
&& $db->countSetVersions($effectiveUserName, $set->set_id)
427-
)
428-
)
429-
{
430-
return $c->maketext("Requested set '[_1]' is not yet open.", $setName);
431-
}
432-
433418
# Check to make sure that the set is visible, and that the user is allowed to view hidden sets.
419+
# If the user can view unopened set information, they will be given warning messages vs error message.
434420
my $visible = $set && $set->visible ne '0' && $set->visible ne '1' ? 1 : $set->visible;
435421
if (!$visible && !$self->hasPermissions($userName, "view_hidden_sets")) {
436-
return $c->maketext("Requested set '[_1]' is not available yet.", $setName);
422+
$c->{viewSetCheck} = 'hidden'
423+
if $node_name eq 'problem_list' && $self->hasPermissions($userName, 'view_unopened_set_info');
424+
return $c->maketext("Requested set '[_1]' is not available.", $setName);
425+
}
426+
427+
# Check to see if the user is trying to access a set that is not open.
428+
if (before($set->open_date) && !$self->hasPermissions($userName, 'view_unopened_sets')) {
429+
# Show problem set info if user has permissions, or there exists any test versions.
430+
$c->{viewSetCheck} = 'unopened'
431+
if $node_name eq 'problem_list'
432+
&& (
433+
$self->hasPermissions($userName, 'view_unopened_set_info')
434+
|| (defined $set->assignment_type
435+
&& $set->assignment_type =~ /gateway/
436+
&& $db->countSetVersions($effectiveUserName, $set->set_id))
437+
);
438+
return $c->maketext("Requested set '[_1]' is not yet open.", $setName);
437439
}
438440

439441
# Check to see if conditional release conditions have been met.
@@ -474,7 +476,11 @@ sub checkSet {
474476

475477
# Check for ip restrictions.
476478
my $badIP = $self->invalidIPAddress($set);
477-
return $badIP if $badIP;
479+
if ($badIP) {
480+
# Allow viewing of set info of restricted sets.
481+
$c->{viewSetCheck} = 'restricted' if $node_name eq 'problem_list';
482+
return $badIP;
483+
}
478484

479485
# If LTI grade passback is enabled and set to 'homework' mode then we need to make sure that there is a sourcedid
480486
# for this set before students access it.
@@ -530,7 +536,9 @@ sub invalidIPAddress {
530536
# if there are no addresses in the locations, return an error that
531537
# says this
532538
return $c->maketext(
533-
"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.",
539+
'Client ip address [_1] is not allowed to work this assignment, because the assignment has ip address '
540+
. 'restrictions and there are no allowed locations associated with the restriction. Contact your '
541+
. 'professor to have this problem resolved.',
534542
$clientIP->ip()
535543
) if (!@restrictAddresses);
536544

@@ -552,17 +560,13 @@ sub invalidIPAddress {
552560
# this is slightly complicated by having to check relax_restrict_ip
553561
my $badIP = '';
554562
if ($restrictType eq 'RestrictTo' && !$inRestrict) {
555-
$badIP =
556-
"Client ip address "
557-
. $clientIP->ip()
558-
. " is not in the list of addresses from "
559-
. "which this assignment may be worked.";
563+
$badIP = $c->maketext(
564+
'Client ip address [_1] is not in the list of addresses from which this assignment may be worked.',
565+
$clientIP->ip());
560566
} elsif ($restrictType eq 'DenyFrom' && $inRestrict) {
561-
$badIP =
562-
"Client ip address "
563-
. $clientIP->ip()
564-
. " is in the list of addresses from "
565-
. "which this assignment may not be worked.";
567+
$badIP = $c->maketext(
568+
'Client ip address [_1] is in the list of addresses from which this assignment may not be worked.',
569+
$clientIP->ip());
566570
} else {
567571
return 0;
568572
}

lib/WeBWorK/ConfigValues.pm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,15 @@ sub getConfigValues ($ce) {
590590
doc2 => x('These users and higher get the "Show Past Answers" button on the problem page.'),
591591
type => 'permission'
592592
},
593+
{
594+
var => 'permissionLevels{view_unopened_set_info}',
595+
doc => x('Allowed to view set information for sets which are not open yet'),
596+
doc2 => x(
597+
'This includes being able to see the set description attached to the links on the assignments '
598+
. 'page, and seeing the set header on the set information page.'
599+
),
600+
type => 'permission'
601+
},
593602
{
594603
var => 'permissionLevels{view_unopened_sets}',
595604
doc => x('Allowed to view problems in sets which are not open yet'),

lib/WeBWorK/ContentGenerator.pm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,6 @@ async sub go ($c) {
110110

111111
my $tx = $c->render_later->tx;
112112

113-
$c->stash->{footerWidthClass} = $c->can('info') ? 'col-md-8' : 'col-12';
114-
115113
if ($c->can('pre_header_initialize')) {
116114
my $pre_header_initialize = $c->pre_header_initialize;
117115
await $pre_header_initialize
@@ -133,6 +131,8 @@ async sub go ($c) {
133131
await $initialize if ref $initialize eq 'Future' || ref $initialize eq 'Mojo::Promise';
134132
}
135133

134+
$c->stash->{footerWidthClass} //= $c->can('info') ? 'col-md-8' : 'col-12';
135+
136136
$c->content;
137137

138138
# 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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,23 @@ use WeBWorK::DB::Utils qw(grok_versionID_from_vsetID_sql);
1818
use WeBWorK::Localize;
1919
use WeBWorK::AchievementItems;
2020

21+
sub can ($c, $arg) {
22+
if ($arg eq 'info') {
23+
return $c->{pg} ? 1 : 0;
24+
}
25+
return $c->SUPER::can($arg);
26+
}
27+
2128
async sub initialize ($c) {
2229
my $db = $c->db;
2330
my $ce = $c->ce;
2431
my $authz = $c->authz;
2532

26-
# $c->{invalidSet} is set in checkSet which is called by ContentGenerator.pm
27-
return
28-
if $c->{invalidSet}
29-
&& ($c->{invalidSet} !~ /^Client ip address .* is not in the list of addresses/
30-
|| $authz->{merged_set}->assignment_type !~ /gateway/);
33+
# $c->{invalidSet} is set in checkSet which is called by ContentGenerator.pm.
34+
# If $c->{viewSetCheck} is also set, we want to view some information unless the set is hidden.
35+
return if $c->{invalidSet} && (!$c->{viewSetCheck} || $c->{viewSetCheck} eq 'hidden');
3136

32-
# This will all be valid if checkSet did not set $c->{invalidSet}.
37+
# This will all be valid if the above check passes.
3338
my $userID = $c->param('user');
3439
my $eUserID = $c->param('effectiveUser');
3540

@@ -161,9 +166,7 @@ sub siblings ($c) {
161166
return $c->include('ContentGenerator/ProblemSet/siblings', setIDs => \@setIDs);
162167
}
163168

164-
sub info {
165-
my ($c) = @_;
166-
return '' unless $c->{pg};
169+
sub info ($c) {
167170
return $c->include('ContentGenerator/ProblemSet/info');
168171
}
169172

lib/WeBWorK/ContentGenerator/ProblemSets.pm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ sub getSetStatus ($c, $set) {
118118
my $db = $c->db;
119119
my $authz = $c->authz;
120120
my $effectiveUser = $c->param('effectiveUser') || $c->param('user');
121-
my $canViewUnopened = $authz->hasPermissions($c->param('user'), 'view_unopened_sets');
121+
my $canViewUnopened = $authz->hasPermissions($c->param('user'), 'view_unopened_set_info');
122122

123123
my @restricted = $ce->{options}{enableConditionalRelease} ? is_restricted($db, $set, $effectiveUser) : ();
124124

@@ -134,6 +134,7 @@ sub getSetStatus ($c, $set) {
134134
$c->maketext('Will open on [_1].', $c->formatDateTime($set->open_date, $ce->{studentDateDisplayFormat}));
135135
push(@$other_messages, $c->restricted_progression_msg(1, $set->restricted_status * 100, @restricted))
136136
if @restricted;
137+
# Test versions are only available on the ProblemSet page, so allow access if there are any test versions.
137138
$link_is_active = 0
138139
unless $canViewUnopened
139140
|| ($set->assignment_type =~ /gateway/ && $db->countSetVersions($effectiveUser, $set->set_id));
Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,44 @@
11
% use WeBWorK::Utils::DateTime qw(before between);
22
%
3-
% if (
4-
% $c->{invalidSet}
5-
% && ($c->{invalidSet} !~ /^Client ip address .* is not in the list of addresses/
6-
% || $authz->{merged_set}->assignment_type !~ /gateway/)
7-
% )
8-
% {
9-
<div class="alert alert-danger">
10-
<p class="mb-3">
11-
<%= maketext(
12-
'The selected problem set ([_1]) is not a valid set for [_2].',
13-
stash('setID'), param('effectiveUser')
14-
) =%>
15-
</p>
16-
<p class="mb-0"><%= $c->{invalidSet} %></p>
17-
</div>
18-
% last;
3+
% if ($c->{invalidSet}) {
4+
% # If $c->{viewSetCheck} is set, show some set information.
5+
% if ($c->{viewSetCheck}) {
6+
% # Do nothing unless the set is hidden, then show a warning message instead of an error.
7+
% if ($c->{viewSetCheck} eq 'hidden') {
8+
<div class="alert alert-warning">
9+
<p class="mb-0"><%= $c->{invalidSet} %></p>
10+
</div>
11+
% last;
12+
% }
13+
% } else {
14+
<div class="alert alert-danger">
15+
<p class="mb-3">
16+
<%= maketext(
17+
'The selected problem set ([_1]) is not a valid set for [_2].',
18+
stash('setID'), param('effectiveUser')
19+
) =%>
20+
</p>
21+
<p class="mb-0"><%= $c->{invalidSet} %></p>
22+
</div>
23+
% last;
24+
% }
1925
% }
2026
%
2127
% my $set = $c->{set};
2228
%
2329
% # Stats message displays the current status of the set and states the next important date.
2430
<%= include 'ContentGenerator/Base/set_status', set => $set =%>
2531
%
32+
% # Shows warning about restricted IP settings.
33+
% if ($c->{viewSetCheck} && $c->{viewSetCheck} eq 'restricted') {
34+
<div class="alert alert-warning"><%= $c->{invalidSet} %></div>
35+
% }
36+
%
2637
<%= include 'ContentGenerator/ProblemSet/auxiliary_tools' =%>
2738
%
28-
<%= $set->assignment_type =~ /gateway/ ? $c->gateway_body : $c->problem_list =%>
39+
% # Always show test versions, but only show problem list if the set has no restrictions.
40+
% if ($set->assignment_type =~ /gateway/) {
41+
<%= $c->gateway_body =%>
42+
% } elsif (!$c->{viewSetCheck}) {
43+
<%= $c->problem_list =%>
44+
% }

templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
showSolutions => '',
1212
) =%>
1313
</div>
14+
% # If viewing a restricted set, don't show anymore buttons.
15+
% if ($c->{viewSetCheck}) {
16+
</div>
17+
% last;
18+
% }
1419
% if ($ce->{achievementsEnabled} && $ce->{achievementItemsEnabled}) {
1520
% my $achievementItems = $c->{achievementItems};
1621
% if ($achievementItems && @$achievementItems) {

templates/ContentGenerator/ProblemSet/version_list.html.ep

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,8 @@
88
%
99
% my $routeName = $set->assignment_type =~ /proctored/ ? 'proctored_gateway_quiz' : 'gateway_quiz';
1010
%
11-
% if ($c->{invalidSet}) {
12-
% # If this is an invalidSet it is because the IP address is not allowed to access the set.
13-
% # Display that message here. Note that the set is valid so the set versions can still be displayed,
14-
% # but the "Start New Test" or "Continue Test" buttons should not be.
15-
<div class="alert alert-warning"><%= $c->{invalidSet} %></div>
11+
% if ($c->{viewSetCheck}) {
12+
% # If we are viewing a restricted set, then only show existing set versions and no other information.
1613
% } elsif ($continueVersion) {
1714
% # Display information about the current test and a continue open test button.
1815
% if ($timeLimit > 0) {
@@ -249,6 +246,6 @@
249246
% } else {
250247
<%= $version_list->() =%>
251248
% }
252-
% } else {
249+
% } elsif (!$c->{viewSetCheck}) {
253250
<div><p><%= maketext('No versions of this test have been taken.') %></p></div>
254251
% }

0 commit comments

Comments
 (0)