exhume!() sets the stage to the previous stage, instead of Stages::AP…

…PROVED

app/models/item.rb

@@ -8,16 +8,15 @@
 # Creates generally happen via {CreateItemCommand}, updates via
 # {UpdateItemCommand}. This will ensure that an appropriate {Event} is created
 # and associated with the instance. Deleting can still be done directly on the
-# instance without use of a {Command}--although {Stages::BURIED burial} is
-# often used instead.
+# instance without use of a {Command}--although {Item#bury! burial} is often
+# used instead.
 #
 # # Lifecycle
 #
-# An item may proceed through several "life stages", indicated by the {stage}
-# attribute and documented in the {Stages} class. {Stages::APPROVED} is where
-# most items spend most of their lives, but note that an approved item may
-# still be embargoed. (An embargo is basically an access limit, which may be
-# perpetual or timed.)
+# An item typically proceeds through several "life stages", indicated by the
+# {stage} attribute and documented in the {Stages} class. {Stages::APPROVED} is
+# where most items spend most of their lives, but note that an approved item
+# may still have one more associated {Embargo embargoes}.
 #
 # # Indexing
 #
@@ -38,6 +37,10 @@
 #                             assigned. The rest of the time, this attribute is
 #                             just a shortcut to avoid having to navigate that
 #                             relationship.
+# * `previous_stage`          Previous lifecycle stage. Updates automatically
+#                             when {stage} changes.
+# * `previous_stage_reason`   Previous lifecycle stage reason. Updates
+#                             automatically when {stage} changes.
 # * `stage`                   Lifecycle stage, whose value is one of the
 #                             {Stages} constant values.
 # * `stage_reason`            Reason for setting the {stage} attribute to its
@@ -188,7 +191,7 @@ def self.label_for(value)
              optional: true
 
   before_save :email_after_submission, :prune_duplicate_elements
-  before_update :set_stage_reason
+  before_update :set_previous_stage
   before_destroy :restrict_in_archive_deletion, :destroy_bitstreams
 
   validates :temp_embargo_kind, inclusion: { in: Embargo::Kind::all },
@@ -693,7 +696,8 @@ def embargoed_for?(user:, client_ip:, client_hostname:)
   def exhume!
     if stage == Item::Stages::BURIED
       transaction do
-        update!(stage: Item::Stages::APPROVED)
+        update!(stage:        self.previous_stage || Item::Stages::APPROVED,
+                stage_reason: self.previous_stage_reason)
         Event.create!(event_type:     Event::Type::UNDELETE,
                       item:           self,
                       before_changes: nil,
@@ -945,12 +949,14 @@ def restrict_in_archive_deletion
     raise "Archived items cannot be deleted" if self.exists_in_medusa?
   end
 
-  ##
-  # Nils out {stage_reason} if the stage changed but the reason didn't. This
-  # ensures that {stage_reason} reflects the last stage change only
-  #
-  def set_stage_reason
-    self.stage_reason = nil if self.stage_changed? && !self.stage_reason_changed?
+  def set_previous_stage
+    if stage_changed?
+      self.previous_stage        = self.stage_was
+      self.previous_stage_reason = self.stage_reason_was
+      # Nil this out if the stage changed but the reason didn't, to ensure that
+      # it reflects the last stage change only.
+      self.stage_reason          = nil unless stage_reason_changed?
+    end
   end
 
   ##

db/migrate/20240311185518_add_previous_stage_column_to_items.rb

@@ -0,0 +1,6 @@
+class AddPreviousStageColumnToItems < ActiveRecord::Migration[7.1]
+  def change
+    add_column :items, :previous_stage, :integer
+    add_column :items, :previous_stage_reason, :text
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_03_08_160256) do
+ActiveRecord::Schema[7.1].define(version: 2024_03_11_185518) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
   enable_extension "unaccent"
@@ -483,6 +483,8 @@
     t.integer "temp_embargo_kind"
     t.bigint "institution_id"
     t.text "deposit_agreement"
+    t.integer "previous_stage"
+    t.text "previous_stage_reason"
     t.index ["institution_id"], name: "index_items_on_institution_id"
     t.index ["stage"], name: "index_items_on_stage"
     t.index ["submitter_id"], name: "index_items_on_submitter_id"

test/models/item_test.rb

@@ -883,7 +883,16 @@ class ItemTest < ActiveSupport::TestCase
 
   # exhume!()
 
-  test "exhume!() sets the stage to APPROVED" do
+  test "exhume!() sets the stage to the previous stage, if set" do
+    @instance = items(:southeast_buried)
+    @instance.update!(previous_stage:        Item::Stages::REJECTED,
+                      previous_stage_reason: "Felt like it")
+    @instance.exhume!
+    assert_equal Item::Stages::REJECTED, @instance.stage
+    assert_equal "Felt like it", @instance.stage_reason
+  end
+
+  test "exhume!() sets the stage to APPROVED if there is no previous stage" do
     @instance = items(:southeast_buried)
     @instance.exhume!
     assert_equal Item::Stages::APPROVED, @instance.stage
@@ -969,6 +978,25 @@ class ItemTest < ActiveSupport::TestCase
     assert_equal expected, @instance.owning_ids
   end
 
+  # previous_stage
+
+  test "previous_stage updates when stage is changed" do
+    assert_nil @instance.previous_stage
+    stage = @instance.stage
+    @instance.update!(stage: Item::Stages::REJECTED)
+    assert_equal stage, @instance.previous_stage
+  end
+
+  # previous_stage_reason
+
+  test "previous_stage_reason updates when stage is changed" do
+    assert_nil @instance.previous_stage_reason
+    stage_reason = @instance.stage_reason
+    @instance.update!(stage:        Item::Stages::REJECTED,
+                      stage_reason: "new reason")
+    assert_equal stage_reason, @instance.previous_stage_reason
+  end
+
   # primary_unit()
 
   test "primary_unit() returns the primary unit" do