Skip to content

Commit a7378bf

Browse files
drgrice1drgrice1
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 f7c2fde commit a7378bf

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
@@ -1340,7 +1340,7 @@ sub path ($c, $args) {
13401340
$courseName => $navigation_allowed ? $c->url_for('set_list') : '',
13411341
$setID eq 'Undefined_Set'
13421342
|| $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation}
1343-
? ($setID => '')
1343+
? ($setID =~ /^(.+),(v\d+)$/ ? ($1 => $c->url_for('problem_list', setID => $1), $2 => '') : ($setID => ''))
13441344
: (
13451345
$c->{set}->set_id => $c->url_for('problem_list', setID => $c->{set}->set_id),
13461346
'v' . $c->{set}->version_id => ''
@@ -1356,7 +1356,7 @@ sub nav ($c, $args) {
13561356
return '' if $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation};
13571357

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

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

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

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

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

14401447
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
#
@@ -839,74 +840,6 @@ sub nav ($c, $args) {
839840
my $mergedSet = $db->getMergedSet($eUserID, $setID);
840841
return '' if !$mergedSet;
841842

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

912845
my ($prevID, $nextID);
@@ -977,7 +910,7 @@ sub nav ($c, $args) {
977910
role => 'navigation',
978911
'aria-label' => 'problem navigation',
979912
$c->c($c->tag('div', class => 'd-flex submit-buttons-container', $c->navMacro($args, \%tail, @links)),
980-
$userNav)->join('')
913+
studentNav($c, $setID))->join('')
981914
);
982915
}
983916

‎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
async sub initialize ($c) {
2223
my $db = $c->db;
@@ -113,17 +114,24 @@ sub nav ($c, $args) {
113114
# Don't show the nav if the user does not have unrestricted navigation permissions.
114115
return '' unless $c->authz->hasPermissions($c->param('user'), 'navigation_allowed');
115116

116-
my @links = (
117-
$c->maketext('Assignments'),
118-
$c->url_for($c->app->routes->lookup($c->current_route)->parent->name),
119-
$c->maketext('Assignments')
120-
);
121117
return $c->tag(
122118
'div',
123119
class => 'row sticky-nav',
124120
role => 'navigation',
125-
'aria-label' => 'problem navigation',
126-
$c->tag('div', $c->navMacro($args, {}, @links))
121+
'aria-label' => 'set navigation',
122+
$c->c(
123+
$c->tag(
124+
'div',
125+
class => 'd-flex submit-buttons-container',
126+
$c->navMacro(
127+
$args, {},
128+
$c->maketext('Assignments'),
129+
$c->url_for($c->app->routes->lookup($c->current_route)->parent->name),
130+
$c->maketext('Assignments')
131+
)
132+
),
133+
$c->{set} ? studentNav($c, $c->{set}->set_id) : ''
134+
)->join('')
127135
);
128136
}
129137

‎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)