From a7378bfad647b4173550c1fce85159450bb2aaf1 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 14 Jan 2026 21:17:09 -0600 Subject: [PATCH] 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 #2875 since it would conflict rather heavily with that pull request if it were not. --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 139 +++++++++--------- lib/WeBWorK/ContentGenerator/Problem.pm | 73 +-------- lib/WeBWorK/ContentGenerator/ProblemSet.pm | 22 ++- lib/WeBWorK/HTML/StudentNav.pm | 83 +++++++++++ .../ContentGenerator/GatewayQuiz/nav.html.ep | 88 ++++++----- .../StudentNav}/student_nav.html.ep | 84 ++++++----- 6 files changed, 271 insertions(+), 218 deletions(-) create mode 100644 lib/WeBWorK/HTML/StudentNav.pm rename templates/{ContentGenerator/Problem => HTML/StudentNav}/student_nav.html.ep (52%) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index e4088251ce..147d4bda71 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1340,7 +1340,7 @@ sub path ($c, $args) { $courseName => $navigation_allowed ? $c->url_for('set_list') : '', $setID eq 'Undefined_Set' || $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation} - ? ($setID => '') + ? ($setID =~ /^(.+),(v\d+)$/ ? ($1 => $c->url_for('problem_list', setID => $1), $2 => '') : ($setID => '')) : ( $c->{set}->set_id => $c->url_for('problem_list', setID => $c->{set}->set_id), 'v' . $c->{set}->version_id => '' @@ -1356,7 +1356,7 @@ sub nav ($c, $args) { return '' if $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation}; # Set up and display a student navigation for those that have permission to act as a student. - if ($c->authz->hasPermissions($userID, 'become_student') && $effectiveUserID ne $userID) { + if ($c->authz->hasPermissions($userID, 'become_student')) { my $setID = $c->{set}->set_id; return '' if $setID eq 'Undefined_Set'; @@ -1365,76 +1365,83 @@ sub nav ($c, $args) { # Find all versions of this set that have been taken (excluding those taken by the current user). my @users = - $db->listSetVersionsWhere({ user_id => { not_like => $userID }, set_id => { like => "$setID,v\%" } }); + $db->listSetVersionsWhere({ user_id => { '!=' => $userID }, set_id => { like => "$setID,v\%" } }); my @allUserRecords = $db->getUsers(map { $_->[0] } @users); - my $filter = $c->param('studentNavFilter'); - - # Format the student names for display, and associate the users with the test versions. - my %filters; - my @userRecords; - for (0 .. $#allUserRecords) { - # Add to the sections and recitations if defined. Also store the first user found in that section or - # recitation. This user will be switched to when the filter is selected. - my $section = $allUserRecords[$_]->section; - $filters{"section:$section"} = - [ $c->maketext('Filter by section [_1]', $section), $allUserRecords[$_]->user_id, $users[$_][2] ] - if $section && !$filters{"section:$section"}; - my $recitation = $allUserRecords[$_]->recitation; - $filters{"recitation:$recitation"} = - [ $c->maketext('Filter by recitation [_1]', $recitation), $allUserRecords[$_]->user_id, $users[$_][2] ] - if $recitation && !$filters{"recitation:$recitation"}; - - # Only keep this user if it satisfies the selected filter if a filter was selected. - next - unless !$filter - || ($filter =~ /^section:(.*)$/ && $allUserRecords[$_]->section eq $1) - || ($filter =~ /^recitation:(.*)$/ && $allUserRecords[$_]->recitation eq $1); - - my $addRecord = $allUserRecords[$_]; - push @userRecords, $addRecord; - - $addRecord->{displayName} = - ($addRecord->last_name || $addRecord->first_name - ? $addRecord->last_name . ', ' . $addRecord->first_name - : $addRecord->user_id); - $addRecord->{setVersion} = $users[$_][2]; - } + if (@allUserRecords) { + my $filter = $c->param('studentNavFilter'); + + # Format the student names for display, and associate the users with the test versions. + my %filters; + my @userRecords; + for (0 .. $#allUserRecords) { + # Add to the sections and recitations if defined. Also store the first user found in that section or + # recitation. This user will be switched to when the filter is selected. + my $section = $allUserRecords[$_]->section; + $filters{"section:$section"} = + [ $c->maketext('Filter by section [_1]', $section), $allUserRecords[$_]->user_id, $users[$_][2] ] + if $section && !$filters{"section:$section"}; + my $recitation = $allUserRecords[$_]->recitation; + $filters{"recitation:$recitation"} = [ + $c->maketext('Filter by recitation [_1]', $recitation), $allUserRecords[$_]->user_id, + $users[$_][2] + ] + if $recitation && !$filters{"recitation:$recitation"}; + + # Only keep this user if it satisfies the selected filter if a filter was selected. + next + unless !$filter + || ($filter =~ /^section:(.*)$/ && $allUserRecords[$_]->section eq $1) + || ($filter =~ /^recitation:(.*)$/ && $allUserRecords[$_]->recitation eq $1); + + my $addRecord = $allUserRecords[$_]; + push @userRecords, $addRecord; + + $addRecord->{displayName} = + ($addRecord->last_name || $addRecord->first_name + ? $addRecord->last_name . ', ' . $addRecord->first_name + : $addRecord->user_id); + $addRecord->{setVersion} = $users[$_][2]; + } - # Sort by last name, then first name, then user_id, then set version. - @userRecords = sort { - lc($a->last_name) cmp lc($b->last_name) - || lc($a->first_name) cmp lc($b->first_name) - || lc($a->user_id) cmp lc($b->user_id) - || lc($a->{setVersion}) <=> lc($b->{setVersion}) - } @userRecords; - - # Find the previous, current, and next test. - my $currentTestIndex = 0; - for (0 .. $#userRecords) { - if ($userRecords[$_]->user_id eq $effectiveUserID && $userRecords[$_]->{setVersion} == $setVersion) { - $currentTestIndex = $_; - last; + # Sort by last name, then first name, then user_id, then set version. + @userRecords = sort { + lc($a->last_name) cmp lc($b->last_name) + || lc($a->first_name) cmp lc($b->first_name) + || lc($a->user_id) cmp lc($b->user_id) + || lc($a->{setVersion}) <=> lc($b->{setVersion}) + } @userRecords; + + # Find the previous, current, and next test. + my $currentTestIndex = 0; + for (0 .. $#userRecords) { + if ($userRecords[$_]->user_id eq $effectiveUserID && $userRecords[$_]->{setVersion} == $setVersion) { + $currentTestIndex = $_; + last; + } } + my $prevTest = $currentTestIndex > 0 ? $userRecords[ $currentTestIndex - 1 ] : 0; + my $nextTest = $currentTestIndex < $#userRecords ? $userRecords[ $currentTestIndex + 1 ] : 0; + + # Mark the current test. + $userRecords[$currentTestIndex]{currentTest} = 1; + + # Show the student nav. + return $c->include( + 'ContentGenerator/GatewayQuiz/nav', + userID => $userID, + eUserID => $effectiveUserID, + userRecords => \@userRecords, + setVersion => $setVersion, + prevTest => $prevTest, + nextTest => $nextTest, + currentTestIndex => $currentTestIndex, + filters => \%filters, + filter => $filter + ); } - my $prevTest = $currentTestIndex > 0 ? $userRecords[ $currentTestIndex - 1 ] : 0; - my $nextTest = $currentTestIndex < $#userRecords ? $userRecords[ $currentTestIndex + 1 ] : 0; - - # Mark the current test. - $userRecords[$currentTestIndex]{currentTest} = 1; - - # Show the student nav. - return $c->include( - 'ContentGenerator/GatewayQuiz/nav', - userRecords => \@userRecords, - setVersion => $setVersion, - prevTest => $prevTest, - nextTest => $nextTest, - currentTestIndex => $currentTestIndex, - filters => \%filters, - filter => $filter - ); } + return ''; } sub warningMessage ($c) { diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index 0711ecf0ca..9e0c5f92f7 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -7,7 +7,6 @@ WeBWorK::ContentGenerator::Problem - Allow a student to interact with a problem. =cut -use WeBWorK::HTML::SingleProblemGrader; use WeBWorK::Debug; use WeBWorK::Utils qw(decodeAnswers wwRound); use WeBWorK::Utils::DateTime qw(before between after); @@ -23,6 +22,8 @@ use WeBWorK::AchievementEvaluator qw(checkForAchievements); use WeBWorK::DB::Utils qw(global2user fake_set fake_problem); use WeBWorK::Localize; use WeBWorK::AchievementEvaluator; +use WeBWorK::HTML::SingleProblemGrader; +use WeBWorK::HTML::StudentNav qw(studentNav); # GET/POST Parameters for this module # @@ -839,74 +840,6 @@ sub nav ($c, $args) { my $mergedSet = $db->getMergedSet($eUserID, $setID); return '' if !$mergedSet; - # Set up a student navigation for those that have permission to act as a student. - my $userNav = ''; - if ($authz->hasPermissions($userID, 'become_student') && $eUserID ne $userID) { - # Find all users for this set (except the current user) sorted by last_name, then first_name, then user_id. - my @allUserRecords = $db->getUsersWhere( - { - user_id => [ - map { $_->[0] } $db->listUserSetsWhere({ set_id => $setID, user_id => { not_like => $userID } }) - ] - }, - [qw/last_name first_name user_id/] - ); - - my $filter = $c->param('studentNavFilter'); - - # Find the previous, current, and next users, and format the student names for display. - # Also create a hash of sections and recitations if there are any for the course. - my @userRecords; - my $currentUserIndex = 0; - my %filters; - for (@allUserRecords) { - # Add to the sections and recitations if defined. Also store the first user found in that section or - # recitation. This user will be switched to when the filter is selected. - my $section = $_->section; - $filters{"section:$section"} = [ $c->maketext('Filter by section [_1]', $section), $_->user_id ] - if $section && !$filters{"section:$section"}; - my $recitation = $_->recitation; - $filters{"recitation:$recitation"} = [ $c->maketext('Filter by recitation [_1]', $recitation), $_->user_id ] - if $recitation && !$filters{"recitation:$recitation"}; - - # Only keep this user if it satisfies the selected filter if a filter was selected. - next - unless !$filter - || ($filter =~ /^section:(.*)$/ && $_->section eq $1) - || ($filter =~ /^recitation:(.*)$/ && $_->recitation eq $1); - - my $addRecord = $_; - $currentUserIndex = @userRecords if $addRecord->user_id eq $eUserID; - push @userRecords, $addRecord; - - # Construct a display name. - $addRecord->{displayName} = - ($addRecord->last_name || $addRecord->first_name - ? $addRecord->last_name . ', ' . $addRecord->first_name - : $addRecord->user_id); - } - my $prevUser = $currentUserIndex > 0 ? $userRecords[ $currentUserIndex - 1 ] : 0; - my $nextUser = $currentUserIndex < $#userRecords ? $userRecords[ $currentUserIndex + 1 ] : 0; - - # Mark the current user. - $userRecords[$currentUserIndex]{currentUser} = 1; - - my $problemPage = $c->url_for('problem_detail', setID => $setID, problemID => $problemID); - - # Set up the student nav. - $userNav = $c->include( - 'ContentGenerator/Problem/student_nav', - eUserID => $eUserID, - problemPage => $problemPage, - userRecords => \@userRecords, - currentUserIndex => $currentUserIndex, - prevUser => $prevUser, - nextUser => $nextUser, - filter => $filter, - filters => \%filters - ); - } - my $isJitarSet = $mergedSet->assignment_type eq 'jitar'; my ($prevID, $nextID); @@ -977,7 +910,7 @@ sub nav ($c, $args) { role => 'navigation', 'aria-label' => 'problem navigation', $c->c($c->tag('div', class => 'd-flex submit-buttons-container', $c->navMacro($args, \%tail, @links)), - $userNav)->join('') + studentNav($c, $setID))->join('') ); } diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm index ba77939ec5..7eebaeb6de 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSet.pm @@ -17,6 +17,7 @@ use WeBWorK::Utils::Sets qw(is_restricted grade_set format_set_name_display use WeBWorK::DB::Utils qw(grok_versionID_from_vsetID_sql); use WeBWorK::Localize; use WeBWorK::AchievementItems; +use WeBWorK::HTML::StudentNav qw(studentNav); async sub initialize ($c) { my $db = $c->db; @@ -113,17 +114,24 @@ sub nav ($c, $args) { # Don't show the nav if the user does not have unrestricted navigation permissions. return '' unless $c->authz->hasPermissions($c->param('user'), 'navigation_allowed'); - my @links = ( - $c->maketext('Assignments'), - $c->url_for($c->app->routes->lookup($c->current_route)->parent->name), - $c->maketext('Assignments') - ); return $c->tag( 'div', class => 'row sticky-nav', role => 'navigation', - 'aria-label' => 'problem navigation', - $c->tag('div', $c->navMacro($args, {}, @links)) + 'aria-label' => 'set navigation', + $c->c( + $c->tag( + 'div', + class => 'd-flex submit-buttons-container', + $c->navMacro( + $args, {}, + $c->maketext('Assignments'), + $c->url_for($c->app->routes->lookup($c->current_route)->parent->name), + $c->maketext('Assignments') + ) + ), + $c->{set} ? studentNav($c, $c->{set}->set_id) : '' + )->join('') ); } diff --git a/lib/WeBWorK/HTML/StudentNav.pm b/lib/WeBWorK/HTML/StudentNav.pm new file mode 100644 index 0000000000..7591712a8d --- /dev/null +++ b/lib/WeBWorK/HTML/StudentNav.pm @@ -0,0 +1,83 @@ +package WeBWorK::HTML::StudentNav; +use Mojo::Base 'Exporter', -signatures; + +=head1 NAME + +WeBWorK::HTML::StudentNav - student navigation for all users assigned to a set. + +=cut + +our @EXPORT_OK = qw(studentNav); + +sub studentNav ($c, $setID) { + my $userID = $c->param('user'); + + return '' unless $c->authz->hasPermissions($userID, 'become_student'); + + # Find all users for the given set (except the current user) sorted by last_name, then first_name, then user_id. + my @allUserRecords = $c->db->getUsersWhere( + { + user_id => + [ map { $_->[0] } $c->db->listUserSetsWhere({ set_id => $setID, user_id => { '!=' => $userID } }) ] + }, + [qw/last_name first_name user_id/] + ); + + return '' unless @allUserRecords; + + my $eUserID = $c->param('effectiveUser'); + + my $filter = $c->param('studentNavFilter'); + + # Find the previous, current, and next users, and format the student names for display. + # Also create a hash of sections and recitations if there are any for the course. + my @userRecords; + my $currentUserIndex = 0; + my %filters; + for (@allUserRecords) { + # Add to the sections and recitations if defined. Also store the first user found in that section or + # recitation. This user will be switched to when the filter is selected. + my $section = $_->section; + $filters{"section:$section"} = [ $c->maketext('Filter by section [_1]', $section), $_->user_id ] + if $section && !$filters{"section:$section"}; + my $recitation = $_->recitation; + $filters{"recitation:$recitation"} = [ $c->maketext('Filter by recitation [_1]', $recitation), $_->user_id ] + if $recitation && !$filters{"recitation:$recitation"}; + + # Only keep this user if it satisfies the selected filter if a filter was selected. + next + unless !$filter + || ($filter =~ /^section:(.*)$/ && $_->section eq $1) + || ($filter =~ /^recitation:(.*)$/ && $_->recitation eq $1); + + my $addRecord = $_; + $currentUserIndex = @userRecords if $addRecord->user_id eq $eUserID; + push @userRecords, $addRecord; + + # Construct a display name. + $addRecord->{displayName} = + ($addRecord->last_name || $addRecord->first_name + ? $addRecord->last_name . ', ' . $addRecord->first_name + : $addRecord->user_id); + } + my $prevUser = $currentUserIndex > 0 ? $userRecords[ $currentUserIndex - 1 ] : 0; + my $nextUser = $currentUserIndex < $#userRecords ? $userRecords[ $currentUserIndex + 1 ] : 0; + + # Mark the current user. + $userRecords[$currentUserIndex]{currentUser} = 1; + + # Set up the student nav. + return $c->include( + 'HTML/StudentNav/student_nav', + userID => $userID, + eUserID => $eUserID, + userRecords => \@userRecords, + currentUserIndex => $currentUserIndex, + prevUser => $prevUser, + nextUser => $nextUser, + filter => $filter, + filters => \%filters + ); +} + +1; diff --git a/templates/ContentGenerator/GatewayQuiz/nav.html.ep b/templates/ContentGenerator/GatewayQuiz/nav.html.ep index 51eb9de764..3a3f7aca81 100644 --- a/templates/ContentGenerator/GatewayQuiz/nav.html.ep +++ b/templates/ContentGenerator/GatewayQuiz/nav.html.ep @@ -9,27 +9,36 @@