diff --git a/app/controllers/api/schools_controller.rb b/app/controllers/api/schools_controller.rb index f4d9956c9..0a144fa11 100644 --- a/app/controllers/api/schools_controller.rb +++ b/app/controllers/api/schools_controller.rb @@ -58,16 +58,6 @@ def update end end - def destroy - result = School::Delete.call(school_id: params[:id]) - - if result.success? - head :no_content - else - render json: { error: result[:error] }, status: :unprocessable_content - end - end - def import authorize! :import, School diff --git a/app/models/ability.rb b/app/models/ability.rb index a11bc4587..c8463d1fa 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -70,7 +70,7 @@ def define_authenticated_non_student_abilities(user) end def define_school_owner_abilities(school:) - can(%i[read update destroy], School, id: school.id) + can(%i[read update], School, id: school.id) can(%i[read], :school_member) can(%i[read create import update destroy regenerate_join_code], SchoolClass, school: { id: school.id }) can(%i[read update show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] }) diff --git a/config/routes.rb b/config/routes.rb index a59a64057..78534ea58 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -66,7 +66,7 @@ resource :project_errors, only: %i[create] resource :school, only: [:show], controller: 'my_school' - resources :schools, only: %i[index show create update destroy] do + resources :schools, only: %i[index show create update] do post :import, on: :collection resources :members, only: %i[index], controller: 'school_members' resources :classes, only: %i[index show create update destroy], controller: 'school_classes' do diff --git a/lib/concepts/school/operations/delete.rb b/lib/concepts/school/operations/delete.rb deleted file mode 100644 index 24b49f7f8..000000000 --- a/lib/concepts/school/operations/delete.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -class School - class Delete - class << self - def call(school_id:) - response = OperationResponse.new - delete_school(school_id) - response - rescue StandardError => e - Sentry.capture_exception(e) - response[:error] = "Error deleting school: #{e}" - response - end - - private - - def delete_school(school_id) - school = School.find(school_id) - school.destroy! - end - end - end -end diff --git a/spec/concepts/school/delete_spec.rb b/spec/concepts/school/delete_spec.rb deleted file mode 100644 index ca3c67659..000000000 --- a/spec/concepts/school/delete_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe School::Delete, type: :unit do - before do - create(:class_student, student_id: student.id, school_class:) - end - - let(:school_class) { build(:school_class, teacher_ids: [teacher.id], school:) } - let(:school) { create(:school) } - let(:school_id) { school.id } - let(:student) { create(:student, school:) } - let(:teacher) { create(:teacher, school:) } - - it 'returns a successful operation response' do - response = described_class.call(school_id:) - expect(response.success?).to be(true) - end - - it 'deletes a school' do - expect { described_class.call(school_id:) }.to change(School, :count).by(-1) - end - - it 'deletes a school classes in the school' do - expect { described_class.call(school_id:) }.to change(SchoolClass, :count).by(-1) - end - - it 'deletes class students in the school' do - expect { described_class.call(school_id:) }.to change(ClassStudent, :count).by(-1) - end - - it 'deletes class teachers in the school' do - expect { described_class.call(school_id:) }.to change(ClassTeacher, :count).by(-1) - end - - context 'when deletion fails' do - let(:school_id) { 'does-not-exist' } - - before do - allow(Sentry).to receive(:capture_exception) - end - - it 'returns a failed operation response' do - response = described_class.call(school_id:) - expect(response.failure?).to be(true) - end - - it 'returns the error message in the operation response' do - response = described_class.call(school_id:) - expect(response[:error]).to include('does-not-exist') - end - - it 'sent the exception to Sentry' do - described_class.call(school_id:) - expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) - end - end -end diff --git a/spec/features/school/deleting_a_school_spec.rb b/spec/features/school/deleting_a_school_spec.rb deleted file mode 100644 index aa54ecf21..000000000 --- a/spec/features/school/deleting_a_school_spec.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'Deleting a school', type: :request do - before do - authenticated_in_hydra_as(owner) - end - - let(:headers) { { Authorization: UserProfileMock::TOKEN } } - let(:school) { create(:school) } - let(:owner) { create(:owner, school:) } - - it 'responds 204 No Content' do - delete("/api/schools/#{school.id}", headers:) - expect(response).to have_http_status(:no_content) - end - - it 'responds 401 Unauthorized when no token is given' do - delete "/api/schools/#{school.id}" - expect(response).to have_http_status(:unauthorized) - end - - it 'responds 403 Forbidden when the user is a school-owner for a different school' do - Role.owner.find_by(user_id: owner.id, school:).delete - school.update!(id: SecureRandom.uuid) - - delete("/api/schools/#{school.id}", headers:) - expect(response).to have_http_status(:forbidden) - end - - it 'responds 403 Forbidden when the user is a school-teacher' do - teacher = create(:teacher, school:) - authenticated_in_hydra_as(teacher) - - delete("/api/schools/#{school.id}", headers:) - expect(response).to have_http_status(:forbidden) - end - - it 'responds 403 Forbidden when the user is a school-student' do - student = create(:student, school:) - authenticated_in_hydra_as(student) - - delete("/api/schools/#{school.id}", headers:) - expect(response).to have_http_status(:forbidden) - end -end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index c8f58ca9d..fb9965129 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -492,7 +492,6 @@ it { is_expected.to be_able_to(:read, school) } it { is_expected.to be_able_to(:update, school) } - it { is_expected.to be_able_to(:destroy, school) } it 'cannot interact with an inactive school' do school.update!(archived_at: Time.current)