feat(courses): block students from suspended courses

- unified suspension check to helper function
- added tests

Author: adi-herwana-nus

app/controllers/course/controller.rb

@@ -57,7 +57,7 @@ def current_ability
   private
 
   def handle_access_denied(exception)
-    return super unless current_course_user&.is_suspended?
+    return super unless current_course_user&.suspended_from_course?(current_ability)
 
     render json: { is_suspended: true, errors: exception.message }, status: :forbidden
   end

app/controllers/course/courses_controller.rb

@@ -10,7 +10,7 @@ def index
   def show
     head :unauthorized and return unless current_user.present? || current_course.published
 
-    return if current_course_user&.is_suspended? && cannot?(:manage, current_course)
+    return if current_course_user&.suspended_from_course?(current_ability)
 
     if can?(:manage, current_course) || current_course.user?(current_user)
       @currently_active_announcements = current_course.announcements.currently_active.includes(:creator)

app/models/components/course/course_ability_component.rb

@@ -35,7 +35,7 @@ def allow_unregistered_users_registering_courses
   end
 
   def allow_registered_users_showing_course
-    can :read, Course, id: course.id unless course_user.is_suspended?
+    can :read, Course, id: course.id unless course_user.is_suspended || course.is_suspended
   end
 
   def allow_staff_show_course_users

app/models/course_user.rb

@@ -213,6 +213,16 @@ def real_student?
     student? && !phantom
   end
 
+  # Test whether this course_user should be blocked from accessing the course.
+  # This can be either because the user is suspended, or the course itself is suspended.
+  # Users with manage permissions (managers, owners, site admins) are unaffected by suspension,
+  # since they need to be able to access the course to unsuspend it.
+  #
+  # @return [Boolean]
+  def suspended_from_course?(ability)
+    ability&.cannot?(:manage, course) && ((student? && course.is_suspended) || is_suspended)
+  end
+
   # Returns my students in the course.
   # If a course_user is the manager of a group, all other users in the group with the group role of
   # normal will be considered as the students of the course_user.

app/views/course/courses/_course_data.json.jbuilder

@@ -15,7 +15,8 @@ json.instructors instructors do |instructor|
   json.imageUrl user_image(instructor)
 end
 
-if can?(:manage, current_course) || (current_course.user?(current_user) && !current_course_user&.is_suspended)
+is_suspended_user = current_course_user&.suspended_from_course?(current_ability)
+if can?(:manage, current_course) || (current_course.user?(current_user) && !is_suspended_user)
   # Announcements
   if @currently_active_announcements && !@currently_active_announcements.empty?
     json.currentlyActiveAnnouncements @currently_active_announcements do |announcement|
@@ -55,6 +56,7 @@ if can?(:manage, current_course) || (current_course.user?(current_user) && !curr
   end
 end
 
-is_suspended_user = current_course_user&.is_suspended && cannot?(:manage, current_course)
+json.isSuspended current_course.is_suspended
 json.isSuspendedUser is_suspended_user
+json.courseSuspensionMessage current_course.course_suspension_message if current_course.is_suspended
 json.userSuspensionMessage current_course.user_suspension_message if is_suspended_user

client/app/bundles/common/ErrorPage.tsx

@@ -243,6 +243,9 @@ const SuspendedPage = (): JSX.Element => {
 
   const dispatch = useAppDispatch();
   const course = useAppSelector((state) => getCourseEntity(state, +courseId!));
+  const suspendedSubtitle = course?.isSuspended
+    ? course?.courseSuspensionMessage
+    : course?.userSuspensionMessage;
 
   useEffectOnce(() => {
     if (sourceURL) window.history.replaceState(null, '', sourceURL);
@@ -288,11 +291,13 @@ const SuspendedPage = (): JSX.Element => {
       ]}
       illustrationAlt="Forbidden illustration"
       illustrationSrc={forbiddenIllustration}
-      subtitle={
-        course?.userSuspensionMessage ?? t(courseTranslations.suspendedSubtitle)
-      }
+      subtitle={suspendedSubtitle ?? t(courseTranslations.suspendedSubtitle)}
       tip={sourceURL}
-      title={t(translations.userSuspended)}
+      title={
+        course?.isSuspended
+          ? t(translations.courseSuspended)
+          : t(translations.userSuspended)
+      }
     />
   );
 };

client/app/types/course/courses.ts

@@ -74,6 +74,8 @@ export interface CourseEntity extends CourseMiniEntity {
   permissions: CourseDataPermissions;
   isSuspendedUser?: boolean;
   userSuspensionMessage?: string;
+  isSuspended: boolean;
+  courseSuspensionMessage?: string;
 }
 
 export interface NewCourseFormData {

spec/controllers/course/admin/admin_controller_spec.rb

@@ -104,6 +104,75 @@
       end
     end
 
+    describe '#suspend' do
+      subject { patch :suspend, params: { course_id: course } }
+
+      context 'when the user is a Course Owner' do
+        let(:user) { create(:course_owner, course: course).user }
+
+        it 'suspends the course' do
+          subject
+          expect(course.reload.is_suspended).to be true
+        end
+      end
+
+      context 'when the user is a Course Manager' do
+        let(:user) { create(:course_manager, course: course).user }
+
+        it 'suspends the course' do
+          subject
+          expect(course.reload.is_suspended).to be true
+        end
+      end
+
+      context 'when the user is a Teaching Assistant' do
+        let(:user) { create(:course_teaching_assistant, course: course).user }
+
+        it { expect { subject }.to raise_exception(CanCan::AccessDenied) }
+      end
+
+      context 'when the user is a Course Student' do
+        let(:user) { create(:course_student, course: course).user }
+
+        it { expect { subject }.to raise_exception(CanCan::AccessDenied) }
+      end
+    end
+
+    describe '#unsuspend' do
+      let(:course) { create(:course, is_suspended: true) }
+      subject { patch :unsuspend, params: { course_id: course } }
+
+      context 'when the user is a Course Owner' do
+        let(:user) { create(:course_owner, course: course).user }
+
+        it 'unsuspends the course' do
+          subject
+          expect(course.reload.is_suspended).to be false
+        end
+      end
+
+      context 'when the user is a Course Manager' do
+        let(:user) { create(:course_manager, course: course).user }
+
+        it 'unsuspends the course' do
+          subject
+          expect(course.reload.is_suspended).to be false
+        end
+      end
+
+      context 'when the user is a Teaching Assistant' do
+        let(:user) { create(:course_teaching_assistant, course: course).user }
+
+        it { expect { subject }.to raise_exception(CanCan::AccessDenied) }
+      end
+
+      context 'when the user is a Course Student' do
+        let(:user) { create(:course_student, course: course).user }
+
+        it { expect { subject }.to raise_exception(CanCan::AccessDenied) }
+      end
+    end
+
     describe '#destroy' do
       subject { delete :destroy, params: { course_id: course } }
       before { controller.instance_variable_set(:@course, course) }

spec/controllers/course/courses_controller_spec.rb

@@ -57,6 +57,41 @@
         end
       end
 
+      context 'when the course is suspended and the user is a student' do
+        before { controller_sign_in(controller, user) }
+        let(:course) { create(:course, published: true, is_suspended: true) }
+        let!(:course_user) { create(:course_student, course: course, user: user) }
+
+        it { is_expected.to have_http_status(:ok) }
+
+        it 'sets isSuspended and isSuspendedUser in the response body' do
+          subject
+          course_data = JSON.parse(response.body).fetch('course')
+          expect(course_data['isSuspended']).to be true
+          expect(course_data['isSuspendedUser']).to be true
+        end
+
+        it 'includes the course suspension message in the response body' do
+          subject
+          expect(JSON.parse(response.body).fetch('course')).to have_key('courseSuspensionMessage')
+        end
+      end
+
+      context 'when the course is suspended and the user is a manager' do
+        before { controller_sign_in(controller, user) }
+        let(:course) { create(:course, published: true, is_suspended: true) }
+        let!(:course_user) { create(:course_manager, course: course, user: user) }
+
+        it { is_expected.to have_http_status(:ok) }
+
+        it 'sets isSuspended but not isSuspendedUser in the response body' do
+          subject
+          course_data = JSON.parse(response.body).fetch('course')
+          expect(course_data['isSuspended']).to be true
+          expect(course_data['isSuspendedUser']).to be false
+        end
+      end
+
       context 'when the user is not enrolled' do
         before { controller_sign_in(controller, user) }
 

spec/models/course_user_spec.rb

@@ -334,6 +334,67 @@
       end
     end
 
+    describe '#suspended_from_course?' do
+      def ability_for(course_user)
+        Ability.new(course_user.user, course, course_user)
+      end
+
+      context 'when the course is not suspended' do
+        it 'returns false for an active student' do
+          expect(student.suspended_from_course?(ability_for(student))).to be false
+        end
+
+        it 'returns true for an individually suspended student' do
+          student.update!(is_suspended: true)
+          expect(student.suspended_from_course?(ability_for(student))).to be true
+        end
+
+        it 'returns false for an individually suspended manager' do
+          manager.update!(is_suspended: true)
+          expect(manager.suspended_from_course?(ability_for(manager))).to be false
+        end
+
+        it 'returns false for an individually suspended owner' do
+          course_owner.update!(is_suspended: true)
+          expect(course_owner.suspended_from_course?(ability_for(course_owner))).to be false
+        end
+
+        it 'returns true for an individually suspended teaching assistant' do
+          teaching_assistant.update!(is_suspended: true)
+          expect(teaching_assistant.suspended_from_course?(ability_for(teaching_assistant))).to be true
+        end
+
+        it 'returns true for an individually suspended observer' do
+          observer.update!(is_suspended: true)
+          expect(observer.suspended_from_course?(ability_for(observer))).to be true
+        end
+      end
+
+      context 'when the course is suspended' do
+        before { course.update!(is_suspended: true) }
+
+        it 'returns true for an active student' do
+          expect(student.suspended_from_course?(ability_for(student))).to be true
+        end
+
+        it 'returns false for a manager' do
+          expect(manager.suspended_from_course?(ability_for(manager))).to be false
+        end
+
+        it 'returns false for an owner' do
+          expect(course_owner.suspended_from_course?(ability_for(course_owner))).to be false
+        end
+
+        it 'returns false for a teaching assistant' do
+          expect(teaching_assistant.suspended_from_course?(ability_for(teaching_assistant))).to be false
+        end
+
+        it 'returns false for an observer' do
+          expect(observer.suspended_from_course?(ability_for(observer))).to be false
+        end
+      end
+    end
+
     describe '#latest_learning_rate_record' do
       it 'returns the latest learning rate record' do
         create(:learning_rate_record, course_user: student, learning_rate: 1)