Can now delete attachments via the attachment manager.

app/assets/javascripts/cms/attachment_manager.js.erb

@@ -1,4 +1,13 @@
+// Allows users to upload files via AJAX as attachments for a given block.
+//
 $(function () {
+
+    // Return the authenticity token for any JS function that needs to do AJAX.
+    // Since AJAX posts will fail if you don't attach this as defined here: http://michaelhendrickx.com/201012_jquery-ajax-with-rails-authenticity-token.html
+    $.cms.auth_token = function () {
+        return $('meta[name=csrf-token]').attr('content');
+    };
+
     $.cms.AttachmentManager = {
         upload:function (file) {
             var assetName = $('#asset_types').val()
@@ -34,13 +43,17 @@ $(function () {
 
         },
 
+        // @param [Integer] id The id of the attachment to delete.
         destroy:function (id) {
-            if (confirm("Are you sure?")) {
-                $.post('/cms/attachments/' + id, {_method:'delete'}, null, 'script');
-                $("#asset_" + id).hide();
-                if ($("#assets_table > table tr:visible").length <= 2) {
-                    $("#assets_table > table").hide();
-                }
+            if (confirm("Are you sure want to delete this attachment?")) {
+                $.post('/cms/attachments/' + id, {_method:'delete', authenticity_token:$.cms.auth_token()}, function (attachment_id) {
+                    console.log(attachment_id);
+                    $("#attachment_" + attachment_id).hide();
+                    if ($("#assets_table > table tr:visible").length <= 2) {
+                        $("#assets_table > table").hide();
+                    }
+                }, 'script');
+
             }
             return false;
         }

app/controllers/cms/attachments_controller.rb

@@ -24,7 +24,7 @@ def create
     def destroy
       @attachment = Attachment.find(params[:id])
       @attachment.destroy
-      render :nothing => true
+      render :json => @attachment.id
     end
   end
 end

app/models/cms/attachment.rb

@@ -20,6 +20,8 @@ class Attachment < ActiveRecord::Base
     before_create :set_cardinality
     belongs_to :attachable, :polymorphic => true
 
+    validates :attachment_name, :attachable_type, :presence => true
+
     include Cms::Addressable
     include Cms::Addressable::DeprecatedPageAccessors
     has_one :section_node, :as => :node, :class_name => 'Cms::SectionNode'

config/routes.rb

@@ -60,8 +60,7 @@
       put :move_to_root
     end
   end
-  match '/attachments/:id', :to => 'attachments#show', :as=>'attachment'
-  resources :attachments, :only=>[:create, :destroy]
+  resources :attachments, :only=>[:show, :create, :destroy]
 
   match '/content_library', :to=>'html_blocks#index', :as=>'content_library'
   content_blocks :html_blocks

lib/cms/behaviors/attaching.rb

@@ -170,18 +170,27 @@ def has_many_attachments(name, options = {})
           end
         end
 
-        # Find all attachments as of the given version for the specified block.
+        # Find all attachments as of the given version for the specified block. Excludes attachments that were
+        # deleted as of a version.
         #
         # @param [Integer] version_number
         # @param [Attaching] attachable The object with attachments
         # @return [Array<Cms::Attachment>]
         def attachments_as_of_version(version_number, attachable)
-          found_versions = Cms::Attachment::Version.where(:attachable_id => attachable.id).where(:attachable_type => attachable.attachable_type).where(:attachable_version => version_number).all
-          found_attachments = []
+          found_versions = Cms::Attachment::Version.where(:attachable_id => attachable.id).
+              where(:attachable_type => attachable.attachable_type).
+              where(:attachable_version => version_number).
+              order(:version).all
+          found_attachments = {}
+
+          # Compress multiple versions into a single record and exclude deleted records.
           found_versions.each do |av|
-            found_attachments << av.build_object_from_version
+            record = av.build_object_from_version
+            found_attachments[record.id] = record
           end
-          found_attachments
+          found_attachments.delete_if { |_, value| value.deleted? }
+
+          found_attachments.values
         end
 
       end
@@ -241,6 +250,7 @@ def after_build_new_version(new_version)
         def after_as_of_version()
           @attachments_as_of = self.class.attachments_as_of_version(version, self)
 
+
           # Override #attachments to return the original attachments for the current version.
           metaclass = class << self;
             self;

test/assumptions_test.rb

@@ -0,0 +1,15 @@
+require "test_helper"
+
+module Cms
+
+  # To avoid hard to debug errors associated with testing,
+  # we verify some assumptions here about the state of the database and other things.
+  class AssumptionsTest < ActiveSupport::TestCase
+
+    test "tests assume there is no data in the database before starting" do
+      assert_equal 0, Section.count
+      assert_equal 0, Page.count
+      assert_equal 0, Permission.count
+    end
+  end
+end

test/factories/attachables.rb

@@ -15,11 +15,21 @@ class VersionedAttachable < ActiveRecord::Base
     end
     m.sequence(:name) { |n| "VersionedAttachable#{n}" }
     m.after_build { |f, evaluator|
-      opts =  {:data => evaluator.attachment_file, :attachment_name => 'document' }
-      opts[:parent] = evaluator.parent if evaluator.parent  # Handle :parent=>nil
+      opts = {:data => evaluator.attachment_file, :attachment_name => 'document'}
+      opts[:parent] = evaluator.parent if evaluator.parent # Handle :parent=>nil
       opts[:data_file_path] = evaluator.attachment_file_path if evaluator.attachment_file_path
       f.attachments.build(opts)
     }
     m.publish_on_save true
   end
+
+  factory :attachment_document, :class => Cms::Attachment do |m|
+    m.attachment_name "document"
+    m.attachable_type "VersionedAttachable"
+    m.data { mock_file }
+    m.parent { find_or_create_root_section }
+    m.data_file_path "/"
+    m.publish_on_save true
+  end
+
 end

test/unit/behaviors/attaching_test.rb

@@ -357,10 +357,25 @@ def setup
       assert_equal file_contents(file2.path), file_contents(@attachable.as_of_version(2).attachments[0].full_file_location)
     end
 
-    test "" do
+    test "delete an attachment should not be found when fetching the draft version of blocks" do
+      doc = @attachable.attachments.first
+      doc.destroy
 
+      current = @attachable.as_of_draft_version()
+      assert_equal 0, current.attachments.size
     end
 
+    test "deleted attachments shouldn't be found'" do
+      @attachable.attachments << create(:attachment_document)
+      @attachable.save!
+
+      assert_equal 2, @attachable.attachments.size
+      doc = @attachable.attachments.first
+      doc.destroy
+
+      current = @attachable.as_of_draft_version()
+      assert_equal 1, current.attachments.size
+    end
 
     private
     def update_attachable_to_version2(new_path)

test/unit/models/attachment_test.rb

@@ -156,3 +156,28 @@ def attachment
     @attachment ||= Cms::Attachment.new
   end
 end
+
+class AttachmentsValidation < ActiveSupport::TestCase
+
+  def setup
+    @valid_attachment = Cms::Attachment.new
+    @valid_attachment.attachment_name = "anything"
+    @valid_attachment.attachable_type = "VersionedAttachable"
+  end
+
+  test "Valid" do
+    assert @valid_attachment.valid?
+  end
+
+  test "Must have an attachment_name" do
+    @valid_attachment.attachment_name = nil
+    refute @valid_attachment.valid?
+  end
+
+  test "Must have content_block_class" do
+    @valid_attachment.attachable_type = nil
+    refute @valid_attachment.valid?
+  end
+
+
+end

test/unit/models/user_test.rb

@@ -141,11 +141,8 @@ class UserAbleToViewTest < ActiveSupport::TestCase
       user = Cms::User.new
       assert(!user.cms_access?, "")
     end
-
-
   end
 
-
   class PageEdittingPermissions < ActiveSupport::TestCase
     def setup
       @content_editor = create(:content_editor) # Create first, so it will have permission to edit root section

todo_list.markdown

@@ -1,7 +1,7 @@
 Tasks:
 
 - [FEATURE] Multiple attachments on a single file works.
-    * [FEATURE] Make deleting attachments work. (It isn't handling auth token issues)
+    * [BUG] Remove/then add an attachment. The new one is not flagged as 'multiple'.
     * [BUG] Uploading files with spaces in them breaks things.
     * [FEATURE] Show be an attachment_manager display helper (for viewing them) or at least document it.