Skip to content

Commit 3567f60

Browse files
drgrice1drgrice1
authored
Merge pull request #2913 from somiaj/remove-student-stats
Remove student statistics.
2 parents c62f680 + 4b24839 commit 3567f60

13 files changed

Lines changed: 106 additions & 146 deletions

File tree

‎lib/WeBWorK/ContentGenerator/Instructor/Index.pm‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,6 @@ sub pre_header_initialize ($c) {
8080
} else {
8181
push @error, E_ONE_SET;
8282
}
83-
} elsif (defined $c->param('user_stats')) {
84-
if ($nusers == 1) {
85-
$route = 'instructor_user_statistics';
86-
$args{userID} = $firstUserID;
87-
} else {
88-
push @error, E_ONE_USER;
89-
}
9083
} elsif (defined $c->param('set_stats')) {
9184
if ($nsets == 1) {
9285
$route = 'instructor_set_statistics';

‎lib/WeBWorK/ContentGenerator/Instructor/Stats.pm‎

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,7 @@ sub initialize ($c) {
2222
# Check permissions
2323
return unless $c->authz->hasPermissions($user, 'access_instructor_tools');
2424

25-
# Cache a list of all users except set level proctors and practice users, and restrict to the sections or
26-
# recitations that are allowed for the user if such restrictions are defined. This list is sorted by last_name,
27-
# then first_name, then user_id. This is used in multiple places in this module, and is guaranteed to be used at
28-
# least once. So it is done here to prevent extra database access.
29-
$c->{student_records} = [
30-
$db->getUsersWhere(
31-
{
32-
user_id => [ -and => { not_like => 'set_id:%' }, { not_like => "$ce->{practiceUserPrefix}\%" } ],
33-
$ce->{viewable_sections}{$user} || $ce->{viewable_recitations}{$user}
34-
? (
35-
-or => [
36-
$ce->{viewable_sections}{$user} ? (section => $ce->{viewable_sections}{$user}) : (),
37-
$ce->{viewable_recitations}{$user} ? (recitation => $ce->{viewable_recitations}{$user}) : ()
38-
]
39-
)
40-
: ()
41-
},
42-
[qw/last_name first_name user_id/]
43-
)
44-
];
45-
46-
if ($c->current_route eq 'instructor_user_statistics') {
47-
$c->{studentID} = $c->stash('userID');
48-
} elsif ($c->current_route =~ /^instructor_(set|problem)_statistics$/) {
25+
if ($c->current_route =~ /^instructor_(set|problem)_statistics$/) {
4926
my $setRecord = $db->getGlobalSet($c->stash('setID'));
5027
return unless $setRecord;
5128
$c->{setRecord} = $setRecord;
@@ -57,6 +34,30 @@ sub initialize ($c) {
5734
return unless $problemRecord;
5835
$c->{problemRecord} = $problemRecord;
5936
}
37+
38+
# Cache a list of all users except set level proctors and practice users, and restrict to the sections
39+
# or recitations that are allowed for the user if such restrictions are defined. This list is sorted by
40+
# last_name, then first_name, then user_id. This is used in multiple places in this module, and is used
41+
# on every page except the main page, so it is done here to prevent extra database access.
42+
$c->{student_records} = [
43+
$db->getUsersWhere(
44+
{
45+
user_id =>
46+
[ -and => { not_like => 'set_id:%' }, { not_like => "$ce->{practiceUserPrefix}\%" } ],
47+
$ce->{viewable_sections}{$user} || $ce->{viewable_recitations}{$user}
48+
? (
49+
-or => [
50+
$ce->{viewable_sections}{$user} ? (section => $ce->{viewable_sections}{$user}) : (),
51+
$ce->{viewable_recitations}{$user}
52+
? (recitation => $ce->{viewable_recitations}{$user})
53+
: ()
54+
]
55+
)
56+
: ()
57+
},
58+
[qw/last_name first_name user_id/]
59+
)
60+
];
6061
}
6162

6263
return;
@@ -67,9 +68,7 @@ sub page_title ($c) {
6768

6869
my $setID = $c->stash('setID') || '';
6970

70-
if ($c->current_route eq 'instructor_user_statistics') {
71-
return $c->maketext('Statistics for student [_1]', $c->{studentID});
72-
} elsif ($c->current_route eq 'instructor_set_statistics') {
71+
if ($c->current_route eq 'instructor_set_statistics') {
7372
return $c->maketext('Statistics for [_1]', $c->tag('span', dir => 'ltr', format_set_name_display($setID)));
7473
} elsif ($c->current_route eq 'instructor_problem_statistics') {
7574
return $c->maketext(
@@ -79,12 +78,11 @@ sub page_title ($c) {
7978
);
8079
}
8180

82-
return $c->maketext('Statistics');
81+
return $c->maketext('Set Statistics');
8382
}
8483

8584
sub siblings ($c) {
86-
# Stats and StudentProgress share this template.
87-
return $c->include('ContentGenerator/Instructor/Stats/siblings', header => $c->maketext('Statistics'));
85+
return $c->include('ContentGenerator/Instructor/Stats/siblings');
8886
}
8987

9088
# Apply the currently selected filter to the student records, and return a reference to the

‎lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ sub page_title ($c) {
6868
}
6969

7070
sub siblings ($c) {
71-
# Stats and StudentProgress share this template.
72-
return $c->include('ContentGenerator/Instructor/Stats/siblings', header => $c->maketext('Student Progress'));
71+
return $c->include('ContentGenerator/Instructor/StudentProgress/siblings');
7372
}
7473

7574
# Display student progress table

‎lib/WeBWorK/Utils/Routes.pm‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ PLEASE FOR THE LOVE OF GOD UPDATE THIS IF YOU CHANGE THE ROUTES BELOW!!!
8787
instructor_statistics /$courseID/instructor/stats
8888
instructor_set_statistics /$courseID/instructor/stats/set/$setID
8989
instructor_problem_statistics /$courseID/instructor/stats/set/$setID/$problemID
90-
instructor_user_statistics /$courseID/instructor/stats/student/$userID
9190
9291
instructor_progress /$courseID/instructor/progress
9392
instructor_set_progress /$courseID/instructor/progress/set/$setID
@@ -491,7 +490,7 @@ my %routeParameters = (
491490
},
492491
instructor_statistics => {
493492
title => x('Statistics'),
494-
children => [qw(instructor_set_statistics instructor_user_statistics)],
493+
children => [qw(instructor_set_statistics)],
495494
module => 'Instructor::Stats',
496495
path => '/stats'
497496
},
@@ -506,11 +505,6 @@ my %routeParameters = (
506505
module => 'Instructor::Stats',
507506
path => '/<problemID:num>'
508507
},
509-
instructor_user_statistics => {
510-
title => '[_1]',
511-
module => 'Instructor::Stats',
512-
path => '/student/#userID'
513-
},
514508
instructor_progress => {
515509
title => x('Student Progress'),
516510
children => [qw(instructor_set_progress instructor_user_progress)],

‎templates/ContentGenerator/Base/links.html.ep‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -221,31 +221,8 @@
221221
% # Statistics
222222
<li class="list-group-item nav-item">
223223
<%= $makelink->('instructor_statistics') %>
224-
% if ($userID ne $eUserID || defined $setID || defined $urlUserID) {
225-
<ul class="nav flex-column">
226-
% if (defined $urlUserID) {
227-
<li class="nav-item">
228-
<%= $makelink->(
229-
'instructor_user_statistics',
230-
text => $urlUserID,
231-
captures => { userID => $urlUserID },
232-
link_attrs => { dir => 'ltr' }
233-
) %>
234-
</li>
235-
% }
236-
% if ($userID ne $eUserID && (!defined $urlUserID || $urlUserID ne $eUserID)) {
237-
<li class="nav-item">
238-
<%= $makelink->(
239-
'instructor_user_statistics',
240-
text => $eUserID,
241-
captures => { userID => $eUserID },
242-
active => current_route eq 'instructor_user_statistics'
243-
&& !defined $urlUserID,
244-
link_attrs => { dir => 'ltr' }
245-
) %>
246-
</li>
247-
% }
248-
% if (defined $setID) {
224+
% if (defined $setID) {
225+
<ul class="nav flex-column">
249226
<li class="nav-item" dir="ltr">
250227
<%= $makelink->(
251228
'instructor_set_statistics',
@@ -270,8 +247,7 @@
270247
</ul>
271248
</li>
272249
% }
273-
% }
274-
</ul>
250+
</ul>
275251
% }
276252
</li>
277253
% # Student Progress

‎templates/ContentGenerator/Instructor/Stats.html.ep‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
11
% use WeBWorK::Utils qw(getAssetURL);
2+
% use WeBWorK::Utils::Sets qw(format_set_name_display);
23
%
34
% unless ($authz->hasPermissions(param('user'), 'access_instructor_tools')) {
45
<div class="alert alert-danger p-1"><%= maketext('You are not authorized to access instructor tools') %></div>
56
% last;
67
% }
78
%
8-
% if (current_route eq 'instructor_user_statistics') {
9-
% # Stats and StudentProgress share this template.
10-
<%= include 'ContentGenerator/Instructor/Stats/student_stats' =%>
11-
% } elsif (current_route eq 'instructor_set_statistics') {
9+
% if (current_route eq 'instructor_set_statistics') {
1210
<%= $c->set_stats =%>
1311
% } elsif (current_route eq 'instructor_problem_statistics') {
1412
<%= $c->problem_stats =%>
1513
% } else {
16-
% # Stats and StudentProgress share this template also.
17-
<%= include 'ContentGenerator/Instructor/Stats/index',
18-
set_header => maketext('View statistics by set'),
19-
student_header => maketext('View statistics by student') =%>
14+
<ul dir="ltr">
15+
% for ($db->listGlobalSetsWhere({}, 'set_id')) {
16+
<li>
17+
<%= link_to format_set_name_display($_->[0]) =>
18+
$c->systemLink(url_for("instructor_set_statistics", setID => $_->[0])) =%>
19+
</li>
20+
% }
21+
</ul>
2022
% }

‎templates/ContentGenerator/Instructor/Stats/siblings.html.ep‎

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
% # Note that this template is used by both WeBWorK::ContentGenerator::Instructor::Stats and
2-
% # WeBWorK::ContentGenerator::Instructor::StudentProgress.
3-
%
41
% use WeBWorK::Utils::JITAR qw(jitar_id_to_seq);
52
% use WeBWorK::Utils::Sets qw(format_set_name_display);
63
%
@@ -9,24 +6,8 @@
96
% }
107
%
118
<div class="info-box bg-light">
12-
<h2><%= $header %></h2>
13-
% if (current_route =~ /^instructor_user_/) {
14-
<ul class="nav flex-column problem-list">
15-
% for (@{ $c->{student_records} }) {
16-
<li class="nav-item">
17-
<%= tag 'a',
18-
$_->user_id eq $c->{studentID}
19-
? (class => 'nav-link active')
20-
: (
21-
href => $c->systemLink(url_for(current_route, userID => $_->user_id)),
22-
class => 'nav-link'
23-
), begin =%>
24-
<%= $_->last_name =%>, <%= $_->first_name %> (<%= $_->user_id %>)
25-
<% end =%>
26-
</li>
27-
% }
28-
</ul>
29-
% } elsif (current_route eq 'instructor_problem_statistics') {
9+
<h2><%= maketext('Statistics') %></h2>
10+
% if (current_route eq 'instructor_problem_statistics') {
3011
<ul class="nav flex-column problem-list">
3112
% for (map { $_->[1] } $db->listGlobalProblemsWhere({ set_id => stash->{setID} }, 'problem_id')) {
3213
<li class="nav-item">
@@ -46,22 +27,8 @@
4627
% }
4728
</ul>
4829
% } else {
49-
<ul class="nav flex-column problem-list" dir="ltr">
50-
% for (map { $_->[0] } $db->listGlobalSetsWhere({}, 'set_id')) {
51-
<li class="nav-item">
52-
<%= tag 'a',
53-
defined stash('setID') && $_ eq stash('setID') ? (class => 'nav-link active')
54-
: (
55-
href => $c->systemLink(
56-
url_for(current_route =~ s/instructor_(progress|statistics)/instructor_set_$1/r,
57-
setID => $_),
58-
params => param('filter') ? { filter => param('filter') } : {}
59-
),
60-
class => 'nav-link'
61-
),
62-
format_set_name_display($_) =%>
63-
</li>
64-
% }
65-
</ul>
30+
% # Shared with StudentProgress siblings.
31+
<%= include 'ContentGenerator/Instructor/Stats/siblings_set_list',
32+
set_link_route => 'instructor_set_statistics' =%>
6633
% }
6734
</div>

‎templates/ContentGenerator/Instructor/Stats/siblings_set_list.html.ep‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
% # This is shared between Stats and StudentProgress siblings page to list all sets.
2+
%
3+
<ul class="nav flex-column problem-list" dir="ltr">
4+
% for (map { $_->[0] } $db->listGlobalSetsWhere({}, 'set_id')) {
5+
<li class="nav-item">
6+
<%= tag 'a',
7+
defined stash('setID') && $_ eq stash('setID') ? (class => 'nav-link active')
8+
: (
9+
href => $c->systemLink(
10+
url_for($set_link_route, setID => $_),
11+
params => param('filter') ? { filter => param('filter') } : {}
12+
),
13+
class => 'nav-link'
14+
),
15+
format_set_name_display($_) =%>
16+
</li>
17+
% }
18+
</ul>

‎templates/ContentGenerator/Instructor/StudentProgress.html.ep‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,9 @@
44
% }
55
%
66
% if (current_route eq 'instructor_user_progress') {
7-
% # Stats and StudentProgress share this template.
8-
<%= include 'ContentGenerator/Instructor/Stats/student_stats' =%>
7+
<%= include 'ContentGenerator/Instructor/StudentProgress/student_progress' =%>
98
% } elsif (current_route eq 'instructor_set_progress') {
109
<%= $c->displaySets =%>
1110
% } else {
12-
% # Stats and StudentProgress share this template also.
13-
<%= include 'ContentGenerator/Instructor/Stats/index',
14-
set_header => maketext('View student progress by set'),
15-
student_header => maketext('View student progress by student') =%>
11+
<%= include 'ContentGenerator/Instructor/StudentProgress/index' =%>
1612
% }

templates/ContentGenerator/Instructor/Stats/index.html.ep renamed to templates/ContentGenerator/Instructor/StudentProgress/index.html.ep

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,17 @@
1-
% # Note that this template is used by both WeBWorK::ContentGenerator::Instructor::Stats and
2-
% # WeBWorK::ContentGenerator::Instructor::StudentProgress.
3-
%
41
% use WeBWorK::Utils::Sets qw(format_set_name_display);
52
%
6-
% my $type = current_route =~ s/instructor_//r;
7-
%
83
% stash->{footerWidthClass} = 'col-lg-10 col-sm-12';
94
%
105
<div class="row">
116
<div class="col-lg-5 col-sm-6 mb-2">
127
<div class="card h-100">
138
<div class="card-body p-2">
14-
<h2 class="card-title text-center fs-3"><%= $set_header %></h2>
9+
<h2 class="card-title text-center fs-3"><%= maketext('View student progress by set') %></h2>
1510
<ul dir="ltr">
1611
% for ($db->listGlobalSetsWhere({}, 'set_id')) {
1712
<li>
1813
<%= link_to format_set_name_display($_->[0]) =>
19-
$c->systemLink(url_for("instructor_set_$type", setID => $_->[0])) =%>
14+
$c->systemLink(url_for("instructor_set_progress", setID => $_->[0])) =%>
2015
</li>
2116
% }
2217
</ul>
@@ -26,12 +21,12 @@
2621
<div class="col-lg-5 col-sm-6 mb-2">
2722
<div class="card h-100">
2823
<div class="card-body p-2">
29-
<h2 class="card-title text-center fs-3"><%= $student_header %></h2>
24+
<h2 class="card-title text-center fs-3"><%= maketext('View student progress by student') %></h2>
3025
<ul>
3126
% for (@{ $c->{student_records} }) {
3227
<li>
3328
<%= link_to $_->last_name . ', ' . $_->first_name . ' (' . $_->user_id . ')' =>
34-
$c->systemLink(url_for("instructor_user_$type", userID => $_->user_id)) =%>
29+
$c->systemLink(url_for("instructor_user_progress", userID => $_->user_id)) =%>
3530
</li>
3631
% }
3732
</ul>

0 commit comments

Comments
 (0)