Merge pull request #2320 from somiaj/student-progress-filter

Filter student progress for single set by section/recitation.

lib/WeBWorK/ContentGenerator/Instructor/Stats.pm

@@ -26,6 +26,7 @@ homework set (including sv graphs).
 use SVG;
 
 use WeBWorK::Utils qw(jitar_id_to_seq jitar_problem_adjusted_status format_set_name_display grade_set);
+use WeBWorK::Utils::FilterRecords qw(getFiltersForClass filterRecords);
 
 sub initialize ($c) {
 	my $db   = $c->db;
@@ -100,39 +101,20 @@ sub siblings ($c) {
 	return $c->include('ContentGenerator/Instructor/Stats/siblings', header => $c->maketext('Statistics'));
 }
 
-# Apply the currently selected filter to the the student records cached in initialize, and return a reference to the
-# list of students and a reference to a hash of section/recitation filters.
+# Apply the currently selected filter to the student records, and return a reference to the
+# list of students and a reference to the array of section/recitation filters.
 sub filter_students ($c) {
-	my $ce   = $c->ce;
-	my $db   = $c->db;
-	my $user = $c->param('user');
+	my $ce       = $c->ce;
+	my $filter   = $c->param('filter') || 'all';
+	my @students = grep { $ce->status_abbrev_has_behavior($_->status, 'include_in_stats') } @{ $c->{student_records} };
 
-	# Create a hash of sections and recitations, if there are any for the course.
-	# Filter out all records except for current/auditing students for stats.
-	# Filter out students not in selected section/recitation.
-	my $filter = $c->param('filter');
-	my %filters;
-	my @outStudents;
-	for my $student (@{ $c->{student_records} }) {
-		# Only include current/auditing students in stats.
-		next
-			unless ($ce->status_abbrev_has_behavior($student->status, 'include_in_stats'));
-
-		my $section = $student->section;
-		$filters{"section:$section"} = $c->maketext('Section [_1]', $section)
-			if $section && !$filters{"section:$section"};
-		my $recitation = $student->recitation;
-		$filters{"recitation:$recitation"} = $c->maketext('Recitation [_1]', $recitation)
-			if $recitation && !$filters{"recitation:$recitation"};
-
-		# Only add users who match the selected section/recitation.
-		push(@outStudents, $student)
-			if (!$filter
-				|| ($filter =~ /^section:(.*)$/    && $section eq $1)
-				|| ($filter =~ /^recitation:(.*)$/ && $recitation eq $1));
-	}
+	# Change visible name of the first 'all' filter.
+	my $filters = getFiltersForClass($c, [ 'section', 'recitation' ], @students);
+	$filters->[0][0] = $c->maketext('All students');
+
+	@students = filterRecords($c, 0, [$filter], @students) unless $filter eq 'all';
 
-	return (\@outStudents, \%filters);
+	return (\@students, $filters);
 }
 
 sub set_stats ($c) {

lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm

@@ -24,6 +24,7 @@ WeBWorK::ContentGenerator::Instructor::StudentProgress - Display Student Progres
 
 use WeBWorK::Utils qw(jitar_id_to_seq wwRound grade_set format_set_name_display);
 use WeBWorK::Utils::Grades qw(list_set_versions);
+use WeBWorK::Utils::FilterRecords qw(getFiltersForClass filterRecords);
 
 sub initialize ($c) {
 	my $db   = $c->db;
@@ -115,12 +116,21 @@ sub displaySets ($c) {
 		: (date => 0, testtime => 0, timeleft => 0, problems => 1, section => 1, recit => 1, login => 1);
 	my $showBestOnly = $setIsVersioned ? $c->param('show_best_only') : 0;
 
+	# Only show students who are included in stats.
+	my @student_records =
+		grep { $ce->status_abbrev_has_behavior($_->status, 'include_in_stats') } @{ $c->{student_records} };
+
+	# Change visible name of the first 'all' filter.
+	my $filter  = $c->param('filter') || 'all';
+	my $filters = getFiltersForClass($c, [ 'section', 'recitation' ], @student_records);
+	$filters->[0][0] = $c->maketext('All students');
+
+	@student_records = filterRecords($c, 0, [$filter], @student_records) unless $filter eq 'all';
+
 	my @score_list;
 	my @user_set_list;
 
-	for my $studentRecord (@{ $c->{student_records} }) {
-		next unless $ce->status_abbrev_has_behavior($studentRecord->status, 'include_in_stats');
-
+	for my $studentRecord (@student_records) {
 		my $studentName = $studentRecord->user_id;
 		my ($allSetVersionNames, $notAssignedSet) =
 			list_set_versions($db, $studentName, $c->stash('setID'), $setIsVersioned);
@@ -277,7 +287,9 @@ sub displaySets ($c) {
 		secondary_sort_method => $secondary_sort_method,
 		ternary_sort_method   => $ternary_sort_method,
 		problems              => \@problems,
-		user_set_list         => \@user_set_list
+		user_set_list         => \@user_set_list,
+		filters               => $filters,
+		filter                => $filter,
 	);
 }
 

lib/WeBWorK/HTML/ScrollingRecordList.pm

@@ -64,7 +64,7 @@ sub scrollingRecordList ($options, @records) {
 		my $format_keywords = join('|', @format_keywords);
 		@$sorts = grep { $_->[0] =~ /$format_keywords/ } @$sorts;
 
-		$filters = getFiltersForClass($c, @records);
+		$filters = getFiltersForClass($c, undef, @records);
 
 		my @selected_filters;
 		if (defined $c->param("$name!filter")) {

lib/WeBWorK/Utils/FilterRecords.pm

@@ -22,26 +22,23 @@ WeBWorK::Utils::FilterRecords - utilities for filtering database records.
 
 =head1 SYNOPSIS
 
-    use WeBWorK::Utils::FilterRecords qw/getFiltersForClass/;
-
-    # Get a list of filters
-    my $filters = getFiltersForClass(@users);
-
-    use WeBWorK::Utils::FilterRecords qw/filterRecords/;
+    use WeBWorK::Utils::FilterRecords qw/getFiltersForClass filterRecords/;
 
     # Start with a list of records
     my @users = $db->getUsers($db->listUsers);
 
-    # Filter the records using a list of provided filters.
-    @filteredUsers = filterRecords([ 'section:1', 'recitation:2' ], @nsers);
+    # Get a list of all filters
+    my $filters = getFiltersForClass($c, undef, @users);
 
-	# Get all records (This isn't useful and just returns the passed in
-	# array of records.  So don't actually do this.)
-    @filteredUsers = filterRecords(undef, @users);
+    # Alternative, get a list of section or recitation filters.
+    my $filters = getFiltersForClass($c, ['section', 'recitation'], @users);
+
+    # Filter the records using a list of provided filters.
+    my @filteredUsers = filterRecords($c, 1, [ 'section:1', 'recitation:2' ], @users);
 
 =head1 DESCRIPTION
 
-This module provides functions for filtering records from the database.
+This module provides functions for filtering user or set records from the database.
 
 =cut
 
@@ -62,27 +59,40 @@ our @EXPORT_OK = qw(
 
 =over
 
-=item getFiltersForClass($c, @records)
+=item getFiltersForClass($c, $include, @records)
+
+Given a list of database records, returns the filters available for those records.
+C<$include> is an array reference that lists the filters to include. If this is
+empty, all possible filters are returned.
 
-Given a list of database records, returns the filters available for those
-records.  For all database records from the WeBWorK::DB::Record::User class
-the filters are by section or recitation or by that user's permission level
-in the permissionLevel table. For all database records from the
-WeBWorK::DB::Record::Set class the filters are by assignment type and by
-visibility. For other classes the only filter is no filter at all.
+For user records (WeBWorK::DB::Record::User), filters can be by section,
+recitation, status, or permission level in the permissionLevel table. The
+possible C<$include> are: 'section', 'recitation', 'status', or 'permission'.
+
+For set records (WeBWorK::DB::Record::Set), filters can be assignment type,
+or visibility. The possible C<$include> are: 'assignment_type', or 'visibility'.
 
 The return value is a reference to a list of two element lists. The first
-element in each list is a a string description of the filter and the second
+element in each list is a string description of the filter and the second
 element is the filter name.  The return value is suitable for passing as the
 second value argument to the Mojolicious select_field tag helper method.
 
 =cut
 
 sub getFiltersForClass {
-	my ($c, @records) = @_;
+	my ($c, $include, @records) = @_;
+	my $blankName = "\x{27E8}" . $c->maketext('blank') . "\x{27E9}";
+
+	my %includes;
+	if (ref $include eq 'ARRAY') {
+		for (@$include) {
+			$includes{$_} = 1;
+		}
+	}
 
 	my @filters;
-	push @filters, [ "\x{27E8}Display all possible records\x{27E9}" => 'all', selected => undef ];
+	push @filters,
+		[ "\x{27E8}" . $c->maketext('Display all possible records') . "\x{27E9}" => 'all', selected => undef ];
 
 	if (ref $records[0] eq 'WeBWorK::DB::Record::User') {
 		my (%sections, %recitations, %permissions, %roles);
@@ -93,35 +103,38 @@ sub getFiltersForClass {
 			++$roles{ $user->status };
 		}
 
-		my %permissionName = reverse %{ $c->ce->{userRoles} };
-		++$permissions{ $permissionName{$_} }
-			for map { $_->permission } $c->db->getPermissionLevelsWhere({ user_id => { not_like => 'set_id:%' } });
+		if (!%includes || $includes{permission}) {
+			my %permissionName = reverse %{ $c->ce->{userRoles} };
+			++$permissions{ $permissionName{$_} }
+				for map { $_->permission } $c->db->getPermissionLevelsWhere({ user_id => { not_like => 'set_id:%' } });
+		}
 
-		if (keys %sections > 1) {
+		if (keys %sections > 1 && (!%includes || $includes{section})) {
 			for my $sec (sortByName(undef, keys %sections)) {
-				push @filters, [ 'Section: ' . ($sec ne '' ? $sec : "\x{27E8}blank\x{27E9}") => "section:$sec" ];
+				push @filters, [ $c->maketext('Section: [_1]', $sec ne '' ? $sec : $blankName) => "section:$sec" ];
 			}
 		}
 
-		if (keys %recitations > 1) {
+		if (keys %recitations > 1 && (!%includes || $includes{recitation})) {
 			for my $rec (sortByName(undef, keys %recitations)) {
-				push @filters, [ 'Recitation: ' . ($rec ne '' ? $rec : "\x{27E8}blank\x{27E9}") => "recitation:$rec" ];
+				push @filters,
+					[ $c->maketext('Recitation: [_1]', $rec ne '' ? $rec : $blankName) => "recitation:$rec" ];
 			}
 		}
 
-		if (keys %roles > 1) {
+		if (keys %roles > 1 && (!%includes || $includes{status})) {
 			for my $role (sortByName(undef, keys %roles)) {
 				my @statuses = keys %{ $c->ce->{statuses} };
 				for (@statuses) {
-					push @filters, [ "Enrollment Status: $_" => "status:$role" ]
+					push @filters, [ $c->maketext('Enrollment Status: [_1]', $_) => "status:$role" ]
 						if ($c->ce->{statuses}{$_}{abbrevs}[0] eq $role);
 				}
 			}
 		}
 
-		if (keys %permissions > 1) {
+		if (keys %permissions > 1 && (!%includes || $includes{permission})) {
 			for my $perm (sortByName(undef, keys %permissions)) {
-				push @filters, [ "Permission Level: $perm" => "permission:$perm" ];
+				push @filters, [ $c->maketext('Permission Level: [_1]', $perm) => "permission:$perm" ];
 			}
 		}
 	} elsif (ref $records[0] eq 'WeBWorK::DB::Record::Set') {
@@ -133,15 +146,15 @@ sub getFiltersForClass {
 				unless (defined $visibles{0} && $set->visible eq '' || defined $visibles{''} && $set->visible eq '0');
 		}
 
-		if (keys %assignment_types > 1) {
+		if (keys %assignment_types > 1 && (!%includes || $includes{assignment_type})) {
 			for my $type (sortByName(undef, keys %assignment_types)) {
 				push @filters, [ FIELD_PROPERTIES()->{assignment_type}{labels}{$type} => "assignment_type:$type" ];
 			}
 		}
 
-		if (keys %visibles > 1) {
+		if (keys %visibles > 1 && (!%includes || $includes{visible})) {
 			for my $vis (sortByName(undef, keys %visibles)) {
-				push @filters, [ ($vis ? 'Visible' : 'Not Visible') => "visible:$vis" ];
+				push @filters, [ ($vis ? $c->maketext('Visible') : $c->maketext('Not Visible')) => "visible:$vis" ];
 			}
 		}
 	}

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

@@ -6,7 +6,7 @@
 %
 <div class="mb-3">
 	<%= maketext('Showing statistics for:') =%>
-	<%= include 'ContentGenerator/Instructor/Stats/student_filter_menu', filters => $filters =%>
+	<%= include 'ContentGenerator/Instructor/Stats/student_filter_menu', filters => $filters, params => {} =%>
 	<%= include 'ContentGenerator/Instructor/Stats/problem_menu', problems => $problems =%>
 </div>
 %

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

@@ -5,7 +5,7 @@
 % # Filter and problem selectors.
 <div class="d-flex align-items-center mt-1 mb-3">
 	<%= maketext('Showing statistics for:') =%>
-	<%= include 'ContentGenerator/Instructor/Stats/student_filter_menu', filters => $filters =%>
+	<%= include 'ContentGenerator/Instructor/Stats/student_filter_menu', filters => $filters, params => {} =%>
 	<%= include 'ContentGenerator/Instructor/Stats/problem_menu', problems => $problems =%>
 </div>
 % # Set information

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

@@ -1,21 +1,20 @@
-% last unless %$filters;
+% last unless @$filters > 1;
 %
 % # Create a section/recitation "filter by" dropdown if there are sections or recitations.
 <div class="btn-group student-nav-filter-selector mx-2">
-	<%= link_to param('filter') ? $filters->{param('filter')} : maketext('All sections') => '#',
-		id => 'filter', class => 'btn btn-primary dropdown-toggle', role => 'button', 'aria-expanded' => 'false',
-		data => { bs_toggle => 'dropdown' } =%>
+	% my $filter         = param('filter') || 'all';
+	% my $current_filter = $filters->[0][0];
+	% for (@$filters) {
+		% $current_filter = $_->[0] if $_->[1] eq $filter;
+	% }
+	<%= link_to $current_filter => '#', id => 'filter', class => 'btn btn-primary dropdown-toggle',
+		role => 'button', 'aria-expanded' => 'false', data => { bs_toggle => 'dropdown' } =%>
 	<ul class="dropdown-menu" role="menu" aria-labelledby="filter">
-		<li>
-			<%= link_to maketext('All sections') => $c->systemLink(url_for),
-				class => 'dropdown-item', param('filter') ? () : (style => 'background-color:#8F8') =%>
-		</li>
-		% for (sort keys %$filters) {
+		% for (@$filters) {
 			<li>
-				<%= link_to $filters->{$_} => $c->systemLink(url_for, params => { filter => $_ }),
-					(param('filter') || '') eq $_ ? (style => 'background-color: #8F8') : (),
-					class => 'dropdown-item' =%>
+				<%= link_to $_->[0] => $c->systemLink(url_for, params => { %$params, filter => $_->[1] }),
+					$_->[1] eq $filter ? (style => 'background-color: #8F8') : (), class => 'dropdown-item' =%>
 			</li>
 		% }
 	</ul>
-</div>\
+</div>