blueprint: don't undelete deleted/unlocked items on sync

- have files, quizzes, learning outcomes, outcome groups,
   and modules in a blueprint course. no items should be
   locked.
 - ensure outcome group changes appear in the unsynced
   changes list correctly
 - sync to an associated course
 - delete items in the associated course
 - change the same items in the blueprint in some way
   (i.e., rename them)
 - re-sync
 - the deleted items should not be undeleted in the
   associated course
 - non-deleted outcome groups should not gain a link
   to a deleted outcome (that shows only in the left pane)

fixes ADMIN-559

Change-Id: Ibe8842353760260b050a6330cd27df986b17b5e7
Reviewed-on: https://gerrit.instructure.com/133374
Tested-by: Jenkins
Reviewed-by: James Williams  <jamesw@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>

app/jsx/blueprint_courses/labels.js

@@ -29,6 +29,7 @@ const itemTypeLabels = {
   assessment_question_bank: I18n.t('Question Bank'),
   calendar_event: I18n.t('Event'),
   learning_outcome: I18n.t('Outcome'),
+  learning_outcome_group: I18n.t('Outcome Group'),
   rubric: I18n.t('Rubric'),
   context_external_tool: I18n.t('External Tool')
 }

app/jsx/blueprint_courses/propTypes.js

@@ -66,7 +66,7 @@ propTypes.migrationExceptionList = arrayOf(propTypes.migrationException)
 
 propTypes.migrationChange = shape({
   asset_id: string.isRequired,
-  asset_type: oneOf(['assignment', 'quiz', 'discussion_topic', 'wiki_page', 'attachment', 'context_module']).isRequired,
+  asset_type: oneOf(['assignment', 'quiz', 'discussion_topic', 'wiki_page', 'attachment', 'context_module', 'learning_outcome', 'learning_outcome_group', 'announcement']).isRequired,
   asset_name: string.isRequired,
   change_type: oneOf(['created', 'updated', 'deleted']).isRequired,
   htnl_url: string,

app/models/context.rb

@@ -199,15 +199,11 @@ module Context
   end
 
   def self.asset_name(asset)
-    if asset.respond_to?(:display_name)
-      asset.display_name
-    elsif asset.respond_to?(:title)
-      asset.title
-     elsif asset.respond_to?(:short_description)
-      asset.short_description
-    else
-      asset.name
-    end
+    name = asset.display_name.presence if asset.respond_to?(:display_name)
+    name ||= asset.title.presence if asset.respond_to?(:title)
+    name ||= asset.short_description.presence if asset.respond_to?(:short_description)
+    name ||= asset.name
+    name
   end
 
   def self.get_account(context)

app/models/importers/learning_outcome_group_importer.rb

@@ -42,6 +42,7 @@ module Importers
           where(migration_clause(hash[:migration_id])).first if hash[:migration_id]
         item ||= context.learning_outcome_groups.temp_record
         item.context = context
+        item.mark_as_importing!(migration)
       end
       item.workflow_state = 'active' # restore deleted ones
       item.migration_id = hash[:migration_id]
@@ -65,6 +66,11 @@ module Importers
       end
 
       item.save!
+
+      # don't import contents of deleted outcome groups
+      # (blueprint migration will not undelete outcome groups deleted downstream)
+      return item if item.deleted?
+
       if hash[:parent_group]
         hash[:parent_group].adopt_outcome_group(item)
       else

app/models/importers/learning_outcome_importer.rb

@@ -82,6 +82,7 @@ module Importers
             where(migration_clause(hash[:migration_id])).first if hash[:migration_id]
           item ||= context.created_learning_outcomes.temp_record
           item.context = context
+          item.mark_as_importing!(migration)
         end
         item.migration_id = hash[:migration_id]
         item.vendor_guid = hash[:vendor_guid]
@@ -113,6 +114,10 @@ module Importers
         item = outcome
       end
 
+      # don't add a deleted outcome to an outcome group, or align it with an assignment
+      # (blueprint migration will not undelete outcomes deleted downstream)
+      return item if item.deleted?
+
       log = hash[:learning_outcome_group] || context.root_outcome_group
       log.add_outcome(item)
 

app/models/importers/quiz_importer.rb

@@ -166,8 +166,10 @@ module Importers
       # there might not be an import id if it's just a text-only type...
       item ||= Quizzes::Quiz.where(context_type: context.class.to_s, context_id: context, id: hash[:id]).first if hash[:id]
       item ||= Quizzes::Quiz.where(context_type: context.class.to_s, context_id: context, migration_id: hash[:migration_id]).first if hash[:migration_id]
-      if item && !allow_update
-        if item.deleted?
+      item ||= context.quizzes.temp_record
+      item.mark_as_importing!(migration)
+      if item
+        if !allow_update && item.deleted?
           item.workflow_state = (hash[:available] || !item.can_unpublish?) ? 'available' : 'unpublished'
           item.saved_by = :migration
           item.quiz_groups.destroy_all
@@ -175,8 +177,6 @@ module Importers
           item.save
         end
       end
-      item ||= context.quizzes.temp_record
-      item.mark_as_importing!(migration)
       new_record = item.new_record? || item.deleted?
 
       hash[:due_at] ||= hash[:due_date] if hash.has_key?(:due_date)

app/models/learning_outcome.rb

@@ -19,6 +19,9 @@
 class LearningOutcome < ActiveRecord::Base
   include Workflow
   include OutcomeAttributes
+  include MasterCourses::Restrictor
+  restrict_columns :state, [:workflow_state]
+
   belongs_to :context, polymorphic: [:account, :course]
   has_many :learning_outcome_results
   has_many :alignments, -> { where("content_tags.tag_type='learning_outcome' AND content_tags.workflow_state<>'deleted'") }, class_name: 'ContentTag'

app/models/learning_outcome_group.rb

@@ -19,6 +19,9 @@
 class LearningOutcomeGroup < ActiveRecord::Base
   include Workflow
   include OutcomeAttributes
+  include MasterCourses::Restrictor
+  restrict_columns :state, [:workflow_state]
+
   belongs_to :learning_outcome_group
   has_many :child_outcome_groups, :class_name => 'LearningOutcomeGroup', :foreign_key => "learning_outcome_group_id"
   has_many :child_outcome_links, -> { where(tag_type: 'learning_outcome_association', content_type: 'LearningOutcome') }, class_name: 'ContentTag', as: :associated_asset

app/models/master_courses.rb

@@ -22,8 +22,8 @@ module MasterCourses
 
   # probably not be a comprehensive list but oh well
   ALLOWED_CONTENT_TYPES = %w{
-    Announcement AssessmentQuestionBank Assignment AssignmentGroup Attachment CalendarEvent
-    DiscussionTopic ContextExternalTool ContextModule ContentTag LearningOutcome Quizzes::Quiz Rubric WikiPage
+    Announcement AssessmentQuestionBank Assignment AssignmentGroup Attachment CalendarEvent DiscussionTopic
+    ContextExternalTool ContextModule ContentTag LearningOutcome LearningOutcomeGroup Quizzes::Quiz Rubric WikiPage
   }.freeze
 
   MIGRATION_ID_PREFIX = "mastercourse_".freeze

app/models/master_courses/child_content_tag.rb

@@ -30,6 +30,7 @@ class MasterCourses::ChildContentTag < ActiveRecord::Base
                                      :content_tag,
                                      :discussion_topic,
                                      :learning_outcome,
+                                     :learning_outcome_group,
                                      :rubric,
                                      :wiki_page,
                                      quiz: 'Quizzes::Quiz'

lib/api/v1/master_courses.rb

@@ -48,6 +48,8 @@ module Api::V1::MasterCourses
       course_external_tool_url(:course_id => asset.context.id, :id => asset.id)
     when 'LearningOutcome'
       course_outcome_url(:course_id => asset.context.id, :id => asset.id)
+    when 'LearningOutcomeGroup'
+      course_outcome_group_url(:course_id => asset.context.id, :id => asset.id)
     else
       polymorphic_url([asset.context, asset])
     end

spec/models/master_courses/master_migration_spec.rb

@@ -381,31 +381,100 @@ describe MasterCourses::MasterMigration do
 
       page1 = @copy_from.wiki_pages.create!(:title => "whee")
       page2 = @copy_from.wiki_pages.create!(:title => "whoo")
+      quiz = @copy_from.quizzes.create!(:title => 'what')
       run_master_migration
 
       page1_to = @copy_to.wiki_pages.where(:migration_id => mig_id(page1)).first
       page1_to.destroy # "manually" delete it
       page2_to = @copy_to.wiki_pages.where(:migration_id => mig_id(page2)).first
+      quiz_to = @copy_to.quizzes.where(:migration_id => mig_id(quiz)).first
+      quiz_to.destroy
 
       Timecop.freeze(3.minutes.from_now) do
         page1.update_attribute(:title, 'new title eh')
         page2.destroy
+        quiz.update_attribute(:title, 'new title wat')
       end
       run_master_migration
 
       expect(page1_to.reload).to be_deleted # shouldn't have restored it
       expect(page2_to.reload).to be_deleted # should still sync the original deletion
+      expect(quiz_to.reload).to be_deleted # shouldn't have restored it neither
 
       Timecop.freeze(5.minutes.from_now) do
         page1.update_attribute(:title, 'another new title srsly')
         @template.content_tag_for(page1).update_attribute(:restrictions, {:content => true}) # lock it down
-
         page2.update_attribute(:workflow_state, "active") # restore the original
+        quiz.update_attribute(:title, 'another new title frd pdq')
+        @template.content_tag_for(quiz).update_attribute(:restrictions, {:content => true}) # lock it down
       end
       run_master_migration
 
       expect(page1_to.reload).to be_active # should be restored because it's locked now
       expect(page2_to.reload).to be_active # should be restored because it hadn't been deleted manually
+      expect(quiz_to.reload).not_to be_deleted # should be restored because it's locked now
+    end
+
+    it "doesn't undelete modules that were deleted downstream" do
+      @copy_to = course_factory
+      @template.add_child_course!(@copy_to)
+
+      mod = @copy_from.context_modules.create! :name => 'teh'
+      run_master_migration
+
+      mod_to = @copy_to.context_modules.where(:migration_id => mig_id(mod)).first
+      mod_to.destroy
+
+      Timecop.freeze(3.minutes.from_now) do
+        mod.touch
+      end
+      run_master_migration
+
+      expect(mod_to.reload).to be_deleted
+    end
+
+    describe "outcomes and groups" do
+      before :once do
+        @copy_to = course_factory
+        @template.add_child_course!(@copy_to)
+
+        root = @copy_from.root_outcome_group
+        @og = @copy_from.learning_outcome_groups.create!({:title => 'outcome group'})
+        root.adopt_outcome_group(@og)
+        @outcome = @copy_from.created_learning_outcomes.create!({:title => 'new outcome'})
+        @og.add_outcome(@outcome)
+        run_master_migration
+
+        @outcome_to = @copy_to.learning_outcomes.where(:migration_id => mig_id(@outcome)).first
+        @og_to = @copy_to.learning_outcome_groups.where(:migration_id => mig_id(@og)).first
+      end
+
+      it "doesn't undelete learning outcomes and outcome groups that were deleted downstream" do
+        @outcome_to.destroy
+        @og_to.destroy
+
+        Timecop.freeze(3.minutes.from_now) do
+          @og.touch
+          @outcome.touch
+        end
+        run_master_migration
+
+        expect(@outcome_to.reload).to be_deleted
+        expect(@og_to.reload).to be_deleted
+      end
+
+      it "doesn't resurrect links to deleted outcomes" do
+        @outcome_to.destroy
+
+        Timecop.freeze(3.minutes.from_now) do
+          @og.touch
+          @outcome.touch
+        end
+        run_master_migration
+
+        expect(@outcome_to.reload).to be_deleted
+        expect(@og_to.child_outcome_links.not_deleted.where(content_type: 'LearningOutcome', content_id: @outcome_to)).not_to be_any
+      end
     end
 
     it "doesn't restore deleted associated files unless relocked" do