Skip to content

Commit 927bb4b

Browse files
somiajsomiaj
committed
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.
1 parent b30b845 commit 927bb4b

9 files changed

Lines changed: 198 additions & 155 deletions

File tree

‎lib/WeBWorK/Authz.pm‎

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ use warnings;
4646
use Carp qw/croak/;
4747

4848
use WeBWorK::Utils::DateTime qw(before);
49-
use WeBWorK::Utils::Sets qw(is_restricted);
49+
use WeBWorK::Utils::Sets qw(restricted_set_message);
5050
use WeBWorK::Authen::Proctor;
5151
use Net::IP;
5252
use Scalar::Util qw(weaken);
@@ -414,34 +414,35 @@ sub checkSet {
414414
# Cache the set for future use as needed. This should probably be more sophisticated than this.
415415
$self->{merged_set} = $set;
416416

417+
# Save restricted set messages to show to instructors if they exist.
418+
my $canViewUnopened = $self->hasPermissions($userName, "view_unopened_sets");
419+
my @restrictedSetMessages;
420+
417421
# 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-
}
422+
# $c->{viewSetCheck} is used to configure what is shown on ProblemSet page.
432423

433424
# Check to make sure that the set is visible, and that the user is allowed to view hidden sets.
434425
my $visible = $set && $set->visible ne '0' && $set->visible ne '1' ? 1 : $set->visible;
435426
if (!$visible && !$self->hasPermissions($userName, "view_hidden_sets")) {
427+
$c->{viewSetCheck} = 'hidden';
428+
return $c->maketext("Requested set '[_1]' is not available.", $setName);
429+
}
430+
431+
# Check to see if the user is trying to access a set that is not open.
432+
if (before($set->open_date) && !$canViewUnopened) {
433+
$c->{viewSetCheck} = 'not-open';
436434
return $c->maketext("Requested set '[_1]' is not available yet.", $setName);
437435
}
438436

439437
# Check to see if conditional release conditions have been met.
440-
if ($ce->{options}{enableConditionalRelease}
441-
&& is_restricted($db, $set, $effectiveUserName)
442-
&& !$self->hasPermissions($userName, "view_unopened_sets"))
443-
{
444-
return $c->maketext("The prerequisite conditions have not been met for set '[_1]'.", $setName);
438+
my $conditional_msg = restricted_set_message($c, $set, 'conditional');
439+
if ($conditional_msg) {
440+
if ($canViewUnopened) {
441+
push(@restrictedSetMessages, $conditional_msg);
442+
} else {
443+
$c->{viewSetCheck} = 'restricted';
444+
return $conditional_msg;
445+
}
445446
}
446447

447448
# Check to be sure that gateways are being sent to the correct content generator.
@@ -474,25 +475,27 @@ sub checkSet {
474475

475476
# Check for ip restrictions.
476477
my $badIP = $self->invalidIPAddress($set);
477-
return $badIP if $badIP;
478-
479-
# If LTI grade passback is enabled and set to 'homework' mode then we need to make sure that there is a sourcedid
480-
# for this set before students access it.
481-
my $LTIGradeMode = $ce->{LTIGradeMode} // '';
482-
483-
if ($LTIGradeMode eq 'homework' && !$self->hasPermissions($userName, "view_unopened_sets")) {
484-
my $LMS =
485-
$ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
486-
? $c->link_to($ce->{LTI}{ $ce->{LTIVersion} }{LMS_name} => $ce->{LTI}{ $ce->{LTIVersion} }{LMS_url})
487-
: $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name};
488-
return $c->b($c->maketext(
489-
'You must use your Learning Management System ([_1]) to access this set. '
490-
. 'Try logging in to the Learning Management System and visiting the set from there.',
491-
$LMS
492-
))
493-
unless $set->lis_source_did || ($ce->{LTIVersion} eq 'v1p3' && $ce->{LTI}{v1p3}{ignoreMissingSourcedID});
478+
if ($badIP) {
479+
if ($self->hasPermissions($userName, 'view_ip_restricted_sets')) {
480+
push(@restrictedSetMessages, $badIP);
481+
} else {
482+
$c->{viewSetCheck} = 'restricted';
483+
return $badIP;
484+
}
485+
}
486+
487+
# Check for lis_source_did if LTI grade passback is 'homework'.
488+
my $lti_msg = restricted_set_message($c, $set, 'lti');
489+
if ($lti_msg) {
490+
if ($canViewUnopened) {
491+
push(@restrictedSetMessages, $lti_msg);
492+
} else {
493+
$c->{viewSetCheck} = 'restricted';
494+
return $lti_msg;
495+
}
494496
}
495497

498+
$c->{restrictedSetMessages} = \@restrictedSetMessages if @restrictedSetMessages;
496499
return 0;
497500
}
498501

@@ -514,8 +517,7 @@ sub invalidIPAddress {
514517
return 0
515518
if (!defined($set->restrict_ip)
516519
|| $set->restrict_ip eq ''
517-
|| $set->restrict_ip eq 'No'
518-
|| $self->hasPermissions($userName, 'view_ip_restricted_sets'));
520+
|| $set->restrict_ip eq 'No');
519521

520522
my $clientIP = new Net::IP($c->tx->remote_address);
521523

@@ -530,7 +532,9 @@ sub invalidIPAddress {
530532
# if there are no addresses in the locations, return an error that
531533
# says this
532534
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.",
535+
'Client ip address [_1] is not allowed to work this assignment, because the assignment has ip address '
536+
. 'restrictions and there are no allowed locations associated with the restriction. Contact your '
537+
. 'professor to have this problem resolved.',
534538
$clientIP->ip()
535539
) if (!@restrictAddresses);
536540

@@ -552,17 +556,13 @@ sub invalidIPAddress {
552556
# this is slightly complicated by having to check relax_restrict_ip
553557
my $badIP = '';
554558
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.";
559+
$badIP = $c->maketext(
560+
'Client ip address [_1] is not in the list of addresses from which this assignment may be worked.',
561+
$clientIP->ip());
560562
} 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.";
563+
$badIP = $c->maketext(
564+
'Client ip address [_1] is in the list of addresses from which this assignment may not be worked.',
565+
$clientIP->ip());
566566
} else {
567567
return 0;
568568
}

‎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: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,23 @@ use WeBWorK::Localize;
1919
use WeBWorK::AchievementItems;
2020
use WeBWorK::HTML::StudentNav qw(studentNav);
2121

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

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

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

@@ -106,6 +111,7 @@ async sub initialize ($c) {
106111

107112
$c->{pg} =
108113
await renderPG($c, $effectiveUser, $c->{set}, $problem, $c->{set}->psvn, {}, { displayMode => $displayMode });
114+
$c->{pg} = '' unless $c->{pg}{body_text} =~ /\S/;
109115

110116
return;
111117
}
@@ -169,10 +175,8 @@ sub siblings ($c) {
169175
return $c->include('ContentGenerator/ProblemSet/siblings', setIDs => \@setIDs);
170176
}
171177

172-
sub info {
173-
my ($c) = @_;
174-
return '' unless $c->{pg};
175-
return $c->include('ContentGenerator/ProblemSet/info');
178+
sub info ($c) {
179+
return $c->{pg} ? $c->include('ContentGenerator/ProblemSet/info') : '';
176180
}
177181

178182
# This is called by the ContentGenerator/ProblemSet/body template for a regular homework set.

‎lib/WeBWorK/ContentGenerator/ProblemSets.pm‎

Lines changed: 10 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use WeBWorK::Debug;
1111
use WeBWorK::Utils qw(sortByName);
1212
use WeBWorK::Utils::DateTime qw(after);
1313
use WeBWorK::Utils::Files qw(readFile path_is_subdir);
14-
use WeBWorK::Utils::Sets qw(is_restricted format_set_name_display);
14+
use WeBWorK::Utils::Sets qw(restricted_set_message);
1515
use WeBWorK::Localize;
1616

1717
# The "default" data in the course_info.txt file.
@@ -114,15 +114,11 @@ sub info ($c) {
114114
}
115115

116116
sub getSetStatus ($c, $set) {
117-
my $ce = $c->ce;
118-
my $db = $c->db;
119-
my $authz = $c->authz;
120-
my $effectiveUser = $c->param('effectiveUser') || $c->param('user');
121-
my $canViewUnopened = $authz->hasPermissions($c->param('user'), 'view_unopened_sets');
122-
123-
my @restricted = $ce->{options}{enableConditionalRelease} ? is_restricted($db, $set, $effectiveUser) : ();
124-
125-
my $link_is_active = 1;
117+
my $ce = $c->ce;
118+
my $db = $c->db;
119+
my $authz = $c->authz;
120+
my $effectiveUser = $c->param('effectiveUser') || $c->param('user');
121+
my $restricted_msg = restricted_set_message($c, $set, 'conditional') || restricted_set_message($c, $set, 'lti');
126122

127123
# Determine set status.
128124
my $status_msg;
@@ -132,11 +128,7 @@ sub getSetStatus ($c, $set) {
132128
$status = 'not-open';
133129
$status_msg =
134130
$c->maketext('Will open on [_1].', $c->formatDateTime($set->open_date, $ce->{studentDateDisplayFormat}));
135-
push(@$other_messages, $c->restricted_progression_msg(1, $set->restricted_status * 100, @restricted))
136-
if @restricted;
137-
$link_is_active = 0
138-
unless $canViewUnopened
139-
|| ($set->assignment_type =~ /gateway/ && $db->countSetVersions($effectiveUser, $set->set_id));
131+
push(@$other_messages, $restricted_msg) if $restricted_msg;
140132
} elsif ($c->submitTime < $set->due_date) {
141133
$status = 'open';
142134

@@ -172,31 +164,8 @@ sub getSetStatus ($c, $set) {
172164
$c->maketext('Open. Due [_1].', $c->formatDateTime($set->due_date, $ce->{studentDateDisplayFormat}));
173165
}
174166

175-
if (@restricted) {
176-
$link_is_active = 0 unless $canViewUnopened;
177-
push(@$other_messages, $c->restricted_progression_msg(0, $set->restricted_status * 100, @restricted));
178-
} elsif (!$canViewUnopened
179-
&& $ce->{LTIVersion}
180-
&& ($ce->{LTIVersion} ne 'v1p3' || !$ce->{LTI}{v1p3}{ignoreMissingSourcedID})
181-
&& defined $ce->{LTIGradeMode}
182-
&& $ce->{LTIGradeMode} eq 'homework'
183-
&& !$set->lis_source_did)
184-
{
185-
# The set shouldn't be shown if LTI grade mode is set to homework and a
186-
# sourced_id is not available to use to send back grades
187-
# (unless we are using LTI 1.3 and $LTI{v1p3}{ignoreMissingSourcedID} is set)
188-
push(
189-
@$other_messages,
190-
$c->maketext(
191-
'You must log into this set via your Learning Management System ([_1]).',
192-
$ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
193-
? $c->link_to(
194-
$ce->{LTI}{ $ce->{LTIVersion} }{LMS_name} => $ce->{LTI}{ $ce->{LTIVersion} }{LMS_url}
195-
)
196-
: $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}
197-
)
198-
);
199-
$link_is_active = 0;
167+
if ($restricted_msg) {
168+
push(@$other_messages, $restricted_msg);
200169
}
201170
} elsif ($c->submitTime < $set->answer_date) {
202171
$status_msg = $c->maketext('Answers available for review on [_1].',
@@ -209,8 +178,7 @@ sub getSetStatus ($c, $set) {
209178
status => $status,
210179
status_msg => $status_msg,
211180
other_messages => $other_messages,
212-
link_is_active => $link_is_active,
213-
is_restricted => scalar(@restricted)
181+
is_restricted => $restricted_msg ? 1 : 0
214182
);
215183
}
216184

@@ -232,20 +200,4 @@ sub byUrgency {
232200
return $a->set_id cmp $b->set_id;
233201
}
234202

235-
sub restricted_progression_msg ($c, $open, $restriction, @restricted) {
236-
if (@restricted == 1) {
237-
return $c->maketext(
238-
'To access this set you must score at least [_1]% on set [_2].',
239-
sprintf('%.0f', $restriction),
240-
$c->tag('span', dir => 'ltr', format_set_name_display($restricted[0]))
241-
);
242-
} else {
243-
return $c->maketext(
244-
'To access this set you must score at least [_1]% on the following sets: [_2].',
245-
sprintf('%.0f', $restriction),
246-
join(', ', map { $c->tag('span', dir => 'ltr', format_set_name_display($_)) } @restricted)
247-
);
248-
}
249-
}
250-
251203
1;

0 commit comments

Comments
 (0)