diff --git a/Changelog.md b/Changelog.md index 27c2a9277b..e604fedd83 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ ## [unreleased] - Allow deletion of assignments with no groups (#6880) - Add new routes for `update`, `show`, and `index` actions of the Sections API Controller (#6955) +- Enable the deletion of Grade Entry Forms that have no grades (#6915) ## [v2.4.5] - Add workaround for CSP in Safari < 16 (#6947) diff --git a/app/controllers/api/grade_entry_forms_controller.rb b/app/controllers/api/grade_entry_forms_controller.rb index 6bed92aa88..efa47903e2 100644 --- a/app/controllers/api/grade_entry_forms_controller.rb +++ b/app/controllers/api/grade_entry_forms_controller.rb @@ -179,5 +179,26 @@ def update_grades end end end + + def destroy + # check if the grade entry form exists + grade_entry_form = record + if grade_entry_form.nil? + render 'shared/http_status', locals: { code: '404', message: + 'Grade Entry Form not found' }, status: :not_found + return + end + # delete the grade entry form + begin + grade_entry_form.destroy! + render 'shared/http_status', + locals: { code: '200', + message: 'Grade Entry Form successfully deleted' }, status: :ok + rescue ActiveRecord::RecordNotDestroyed + render 'shared/http_status', + locals: { code: :conflict, + message: 'Grade Entry Form contains non-nil grades' }, status: :conflict + end + end end end diff --git a/app/controllers/grade_entry_forms_controller.rb b/app/controllers/grade_entry_forms_controller.rb index 8212a48a01..aa53c264ee 100644 --- a/app/controllers/grade_entry_forms_controller.rb +++ b/app/controllers/grade_entry_forms_controller.rb @@ -299,4 +299,16 @@ def switch redirect_to student_interface_course_grade_entry_form_path(current_course, params[:id]) end end + + def destroy + @grade_entry_form = record + begin + @grade_entry_form.destroy! + redirect_to course_assignments_path(@current_course) + flash_message(:success, t('grade_entry_forms.successfully_deleted')) + rescue ActiveRecord::RecordNotDestroyed + redirect_to edit_course_grade_entry_form_path(@current_course, params[:id]) + flash_message(:error, t('grade_entry_forms.failed_deletion')) + end + end end diff --git a/app/models/grade_entry_form.rb b/app/models/grade_entry_form.rb index 3e470c7410..6630e85ec1 100644 --- a/app/models/grade_entry_form.rb +++ b/app/models/grade_entry_form.rb @@ -20,6 +20,8 @@ class GradeEntryForm < Assessment after_create :create_all_grade_entry_students + before_destroy -> { throw(:abort) if self.grades.where.not(grade: nil).exists? }, prepend: true + # Set the default order of spreadsheets: in ascending order of id default_scope { order('id ASC') } diff --git a/app/views/grade_entry_forms/_form.html.erb b/app/views/grade_entry_forms/_form.html.erb index e383a649ba..bb209ce2c8 100644 --- a/app/views/grade_entry_forms/_form.html.erb +++ b/app/views/grade_entry_forms/_form.html.erb @@ -89,3 +89,13 @@ data: { disable_with: t('working') } %>

<% end %> + +<% unless @grade_entry_form.new_record? %> + <% not_deletable = @grade_entry_form.grades.where.not(grade: nil).exists? %> + <%= button_to t('delete'), + course_grade_entry_form_path(@current_course.id, @grade_entry_form.id), + method: :delete, + title: not_deletable ? t('grade_entry_forms.grades.non_nil_grade_exists') : nil, + disabled: not_deletable + %> +<% end %> diff --git a/config/locales/views/grade_entry_forms/en.yml b/config/locales/views/grade_entry_forms/en.yml index 763386d859..6a43c51a19 100644 --- a/config/locales/views/grade_entry_forms/en.yml +++ b/config/locales/views/grade_entry_forms/en.yml @@ -4,9 +4,11 @@ en: add_column: Add a column csv: overwrite: Overwrite existing grades and columns? + failed_deletion: Failed to delete Marks Spreadsheet. grade_entry_item_distribution: Column Grade Distribution grades: marking_state: State + non_nil_grade_exists: Marks Spreadsheet contains non-nil grades. select_a_student: You must select at least one student to update. successfully_changed: Successfully updated %{numGradeEntryStudentsChanged} students. message_placeholder: Please enter a message that you would like to have displayed to the students. Note it is displayed regardless of marks being released. @@ -16,3 +18,4 @@ en: mark_message: 'Your mark:' no_results: There are no marks available yet. title: 'Marks:' + successfully_deleted: Marks Spreadsheet was successfully deleted. diff --git a/config/routes.rb b/config/routes.rb index 496455a70f..1a0d7a79ba 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,7 +26,7 @@ put 'update_by_username' end end - resources :grade_entry_forms, only: [:show, :index, :create, :update] do + resources :grade_entry_forms, only: [:show, :index, :create, :update, :destroy] do member do put 'update_grades' end @@ -390,7 +390,7 @@ end end - resources :grade_entry_forms, except: [:index, :destroy] do + resources :grade_entry_forms, except: [:index] do collection do get 'student_interface' end diff --git a/spec/controllers/api/grade_entry_forms_controller_spec.rb b/spec/controllers/api/grade_entry_forms_controller_spec.rb index 3c35065da9..c97b40ab56 100644 --- a/spec/controllers/api/grade_entry_forms_controller_spec.rb +++ b/spec/controllers/api/grade_entry_forms_controller_spec.rb @@ -313,5 +313,37 @@ expect(grade_entry_form.grade_entry_students.find_by(role: student).grades.first.grade).to eq(5) end end + context 'DELETE Destroy' do + it 'does not delete a non-existing grade entry form' do + delete :destroy, params: { course_id: course.id, id: -1 } + expect(response).to have_http_status(404) + end + it 'successfully deletes a grade entry form with no non-nil grades' do + form = create :grade_entry_form, course_id: course.id, id: 4 + first_student = create(:student) + second_student = create(:student) + grade_entry_item = create(:grade_entry_item, out_of: 10, grade_entry_form: form) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: first_student), + grade_entry_item: grade_entry_item, grade: nil) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: second_student), + grade_entry_item: grade_entry_item, grade: nil) + delete :destroy, params: { course_id: course.id, id: 4 } + expect(response).to have_http_status(:ok) + expect(course.grade_entry_forms.exists?(form.id)).to be_falsey + end + it 'does not delete a grade entry form with non-nil grades' do + form = create :grade_entry_form, course_id: course.id, id: 4 + first_student = create(:student) + second_student = create(:student) + grade_entry_item = create(:grade_entry_item, out_of: 10, grade_entry_form: form) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: first_student), + grade_entry_item: grade_entry_item, grade: nil) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: second_student), + grade_entry_item: grade_entry_item, grade: 0.2) + delete :destroy, params: { course_id: course.id, id: 4 } + expect(response).to have_http_status(:conflict) + expect(course.grade_entry_forms.exists?(form.id)).to be_truthy + end + end end end diff --git a/spec/controllers/grade_entry_forms_controller_spec.rb b/spec/controllers/grade_entry_forms_controller_spec.rb index 596b75b5aa..52a1c20a5d 100644 --- a/spec/controllers/grade_entry_forms_controller_spec.rb +++ b/spec/controllers/grade_entry_forms_controller_spec.rb @@ -729,4 +729,35 @@ end end end + context 'DELETE Destroy' do + let(:user) { create(:instructor) } + it 'does not delete a non-existing grade entry form' do + delete_as user, :destroy, params: { course_id: course.id, id: -1 } + expect(response).to have_http_status(404) + end + it 'successfully deletes a grade entry form with no non-nil grades' do + form = create :grade_entry_form, course_id: course.id, id: 4 + first_student = create(:student) + second_student = create(:student) + grade_entry_item = create(:grade_entry_item, out_of: 10, grade_entry_form: form) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: first_student), + grade_entry_item: grade_entry_item, grade: nil) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: second_student), + grade_entry_item: grade_entry_item, grade: nil) + delete_as user, :destroy, params: { course_id: course.id, id: 4 } + expect(course.grade_entry_forms.exists?(form.id)).to be_falsey + end + it 'does not delete a grade entry form with non-nil grades' do + form = create :grade_entry_form, course_id: course.id, id: 4 + first_student = create(:student) + second_student = create(:student) + grade_entry_item = create(:grade_entry_item, out_of: 10, grade_entry_form: form) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: first_student), + grade_entry_item: grade_entry_item, grade: nil) + create(:grade, grade_entry_student: form.grade_entry_students.find_by(role: second_student), + grade_entry_item: grade_entry_item, grade: 0.2) + delete_as user, :destroy, params: { course_id: course.id, id: 4 } + expect(course.grade_entry_forms.exists?(form.id)).to be_truthy + end + end end