Make Lesson Deletions Hard Deletions

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

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

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
 

app/views/api/lessons/_lesson.json.jbuilder

@@ -11,7 +11,6 @@ json.call(
   :description,
   :visibility,
   :due_date,
-  :archived_at,
   :created_at,
   :updated_at
 )

spec/features/lesson/archiving_a_lesson_spec.rb → 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

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)

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)