Enabled instructors to assign inactive students to groups (#7757)

Author: Naragod

Changelog.md

@@ -7,6 +7,7 @@
 ### ✨ New features and improvements
 - Enable test results downloads through the API (#7754)
 - Provide suggestions for partial student matching scans (#7760)
+- Allow inactive students to join groups (#7757)
 
 ### 🐛 Bug fixes
 

app/controllers/groups_controller.rb

@@ -306,6 +306,7 @@ def upload
 
         group_rows << row.compact_blank
       end
+
       if result[:invalid_lines].empty?
         @current_job = CreateGroupsJob.perform_later assignment, group_rows
         session[:job_id] = @current_job.job_id
@@ -489,8 +490,8 @@ def invite_member
       if errors.blank?
         to_invite.each do |i|
           i = i.strip
-          invited_user = current_course.students.joins(:user).where(hidden: false).find_by('users.user_name': i)
-          if invited_user.receives_invite_emails?
+          invited_user = current_course.students.joins(:user).find_by('users.user_name': i)
+          if invited_user&.receives_invite_emails?
             NotificationMailer.with(inviter: current_role,
                                     invited: invited_user,
                                     grouping: @grouping).grouping_invite_email.deliver_later
@@ -699,9 +700,6 @@ def add_member(student, grouping, assignment)
                             end
     @bad_user_names = []
 
-    if student.hidden
-      raise I18n.t('groups.invite_member.errors.not_found', user_name: student.user_name)
-    end
     if student.has_accepted_grouping_for?(assignment.id)
       raise I18n.t('groups.invite_member.errors.already_grouped', user_name: student.user_name)
     end

app/models/grouping.rb

@@ -288,7 +288,7 @@ def invite(members,
     all_errors = []
     members.each do |m|
       m = m.strip
-      user = course.students.joins(:user).where(hidden: false).find_by('users.user_name': m)
+      user = course.students.joins(:user).find_by('users.user_name': m)
       begin
         if user.nil?
           raise I18n.t('groups.invite_member.errors.not_found', user_name: m)
@@ -305,7 +305,7 @@ def invite(members,
 
   # Add a new member to base
   def add_member(role, set_membership_status = StudentMembership::STATUSES[:accepted])
-    if role.has_accepted_grouping_for?(self.assessment_id) || role.hidden
+    if role.has_accepted_grouping_for?(self.assessment_id)
       nil
     else
       member = StudentMembership.new(role: role, membership_status:
@@ -329,6 +329,8 @@ def add_member(role, set_membership_status = StudentMembership::STATUSES[:accept
   def can_invite?(role)
     if self.inviter == role
       raise I18n.t('groups.invite_member.errors.inviting_self')
+    elsif role.hidden
+      raise I18n.t('groups.invite_member.errors.inactive_student', user_name: role.user_name)
     elsif !extension.nil?
       raise I18n.t('groups.invite_member.errors.extension_exists')
     elsif self.student_membership_number >= self.assignment.group_max

config/locales/views/groups/en.yml

@@ -42,6 +42,7 @@ en:
         extension_exists: You cannot modify a group with an extension to the due date. Please contact your instructor if you wish to make changes.
         group_max_reached: 'Could not invite %{user_name}: group maximum has been reached.'
         inactive_grader: Cannot assign inactive grader(s) %{user_names}.
+        inactive_student: 'Could not invite %{user_name}: this student is inactive.'
         inviting_self: You cannot invite yourself to your own group.
         need_to_create_group: You must create a group before you can invite members.
         not_found: No student '%{user_name}' could be found.

spec/controllers/groups_controller_spec.rb

@@ -532,7 +532,8 @@
         # Setup for Git Repository
         allow(Settings.repository).to receive(:type).and_return('git')
 
-        @assignment = create(:assignment)
+        # Create assignment with group_max of 3 to accommodate test files
+        @assignment = create(:assignment, assignment_properties_attributes: { group_max: 3 })
 
         # Create students corresponding to the file_good
         @student_user_names = %w[c8shosta c5bennet]
@@ -570,6 +571,67 @@
         expect(flash[:error]).not_to be_blank
         expect(response).to redirect_to(action: 'index')
       end
+
+      it 'accepts groups within the maximum group size' do
+        @assignment.update!(assignment_properties_attributes: { group_max: 2 })
+        # Students c8shosta and c5bennet are already created in the outer before block
+        expect do
+          post_as instructor, :upload, params: {
+            course_id: course.id,
+            assignment_id: @assignment.id,
+            upload_file: fixture_file_upload('groups/form_good.csv', 'text/csv')
+          }
+        end.to have_enqueued_job(CreateGroupsJob)
+
+        expect(response).to have_http_status(:found)
+        expect(flash[:error]).to be_blank
+        expect(response).to redirect_to(action: 'index')
+      end
+
+      it 'accepts single-member groups' do
+        @assignment.update!(assignment_properties_attributes: { group_max: 1 })
+        create(:student, user: create(:end_user, user_name: 'solo_student'))
+
+        csv_content = "group1,solo_student\n"
+        file = Tempfile.new(['test_upload', '.csv'])
+        file.write(csv_content)
+        file.rewind
+
+        expect do
+          post_as instructor, :upload, params: { course_id: course.id,
+                                                 assignment_id: @assignment.id,
+                                                 upload_file: fixture_file_upload(file.path, 'text/csv') }
+        end.to have_enqueued_job(CreateGroupsJob)
+
+        file.close
+        file.unlink
+
+        expect(flash[:error]).to be_blank
+      end
+
+      context 'when uploading groups with inactive students' do
+        before do
+          @assignment.update!(assignment_properties_attributes: { group_max: 2 })
+          # Create an active student
+          @active_student = create(:student, user: create(:end_user, user_name: 'active_student'))
+          # Create an inactive student (hidden: true)
+          @inactive_student = create(:student, hidden: true, user: create(:end_user, user_name: 'inactive_student'))
+        end
+
+        it 'allows importing groups with inactive students (ISSUE-7743)' do
+          expect do
+            post_as instructor, :upload, params: {
+              course_id: course.id,
+              assignment_id: @assignment.id,
+              upload_file: fixture_file_upload('groups/form_with_inactive_students.csv', 'text/csv')
+            }
+          end.to have_enqueued_job(CreateGroupsJob)
+
+          expect(response).to have_http_status(:found)
+          expect(flash[:error]).to be_blank
+          expect(response).to redirect_to(action: 'index')
+        end
+      end
     end
 
     describe '#create_groups_when_students_work_alone' do

spec/fixtures/files/groups/form_group_size_exceeded.csv

@@ -0,0 +1 @@
+group1,student1,student2,student3

spec/fixtures/files/groups/form_multiple_oversized.csv

@@ -0,0 +1,5 @@
+group1,student1,student2,student3
+group2,student4,student5
+group3,student6,student7,student8
+group4,student9,student10,student11
+group5,student12,student13,student14

spec/fixtures/files/groups/form_with_inactive_students.csv

@@ -0,0 +1 @@
+group1,active_student,inactive_student

spec/models/grouping_spec.rb

@@ -45,15 +45,25 @@
     context 'hidden students' do
       let(:hidden) { create(:student, hidden: true) }
 
-      it 'cannot be invited' do
-        grouping.invite(hidden.user.user_name)
-        expect(grouping.memberships.count).to eq(0)
+      it 'can be invited by instructors' do
+        grouping.invite(hidden.user.user_name, StudentMembership::STATUSES[:accepted], invoked_by_instructor: true)
+        expect(grouping.memberships.count).to eq(1)
+        expect(grouping.memberships.first.role).to eq(hidden)
       end
 
-      it 'cannot be added' do
-        grouping.add_member(hidden)
+      it 'cannot be invited by students' do
+        errors = grouping.invite(hidden.user.user_name)
+        expect(errors).not_to be_empty
+        expect(errors.first).to include('inactive')
         expect(grouping.memberships.count).to eq(0)
       end
+
+      it 'can be added' do
+        result = grouping.add_member(hidden)
+        expect(result).not_to be_nil
+        expect(grouping.memberships.count).to eq(1)
+        expect(grouping.memberships.first.role).to eq(hidden)
+      end
     end
 
     it 'displays Empty Group since no students in the group' do