Skip to content

Commit a51270e

Browse files
drgrice1somiaj
drgrice1
authored and
somiaj
committed
Make it much easier to act as a student.
This is achieved by displaying the student nav on the problem set page for users that have the permission to act as a user as well as always showing the student nav in problems and in tests. If you are acting as a user, then the student nav looks the same as before with the next and previous buttons and the name of the user that is currently being acted as shown on the button. However, if you are not currently acting as another user, then the next and previous buttons are not shown and the button says "Select Student to Act As" (or in a test it says "Select a Test to Review"). Also fix the breadcrumb in a test when the set is not valid, but the setID does have the `setID,v?` format. Currently if you are acting as a student user and reviewing a test version, say "test,v1", but you have not worked the test, and you click the "Stop Acting" button, then the message stating that the selected test is not valid for the user is shown (it would be nice to not show this even) and the breadcrumb ends with the inactive link "test,v1", and you can only go back to the assignments page. Now, the "setID" link is shown and works, and the inactive "v1" link is at the end. This is built on top of openwebwork#2875 since it would conflict rather heavily with that pull request if it were not.
1 parent 57d6817 commit a51270e

6 files changed

Lines changed: 271 additions & 218 deletions

File tree

‎lib/WeBWorK/ContentGenerator/GatewayQuiz.pm‎

Lines changed: 73 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,7 +1343,7 @@ sub path ($c, $args) {
13431343
$courseName => $navigation_allowed ? $c->url_for('set_list') : '',
13441344
$setID eq 'Undefined_Set'
13451345
|| $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation}
1346-
? ($setID => '')
1346+
? ($setID =~ /^(.+),(v\d+)$/ ? ($1 => $c->url_for('problem_list', setID => $1), $2 => '') : ($setID => ''))
13471347
: (
13481348
$c->{set}->set_id => $c->url_for('problem_list', setID => $c->{set}->set_id),
13491349
'v' . $c->{set}->version_id => ''
@@ -1359,7 +1359,7 @@ sub nav ($c, $args) {
13591359
return '' if $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation};
13601360

13611361
# Set up and display a student navigation for those that have permission to act as a student.
1362-
if ($c->authz->hasPermissions($userID, 'become_student') && $effectiveUserID ne $userID) {
1362+
if ($c->authz->hasPermissions($userID, 'become_student')) {
13631363
my $setID = $c->{set}->set_id;
13641364

13651365
return '' if $setID eq 'Undefined_Set';
@@ -1368,76 +1368,83 @@ sub nav ($c, $args) {
13681368

13691369
# Find all versions of this set that have been taken (excluding those taken by the current user).
13701370
my @users =
1371-
$db->listSetVersionsWhere({ user_id => { not_like => $userID }, set_id => { like => "$setID,v\%" } });
1371+
$db->listSetVersionsWhere({ user_id => { '!=' => $userID }, set_id => { like => "$setID,v\%" } });
13721372
my @allUserRecords = $db->getUsers(map { $_->[0] } @users);
13731373

1374-
my $filter = $c->param('studentNavFilter');
1375-
1376-
# Format the student names for display, and associate the users with the test versions.
1377-
my %filters;
1378-
my @userRecords;
1379-
for (0 .. $#allUserRecords) {
1380-
# Add to the sections and recitations if defined. Also store the first user found in that section or
1381-
# recitation. This user will be switched to when the filter is selected.
1382-
my $section = $allUserRecords[$_]->section;
1383-
$filters{"section:$section"} =
1384-
[ $c->maketext('Filter by section [_1]', $section), $allUserRecords[$_]->user_id, $users[$_][2] ]
1385-
if $section && !$filters{"section:$section"};
1386-
my $recitation = $allUserRecords[$_]->recitation;
1387-
$filters{"recitation:$recitation"} =
1388-
[ $c->maketext('Filter by recitation [_1]', $recitation), $allUserRecords[$_]->user_id, $users[$_][2] ]
1389-
if $recitation && !$filters{"recitation:$recitation"};
1390-
1391-
# Only keep this user if it satisfies the selected filter if a filter was selected.
1392-
next
1393-
unless !$filter
1394-
|| ($filter =~ /^section:(.*)$/ && $allUserRecords[$_]->section eq $1)
1395-
|| ($filter =~ /^recitation:(.*)$/ && $allUserRecords[$_]->recitation eq $1);
1396-
1397-
my $addRecord = $allUserRecords[$_];
1398-
push @userRecords, $addRecord;
1399-
1400-
$addRecord->{displayName} =
1401-
($addRecord->last_name || $addRecord->first_name
1402-
? $addRecord->last_name . ', ' . $addRecord->first_name
1403-
: $addRecord->user_id);
1404-
$addRecord->{setVersion} = $users[$_][2];
1405-
}
1374+
if (@allUserRecords) {
1375+
my $filter = $c->param('studentNavFilter');
1376+
1377+
# Format the student names for display, and associate the users with the test versions.
1378+
my %filters;
1379+
my @userRecords;
1380+
for (0 .. $#allUserRecords) {
1381+
# Add to the sections and recitations if defined. Also store the first user found in that section or
1382+
# recitation. This user will be switched to when the filter is selected.
1383+
my $section = $allUserRecords[$_]->section;
1384+
$filters{"section:$section"} =
1385+
[ $c->maketext('Filter by section [_1]', $section), $allUserRecords[$_]->user_id, $users[$_][2] ]
1386+
if $section && !$filters{"section:$section"};
1387+
my $recitation = $allUserRecords[$_]->recitation;
1388+
$filters{"recitation:$recitation"} = [
1389+
$c->maketext('Filter by recitation [_1]', $recitation), $allUserRecords[$_]->user_id,
1390+
$users[$_][2]
1391+
]
1392+
if $recitation && !$filters{"recitation:$recitation"};
1393+
1394+
# Only keep this user if it satisfies the selected filter if a filter was selected.
1395+
next
1396+
unless !$filter
1397+
|| ($filter =~ /^section:(.*)$/ && $allUserRecords[$_]->section eq $1)
1398+
|| ($filter =~ /^recitation:(.*)$/ && $allUserRecords[$_]->recitation eq $1);
1399+
1400+
my $addRecord = $allUserRecords[$_];
1401+
push @userRecords, $addRecord;
1402+
1403+
$addRecord->{displayName} =
1404+
($addRecord->last_name || $addRecord->first_name
1405+
? $addRecord->last_name . ', ' . $addRecord->first_name
1406+
: $addRecord->user_id);
1407+
$addRecord->{setVersion} = $users[$_][2];
1408+
}
14061409

1407-
# Sort by last name, then first name, then user_id, then set version.
1408-
@userRecords = sort {
1409-
lc($a->last_name) cmp lc($b->last_name)
1410-
|| lc($a->first_name) cmp lc($b->first_name)
1411-
|| lc($a->user_id) cmp lc($b->user_id)
1412-
|| lc($a->{setVersion}) <=> lc($b->{setVersion})
1413-
} @userRecords;
1414-
1415-
# Find the previous, current, and next test.
1416-
my $currentTestIndex = 0;
1417-
for (0 .. $#userRecords) {
1418-
if ($userRecords[$_]->user_id eq $effectiveUserID && $userRecords[$_]->{setVersion} == $setVersion) {
1419-
$currentTestIndex = $_;
1420-
last;
1410+
# Sort by last name, then first name, then user_id, then set version.
1411+
@userRecords = sort {
1412+
lc($a->last_name) cmp lc($b->last_name)
1413+
|| lc($a->first_name) cmp lc($b->first_name)
1414+
|| lc($a->user_id) cmp lc($b->user_id)
1415+
|| lc($a->{setVersion}) <=> lc($b->{setVersion})
1416+
} @userRecords;
1417+
1418+
# Find the previous, current, and next test.
1419+
my $currentTestIndex = 0;
1420+
for (0 .. $#userRecords) {
1421+
if ($userRecords[$_]->user_id eq $effectiveUserID && $userRecords[$_]->{setVersion} == $setVersion) {
1422+
$currentTestIndex = $_;
1423+
last;
1424+
}
14211425
}
1426+
my $prevTest = $currentTestIndex > 0 ? $userRecords[ $currentTestIndex - 1 ] : 0;
1427+
my $nextTest = $currentTestIndex < $#userRecords ? $userRecords[ $currentTestIndex + 1 ] : 0;
1428+
1429+
# Mark the current test.
1430+
$userRecords[$currentTestIndex]{currentTest} = 1;
1431+
1432+
# Show the student nav.
1433+
return $c->include(
1434+
'ContentGenerator/GatewayQuiz/nav',
1435+
userID => $userID,
1436+
eUserID => $effectiveUserID,
1437+
userRecords => \@userRecords,
1438+
setVersion => $setVersion,
1439+
prevTest => $prevTest,
1440+
nextTest => $nextTest,
1441+
currentTestIndex => $currentTestIndex,
1442+
filters => \%filters,
1443+
filter => $filter
1444+
);
14221445
}
1423-
my $prevTest = $currentTestIndex > 0 ? $userRecords[ $currentTestIndex - 1 ] : 0;
1424-
my $nextTest = $currentTestIndex < $#userRecords ? $userRecords[ $currentTestIndex + 1 ] : 0;
1425-
1426-
# Mark the current test.
1427-
$userRecords[$currentTestIndex]{currentTest} = 1;
1428-
1429-
# Show the student nav.
1430-
return $c->include(
1431-
'ContentGenerator/GatewayQuiz/nav',
1432-
userRecords => \@userRecords,
1433-
setVersion => $setVersion,
1434-
prevTest => $prevTest,
1435-
nextTest => $nextTest,
1436-
currentTestIndex => $currentTestIndex,
1437-
filters => \%filters,
1438-
filter => $filter
1439-
);
14401446
}
1447+
return '';
14411448
}
14421449

14431450
sub warningMessage ($c) {

‎lib/WeBWorK/ContentGenerator/Problem.pm‎

Lines changed: 3 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ WeBWorK::ContentGenerator::Problem - Allow a student to interact with a problem.
77
88
=cut
99

10-
use WeBWorK::HTML::SingleProblemGrader;
1110
use WeBWorK::Debug;
1211
use WeBWorK::Utils qw(decodeAnswers wwRound);
1312
use WeBWorK::Utils::DateTime qw(before between after);
@@ -23,6 +22,8 @@ use WeBWorK::AchievementEvaluator qw(checkForAchievements);
2322
use WeBWorK::DB::Utils qw(global2user fake_set fake_problem);
2423
use WeBWorK::Localize;
2524
use WeBWorK::AchievementEvaluator;
25+
use WeBWorK::HTML::SingleProblemGrader;
26+
use WeBWorK::HTML::StudentNav qw(studentNav);
2627

2728
# GET/POST Parameters for this module
2829
#
@@ -836,74 +837,6 @@ sub nav ($c, $args) {
836837
my $mergedSet = $db->getMergedSet($eUserID, $setID);
837838
return '' if !$mergedSet;
838839

839-
# Set up a student navigation for those that have permission to act as a student.
840-
my $userNav = '';
841-
if ($authz->hasPermissions($userID, 'become_student') && $eUserID ne $userID) {
842-
# Find all users for this set (except the current user) sorted by last_name, then first_name, then user_id.
843-
my @allUserRecords = $db->getUsersWhere(
844-
{
845-
user_id => [
846-
map { $_->[0] } $db->listUserSetsWhere({ set_id => $setID, user_id => { not_like => $userID } })
847-
]
848-
},
849-
[qw/last_name first_name user_id/]
850-
);
851-
852-
my $filter = $c->param('studentNavFilter');
853-
854-
# Find the previous, current, and next users, and format the student names for display.
855-
# Also create a hash of sections and recitations if there are any for the course.
856-
my @userRecords;
857-
my $currentUserIndex = 0;
858-
my %filters;
859-
for (@allUserRecords) {
860-
# Add to the sections and recitations if defined. Also store the first user found in that section or
861-
# recitation. This user will be switched to when the filter is selected.
862-
my $section = $_->section;
863-
$filters{"section:$section"} = [ $c->maketext('Filter by section [_1]', $section), $_->user_id ]
864-
if $section && !$filters{"section:$section"};
865-
my $recitation = $_->recitation;
866-
$filters{"recitation:$recitation"} = [ $c->maketext('Filter by recitation [_1]', $recitation), $_->user_id ]
867-
if $recitation && !$filters{"recitation:$recitation"};
868-
869-
# Only keep this user if it satisfies the selected filter if a filter was selected.
870-
next
871-
unless !$filter
872-
|| ($filter =~ /^section:(.*)$/ && $_->section eq $1)
873-
|| ($filter =~ /^recitation:(.*)$/ && $_->recitation eq $1);
874-
875-
my $addRecord = $_;
876-
$currentUserIndex = @userRecords if $addRecord->user_id eq $eUserID;
877-
push @userRecords, $addRecord;
878-
879-
# Construct a display name.
880-
$addRecord->{displayName} =
881-
($addRecord->last_name || $addRecord->first_name
882-
? $addRecord->last_name . ', ' . $addRecord->first_name
883-
: $addRecord->user_id);
884-
}
885-
my $prevUser = $currentUserIndex > 0 ? $userRecords[ $currentUserIndex - 1 ] : 0;
886-
my $nextUser = $currentUserIndex < $#userRecords ? $userRecords[ $currentUserIndex + 1 ] : 0;
887-
888-
# Mark the current user.
889-
$userRecords[$currentUserIndex]{currentUser} = 1;
890-
891-
my $problemPage = $c->url_for('problem_detail', setID => $setID, problemID => $problemID);
892-
893-
# Set up the student nav.
894-
$userNav = $c->include(
895-
'ContentGenerator/Problem/student_nav',
896-
eUserID => $eUserID,
897-
problemPage => $problemPage,
898-
userRecords => \@userRecords,
899-
currentUserIndex => $currentUserIndex,
900-
prevUser => $prevUser,
901-
nextUser => $nextUser,
902-
filter => $filter,
903-
filters => \%filters
904-
);
905-
}
906-
907840
my $isJitarSet = $mergedSet->assignment_type eq 'jitar';
908841

909842
my ($prevID, $nextID);
@@ -974,7 +907,7 @@ sub nav ($c, $args) {
974907
role => 'navigation',
975908
'aria-label' => 'problem navigation',
976909
$c->c($c->tag('div', class => 'd-flex submit-buttons-container', $c->navMacro($args, \%tail, @links)),
977-
$userNav)->join('')
910+
studentNav($c, $setID))->join('')
978911
);
979912
}
980913

‎lib/WeBWorK/ContentGenerator/ProblemSet.pm‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use WeBWorK::Utils::Sets qw(is_restricted grade_set format_set_name_display
1717
use WeBWorK::DB::Utils qw(grok_versionID_from_vsetID_sql);
1818
use WeBWorK::Localize;
1919
use WeBWorK::AchievementItems;
20+
use WeBWorK::HTML::StudentNav qw(studentNav);
2021

2122
sub can ($c, $arg) {
2223
if ($arg eq 'info') {
@@ -119,17 +120,24 @@ sub nav ($c, $args) {
119120
# Don't show the nav if the user does not have unrestricted navigation permissions.
120121
return '' unless $c->authz->hasPermissions($c->param('user'), 'navigation_allowed');
121122

122-
my @links = (
123-
$c->maketext('Assignments'),
124-
$c->url_for($c->app->routes->lookup($c->current_route)->parent->name),
125-
$c->maketext('Assignments')
126-
);
127123
return $c->tag(
128124
'div',
129125
class => 'row sticky-nav',
130126
role => 'navigation',
131-
'aria-label' => 'problem navigation',
132-
$c->tag('div', $c->navMacro($args, {}, @links))
127+
'aria-label' => 'set navigation',
128+
$c->c(
129+
$c->tag(
130+
'div',
131+
class => 'd-flex submit-buttons-container',
132+
$c->navMacro(
133+
$args, {},
134+
$c->maketext('Assignments'),
135+
$c->url_for($c->app->routes->lookup($c->current_route)->parent->name),
136+
$c->maketext('Assignments')
137+
)
138+
),
139+
$c->{set} ? studentNav($c, $c->{set}->set_id) : ''
140+
)->join('')
133141
);
134142
}
135143

‎lib/WeBWorK/HTML/StudentNav.pm‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package WeBWorK::HTML::StudentNav;
2+
use Mojo::Base 'Exporter', -signatures;
3+
4+
=head1 NAME
5+
6+
WeBWorK::HTML::StudentNav - student navigation for all users assigned to a set.
7+
8+
=cut
9+
10+
our @EXPORT_OK = qw(studentNav);
11+
12+
sub studentNav ($c, $setID) {
13+
my $userID = $c->param('user');
14+
15+
return '' unless $c->authz->hasPermissions($userID, 'become_student');
16+
17+
# Find all users for the given set (except the current user) sorted by last_name, then first_name, then user_id.
18+
my @allUserRecords = $c->db->getUsersWhere(
19+
{
20+
user_id =>
21+
[ map { $_->[0] } $c->db->listUserSetsWhere({ set_id => $setID, user_id => { '!=' => $userID } }) ]
22+
},
23+
[qw/last_name first_name user_id/]
24+
);
25+
26+
return '' unless @allUserRecords;
27+
28+
my $eUserID = $c->param('effectiveUser');
29+
30+
my $filter = $c->param('studentNavFilter');
31+
32+
# Find the previous, current, and next users, and format the student names for display.
33+
# Also create a hash of sections and recitations if there are any for the course.
34+
my @userRecords;
35+
my $currentUserIndex = 0;
36+
my %filters;
37+
for (@allUserRecords) {
38+
# Add to the sections and recitations if defined. Also store the first user found in that section or
39+
# recitation. This user will be switched to when the filter is selected.
40+
my $section = $_->section;
41+
$filters{"section:$section"} = [ $c->maketext('Filter by section [_1]', $section), $_->user_id ]
42+
if $section && !$filters{"section:$section"};
43+
my $recitation = $_->recitation;
44+
$filters{"recitation:$recitation"} = [ $c->maketext('Filter by recitation [_1]', $recitation), $_->user_id ]
45+
if $recitation && !$filters{"recitation:$recitation"};
46+
47+
# Only keep this user if it satisfies the selected filter if a filter was selected.
48+
next
49+
unless !$filter
50+
|| ($filter =~ /^section:(.*)$/ && $_->section eq $1)
51+
|| ($filter =~ /^recitation:(.*)$/ && $_->recitation eq $1);
52+
53+
my $addRecord = $_;
54+
$currentUserIndex = @userRecords if $addRecord->user_id eq $eUserID;
55+
push @userRecords, $addRecord;
56+
57+
# Construct a display name.
58+
$addRecord->{displayName} =
59+
($addRecord->last_name || $addRecord->first_name
60+
? $addRecord->last_name . ', ' . $addRecord->first_name
61+
: $addRecord->user_id);
62+
}
63+
my $prevUser = $currentUserIndex > 0 ? $userRecords[ $currentUserIndex - 1 ] : 0;
64+
my $nextUser = $currentUserIndex < $#userRecords ? $userRecords[ $currentUserIndex + 1 ] : 0;
65+
66+
# Mark the current user.
67+
$userRecords[$currentUserIndex]{currentUser} = 1;
68+
69+
# Set up the student nav.
70+
return $c->include(
71+
'HTML/StudentNav/student_nav',
72+
userID => $userID,
73+
eUserID => $eUserID,
74+
userRecords => \@userRecords,
75+
currentUserIndex => $currentUserIndex,
76+
prevUser => $prevUser,
77+
nextUser => $nextUser,
78+
filter => $filter,
79+
filters => \%filters
80+
);
81+
}
82+
83+
1;

0 commit comments

Comments
 (0)