Enable removal of a student from a course

Changelog.md

@@ -18,6 +18,7 @@
 - Improved styling of markdown previews in `MarkdownPreview` component and annotation displays (#7487)
 - Allow creating image annotations from a test run's outputs (#7486)
 - Added an API that collects a single submission (#7494)
+- Enable removal of a student from a course (#7480)
 
 ### 🐛 Bug fixes
 

app/controllers/students_controller.rb

@@ -135,6 +135,22 @@ def update_settings
     redirect_to action: 'settings'
   end
 
+  def destroy
+    @role = record
+    begin
+      @role.destroy!
+    rescue ActiveRecord::DeleteRestrictionError => e
+      flash_message(:error, I18n.t('flash.students.destroy.restricted', user_name: @role.user_name, message: e.message))
+      head :conflict
+    rescue ActiveRecord::RecordNotDestroyed => e
+      flash_message(:error, I18n.t('flash.students.destroy.error', user_name: @role.user_name, message: e.message))
+      head :bad_request
+    else
+      flash_now(:success, I18n.t('flash.students.destroy.success', user_name: @role.user_name))
+      head :no_content
+    end
+  end
+
   private
 
   def role_params

app/controllers/tas_controller.rb

@@ -55,7 +55,7 @@ def destroy
       head :bad_request
     else
       flash_now(:success, I18n.t('flash.tas.destroy.success', user_name: @role.user_name))
-      head :ok
+      head :no_content
     end
   end
 

app/javascript/Components/__tests__/student_table.test.jsx

@@ -3,7 +3,7 @@
  */
 
 import {StudentTable} from "../student_table";
-import {render, screen, within} from "@testing-library/react";
+import {render, screen, within, fireEvent, waitFor} from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 
 describe("For the StudentTable component's states and props", () => {
@@ -267,4 +267,55 @@ describe("For the StudentTable's display of students", () => {
       await screen.findByText("No rows found");
     });
   });
+
+  describe("When the remove button is pressed", () => {
+    let mock_course_id = 1;
+    let mock_student_id = 42;
+
+    beforeEach(() => {
+      jest.clearAllMocks();
+      jest.spyOn(global, "fetch").mockResolvedValue({
+        ok: true,
+        json: jest.fn().mockResolvedValue({
+          students: [
+            {
+              _id: mock_student_id,
+              user_name: "testtest",
+              first_name: "Test",
+              last_name: "Test",
+              email: "test@test.com",
+              hidden: false,
+            },
+          ],
+          sections: {},
+          counts: {all: 1, active: 1, inactive: 0},
+        }),
+      });
+
+      document.querySelector = jest.fn().mockReturnValue({
+        content: "mocked-csrf-token",
+      });
+    });
+
+    it("calls the correct endpoint when removeStudent is triggered", async () => {
+      render(<StudentTable course_id={mock_course_id} />);
+
+      await screen.findByText("testtest");
+
+      fireEvent.click(screen.getByText(I18n.t("remove")));
+
+      await waitFor(() => {
+        expect(fetch).toHaveBeenCalledWith(
+          Routes.course_student_path(mock_course_id, mock_student_id),
+          expect.objectContaining({
+            method: "DELETE",
+            headers: expect.objectContaining({
+              "Content-Type": "application/json",
+              "X-CSRF-Token": expect.any(String),
+            }),
+          })
+        );
+      });
+    });
+  });
 });

app/javascript/Components/__tests__/ta_table.test.jsx

@@ -103,7 +103,7 @@ describe("For the TATable's display of TAs", () => {
     });
   });
 
-  describe("When the delete button is pressed", () => {
+  describe("When the remove button is pressed", () => {
     let mock_course_id = 1;
     let mock_ta_id = 42;
 
@@ -136,7 +136,7 @@ describe("For the TATable's display of TAs", () => {
 
       await screen.findByText("testtest");
 
-      fireEvent.click(screen.getByText(I18n.t("delete")));
+      fireEvent.click(screen.getByText(I18n.t("remove")));
 
       await waitFor(() => {
         expect(fetch).toHaveBeenCalledWith(

app/javascript/Components/student_table.jsx

@@ -62,6 +62,24 @@ class RawStudentTable extends React.Component {
     }).then(this.fetchData);
   };
 
+  removeStudent = student_id => {
+    fetch(Routes.course_student_path(this.props.course_id, student_id), {
+      method: "DELETE",
+      headers: {
+        "Content-Type": "application/json",
+        "X-CSRF-Token": document.querySelector('[name="csrf-token"]').content,
+      },
+    })
+      .then(response => {
+        if (response.ok) {
+          this.fetchData();
+        }
+      })
+      .catch(error => {
+        console.error("Error removing student:", error);
+      });
+  };
+
   render() {
     const {data, loading} = this.state;
 
@@ -178,12 +196,19 @@ class RawStudentTable extends React.Component {
               Header: I18n.t("actions"),
               accessor: "_id",
               Cell: data => (
-                <span>
-                  <a href={Routes.edit_course_student_path(this.props.course_id, data.value)}>
-                    {I18n.t("edit")}
-                  </a>
-                  &nbsp;
-                </span>
+                <>
+                  <span>
+                    <a href={Routes.edit_course_student_path(this.props.course_id, data.value)}>
+                      {I18n.t("edit")}
+                    </a>
+                  </span>
+                  &nbsp;|&nbsp;
+                  <span>
+                    <a href="#" onClick={() => this.removeStudent(data.value)}>
+                      {I18n.t("remove")}
+                    </a>
+                  </span>
+                </>
               ),
               sortable: false,
               filterable: false,

app/javascript/Components/ta_table.jsx

@@ -50,7 +50,7 @@ class TATable extends React.Component {
         }
       })
       .catch(error => {
-        console.error("Error deleting TA:", error);
+        console.error("Error removing TA:", error);
       });
   };
 
@@ -116,7 +116,7 @@ class TATable extends React.Component {
                  | 
                 <span>
                   <a href="#" onClick={() => this.removeTA(data.value)}>
-                    {I18n.t("delete")}
+                    {I18n.t("remove")}
                   </a>
                 </span>
               </>

app/models/grade_entry_student.rb

@@ -24,6 +24,8 @@ class GradeEntryStudent < ApplicationRecord
 
   validate :courses_should_match
 
+  before_destroy -> { throw(:abort) if self.grades.where.not(grade: nil).exists? }, prepend: true
+
   # Merges records of GradeEntryStudent that do not exist yet using a caller-
   # specified block. The block is given the passed-in student IDs and grade
   # entry form IDs and must return a list of (student ID, grade entry form IDs)

app/models/instructor.rb

@@ -3,4 +3,6 @@ class Instructor < Role
   after_create { Repository.get_class.update_permissions }
   after_destroy { Repository.get_class.update_permissions }
   validate :associated_user_is_an_end_user, unless: -> { self.admin_role? }
+
+  has_many :memberships, dependent: :delete_all, foreign_key: 'role_id', inverse_of: :role
 end

app/models/role.rb

@@ -8,7 +8,7 @@ class Role < ApplicationRecord
   delegate_missing_to :user
 
   # Group relationships
-  has_many :memberships, dependent: :delete_all
+  has_many :memberships
   has_many :groupings, through: :memberships
   has_many :notes, as: :noteable, dependent: :destroy
   has_many :annotations, as: :creator

app/models/student.rb

@@ -1,6 +1,6 @@
 # Student user for a given course.
 class Student < Role
-  has_many :grade_entry_students, foreign_key: :role_id, inverse_of: :role
+  has_many :grade_entry_students, dependent: :destroy, foreign_key: :role_id, inverse_of: :role
   has_many :accepted_memberships,
            -> {
              where membership_status: [StudentMembership::STATUSES[:accepted],
@@ -30,7 +30,7 @@ class Student < Role
            through: :memberships,
            source: :grouping
 
-  has_many :student_memberships, foreign_key: 'role_id', inverse_of: :role
+  has_many :student_memberships, dependent: :restrict_with_exception, foreign_key: 'role_id', inverse_of: :role
 
   has_many :grace_period_deductions, through: :memberships
 

app/models/ta.rb

@@ -6,6 +6,8 @@ class Ta < Role
   validate :associated_user_is_an_end_user
   accepts_nested_attributes_for :grader_permission
 
+  has_many :ta_memberships, dependent: :delete_all, foreign_key: 'role_id', inverse_of: :role
+
   has_many :annotation_texts, dependent: :nullify, inverse_of: :creator, foreign_key: :creator_id
   has_many :annotations, dependent: :nullify, inverse_of: :creator, foreign_key: :creator_id
 

config/locales/common/en.yml

@@ -68,6 +68,7 @@ en:
   please_wait: Please wait…
   preview: Preview
   refresh: Refresh
+  remove: Remove
   save: Save
   search: Search
   select_filename: Select Filename

config/locales/defaults/responders/en.yml

@@ -34,14 +34,18 @@ en:
     students:
       create:
         error: "%{resource_name} could not be created due to the following error(s): %{errors}."
+      destroy:
+        error: 'Error removing student (%{user_name}): %{message}'
+        restricted: 'Cannot remove Student (%{user_name}): %{message}'
+        success: Succesfully removed Student (%{user_name}) from course.
       update:
         error: "%{resource_name} could not be updated due to the following error(s): %{errors}."
     tas:
       create:
         error: "%{resource_name} could not be created due to the following error(s): %{errors}."
       destroy:
-        error: 'Error deleting TA (%{user_name}): %{message}'
-        restricted: 'Cannot delete TA (%{user_name}): %{message}'
-        success: Succesfully deleted TA (%{user_name}) from course.
+        error: 'Error removing TA (%{user_name}): %{message}'
+        restricted: 'Cannot remove TA (%{user_name}): %{message}'
+        success: Succesfully removed TA (%{user_name}) from course.
       update:
         error: "%{resource_name} could not be updated due to the following error(s): %{errors}."

config/routes.rb

@@ -455,7 +455,7 @@
       end
     end
 
-    resources :students, only: [:create, :new, :index, :edit, :update] do
+    resources :students, only: [:create, :new, :index, :edit, :update, :destroy] do
       collection do
         patch 'bulk_modify'
         get 'manage'