Some minor cleanup to previous commit

Instead of making use_transactions a special instance method, keep
it as previously defined and add a use_transaction? private method
that takes an option hash.  I think this is a bit cleaner.

Also, make a few minor changes to the specs and update the
CHANGELOG.

CHANGELOG

@@ -1,5 +1,9 @@
 === HEAD
 
+* Allow Model#destroy to take an options hash and respect a :transaction option (john_firebaugh)
+
+* If a transaction is being used, raise_on_save_failure is false, and a before hook returns false, rollback the transaction (john_firebaugh, jeremyevans)
+
 * In the schema_dumper, explicitly specify the :type option if it isn't Integer (jeremyevans)
 
 * On postgres, use bigserial type if :type=>Bignum is given as an option to primary_key (jeremyevans)

lib/sequel/model/base.rb

@@ -506,19 +506,7 @@ def self.class_attr_reader(*meths) # :nodoc:
       private_class_method :class_attr_overridable, :class_attr_reader
 
       class_attr_reader :columns, :db, :primary_key, :db_schema
-      class_attr_overridable :raise_on_save_failure, :raise_on_typecast_failure, :strict_param_setting, :typecast_empty_string_to_nil, :typecast_on_assignment
-
-      attr_writer :use_transactions
-      def use_transactions(opts = {})
-        case
-          when opts.include?(:transaction)
-            opts[:transaction]
-          when defined?(@use_transactions)
-            @use_transactions
-          else
-            self.class.use_transactions
-        end
-      end
+      class_attr_overridable :raise_on_save_failure, :raise_on_typecast_failure, :strict_param_setting, :typecast_empty_string_to_nil, :typecast_on_assignment, :use_transactions
 
       # The hash of attribute values.  Keys are symbols with the names of the
       # underlying database columns.
@@ -619,9 +607,10 @@ def delete
       # If before_destroy returns false, returns false without
       # deleting the object the the database. Otherwise, deletes
       # the item from the database and returns self.  Uses a transaction
-      # if use_transactions is true.
+      # if use_transactions is true or if the :transaction option is given and
+      # true.
       def destroy(opts = {})
-        use_transactions(opts) ? db.transaction{_destroy(opts)} : _destroy(opts)
+        use_transaction?(opts) ? db.transaction{_destroy(opts)} : _destroy(opts)
       end
 
       # Iterates through all of the current values using each.
@@ -734,7 +723,7 @@ def reload
       def save(*columns)
         opts = columns.last.is_a?(Hash) ? columns.pop : {}
         return save_failure(:invalid) if opts[:validate] != false and !valid?
-        use_transactions(opts) ? db.transaction(opts){_save(columns, opts)} : _save(columns, opts)
+        use_transaction?(opts) ? db.transaction(opts){_save(columns, opts)} : _save(columns, opts)
       end
 
       # Saves only changed columns if the object has been modified.
@@ -908,7 +897,7 @@ def save_failure(type, opts = {})
           else
             raise BeforeHookFailed, "one of the before_#{type} hooks returned false"
           end
-        elsif type != :invalid && use_transactions(opts)
+        elsif type != :invalid && use_transaction?(opts)
           raise Rollback
         end
       end
@@ -981,6 +970,13 @@ def update_restricted(hash, only, except)
         set_restricted(hash, only, except)
         save_changes
       end
+
+      # Whether to use a transaction for this action.  If the :transaction
+      # option is present in the hash, use that, otherwise, fallback to the
+      # object's default (if set), or class's default (if not).
+      def use_transaction?(opts = {})
+        opts.include?(:transaction) ? opts[:transaction] : use_transactions
+      end
     end
 
     # Dataset methods are methods that the model class extends its dataset with in

spec/model/model_spec.rb

@@ -569,23 +569,25 @@ def @c.columns; [:x]; end
 context "Model#use_transactions" do
   before do
     @c = Class.new(Sequel::Model(:items))
-    @instance = @c.new
   end
 
   specify "should return class value by default" do
-    @instance.use_transactions.should == Sequel::Model.use_transactions
+    @c.use_transactions = true
+    @c.new.use_transactions.should == true
+    @c.use_transactions = false
+    @c.new.use_transactions.should == false
   end
 
   specify "should return set value if manually set" do
-    @instance.use_transactions = false
-    @instance.use_transactions.should == false
-    @instance.use_transactions = true
-    @instance.use_transactions.should == true
-  end
-
-  specify "should return :transaction option if given" do
-    @instance.use_transactions(:transaction => false).should == false
-    @instance.use_transactions = false
-    @instance.use_transactions(:transaction => true).should == true
+    instance = @c.new
+    instance.use_transactions = false
+    instance.use_transactions.should == false
+    @c.use_transactions = true
+    instance.use_transactions.should == false
+    
+    instance.use_transactions = true
+    instance.use_transactions.should == true
+    @c.use_transactions = false
+    instance.use_transactions.should == true
   end
 end

spec/model/record_spec.rb

@@ -236,9 +236,9 @@ def o.before_save
     o = @c.load(:id => 3, :x => 1, :y => nil)
     o.use_transactions = false
     def o.before_save
-      raise Sequel::Error::Rollback
+      raise Sequel::Rollback
     end
-    proc { o.save(:y) }.should raise_error(Sequel::Error::Rollback)
+    proc { o.save(:y) }.should raise_error(Sequel::Rollback)
     MODEL_DB.sqls.should == []
     MODEL_DB.reset
   end