diff --git a/app/models/importers/link_replacer.rb b/app/models/importers/link_replacer.rb index f3cc619b91f..cf9ab080cef 100644 --- a/app/models/importers/link_replacer.rb +++ b/app/models/importers/link_replacer.rb @@ -119,13 +119,26 @@ module Importers subbed = false links.each do |link| new_value = link[:new_value] || link[:old_value] - if html.sub!(link[:placeholder], new_value) + if html.gsub!(link[:placeholder], new_value) subbed = true end end subbed end + def recursively_sub_placeholders!(object, links) + subbed = false + case object + when Hash + object.values.each { |o| subbed = true if recursively_sub_placeholders!(o, links) } + when Array + object.each { |o| subbed = true if recursively_sub_placeholders!(o, links) } + when String + subbed = sub_placeholders!(object, links) + end + subbed + end + def process_assignment_types!(item, links) case item when Assignment @@ -150,18 +163,16 @@ module Importers # we have to do a little bit more here because the question_data can get copied all over quiz_ids = [] Quizzes::QuizQuestion.where(:assessment_question_id => aq.id).find_each do |qq| - qq_yaml = qq['question_data'].to_yaml - if sub_placeholders!(qq_yaml, links) - Quizzes::QuizQuestion.where(:id => qq.id).update_all(:question_data => qq_yaml) + if recursively_sub_placeholders!(qq['question_data'], links) + Quizzes::QuizQuestion.where(:id => qq.id).update_all(:question_data => qq['question_data'].to_yaml) quiz_ids << qq.quiz_id end end if quiz_ids.any? Quizzes::Quiz.where(:id => quiz_ids.uniq).where("quiz_data IS NOT NULL").find_each do |quiz| - quiz_yaml = quiz['quiz_data'].to_yaml - if sub_placeholders!(quiz_yaml, links) - Quizzes::Quiz.where(:id => quiz.id).update_all(:quiz_data => quiz_yaml) + if recursively_sub_placeholders!(quiz['quiz_data'], links) + Quizzes::Quiz.where(:id => quiz.id).update_all(:quiz_data => quiz['quiz_data'].to_yaml) end end end @@ -174,23 +185,20 @@ module Importers link[:new_value] = aq.translate_file_link(link[:new_value]) end - aq_yaml = aq['question_data'].to_yaml - if sub_placeholders!(aq_yaml, links) - AssessmentQuestion.where(:id => aq.id).update_all(:question_data => aq_yaml) + if recursively_sub_placeholders!(aq['question_data'], links) + AssessmentQuestion.where(:id => aq.id).update_all(:question_data => aq['question_data'].to_yaml) end end def process_quiz_question!(qq, links) - qq_yaml = qq['question_data'].to_yaml - if sub_placeholders!(qq_yaml, links) - Quizzes::QuizQuestion.where(:id => qq.id).update_all(:question_data => qq_yaml) + if recursively_sub_placeholders!(qq['question_data'], links) + Quizzes::QuizQuestion.where(:id => qq.id).update_all(:question_data => qq['question_data'].to_yaml) end quiz = Quizzes::Quiz.where(:id => qq.quiz_id).where("quiz_data IS NOT NULL").first if quiz - quiz_yaml = quiz['quiz_data'].to_yaml - if sub_placeholders!(quiz_yaml, links) - Quizzes::Quiz.where(:id => quiz.id).update_all(:quiz_data => quiz_yaml) + if recursively_sub_placeholders!(quiz['quiz_data'], links) + Quizzes::Quiz.where(:id => quiz.id).update_all(:quiz_data => quiz['quiz_data'].to_yaml) end end end diff --git a/db/migrate/20150922142651_fix_imported_question_media_comments.rb b/db/migrate/20150922142651_fix_imported_question_media_comments.rb new file mode 100644 index 00000000000..f0ce6903cb4 --- /dev/null +++ b/db/migrate/20150922142651_fix_imported_question_media_comments.rb @@ -0,0 +1,10 @@ +class FixImportedQuestionMediaComments < ActiveRecord::Migration + tag :postdeploy + def up + DataFixup::FixImportedQuestionMediaComments.send_later_if_production_enqueue_args(:run, + :priority => Delayed::LOW_PRIORITY, :n_strand => 'long_datafixups') + end + + def down + end +end diff --git a/lib/data_fixup/fix_imported_question_media_comments.rb b/lib/data_fixup/fix_imported_question_media_comments.rb new file mode 100644 index 00000000000..9fc19cb3220 --- /dev/null +++ b/lib/data_fixup/fix_imported_question_media_comments.rb @@ -0,0 +1,82 @@ +module DataFixup + module FixImportedQuestionMediaComments + def self.get_fixed_yaml(bad_yaml) + return unless bad_yaml + placeholders = [] + + # tl;dr - search for the imported media comment links and re-substitute them in the right way so the yaml still works + # so first make a placeholder without quotes so we deserialize the yaml again + bad_yaml.gsub!(/\/m) do |link_str| + placeholder = "somuchsadness_#{Digest::MD5.hexdigest(link_str)}" + placeholders << {:placeholder => placeholder, :new_value => link_str} + placeholder + end + + bad_yaml.gsub!(/\/m) do |link_str| # for empty/unmatched tags + placeholder = "somuchsadness_#{Digest::MD5.hexdigest(link_str)}" + placeholders << {:placeholder => placeholder, :new_value => link_str} + placeholder + end + + return unless hash = (YAML.load(bad_yaml) rescue nil) + + # now make the substitutions correctly and return the serialized yaml + Importers::LinkReplacer.new(nil).recursively_sub_placeholders!(hash, placeholders) + hash.to_yaml + end + + def self.run + quiz_ids_to_fix = [] + still_broken_aq_ids = [] + still_broken_qq_ids = [] + still_broken_quiz_ids = [] + + date_of_sadness = DateTime.parse("2015-07-17") # day before the borked link refactoring was released + + AssessmentQuestion.find_ids_in_ranges(:batch_size => 10000) do |min_id, max_id| + AssessmentQuestion.where(id: min_id..max_id).where("migration_id IS NOT NULL"). + where("updated_at > ?", date_of_sadness).where("question_data LIKE ?", "%media_comment%").each do |aq| + next unless (aq['question_data'] rescue nil).nil? # deserializing the attribute will fail silently in Rails 3 but not Rails 4 + + unless yaml = get_fixed_yaml(aq.attributes_before_type_cast['question_data']) + still_broken_aq_ids << aq.id + next + end + + AssessmentQuestion.where(:id => aq).update_all(:question_data => yaml) + end + end + + Quizzes::QuizQuestion.find_ids_in_ranges(:batch_size => 10000) do |min_id, max_id| + Quizzes::QuizQuestion.where(id: min_id..max_id).where("migration_id IS NOT NULL"). + where("updated_at > ?", date_of_sadness).where("question_data LIKE ?", "%media_comment%").each do |qq| + next unless (qq['question_data'] rescue nil).nil? + + unless yaml = get_fixed_yaml(qq.attributes_before_type_cast['question_data']) + still_broken_qq_ids << qq.id + next + end + + quiz_ids_to_fix << qq.quiz_id + Quizzes::QuizQuestion.where(:id => qq).update_all(:question_data => yaml) + end + end + + quiz_ids_to_fix.uniq.each_slice(100) do |quiz_ids| + Quizzes::Quiz.where("quiz_data IS NOT NULL").where(:id => quiz_ids).each do |quiz| + next unless (quiz['quiz_data'] rescue nil).nil? + unless yaml = get_fixed_yaml(quiz.attributes_before_type_cast['quiz_data']) + still_broken_quiz_ids << quiz.id + next + end + + Quizzes::Quiz.where(:id => quiz).update_all(:quiz_data => yaml) + end + end + + Rails.logger.error("Problem running FixImportedQuestionMediaComments: could not fix quiz questions #{still_broken_qq_ids}") if still_broken_qq_ids.any? + Rails.logger.error("Problem running FixImportedQuestionMediaComments: could not fix assessment questions #{still_broken_aq_ids}") if still_broken_aq_ids.any? + Rails.logger.error("Problem running FixImportedQuestionMediaComments: could not fix quizzes #{still_broken_quiz_ids}") if still_broken_quiz_ids.any? + end + end +end diff --git a/spec/fixtures/migration/canvas_quiz_media_comment.zip b/spec/fixtures/migration/canvas_quiz_media_comment.zip new file mode 100644 index 00000000000..f722211c86d Binary files /dev/null and b/spec/fixtures/migration/canvas_quiz_media_comment.zip differ diff --git a/spec/migrations/fix_imported_question_media_comments_spec.rb b/spec/migrations/fix_imported_question_media_comments_spec.rb new file mode 100644 index 00000000000..e5b53810065 --- /dev/null +++ b/spec/migrations/fix_imported_question_media_comments_spec.rb @@ -0,0 +1,54 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb') + +describe DataFixup::FixImportedQuestionMediaComments do + it 'should fix broken yaml in questions and quizzes' do + course + placeholder = "SOMETEXT" + bank = @course.assessment_question_banks.create!(:title => 'bank') + data = {'question_name' => 'test question', 'question_type' => 'essay_question', + 'question_text' => "\n SOME OTHER TEXT \n #{placeholder} \n blahblahblah " } + aq = bank.assessment_questions.build(:question_data => data) + aq.migration_id = 'something' + aq.save! + + quiz = @course.quizzes.create!(:title => "other quiz") + qq = quiz.quiz_questions.build(:question_data => data) + qq.migration_id = 'somethingelse' + qq.save! + quiz.generate_quiz_data + quiz.published_at = Time.now + quiz.workflow_state = 'available' + quiz.save! + + broken_link = "stuff" + + # deliberately create broken yaml + AssessmentQuestion.where(:id => aq).update_all( + :question_data => aq['question_data'].to_yaml.gsub(placeholder, broken_link), + :updated_at => DateTime.parse('2015-10-16')) #just in case someone tries to run this spec in the past + Quizzes::QuizQuestion.where(:id => qq).update_all( + :question_data => qq['question_data'].to_yaml.gsub(placeholder, broken_link), + :updated_at => DateTime.parse('2015-10-16')) + Quizzes::Quiz.where(:id => quiz).update_all( + :quiz_data => quiz['quiz_data'].to_yaml.gsub(placeholder, broken_link), + :updated_at => DateTime.parse('2015-10-16')) + + aq = AssessmentQuestion.where(:id => aq).first + qq = Quizzes::QuizQuestion.where(:id => qq).first + quiz = Quizzes::Quiz.where(:id => quiz).first + + expect((aq['question_data'] rescue nil)).to be_nil # Rails 4 raises errors when trying to deserialize + expect((qq['question_data'] rescue nil)).to be_nil + expect((quiz['quiz_data'] rescue nil)).to be_nil + + DataFixup::FixImportedQuestionMediaComments.run + + aq = AssessmentQuestion.where(:id => aq).first + qq = Quizzes::QuizQuestion.where(:id => qq).first + quiz = Quizzes::Quiz.where(:id => quiz).first + + expect(aq['question_data']['question_text']).to include(broken_link) + expect(qq['question_data']['question_text']).to include(broken_link) + expect(quiz['quiz_data'].first['question_text']).to include(broken_link) + end +end \ No newline at end of file diff --git a/spec/models/content_migration_spec.rb b/spec/models/content_migration_spec.rb index e6e1beef431..78ff96f1024 100644 --- a/spec/models/content_migration_spec.rb +++ b/spec/models/content_migration_spec.rb @@ -397,4 +397,35 @@ describe ContentMigration do expect(cm.migration_issues).to be_empty end + + it "should correclty handle media comment resolution in quizzes" do + course_with_teacher + cm = ContentMigration.new(:context => @course, :user => @user) + cm.migration_type = 'canvas_cartridge_importer' + cm.migration_settings['import_immediately'] = true + cm.save! + + package_path = File.join(File.dirname(__FILE__) + "/../fixtures/migration/canvas_quiz_media_comment.zip") + attachment = Attachment.new + attachment.context = cm + attachment.uploaded_data = File.open(package_path, 'rb') + attachment.filename = 'file.zip' + attachment.save! + + cm.attachment = attachment + cm.save! + + cm.queue_migration + run_jobs + + expect(cm.migration_issues).to be_empty + quiz = @course.quizzes.available.first + expect(quiz.quiz_data).to be_present + expect(quiz.quiz_data.to_yaml).to include("/media_objects/m-5U5Jww6HL7zG35CgyaYGyA5bhzsremxY") + + qq = quiz.quiz_questions.first + expect(qq.question_data).to be_present + expect(qq.question_data.to_yaml).to include("/media_objects/m-5U5Jww6HL7zG35CgyaYGyA5bhzsremxY") + + end end