From d25d9e23bce8edc193c30b62fda5e7876ae363cc Mon Sep 17 00:00:00 2001
From: James Williams <jamesw@instructure.com>
Date: Fri, 15 Aug 2014 14:05:10 -0600
Subject: [PATCH] fix importing module items with missing content

fixes an issue that arose with rails 3, where
module items that were not set with content (and thus
shouldn't have been saved) were being autosaved and
breaking the entire modules page

test plan:
* import the package(s) referenced in the ticket
* the modules page should still be functional

fixes #CNVS-14777

Change-Id: I82fe287a4db9492489461e971911a0f7c15fbcb0
Reviewed-on: https://gerrit.instructure.com/39404
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
---
 .../importers/context_module_importer.rb      |  2 +-
 ...192313_fix_content_tags_without_content.rb | 10 +++++++
 .../fix_content_tags_without_content.rb       |  7 +++++
 .../fix_content_tags_without_content_spec.rb  | 16 +++++++++++
 .../importers/context_module_importer_spec.rb | 27 +++++++++++++++++++
 5 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20140815192313_fix_content_tags_without_content.rb
 create mode 100644 lib/data_fixup/fix_content_tags_without_content.rb
 create mode 100644 spec/migrations/fix_content_tags_without_content_spec.rb

diff --git a/app/models/importers/context_module_importer.rb b/app/models/importers/context_module_importer.rb
index 0e4390a1bde..9e3559f2597 100644
--- a/app/models/importers/context_module_importer.rb
+++ b/app/models/importers/context_module_importer.rb
@@ -93,7 +93,7 @@ module Importers
       hash[:migration_id] ||= Digest::MD5.hexdigest(hash[:title]) if hash[:title]
       existing_item = context_module.content_tags.find_by_id(hash[:id]) if hash[:id].present?
       existing_item ||= context_module.content_tags.find_by_migration_id(hash[:migration_id]) if hash[:migration_id]
-      existing_item ||= context_module.content_tags.new(:context => context)
+      existing_item ||= ContentTag.new(:context_module => context_module, :context => context)
       if hash[:workflow_state] == 'unpublished'
         existing_item.workflow_state = 'unpublished'
       else
diff --git a/db/migrate/20140815192313_fix_content_tags_without_content.rb b/db/migrate/20140815192313_fix_content_tags_without_content.rb
new file mode 100644
index 00000000000..4d43b930a94
--- /dev/null
+++ b/db/migrate/20140815192313_fix_content_tags_without_content.rb
@@ -0,0 +1,10 @@
+class FixContentTagsWithoutContent < ActiveRecord::Migration
+  tag :postdeploy
+
+  def up
+    DataFixup::FixContentTagsWithoutContent.send_later_if_production(:run)
+  end
+
+  def down
+  end
+end
diff --git a/lib/data_fixup/fix_content_tags_without_content.rb b/lib/data_fixup/fix_content_tags_without_content.rb
new file mode 100644
index 00000000000..c43fbcbb234
--- /dev/null
+++ b/lib/data_fixup/fix_content_tags_without_content.rb
@@ -0,0 +1,7 @@
+module DataFixup::FixContentTagsWithoutContent
+  def self.run
+    while ContentTag.where("context_module_id IS NOT NULL AND content_id IS NULL AND
+                            content_type IS NULL AND tag_type = ?", "default").limit(1000).delete_all > 0
+    end
+  end
+end
\ No newline at end of file
diff --git a/spec/migrations/fix_content_tags_without_content_spec.rb b/spec/migrations/fix_content_tags_without_content_spec.rb
new file mode 100644
index 00000000000..b7eeb43a69f
--- /dev/null
+++ b/spec/migrations/fix_content_tags_without_content_spec.rb
@@ -0,0 +1,16 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
+require 'db/migrate/20140815192313_fix_content_tags_without_content.rb'
+
+describe 'FixContentTagsWithoutContents' do
+  describe "up" do
+    it "should delete corrupt content tags from migrations" do
+      course
+      mod = @course.context_modules.create!
+      tag = mod.content_tags.new(:context => @course)
+      tag.save(:validate => false)
+      FixContentTagsWithoutContent.new.up
+
+      ContentTag.find_by_id(tag.id).should be_nil
+    end
+  end
+end
diff --git a/spec/models/importers/context_module_importer_spec.rb b/spec/models/importers/context_module_importer_spec.rb
index 64acfbdda1b..79953e5bd49 100644
--- a/spec/models/importers/context_module_importer_spec.rb
+++ b/spec/models/importers/context_module_importer_spec.rb
@@ -109,4 +109,31 @@ describe "Importing modules" do
     tag2.content.should == tool2
   end
 
+  it "should not create a blank tag if the content is not found" do
+    data = { :migration_id => "1", :title => "derp",
+             :items => [{
+                :migration_id => 'mig1',
+                :type => "linked_resource",
+                :linked_resource_title => "whatevs",
+                :linked_resource_type => "externalurl",
+                :url => "http://exmpale.com/stuff"
+              },
+              {
+                :migration_id => 'mig2',
+                :type => "linked_resource",
+                :linked_resource_title => "whatevs",
+                :linked_resource_type => "WikiPage",
+                :linked_resource_id => '2'
+              }],
+             :completion_requirements => [{:type => "must_view", :item_migration_id => "mig1"}]
+    }
+
+    course_model
+    migration = ContentMigration.new
+    mod = Importers::ContextModuleImporter.import_from_migration(data, @course, migration)
+    mod.reload
+
+    mod.content_tags.count.should == 1
+  end
+
 end