Allow inactive groups in the graders table to be toggled for display #6778

Bruce-8 · 2023-10-20T04:01:53Z

Motivation and Context

Currently on the assignment groups tab, it is possible to hide students and groups that are inactive. This pull request adds the same kind of filtering to the graders tab, to toggle whether to display inactive groups on the right-hand pane.

Your Changes

Description:
Front-end:

Modified graders_manager.jsx to allow for RawGroupsTable to display inactive groups when the icon Display inactive groups is clicked. Created a Display inactive groups icon on the right-hand pane of the web interface.
Updated the hidden tooltip for the Display inactive groups icon to include the amount of hidden groups.

Back-end:

Modified the index action of graders_controller.rb to include the members attribute of groups in the grader data so the front-end is able to determine whether a group is inactive or not based on its members (students).

Type of change (select all that apply):

New feature (non-breaking change which adds functionality)

Testing

Manually tested changes through the web interface.

Questions and Comments (if applicable)

Checklist

I have performed a self-review of my own code.
I have verified that the pre-commit.ci checks have passed.
I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported on Coveralls.
I have updated the Changelog.md file.

Pull request to make documentation changes (if applicable)

Implementing the following functionality: When an instructor tries to add an inactive grader to a group, the system prevents him from doing so and throws a red flash-message saying the user is inactive and thus cannot be added.

Our goal is to be able to determine in the ffront end which groups should be hidden because they are inactive. Our definition of an inactive group is that all of its members are inactive. Thus, (along with the graders associated to each group, which we already passing to the front end) we need to pass the (student) members associated with each group. This will allow the front end for this graders use case to act similarly to the front end of the groups use case (by filtering which groups should be visible or not based on the activity/inacticity of their users)

… display-inactive-graders Resolve merge conflicts

coveralls · 2023-10-28T22:28:35Z

Pull Request Test Coverage Report for Build 7159783050

52 of 53 (98.11%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.3%) to 91.563%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
app/assets/javascripts/Components/graders_manager.jsx	19	20	95.0%

Totals
Change from base Build 7104317343:	-0.3%
Covered Lines:	38537
Relevant Lines:	41467

💛 - Coveralls

david-yz-liu

@Bruce-8 and @mimischly7 good work. I left some comments for you guys to work on; additionally, make sure to add test cases for both the front-end and back-end changes.

david-yz-liu · 2023-11-10T14:32:55Z

Changelog.md

@@ -15,6 +15,7 @@
 ## [v2.3.3]
 - Fix bug where uploading scanned exam pages with overwriting option selected did not update submission files (#6768)
 - Fix bug: "Download Submissions" download link was not being rendered from partial view (#6779)
+- Allow inactive groups in the graders tab to be displayed (#6778)


This entry should go under the "unreleased" section above.

Also, please reword to "Allow inactive groups in the graders table to be toggled for display". (Since they were originally already displayed, this change makes it so that you can hide these groups.)

david-yz-liu · 2023-11-10T14:34:11Z

app/assets/javascripts/Components/graders_manager.jsx

@@ -66,6 +68,15 @@ class GradersManager extends React.Component {
 if (this.groupsTable) this.groupsTable.resetSelection();
 if (this.criteriaTable) this.criteriaTable.resetSelection();

+ var inactive_groups_count = 0;


In general use let instead of var for variable declarations. (See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let)

david-yz-liu · 2023-11-10T14:37:11Z

app/assets/javascripts/Components/graders_manager.jsx

- });
+ let showHiddenGraderTooltip = "";
+ let showHiddenGroupsTooltip = "";
+ if (this.props.hiddenStudentsCount !== null && this.props.hiddenGroupsCount !== null) {


hiddenStudentsCount is not correct (make sure you are testing this carefully)

david-yz-liu · 2023-11-10T14:45:55Z

app/controllers/graders_controller.rb

@@ -15,10 +15,30 @@ class GradersController < ApplicationController
 def index
 @assignment = Assignment.find(params[:assignment_id])

+ # We will complement the data hash returned by the Assignment.current_grader_data method by adding 'groups.members'


This code has the right idea, but I have a few comments for improving it:

You shouldn't add additional code in the controller method; instead, add code to the current_grader_data method instead.

Right now the loop (data[:groups].each) has an N + 1 query problem (I recommend looking up what that means). Instead of doing this, a better approach is to make a single database query using joins that combines the grouping and roles (as student members) tables together. After doing so you can then group the results by grouping_id and then merge with the existing grouping data.

As a tip, I strongly recommend using the Rails console to experiment with making more complex active record queries, since you'll likely need to iterate a bunch to get this right.

converting left outer join to inner join, setting default behavior for some hashes (for when trying to access a non-existent key), and adding documentation

A simple rspec test for the Assignment.current_grader_data method that ensures the returned object has the structure required by frontend code to operate correctly.

… display-inactive-graders Resolve merge conflicts

rspec test that checks the members contained in the object returned by Assignment.current_grader_data is correct.

david-yz-liu · 2023-12-01T03:35:41Z

Changelog.md

@@ -15,6 +15,7 @@
 - Ensure bonus marks are not included in assignment "out of" in submissions table (#6836)
 - Ensure assignment "out of" in submissions table is rounded to two decimal places (#6836)
 - Added POST API for Group Creation (#6834)
+- Allow inactive groups in the graders table to be toggled for display (#6778)


We've updated the changelog with a new minor release. Please do a merge from master; after that, make sure to move this entry so it appears under the "Unreleased" section.

david-yz-liu · 2023-12-01T03:39:23Z

app/models/assignment.rb

@@ -1091,6 +1091,39 @@ def current_grader_data
 hide_unassigned_criteria: self.hide_unassigned_criteria,
 sections: assignment.course.sections.pluck(:id, :name).to_h
 }
+
+ # ---- NEW CODE TO ADD MEMBERS TO EACH GROUP IN THE CURRENT DATA SET -----


Delete the comments in this section

david-yz-liu · 2023-12-01T03:39:36Z

bin/public/javascripts/translations.json

@@ -0,0 +1,2 @@
+{


Delete this file

david-yz-liu · 2023-12-01T03:39:58Z

spec/models/assignment_spec.rb

 let(:grouping) { create :grouping_with_inviter, inviter: student, assignment: assignment }
+


delete this line

…/Markus into display-inactive-graders Fixing merge conflicts

david-yz-liu · 2023-12-07T22:00:07Z

app/models/assignment.rb

@@ -1091,6 +1091,19 @@ def current_grader_data
 hide_unassigned_criteria: self.hide_unassigned_criteria,
 sections: assignment.course.sections.pluck(:id, :name).to_h
 }
+
+ result.default_proc = proc { |h, k| h[k] = [] }


This isn't necessary

david-yz-liu · 2023-12-07T22:00:45Z

app/models/assignment.rb

+
+ grouped_data = members_data.group_by { |x| x[0] }
+ grouped_data.each_value { |a| a.each { |b| b.delete_at(0) } }
+ grouped_data.default_proc = proc { |h, k| h[k] = [] }


This isn't necessary

david-yz-liu · 2023-12-07T22:01:50Z

Changelog.md

@@ -18,6 +18,7 @@
 - Ensure starter files are passed to autotester in sorted order (#6771)
 - Resolved issue 6677 by taking timed assessment's duration into account when determining when grading can begin (#6845)
 - Fix loading results page when group is created after the due date (#6863)
+- Allow inactive groups in the graders table to be toggled for display (#6778)


There's been another release! Please do another pull from upstream master and put this entry under an [unreleased] section.

I was initially defining some default behavior for some hashes in the Assignment model, but this was redundant.

…arkUsProject#6778) Co-authored-by: Mimis Chlympatsos <[email protected]> Co-authored-by: David Liu <[email protected]>

Allow inactive grader groups to be displayed

fa0d6bc

Bruce-8 marked this pull request as draft October 20, 2023 04:02

Bruce-8 and others added 7 commits October 20, 2023 00:06

Updated Changelog and moved checkbox to righthand pane

6359ef0

flash-message-for-inactive-grader

08e6e93

Implementing the following functionality: When an instructor tries to add an inactive grader to a group, the system prevents him from doing so and throws a red flash-message saying the user is inactive and thus cannot be added.

updating-Changelog

5f8f4ed

Changed front-end logic with filtering groups

1bb7085

Changed location of group string

4d02bf7

Changed location of group string

59c1898

Bruce-8 changed the title ~~Allow inactive grader groups to be displayed~~ Allow inactive grader groups to be displayed and prevent inactive graders to join a group Oct 27, 2023

Modified Changelog

3a70425

Bruce-8 changed the title ~~Allow inactive grader groups to be displayed and prevent inactive graders to join a group~~ Allow inactive groups in the graders tab to be displayed and prevent inactive graders from joining a group Oct 27, 2023

Bruce-8 added 2 commits October 28, 2023 17:50

Revert changes for flash message

05630bb

Merge branch 'master' of https:/MarkUsProject/Markus into…

82b578c

… display-inactive-graders Resolve merge conflicts

Bruce-8 marked this pull request as ready for review October 28, 2023 21:55

Re-added deleted space in graders_manager

b6226ed

Bruce-8 requested a review from david-yz-liu October 28, 2023 21:59

Bruce-8 changed the title ~~Allow inactive groups in the graders tab to be displayed and prevent inactive graders from joining a group~~ Allow inactive groups in the graders tab to be displayed Oct 28, 2023

david-yz-liu reviewed Nov 10, 2023

View reviewed changes

mimischly7 and others added 4 commits November 10, 2023 22:58

working-code-for-getting-members-efficiently

a1e9313

finalizing-backend-improvements

f6e81b7

converting left outer join to inner join, setting default behavior for some hashes (for when trying to access a non-existent key), and adding documentation

rspec-test-for-Assignment-method

0ae0368

A simple rspec test for the Assignment.current_grader_data method that ensures the returned object has the structure required by frontend code to operate correctly.

Merge branch 'master' of https:/MarkUsProject/Markus into…

d7b4fbd

… display-inactive-graders Resolve merge conflicts

Bruce-8 changed the title ~~Allow inactive groups in the graders tab to be displayed~~ Allow inactive groups in the graders table to be toggled for display Nov 21, 2023

Bruce-8 and others added 5 commits November 20, 2023 21:24

Fixed errors and changelog

f23392a

Added front-end tests

5720893

Increased coverage of new lines of code

289f81d

Final test fixes (front-end)

09561aa

rspec-test-for-assign-all-graders

ab7f5dc

rspec test that checks the members contained in the object returned by Assignment.current_grader_data is correct.

mimischly7 requested a review from david-yz-liu November 24, 2023 20:36

david-yz-liu reviewed Dec 1, 2023

View reviewed changes

Bruce-8 and others added 3 commits November 30, 2023 23:05

Fixed Changelog and merge conflicts

196be58

Merge branch 'display-inactive-graders' of https:/Bruce-8…

87681b2

…/Markus into display-inactive-graders Fixing merge conflicts

final-touches

7723b9b

mimischly7 requested a review from david-yz-liu December 5, 2023 20:26

conflict-resolution

ca70459

david-yz-liu reviewed Dec 7, 2023

View reviewed changes

Bruce-8 and others added 2 commits December 8, 2023 05:48

Modified Changelog

29719a9

removing-unnecessary-lines

e2c9a24

I was initially defining some default behavior for some hashes in the Assignment model, but this was redundant.

Bruce-8 requested a review from david-yz-liu December 10, 2023 19:21

david-yz-liu added 2 commits December 14, 2023 11:14

Merge branch 'master' into display-inactive-graders

61e5dee

Pin CI chromedriver version

d659e07

david-yz-liu approved these changes Dec 15, 2023

View reviewed changes

david-yz-liu merged commit 51edbf2 into MarkUsProject:master Dec 15, 2023
6 checks passed

david-yz-liu added this to the v2.4.2 milestone Dec 18, 2023

Bruce-8 deleted the display-inactive-graders branch January 9, 2024 15:57

pranavrao145 mentioned this pull request Mar 26, 2024

Hide Inactive Groups in Submission Table #7000

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow inactive groups in the graders table to be toggled for display #6778

Allow inactive groups in the graders table to be toggled for display #6778

Bruce-8 commented Oct 20, 2023 •

edited

Loading

coveralls commented Oct 28, 2023 •

edited

Loading

david-yz-liu left a comment

david-yz-liu Nov 10, 2023

david-yz-liu Nov 10, 2023

david-yz-liu Nov 10, 2023

david-yz-liu Nov 10, 2023

david-yz-liu Dec 1, 2023

david-yz-liu Dec 1, 2023

david-yz-liu Dec 1, 2023

david-yz-liu Dec 1, 2023

david-yz-liu Dec 7, 2023

david-yz-liu Dec 7, 2023

david-yz-liu Dec 7, 2023

		let(:grouping) { create :grouping_with_inviter, inviter: student, assignment: assignment }

Allow inactive groups in the graders table to be toggled for display #6778

Allow inactive groups in the graders table to be toggled for display #6778

Conversation

Bruce-8 commented Oct 20, 2023 • edited Loading

Motivation and Context

Your Changes

Testing

Questions and Comments (if applicable)

Checklist

Pull request to make documentation changes (if applicable)

coveralls commented Oct 28, 2023 • edited Loading

Pull Request Test Coverage Report for Build 7159783050

💛 - Coveralls

david-yz-liu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bruce-8 commented Oct 20, 2023 •

edited

Loading

coveralls commented Oct 28, 2023 •

edited

Loading