diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 7071c1f50..59362570f 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -62,21 +62,14 @@ def update end def destroy - operation = params[:undo] == 'true' ? Lesson::Unarchive : Lesson::Archive - result = operation.call(lesson: @lesson) - - if result.success? - head :no_content - else - render json: { error: result[:error] }, status: :unprocessable_entity - end + @lesson.destroy! + head :no_content end private def filtered_lessons_scope - archive_scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived - scope = params[:school_class_id] ? archive_scope.where(school_class_id: params[:school_class_id]) : archive_scope + scope = params[:school_class_id] ? Lesson.where(school_class_id: params[:school_class_id]) : Lesson.all scope = scope.joins(:project).where(projects: { identifier: params[:project_identifier] }) if params[:project_identifier].present? scope.order(created_at: :asc) end diff --git a/app/dashboards/lesson_dashboard.rb b/app/dashboards/lesson_dashboard.rb index 2a5775315..f8e3b719b 100644 --- a/app/dashboards/lesson_dashboard.rb +++ b/app/dashboards/lesson_dashboard.rb @@ -22,7 +22,6 @@ class LessonDashboard < Administrate::BaseDashboard description: Field::String, visibility: Field::String, due_date: Field::DateTime, - archived_at: Field::DateTime, created_at: Field::DateTime, updated_at: Field::DateTime }.freeze @@ -53,7 +52,6 @@ class LessonDashboard < Administrate::BaseDashboard description visibility due_date - archived_at created_at updated_at ].freeze diff --git a/app/models/lesson.rb b/app/models/lesson.rb index 4578adbb5..e3b7a433d 100644 --- a/app/models/lesson.rb +++ b/app/models/lesson.rb @@ -1,15 +1,16 @@ # frozen_string_literal: true class Lesson < ApplicationRecord + self.ignored_columns += [:archived_at] + belongs_to :school, optional: true belongs_to :school_class, optional: true belongs_to :parent, optional: true, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :copies has_many :copies, dependent: :nullify, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :parent - has_one :project, dependent: :nullify + has_one :project, dependent: :destroy accepts_nested_attributes_for :project before_validation :assign_school_from_school_class - before_destroy -> { throw :abort } validates :user_id, presence: true validates :name, presence: true @@ -18,9 +19,6 @@ class Lesson < ApplicationRecord validate :user_has_the_school_owner_or_school_teacher_role_for_the_school validate :user_is_the_school_teacher_for_the_school_class - scope :archived, -> { where.not(archived_at: nil) } - scope :unarchived, -> { where(archived_at: nil) } - def self.users User.from_userinfo(ids: pluck(:user_id)) end @@ -34,24 +32,6 @@ def with_user [self, User.from_userinfo(ids: user_id).first] end - def archived? - archived_at.present? - end - - def archive! - return if archived? - - self.archived_at = Time.now.utc - save!(validate: false) - end - - def unarchive! - return unless archived? - - self.archived_at = nil - save!(validate: false) - end - def submitted_count return 0 unless project diff --git a/app/views/api/lessons/_lesson.json.jbuilder b/app/views/api/lessons/_lesson.json.jbuilder index f34f0f861..c46dc21dc 100644 --- a/app/views/api/lessons/_lesson.json.jbuilder +++ b/app/views/api/lessons/_lesson.json.jbuilder @@ -11,7 +11,6 @@ json.call( :description, :visibility, :due_date, - :archived_at, :created_at, :updated_at ) diff --git a/lib/concepts/lesson/operations/archive.rb b/lib/concepts/lesson/operations/archive.rb deleted file mode 100644 index 3bd8bd547..000000000 --- a/lib/concepts/lesson/operations/archive.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class Lesson - class Archive - class << self - def call(lesson:) - response = OperationResponse.new - lesson.archive! - response - rescue StandardError => e - Sentry.capture_exception(e) - response[:error] = "Error archiving lesson: #{e}" - response - end - end - end -end diff --git a/lib/concepts/lesson/operations/unarchive.rb b/lib/concepts/lesson/operations/unarchive.rb deleted file mode 100644 index f7c3170dc..000000000 --- a/lib/concepts/lesson/operations/unarchive.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class Lesson - class Unarchive - class << self - def call(lesson:) - response = OperationResponse.new - lesson.unarchive! - response - rescue StandardError => e - Sentry.capture_exception(e) - response[:error] = "Error unarchiving lesson: #{e}" - response - end - end - end -end diff --git a/spec/concepts/lesson/archive_spec.rb b/spec/concepts/lesson/archive_spec.rb deleted file mode 100644 index 14113f6b0..000000000 --- a/spec/concepts/lesson/archive_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Lesson::Archive, type: :unit do - let(:lesson) { create(:lesson) } - - it 'returns a successful operation response' do - response = described_class.call(lesson:) - expect(response.success?).to be(true) - end - - it 'archives the lesson' do - described_class.call(lesson:) - expect(lesson.reload.archived?).to be(true) - end -end diff --git a/spec/concepts/lesson/unarchive_spec.rb b/spec/concepts/lesson/unarchive_spec.rb deleted file mode 100644 index 99dac2333..000000000 --- a/spec/concepts/lesson/unarchive_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Lesson::Unarchive, type: :unit do - let(:lesson) { create(:lesson, archived_at: Time.now.utc) } - - it 'returns a successful operation response' do - response = described_class.call(lesson:) - expect(response.success?).to be(true) - end - - it 'unarchives the lesson' do - described_class.call(lesson:) - expect(lesson.reload.archived?).to be(false) - end -end diff --git a/spec/features/lesson/archiving_a_lesson_spec.rb b/spec/features/lesson/destroying_a_lesson_spec.rb similarity index 80% rename from spec/features/lesson/archiving_a_lesson_spec.rb rename to spec/features/lesson/destroying_a_lesson_spec.rb index dd95cd434..807c1bfa3 100644 --- a/spec/features/lesson/archiving_a_lesson_spec.rb +++ b/spec/features/lesson/destroying_a_lesson_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe 'Archiving a lesson', type: :request do +RSpec.describe 'Destroying a lesson', type: :request do before do authenticated_in_hydra_as(owner) end @@ -19,23 +19,16 @@ expect(response).to have_http_status(:no_content) end - it 'responds 204 No Content if the lesson is already archived' do - lesson.archive! - - delete("/api/lessons/#{lesson.id}", headers:) - expect(response).to have_http_status(:no_content) + it 'destroys the lesson' do + lesson_id = lesson.id + delete("/api/lessons/#{lesson_id}", headers:) + expect(Lesson.exists?(lesson_id)).to be(false) end - it 'archives the lesson' do + it 'destroys the associated project' do + project_id = lesson.project.id delete("/api/lessons/#{lesson.id}", headers:) - expect(lesson.reload.archived?).to be(true) - end - - it 'unarchives the lesson when the ?undo=true query parameter is set' do - lesson.archive! - - delete("/api/lessons/#{lesson.id}?undo=true", headers:) - expect(lesson.reload.archived?).to be(false) + expect(Project.exists?(project_id)).to be(false) end it 'responds 401 Unauthorized when no token is given' do diff --git a/spec/features/lesson/listing_lessons_spec.rb b/spec/features/lesson/listing_lessons_spec.rb index 30817538e..4128991f9 100644 --- a/spec/features/lesson/listing_lessons_spec.rb +++ b/spec/features/lesson/listing_lessons_spec.rb @@ -61,24 +61,6 @@ expect(data.first[:user_name]).to be_nil end - it 'does not include archived lessons' do - lesson.archive! - - get('/api/lessons', headers:) - data = JSON.parse(response.body, symbolize_names: true) - - expect(data.size).to eq(0) - end - - it 'includes archived lessons if ?include_archived=true is set' do - lesson.archive! - - get('/api/lessons?include_archived=true', headers:) - data = JSON.parse(response.body, symbolize_names: true) - - expect(data.size).to eq(1) - end - it 'does not include lessons with no class if school_class_id provided' do get("/api/lessons?school_class_id=#{school_class.id}", headers:) data = JSON.parse(response.body, symbolize_names: true) @@ -103,24 +85,6 @@ expect(data.size).to eq(1) end - it 'defaults to not including archived lessons from the class if school_class_id provided' do - lesson.archive! - lesson.update!(school_class_id: school_class.id) - get("/api/lessons?school_class_id=#{school_class.id}", headers:) - data = JSON.parse(response.body, symbolize_names: true) - - expect(data.size).to eq(0) - end - - it 'includes archived lessons from class if include_archived=true and school_class_id provided' do - lesson.archive! - lesson.update!(school_class_id: school_class.id) - get("/api/lessons?include_archived=true&school_class_id=#{school_class.id}", headers:) - data = JSON.parse(response.body, symbolize_names: true) - - expect(data.size).to eq(1) - end - it 'includes the submitted_count for each lesson' do lesson.update!(school_class_id: school_class.id) remix = create(:project, school:, remixed_from_id: lesson.project.id, user_id: student.id) diff --git a/spec/models/lesson_spec.rb b/spec/models/lesson_spec.rb index 05d7b3568..c5e581e87 100644 --- a/spec/models/lesson_spec.rb +++ b/spec/models/lesson_spec.rb @@ -41,9 +41,16 @@ end describe 'callbacks' do - it 'cannot be destroyed and should be archived instead' do + it 'can be destroyed' do lesson = create(:lesson) - expect { lesson.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed) + expect { lesson.destroy! }.not_to raise_error + end + + it 'destroys associated project' do + lesson = create(:lesson) + project_id = lesson.project.id + lesson.destroy! + expect(Project.exists?(project_id)).to be(false) end end @@ -112,32 +119,6 @@ end end - describe '.archived' do - let!(:archived_lesson) { create(:lesson, archived_at: Time.now.utc) } - let!(:unarchived_lesson) { create(:lesson) } - - it 'includes archived lessons' do - expect(described_class.archived).to include(archived_lesson) - end - - it 'excludes unarchived lessons' do - expect(described_class.archived).not_to include(unarchived_lesson) - end - end - - describe '.unarchived' do - let!(:archived_lesson) { create(:lesson, archived_at: Time.now.utc) } - let!(:unarchived_lesson) { create(:lesson) } - - it 'includes unarchived lessons' do - expect(described_class.unarchived).to include(unarchived_lesson) - end - - it 'excludes archived lessons' do - expect(described_class.unarchived).not_to include(archived_lesson) - end - end - describe '#school' do let(:school) { create(:school) } @@ -225,69 +206,6 @@ end end - describe '#archive!' do - let(:lesson) { build(:lesson) } - - it 'archives the lesson' do - lesson.archive! - expect(lesson.archived?).to be(true) - end - - it 'sets archived_at' do - lesson.archive! - expect(lesson.archived_at).to be_present - end - - it 'does not set archived_at if it was already set' do - lesson.update!(archived_at: 1.day.ago) - - lesson.archive! - expect(lesson.archived_at).to be < 23.hours.ago - end - - it 'saves the record' do - lesson.archive! - expect(lesson).to be_persisted - end - - it 'is infallible to other validation errors' do - lesson.save! - lesson.name = ' ' - lesson.save!(validate: false) - - lesson.archive! - expect(lesson.archived?).to be(true) - end - end - - describe '#unarchive!' do - let(:lesson) { build(:lesson, archived_at: Time.now.utc) } - - it 'unarchives the lesson' do - lesson.unarchive! - expect(lesson.archived?).to be(false) - end - - it 'clears archived_at' do - lesson.unarchive! - expect(lesson.archived_at).to be_nil - end - - it 'saves the record' do - lesson.unarchive! - expect(lesson).to be_persisted - end - - it 'is infallible to other validation errors' do - lesson.archive! - lesson.name = ' ' - lesson.save!(validate: false) - - lesson.unarchive! - expect(lesson.archived?).to be(false) - end - end - describe '#submitted_count' do it 'returns 0 if there is no project' do lesson = create(:lesson, project: nil)