Fixed issue where attachments were not correctly saving due to versio…

…ning behavior

app/models/attachment.rb

@@ -31,8 +31,7 @@ class Attachment < ActiveRecord::Base
 
   #----- Validations -----------------------------------------------------------
 
-  validates_presence_of :temp_file, 
-    :message => "You must upload a file", :on => :create
+  validates_presence_of :temp_file, :message => "You must upload a file", :on => :create
   validates_presence_of :file_path
   validates_uniqueness_of :file_path
   validates_presence_of :section_id
@@ -55,6 +54,7 @@ def section
   end
 
   def section=(section)
+    logger.debug {"Attachment#section=(#{section})"}
     if @section != section
       dirty!
       @section_id = section ? section.id : nil

lib/cms/behaviors/attaching.rb

@@ -94,7 +94,7 @@ def process_attachment
               return false
             end              
           else
-            build_attachment if attachment.nil?  
+            build_attachment if attachment.nil?
             attachment.temp_file = attachment_file 
             set_attachment_file_path
             if attachment.file_path.blank?
@@ -137,6 +137,7 @@ def sanitize_file_path(file_path)
         end
 
         def update_attachment_if_changed
+          logger.debug {"UPDATE ATTACHMENT if changed" }
           if attachment
             attachment.archived = archived if self.class.archivable?
             if respond_to?(:revert_to_version) && revert_to_version

lib/cms/behaviors/versioning.rb

@@ -22,6 +22,7 @@ def is_versioned(options={})
           has_many :versions, :class_name  => version_class_name, :foreign_key => version_foreign_key
 
           before_validation :initialize_version
+          before_create :build_new_version
           attr_accessor :skip_callbacks
 
           attr_accessor :revert_to_version
@@ -80,7 +81,8 @@ def initialize_version
           self.version = 1 if new_record?
         end
 
-        def build_new_version
+
+        def build_new_version_and_add_to_versions_list_for_saving
           # First get the values from the draft
           attrs = draft_attributes
 
@@ -98,7 +100,7 @@ def build_new_version
         end
 
         def draft_attributes
-          # When there is no draft, we'll just copy the attibutes from this object
+          # When there is no draft, we'll just copy the attributes from this object
           # Otherwise we need to use the draft
           d = new_record? ? self : draft
           self.class.versioned_columns.inject({}) { |attrs, col| attrs[col] = d.send(col); attrs }
@@ -113,7 +115,7 @@ def default_version_comment
         end
 
         def publish_if_needed
-          logger.warn "Checking if publishing should occur. publish='#{@publish_on_save}'"
+          logger.debug { "#{self.class}#publish_if_needed. publish? = '#{!!@publish_on_save}'"}
 
           if @publish_on_save
             publish
@@ -146,21 +148,22 @@ def publish_if_needed
         # 2. If its an update, a new version is created and that is saved.
         # 3. If new record, its version is set to 1, and its published if needed.
         def create_or_update
-          logger.debug {"create_or_update called. Publishing? = #{publish_on_save}"}
+          logger.debug {"#{self.class}#create_or_update called. Published = #{!!publish_on_save}"}
           self.skip_callbacks = false
           unless different_from_last_draft?
             logger.debug {"No difference between this version and last. Skipping save"}
             self.skip_callbacks = true
             return true
           end
-          @new_version = build_new_version
+          logger.debug {"Saving #{self.class} #{self.attributes}"}
           if new_record?
             self.version = 1
             # This should call ActiveRecord::Callbacks#create_or_update, which will correctly trigger the :save callback_chain
             saved_correctly = super
             changed_attributes.clear
           else
-            logger.debug {"Updating"}               
+            logger.debug {"#{self.class}#update"}
+            build_new_version
             # Because we are 'skipping' the normal ActiveRecord update here, we must manually call the save callback chain.
             run_callbacks :save do
               saved_correctly = @new_version.save
@@ -170,6 +173,14 @@ def create_or_update
           return saved_correctly
         end
 
+        # Build a new version of this record and associate it with this record.
+        #
+        # Called as a before_create in order to correctly allow any other associations to be saved correctly.
+        # Called explicitly during update, where it will just define the new_version to be saved.
+        def build_new_version
+          @new_version = build_new_version_and_add_to_versions_list_for_saving
+          logger.debug {"New version of #{self.class}::Version is #{@new_version.attributes}"}
+        end
         # Implementation from BrowserCMS 3.1. Left for reference while tests are being fixed for Rails 3 upgrade.
         #
         #

rails3_buglist.txt

@@ -1,5 +1,12 @@
-How to adding ActiveRecord SQL logging to the console:
-    ActiveRecord::Base.logger = Logger.new(STDOUT)
+Start with: rake test:units TEST=unit/behaviors/attaching_test.rb
+Issue #1: Creating blocks with attachments is not correctly saving the default section.
+    A SectionNode is being created. However, it may be the case that Attachment#section is not correctly returning the appropriate object.
+
+    * Probably appears to be that attachmentable objects are not being saved.
+    * Default attachable is doing insert correctly, but the versions table is not being updated.
+
+    How to adding ActiveRecord SQL logging to the console:
+    require 'test/console_helper'
 
 Issue #2: (master_helper_module has gone apparently been removed.)
     rake test:units TEST=unit/behaviors/rendering_test.rb

test/console_helper.rb

@@ -0,0 +1,5 @@
+# Used to expose behavior to the console in a single statement...
+# require 'test/console_helper'
+#
+require 'test/test_file'
+Cms.activate_logging

test/test_file.rb

@@ -0,0 +1,17 @@
+module Cms
+
+  # Simplified handling for creating files for writing tests around multipart uploads.
+  module TestFile
+    FILES_DIR = File.dirname(__FILE__) + '/fixtures/multipart'
+
+    # Creates a file to test uploading to a sample file.
+    def self.new_file(file_name='foo.jpg', content_type="image/jpeg")
+      Rack::Test::UploadedFile.new("#{FILES_DIR}/#{file_name}", content_type, false)
+    end
+  end
+
+  # For activating logging on the console 
+  def self.activate_logging
+    ActiveRecord::Base.logger = Logger.new(STDOUT)
+  end
+end

test/test_helper.rb

@@ -94,11 +94,10 @@ def create_admin_user(attrs={})
     user  
   end
 
-  FILES_DIR = File.dirname(__FILE__) + '/fixtures/multipart'
-
+  require 'test_file'
   # Creates a TempFile attached to an uploaded file. Used to test attachments
   def file_upload_object(options)
-    Rack::Test::UploadedFile.new("#{FILES_DIR}/#{options[:original_filename]}", options[:content_type], false)
+    Cms::TestFile.new_file(options[:original_filename], options[:content_type])
   end
 
   def guest_group