Make course syllabus optional when copying courses

fixes #9185

This adds a new version of export files and ensures
older exports will work. We are now using a
canvas_export.txt resource to identify if this is a
canvas cartridge or a common cartridge.

Test Plan
1. Copy a course
2. When copying, unselect copy all and course syllabus
3. The syllabus shouldn't be copied over.

Change-Id: I37c9aa12aabe453ef4481c6f39b7b33c97b130bb
Reviewed-on: https://gerrit.instructure.com/14971
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>

app/models/content_export.rb

@@ -109,6 +109,11 @@ class ContentExport < ActiveRecord::Base
     self.settings[:selected_content] ||= {}
   end
 
+  # Method Summary
+  #   Takes in an ActiveRecord object. Determines if the item being 
+  #   checked should be exported or not. 
+  #
+  #   Returns: bool
   def export_object?(obj)
     return false unless obj
     return true if selected_content.empty?
@@ -123,6 +128,16 @@ class ContentExport < ActiveRecord::Base
     false
   end
 
+  # Method Summary
+  #   Takes a symbol containing the items that were selected to export.
+  #   is_set? will return true if the item is selected. Also handles
+  #   a case where 'everything' is set and returns true
+  #   
+  # Returns: bool
+  def export_symbol?(symbol)
+    is_set?(selected_content[symbol]) || is_set?(selected_content[:everything])
+  end
+
   def add_item_to_export(obj)
     return unless obj && obj.class.respond_to?(:table_name)
     return if selected_content.empty?

app/models/course.rb

@@ -1795,7 +1795,7 @@ class Course < ActiveRecord::Base
       end
     end
 
-    migration.fast_update_progress(30)
+    migration.fast_update_progress(31)
     question_data = AssessmentQuestion.process_migration(data, migration); migration.fast_update_progress(35)
     Group.process_migration(data, migration); migration.fast_update_progress(36)
     LearningOutcome.process_migration(data, migration); migration.fast_update_progress(37)
@@ -1821,10 +1821,17 @@ class Course < ActiveRecord::Base
     CalendarEvent.process_migration(data, migration);migration.fast_update_progress(90)
     WikiPage.process_migration_course_outline(data, migration);migration.fast_update_progress(95)
 
-    if !migration.copy_options || migration.is_set?(migration.copy_options[:everything]) || migration.is_set?(migration.copy_options[:all_course_settings])
+    everything_selected = !migration.copy_options || migration.is_set?(migration.copy_options[:everything])
+    if everything_selected || migration.is_set?(migration.copy_options[:all_course_settings])
       import_settings_from_migration(data, migration); migration.fast_update_progress(96)
     end
 
+    syllabus_should_be_added = everything_selected || migration.copy_options[:syllabus_body]
+    if syllabus_should_be_added
+      syllabus_body = data[:course][:syllabus_body] if data[:course]
+      import_syllabus_from_migration(syllabus_body) if syllabus_body
+    end
+
     begin
       #Adjust dates
       if bool_res(params[:copy][:shift_dates])
@@ -1875,10 +1882,13 @@ class Course < ActiveRecord::Base
   attr_accessor :imported_migration_items, :full_migration_hash, :external_url_hash, :content_migration
   attr_accessor :folder_name_lookups, :attachment_path_id_lookup, :attachment_path_id_lookup_lower, :assignment_group_no_drop_assignments
 
+  def import_syllabus_from_migration(syllabus_body)
+    self.syllabus_body = ImportedHtmlConverter.convert(syllabus_body, self) 
+  end
+
   def import_settings_from_migration(data, migration)
     return unless data[:course]
     settings = data[:course]
-    self.syllabus_body = ImportedHtmlConverter.convert(settings[:syllabus_body], self) if settings[:syllabus_body]
     if settings[:tab_configuration] && settings[:tab_configuration].is_a?(Array)
       self.tab_configuration = settings[:tab_configuration]
     end

app/views/content_imports/_copy_course_item_selection.html.erb

@@ -1,5 +1,10 @@
 <h3><%= check_box :copy, :all_course_settings, :class => "copy_all", :checked => false %><%= label :copy, :all_course_settings, image_tag('file_multiple.png') + " " + t('labels.copy_settings', "Settings from %{course}", :course => @source_course.name) %></h3>
 
+<h3>
+  <%= check_box :copy, :syllabus_body, :class => "copy_all", :checked => false %>
+  <%= label :copy, :syllabus_body, image_tag('file_multiple.png') + " " + t('labels.copy_syllabus', "Syllabus description from %{course}", :course => @source_course.name) %>
+</h3>
+
 <% if @source_course.assignment_groups.active.length > 0 %>
   <h3><%= check_box :copy, :all_assignments, :class => "copy_all", :checked => true %><%= label :copy, :all_assignments, image_tag('assignment.png') + " " + t('labels.assignment', "Assignments for %{course}", :course => @source_course.name) %></h3>
   <ul class="unstyled_list root_asset_list">

lib/canvas/migration/package_identifier.rb

@@ -32,6 +32,8 @@ module Canvas::Migration
         if get_node_val(doc, 'metadata schema') =~ /IMS Common Cartridge/i
           if !!doc.at_css(%{resources resource[href="#{CC::CCHelper::COURSE_SETTINGS_DIR}/#{CC::CCHelper::SYLLABUS}"] file[href="#{CC::CCHelper::COURSE_SETTINGS_DIR}/#{CC::CCHelper::COURSE_SETTINGS}"]})
             :canvas_cartridge
+          elsif !!doc.at_css(%{resources resource[href="#{CC::CCHelper::COURSE_SETTINGS_DIR}/#{CC::CCHelper::CANVAS_EXPORT_FLAG}"]})
+            :canvas_cartridge
           elsif get_node_val(doc, 'metadata schemaversion') == "1.0.0"
             :common_cartridge_1_0
           elsif get_node_val(doc, 'metadata schemaversion') == "1.1.0"
@@ -81,7 +83,5 @@ module Canvas::Migration
       end
       raise "Unsupported content package"
     end
-
-
   end
-end
+end

lib/cc/canvas_resource.rb

@@ -29,10 +29,9 @@ module CC
       migration_id = create_key(@course)
       
       @canvas_resource_dir = File.join(@export_dir, CCHelper::COURSE_SETTINGS_DIR)
+      canvas_export_path = File.join(CCHelper::COURSE_SETTINGS_DIR, CCHelper::CANVAS_EXPORT_FLAG)
       FileUtils::mkdir_p @canvas_resource_dir
       
-      syl_rel_path = create_syllabus
-      
       resources = []
       resources << run_and_set_progress(:create_course_settings, nil, I18n.t('course_exports.errors.course_settings', "Failed to export course settings"), migration_id)
       resources << run_and_set_progress(:create_module_meta, nil, I18n.t('course_exports.errors.module_meta', "Failed to export module meta data"))
@@ -44,17 +43,53 @@ module CC
       resources << run_and_set_progress(:files_meta_path, nil, I18n.t('course_exports.errors.file_meta', "Failed to export file meta data"))
       resources << run_and_set_progress(:create_events, 25, I18n.t('course_exports.errors.events', "Failed to export calendar events"))
       
+
+      # Create the syllabus resource
+      if export_symbol?(:syllabus_body)
+        syl_rel_path = create_syllabus
+        @resources.resource(
+          :identifier => migration_id + "_syllabus",
+          "type" => Manifest::LOR,
+          :href => syl_rel_path,
+          :intendeduse => "syllabus"
+        ) do |res|
+          res.file(:href=>syl_rel_path)
+        end
+      end
+
+      create_canvas_export_flag
+
+      # Create other resources
       @resources.resource(
-              :identifier => migration_id,
-              "type" => Manifest::LOR,
-              :href => syl_rel_path,
-              :intendeduse => "syllabus"
+        :identifier => migration_id,
+        "type" => Manifest::LOR,
+        :href => canvas_export_path
       ) do |res|
-        res.file(:href=>syl_rel_path)
+
         resources.each do |resource|
           res.file(:href=>resource) if resource
         end
+
+        res.file(:href => canvas_export_path)
       end
+
+    end
+
+    # Method Summary
+    #   The canvas export flag is just a txt file we can use to 
+    #   verify this is a canvas flavor of common cartridge. We 
+    #   do this because we can't change the structure of the xml
+    #   but still need some type of flag.
+    def create_canvas_export_flag
+      path = File.join(@canvas_resource_dir, 'canvas_export.txt')
+      canvas_export_file = File.open(path, 'w')
+
+      # Fun panda joke!
+      canvas_export_file << <<-JOKE
+Q: What did the panda say when he was forced out of his natural habitat?
+A: This is un-BEAR-able
+JOKE
+      canvas_export_file.close
     end
     
     def create_syllabus(io_object=nil)

lib/cc/cc_exporter.rb

@@ -99,6 +99,10 @@ module CC
     def export_object?(obj)
       @content_export ? @content_export.export_object?(obj) : true
     end
+
+    def export_symbol?(obj)
+      @content_export ? @content_export.export_symbol?(obj) : true
+    end
     
     private
     
@@ -141,4 +145,4 @@ module CC
     end
 
   end
-end
+end

lib/cc/cc_helper.rb

@@ -83,6 +83,7 @@ module CCHelper
   WEB_RESOURCES_FOLDER = 'web_resources'
   WIKI_FOLDER = 'wiki_content'
   MEDIA_OBJECTS_FOLDER = 'media_objects'
+  CANVAS_EXPORT_FLAG = 'canvas_export.txt'
   
   def create_key(object, prepend="")
     CCHelper.create_key(object, prepend)

lib/cc/importer/canvas/course_settings.rb

@@ -34,7 +34,9 @@ module CC::Importer::Canvas
     
     def convert_all_course_settings
       @course[:course] = convert_course_settings(settings_doc(COURSE_SETTINGS))
-      @course[:course][:syllabus_body] = convert_syllabus(settings_doc(SYLLABUS, true))
+      if doc = settings_doc(SYLLABUS, true)
+        @course[:course][:syllabus_body] = convert_syllabus(doc)
+      end
       @course[:assignment_groups] = convert_assignment_groups(settings_doc(ASSIGNMENT_GROUPS))
       @course[:external_tools] = convert_external_tools(settings_doc(EXTERNAL_TOOLS))
       @course[:external_feeds] = convert_external_feeds(settings_doc(EXTERNAL_FEEDS))

lib/cc/manifest.rb

@@ -20,7 +20,7 @@ module CC
     include CCHelper
     
     attr_accessor :exporter, :weblinks, :basic_ltis
-    delegate :add_error, :set_progress, :export_object?, :for_course_copy, :add_item_to_export, :user, :to => :exporter
+    delegate :add_error, :set_progress, :export_object?, :export_symbol?, :for_course_copy, :add_item_to_export, :user, :to => :exporter
 
     def initialize(exporter)
       @exporter = exporter

lib/cc/resource.rb

@@ -28,7 +28,7 @@ module CC
     include WebLinks
     include BasicLTILinks
     
-    delegate :add_error, :set_progress, :export_object?, :for_course_copy, :add_item_to_export, :to => :@manifest
+    delegate :add_error, :set_progress, :export_object?, :export_symbol?, :for_course_copy, :add_item_to_export, :to => :@manifest
     delegate :referenced_files, :to => :@html_exporter
 
     def initialize(manifest, manifest_node)

spec/apis/v1/courses_api_spec.rb

@@ -1130,16 +1130,20 @@ describe ContentImportsController, :type => :integration do
   end
   
   it "should only copy course settings" do
-    run_only_copy(:course_settings)
-    check_counts 0
-    @copy_to.reload
-    @copy_to.syllabus_body.should == "<p>haha</p>"
+    @copy_from.default_view = 'modules' 
+    @copy_from.save!
+    run_only_copy(:course_settings) 
+    check_counts 0 
+    @copy_to.reload 
+    @copy_to.default_view.should == 'modules'
   end
+
   it "should only copy wiki pages" do
     run_only_copy(:wiki_pages)
     check_counts 0
     @copy_to.wiki.wiki_pages.count.should == 1
   end
+
   each_copy_option do |option, association|
     it "should only copy #{option}" do
       pending if !Qti.qti_enabled? && association == :quizzes

spec/lib/canvas/migration_spec.rb

@@ -10,6 +10,7 @@ describe "Migration package importers" do
     end
     
     supported = {
+            "Old Canvas Cartridge" => [:old_canvas, CC::Importer::Canvas::Converter],
             "Canvas Cartridge" => [:canvas, CC::Importer::Canvas::Converter],
             "Common Cartridge 1.0" => ["cc1-0", CC::Importer::Standard::Converter],
             "Common Cartridge 1.1" => ["cc1-1", CC::Importer::Standard::Converter],
@@ -63,4 +64,4 @@ describe "Migration package importers" do
     end
   end
   
-end
+end

spec/lib/cc/cc_exporter_spec.rb

@@ -329,5 +329,30 @@ describe "Common Cartridge exporting" do
       doc.at_css("assignmentGroup[identifier=#{mig_id(@ag2)}]").should_not be_nil
     end
 
+    it "should not export syllabus if not selected" do
+      @course.syllabus_body = "<p>Bodylicious</p>"
+
+      run_export
+      @manifest_doc.at_css('resource[href="course_settings/syllabus.html"]').should be_nil
+    end
+
+    it "should export syllabus when selected" do 
+      @course.syllabus_body = "<p>Bodylicious</p>"
+
+      @ce.selected_content = {
+        :syllabus_body => "1"
+      }
+      @ce.save!
+
+      run_export
+      @manifest_doc.at_css('resource[href="course_settings/syllabus.html"]').should_not be_nil
+    end
+
+    it "should use canvas_export.txt as flag" do
+      run_export
+
+      @manifest_doc.at_css('resource[href="course_settings/canvas_export.txt"]').should_not be_nil
+      @zip_file.find_entry('course_settings/canvas_export.txt').should_not be_nil
+    end
   end
 end

spec/models/content_migration_spec.rb

@@ -76,6 +76,7 @@ describe ContentMigration do
 
     it "should migrate syllabus links on copy" do
       course_model
+
       topic = @copy_from.discussion_topics.create!(:title => "some topic", :message => "<p>some text</p>")
       @copy_from.syllabus_body = "<a href='/courses/#{@copy_from.id}/discussion_topics/#{topic.id}'>link</a>"
       @copy_from.save!
@@ -88,6 +89,31 @@ describe ContentMigration do
       @copy_to.syllabus_body.should match(/\/courses\/#{@copy_to.id}\/discussion_topics\/#{new_topic.id}/)
     end
 
+    it "should copy course syllabus when the everything option is selected" do 
+      course_model
+
+      @copy_from.syllabus_body = "What up"
+      @copy_from.save!
+
+      run_course_copy
+
+      @copy_to.syllabus_body.should =~ /#{@copy_from.syllabus_body}/
+    end
+
+    it "should not migrate syllabus when not selected" do
+      course_model
+      @copy_from.syllabus_body = "<p>wassup</p>"
+
+      @cm.copy_options = {
+        :course => {'syllabus_body' => false}
+      }
+      @cm.save!
+
+      run_course_copy
+
+      @copy_to.syllabus_body.should == nil
+    end
+
     def make_grading_standard(context)
       gs = context.grading_standards.new
       gs.title = "Standard eh"
@@ -125,19 +151,11 @@ describe ContentMigration do
       @copy_from.grading_standard_enabled = true
       @copy_from.save!
 
-      body_with_link = %{<p>Watup? <strong>eh?</strong><a href="/courses/%s/assignments">Assignments</a></p>
-  <div>
-    <div><img src="http://www.instructure.com/images/header-logo.png"></div>
-    <div><img src="http://www.instructure.com/images/header-logo.png"></div>
-  </div>}
-      @copy_from.syllabus_body = body_with_link % @copy_from.id
-
       run_course_copy
 
       #compare settings
       @copy_to.conclude_at.should == nil
       @copy_to.start_at.should == nil
-      @copy_to.syllabus_body.should == (body_with_link % @copy_to.id)
       @copy_to.storage_quota.should == 444
       @copy_to.settings[:hide_final_grade].should == true
       @copy_to.grading_standard_enabled.should == true
@@ -838,6 +856,7 @@ describe ContentMigration do
 
     it "items in the root folder should be in the root in the new course" do
       att = Attachment.create!(:filename => 'dummy.txt', :uploaded_data => StringIO.new('fakety'), :folder => Folder.root_folders(@copy_from).first, :context => @copy_from)
+
       @copy_from.syllabus_body = "<a href='/courses/#{@copy_from.id}/files/#{att.id}/download?wrap=1'>link</a>"
       @copy_from.save!
 
@@ -853,6 +872,7 @@ describe ContentMigration do
 
     it "should preserve media comment links" do
       pending unless Qti.qti_enabled?
+
       @copy_from.media_objects.create!(:media_id => '0_12345678')
       @copy_from.syllabus_body = <<-HTML.strip
       <p>
@@ -1392,7 +1412,6 @@ equation: <img class="equation_image" title="Log_216" src="/equation_images/Log_
         @copy_to.assignments.count.should == 0
         @copy_to.quizzes.count.should == 0
         @copy_to.discussion_topics.count.should == 0
-
         @cm.content_export.error_messages.should == [
                 ["The assignment \"lock locky\" could not be copied because it is locked.", nil],
                 ["The topic \"topic\" could not be copied because it is locked.", nil],

spec/selenium/course_copy_spec.rb

@@ -216,7 +216,6 @@ describe "course copy" do
 
     it "should not copy course settings if not checked" do
       @second_course = Course.create!(:name => 'second course')
-      @second_course.syllabus_body = "<p>haha</p>"
       @second_course.tab_configuration = [{"id" => 0}, {"id" => 14}, {"id" => 8}, {"id" => 5}, {"id" => 6}, {"id" => 2}, {"id" => 3, "hidden" => true}]
       @second_course.default_view = 'modules'
 
@@ -225,11 +224,22 @@ describe "course copy" do
         wait_for_ajaximations
       end
 
-      @course.syllabus_body.should == nil
       @course.tab_configuration.should == []
       @course.default_view.should == 'feed'
     end
 
+    it "should not copy syllabus body if not selected" do
+      @second_course = Course.create!(:name => 'second course')
+      @second_course.syllabus_body = "<p>haha</p>"
+
+      course_copy_helper do
+        f('#copy_everything').click
+        wait_for_ajaximations
+      end
+
+      @course.syllabus_body.should == nil
+    end
+
     it "should correctly copy content from a completed course" do
       course_with_teacher_logged_in
       @course.wiki.wiki_pages.count.should == 0
@@ -290,4 +300,4 @@ describe "course copy" do
       upload_helper(true)
     end
   end
-end
+end