Implement back-end features for "Remove a Classroom"

app/controllers/api/school_classes_controller.rb

@@ -7,7 +7,7 @@ class SchoolClassesController < ApiController
     before_action :load_and_authorize_school_class
 
     def index
-      school_classes = accessible_school_classes
+      school_classes = accessible_school_classes.active
       school_classes = school_classes.joins(:teachers).where(teachers: { teacher_id: current_user&.id }) if params[:my_classes] == 'true'
       @school_classes_with_teachers = school_classes.with_teachers
 

app/models/ability.rb

@@ -62,7 +62,7 @@ def define_authenticated_non_student_abilities(user)
   def define_school_owner_abilities(school:)
     can(%i[read update destroy], School, id: school.id)
     can(%i[read], :school_member)
-    can(%i[read create import update destroy], SchoolClass, school: { id: school.id })
+    can(%i[read create import update destroy], SchoolClass, school: { id: school.id }, deleted: false)
     can(%i[read show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
     can(%i[read create create_batch destroy], ClassStudent, school_class: { school: { id: school.id } })
     can(%i[read create destroy], :school_owner)
@@ -78,7 +78,7 @@ def define_school_teacher_abilities(user:, school:)
     can(%i[read], School, id: school.id)
     can(%i[read], :school_member)
     can(%i[create import], SchoolClass, school: { id: school.id })
-    can(%i[read update destroy], SchoolClass, school: { id: school.id }, teachers: { teacher_id: user.id })
+    can(%i[read update destroy], SchoolClass, school: { id: school.id }, teachers: { teacher_id: user.id }, deleted: false)
     can(%i[read create create_batch destroy], ClassStudent, school_class: { school: { id: school.id }, teachers: { teacher_id: user.id } })
     can(%i[read], :school_owner)
     can(%i[read], :school_teacher)
@@ -112,7 +112,7 @@ def define_school_student_abilities(user:, school:)
       ).select(:id)
     ).pluck(:id)
     can(%i[read], School, id: school.id)
-    can(%i[read], SchoolClass, school: { id: school.id }, students: { student_id: user.id })
+    can(%i[read], SchoolClass, school: { id: school.id }, students: { student_id: user.id }, deleted: false)
     # Ensure no access to ClassMember resources, relationships otherwise allow access in some circumstances.
     can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { students: { student_id: user.id } })
     can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil, remixed_from_id: visible_lesson_project_ids)

app/models/school_class.rb

@@ -8,6 +8,8 @@ class SchoolClass < ApplicationRecord
   accepts_nested_attributes_for :teachers
 
   scope :with_teachers, ->(user_id) { joins(:teachers).where(teachers: { id: user_id }) }
+  scope :active, -> { where(deleted: false) }
+  scope :deleted, -> { where(deleted: true) }
 
   before_validation :assign_class_code, on: %i[create import]
 
@@ -73,6 +75,17 @@ def submitted_count
       .count
   end
 
+  def deleted?
+    deleted
+  end
+
+  def mark_as_deleted!
+    return if deleted?
+
+    self.deleted = true
+    save!(validate: false)
+  end
+
   private
 
   def school_class_has_at_least_one_teacher

db/migrate/20260113131231_add_deleted_to_school_classes.rb

@@ -0,0 +1,6 @@
+class AddDeletedToSchoolClasses < ActiveRecord::Migration[7.2]
+  def change
+    add_column :school_classes, :deleted, :boolean, default: false, null: false
+    add_index :school_classes, :deleted
+  end
+end

lib/concepts/school_class/operations/delete.rb

@@ -5,7 +5,7 @@ class Delete
     class << self
       def call(school:, school_class_id:)
         response = OperationResponse.new
-        delete_school_class(school, school_class_id)
+        mark_school_class_as_deleted(school, school_class_id)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -15,9 +15,9 @@ def call(school:, school_class_id:)
 
       private
 
-      def delete_school_class(school, school_class_id)
+      def mark_school_class_as_deleted(school, school_class_id)
         school_class = school.classes.find(school_class_id)
-        school_class.destroy!
+        school_class.mark_as_deleted!
       end
     end
   end

spec/concepts/school_class/delete_spec.rb

@@ -18,16 +18,21 @@
     expect(response.success?).to be(true)
   end
 
-  it 'deletes a school class' do
-    expect { described_class.call(school:, school_class_id:) }.to change(SchoolClass, :count).by(-1)
+  it 'marks the school class as deleted' do
+    described_class.call(school:, school_class_id:)
+    expect(school_class.reload.deleted?).to be true
   end
 
-  it 'deletes class students in the school class' do
-    expect { described_class.call(school:, school_class_id:) }.to change(ClassStudent, :count).by(-1)
+  it 'does not delete the school class record' do
+    expect { described_class.call(school:, school_class_id:) }.not_to change(SchoolClass, :count)
   end
 
-  it 'deletes class teachers in the school class' do
-    expect { described_class.call(school:, school_class_id:) }.to change(ClassTeacher, :count).by(-1)
+  it 'does not delete class students in the school class' do
+    expect { described_class.call(school:, school_class_id:) }.not_to change(ClassStudent, :count)
+  end
+
+  it 'does not delete class teachers in the school class' do
+    expect { described_class.call(school:, school_class_id:) }.not_to change(ClassTeacher, :count)
   end
 
   context 'when deletion fails' do

spec/features/school_class/deleting_a_school_class_spec.rb

@@ -18,6 +18,11 @@
     expect(response).to have_http_status(:no_content)
   end
 
+  it 'marks the class as deleted instead of destroying it' do
+    delete("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
+    expect(school_class.reload.deleted?).to be true
+  end
+
   it 'responds 204 No Content when the user is the class teacher' do
     authenticated_in_hydra_as(teacher)
 
@@ -53,4 +58,11 @@
     delete("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
     expect(response).to have_http_status(:forbidden)
   end
+
+  it 'responds 403 Forbidden when the class is already deleted' do
+    school_class.update!(deleted: true)
+
+    delete("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
+    expect(response).to have_http_status(:forbidden)
+  end
 end

spec/features/school_class/listing_school_classes_spec.rb

@@ -102,6 +102,30 @@ def create_remix_with_feedback(school_class:, student:, feedback_attrs: [])
       data = get_classes
       expect(data.size).to eq(1)
     end
+
+    it 'does not include deleted classes' do
+      school_class.update!(deleted: true)
+
+      data = get_classes
+      expect(data.pluck(:name)).not_to include('Test School Class')
+    end
+
+    it 'does not include deleted classes for school-teachers' do
+      authenticated_in_hydra_as(teacher)
+      school_class.update!(deleted: true)
+
+      data = get_classes
+      expect(data).to be_empty
+    end
+
+    it 'does not include deleted classes for school-students' do
+      authenticated_in_hydra_as(student)
+      stub_user_info_api_for(teacher)
+      school_class.update!(deleted: true)
+
+      data = get_classes
+      expect(data).to be_empty
+    end
   end
 
   describe 'submitted_count' do

spec/features/school_class/showing_a_school_class_spec.rb

@@ -104,6 +104,31 @@
       get("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
       expect(response).to have_http_status(:forbidden)
     end
+
+    it 'responds 403 Forbidden when the class is deleted even if student is a member' do
+      student = create(:student, school:)
+      authenticated_in_hydra_as(student)
+      create(:class_student, school_class:, student_id: student.id)
+      school_class.update!(deleted: true)
+
+      get("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it 'responds 403 Forbidden when the class is deleted even if user is the teacher' do
+      authenticated_in_hydra_as(teacher)
+      school_class.update!(deleted: true)
+
+      get("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it 'responds 403 Forbidden when the class is deleted even if user is a school owner' do
+      school_class.update!(deleted: true)
+
+      get("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
   end
 
   context 'when school and class codes are provided' do
@@ -201,5 +226,30 @@
       get("/api/schools/#{school.code}/classes/#{school_class.code}", headers:)
       expect(response).to have_http_status(:forbidden)
     end
+
+    it 'responds 403 Forbidden when the class is deleted even if student is a member' do
+      student = create(:student, school:)
+      authenticated_in_hydra_as(student)
+      create(:class_student, school_class:, student_id: student.id)
+      school_class.update!(deleted: true)
+
+      get("/api/schools/#{school.code}/classes/#{school_class.code}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it 'responds 403 Forbidden when the class is deleted even if user is the teacher' do
+      authenticated_in_hydra_as(teacher)
+      school_class.update!(deleted: true)
+
+      get("/api/schools/#{school.code}/classes/#{school_class.code}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it 'responds 403 Forbidden when the class is deleted even if user is a school owner' do
+      school_class.update!(deleted: true)
+
+      get("/api/schools/#{school.code}/classes/#{school_class.code}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
   end
 end

spec/features/school_class/updating_a_school_class_spec.rb

@@ -84,4 +84,11 @@
     put("/api/schools/#{school.id}/classes/#{school_class.id}", headers:, params:)
     expect(response).to have_http_status(:forbidden)
   end
+
+  it 'responds 403 Forbidden when the class is deleted even if user is the teacher' do
+    school_class.update!(deleted: true)
+
+    put("/api/schools/#{school.id}/classes/#{school_class.id}", headers:, params:)
+    expect(response).to have_http_status(:forbidden)
+  end
 end

spec/models/school_class_spec.rb

@@ -285,6 +285,50 @@
     end
   end
 
+  describe '#deleted?' do
+    it 'is not deleted by default' do
+      school_class = create(:school_class, teacher_ids: [teacher.id], school:)
+      expect(school_class.deleted?).to be false
+    end
+
+    it 'returns true when deleted' do
+      school_class = create(:school_class, teacher_ids: [teacher.id], school:)
+      school_class.update!(deleted: true)
+      expect(school_class.deleted?).to be true
+    end
+  end
+
+  describe '#mark_as_deleted!' do
+    it 'sets deleted to true' do
+      school_class = create(:school_class, teacher_ids: [teacher.id], school:)
+      school_class.mark_as_deleted!
+      expect(school_class.deleted).to be true
+    end
+
+    it 'does nothing if already deleted' do
+      school_class = create(:school_class, teacher_ids: [teacher.id], school:, deleted: true)
+      expect { school_class.mark_as_deleted! }.not_to(change { school_class.reload.updated_at })
+    end
+  end
+
+  describe '.active scope' do
+    it 'only returns active (non-deleted) classes' do
+      active_class = create(:school_class, teacher_ids: [teacher.id], school:, deleted: false)
+      create(:school_class, teacher_ids: [teacher.id], school:, deleted: true)
+
+      expect(described_class.active).to contain_exactly(active_class)
+    end
+  end
+
+  describe '.deleted scope' do
+    it 'only returns deleted classes' do
+      create(:school_class, teacher_ids: [teacher.id], school:, deleted: false)
+      deleted_class = create(:school_class, teacher_ids: [teacher.id], school:, deleted: true)
+
+      expect(described_class.deleted).to contain_exactly(deleted_class)
+    end
+  end
+
   describe 'auditing' do
     subject(:school_class) { create(:school_class, teacher_ids: [teacher.id], school:) }