diff --git a/app/controllers/api/roles_controller.rb b/app/controllers/api/roles_controller.rb index e2b31f3a8a..a452958bf9 100644 --- a/app/controllers/api/roles_controller.rb +++ b/app/controllers/api/roles_controller.rb @@ -115,14 +115,20 @@ def create_role ApplicationRecord.transaction do user = User.find_by(user_name: params[:user_name]) role = Role.new(**role_params, user: user, course: @current_course) - role.section = @current_course.sections.find_by(name: params[:section_name]) if params[:section_name] + if params[:section_name] + if params[:section_name].empty? + role.section = nil + else + role.section = @current_course.sections.find_by!(name: params[:section_name]) + end + end role.grace_credits = params[:grace_credits] if params[:grace_credits] role.hidden = params[:hidden].to_s.casecmp('true').zero? if params[:hidden] role.save! render 'shared/http_status', locals: { code: '201', message: HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created end - rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e + rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity rescue StandardError render 'shared/http_status', locals: { code: '500', message: @@ -131,14 +137,20 @@ def create_role def update_role(role) ApplicationRecord.transaction do - role.section = @current_course.sections.find_by(name: params[:section_name]) if params[:section_name] + if params[:section_name] + if params[:section_name].empty? + role.section = nil + else + role.section = @current_course.sections.find_by!(name: params[:section_name]) + end + end role.grace_credits = params[:grace_credits] if params[:grace_credits] role.hidden = params[:hidden].to_s.casecmp('true').zero? if params[:hidden] role.save! end render 'shared/http_status', locals: { code: '200', message: HttpStatusHelper::ERROR_CODE['message']['200'] }, status: :ok - rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e + rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity rescue StandardError render 'shared/http_status', locals: { code: '500', message: diff --git a/spec/controllers/api/roles_controller_spec.rb b/spec/controllers/api/roles_controller_spec.rb index 53d654da74..5ac84352d5 100644 --- a/spec/controllers/api/roles_controller_spec.rb +++ b/spec/controllers/api/roles_controller_spec.rb @@ -187,6 +187,22 @@ expect(response).to have_http_status(422) end end + context 'with an invalid section name' do + let(:other_params) { { section_name: 'section.name' } } + it 'should raise a 422 error' do + expect(response).to have_http_status(422) + end + end + context 'with a nil section name' do + let(:created_student) { Student.joins(:user).where('users.user_name': student.user_name).first } + let(:other_params) { { section_name: nil } } + it 'should be successful' do + expect(response).to have_http_status(201) + end + it 'should not set a section' do + expect(created_student.section).to be_nil + end + end end end @@ -240,6 +256,27 @@ student.reload expect(student.hidden).to eq(true) end + context 'with an invalid section name' do + it 'should raise a 422 error' do + put :update, params: { id: student.id, course_id: course.id, section_name: 'section.name' } + expect(response).to have_http_status(422) + end + end + context 'with a nil section name' do + let(:section) { create :section } + before :each do + student.update!(section: section) + end + it 'should be successful' do + put :update, params: { id: student.id, course_id: course.id, section_name: nil } + expect(response).to have_http_status(200) + end + it 'should set the section to nil' do + put :update, params: { id: student.id, course_id: course.id, section_name: nil } + student.reload + expect(student.section).to be_nil + end + end end context 'when updating a user that does not exist' do