Refactoring

1.rb

  def validate_status_changes
    return if changes.keys == ["status"]
    return if changes["status"].present? && changes["status"].second == "open"
    return if changes["status"].present? && changes["status"].first == "open"

    errors.add_to_base(:cant_update_locked_deliverable) if locked?
    errors.add_to_base(:cant_update_closed_deliverable) if closed?
  end

2.rb

  def validate_status_changes
    if changes.keys == ["status"]
      noop("Allow changes to the status only")
    elsif changes["status"].present? && changes["status"].second == "open"
      noop("Allow any changes when going to 'open'")
    elsif changes["status"].present? && changes["status"].first == "open"
      noop("Allow any changes when going from 'open' to another status")
    else
      errors.add_to_base(:cant_update_locked_deliverable) if locked?
      errors.add_to_base(:cant_update_closed_deliverable) if closed?
    end
  end

class << changes
  def open?
    ["status"] == self.keys ||
      self["status"].present? && ("open" == self["status"].second || "open" == self["status"].first)
  end
end

def validate_status_changes
  return if changes.open?

  errors.add_to_base(:cant_update_locked_deliverable) if locked?
  errors.add_to_base(:cant_update_closed_deliverable) if closed?
end

not literally, necessarily - would prefer seeing it on some kind of object

Interesting, didn't think about defining a method on the changes object (it's from ActiveRecord's dirty tracking). I'll try something like that out and see (open? is used in the main class so I don't want to reuse that method name here).

+1 to Giles' version. I'm a big fan of the "extracting boolean expressions into predicate methods" idiom.

Can't get that version to work. changes is an instance method that isn't available.

undefined local variable or method `changes' for #Class:0xb5cfa6b0 (NameError)

That got me unblocked, here is what I came up with (tests all green)

  def valid_status_change?
    change_to_status_only? || changing_to_the_open_status? || changing_from_the_open_status?
  end

  def change_to_status_only?
    ["status"] == changes.keys
  end

  def changing_to_the_open_status?
    changes["status"].present? && "open" == changes["status"].second
  end

  def changing_from_the_open_status?
    changes["status"].present? && "open" == changes["status"].first
  end

  def validate_status_changes
    return if valid_status_change?

    errors.add_to_base(:cant_update_locked_deliverable) if locked?
    errors.add_to_base(:cant_update_closed_deliverable) if closed?
  end