ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix up module positions after import 

test plan:
  CNVS-4067
   - create a course (A) with three modules
   - use the modules API to retrieve positions, and verify
     they're unique (1, 2, 3)
   - export course A
   - create a second course (B) with three modules having
     different names from course A
   - import course A into course B
   - use the modules API to retrieve positions in course B,
     and verify that the imported modules' positions from A
     do not overlap course B's module positions
     (e.g., they should be 4, 5, 6)
   - also test positions of imported assignment groups
  CNVS-8256
   - import the course attached to the ticket
   - use the modules API to list the modules in the course
   - the positions should be unique: (1, 2, 3), not (1, 1, 1)

fixes CNVS-4067
fixes CNVS-8256

Change-Id: Ib81a238e073e46eb01d72565a569ab24762d37bf
Reviewed-on: https://gerrit.instructure.com/24525
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Product-Review: Bracken Mosbacker <bracken@instructure.com>
QA-Review: Matt Fairbourn <mfairbourn@instructure.com>
This commit is contained in:
Jeremy Stanley 2013-09-19 10:09:07 -06:00
parent 56ac233467
commit 5f5166d37d
5 changed files with 158 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/models/assignment_group.rb
View File

@ -22,7 +22,7 @@ class AssignmentGroup < ActiveRecord::Base
attr_accessible :name, :rules, :assignment_weighting_scheme, :group_weight, :position, :default_assignment_name
attr_readonly :context_id, :context_type
acts_as_list :scope => :context
acts_as_list :scope => 'assignment_groups.context_type = #{connection.quote(context_type)} AND assignment_groups.context_id = #{context_id} AND assignment_groups.workflow_state <> \'deleted\''
has_a_broadcast_policy
has_many :assignments, :order => 'position, due_at, title', :dependent => :destroy
@ -200,6 +200,7 @@ class AssignmentGroup < ActiveRecord::Base
end
end
end
migration.context.assignment_groups.first.try(:fix_position_conflicts)
end
def self.add_groups_for_imported_assignments(data, migration)

12
app/models/context_module.rb
View File

@ -24,7 +24,7 @@ class ContextModule < ActiveRecord::Base
belongs_to :cloned_item
has_many :context_module_progressions, :dependent => :destroy
has_many :content_tags, :dependent => :destroy, :order => 'content_tags.position, content_tags.title'
acts_as_list :scope => :context
acts_as_list :scope => 'context_modules.context_type = #{connection.quote(context_type)} AND context_modules.context_id = #{context_id} AND context_modules.workflow_state <> \'deleted\''
serialize :prerequisites
serialize :completion_requirements
@ -673,14 +673,8 @@ class ContextModule < ActiveRecord::Base
end
end
end
migration_ids = modules.map{|m| m['module_id'] }.compact
conn = self.connection
cases = []
max = migration.context.context_modules.not_deleted.map(&:position).compact.max || 0
modules.each_with_index{|m, idx| cases << " WHEN migration_id=#{conn.quote(m['module_id'])} THEN #{max + idx + 1} " if m['module_id'] }
unless cases.empty?
conn.execute("UPDATE context_modules SET position=CASE #{cases.join(' ')} ELSE NULL END WHERE context_id=#{migration.context.id} AND context_type=#{conn.quote(migration.context.class.to_s)} AND migration_id IN (#{migration_ids.map{|id| conn.quote(id)}.join(',')})")
end
migration.context.context_modules.first.try(:fix_position_conflicts)
migration.context.touch
end
def self.import_from_migration(hash, context, item=nil)

13
config/initializers/acts_as_list.rb
View File

@ -62,6 +62,19 @@ module ActiveRecord
return false
end
end
def fix_position_conflicts
list = acts_as_list_class.where(scope_condition).select([:id, position_column.to_sym]).sort_by{|o| [o.send(position_column) || SortLast, o.id] }
updates = []
last_position = 0
list.each do |obj|
new_position = (obj.position && obj.position > last_position) ? obj.position : last_position + 1
updates << "WHEN id=#{obj.id} THEN #{new_position}"
last_position = new_position
end
acts_as_list_class.update_all("position=CASE #{updates.join(" ")} ELSE NULL END", scope_condition) unless updates.empty?
end
end
end
end

39
spec/lib/acts_as_list.rb
View File

@ -76,5 +76,42 @@ describe "acts_as_list" do
@modules.map(&:position).should == [1, 2, 3]
end
end
end
describe "#fix_position_conflicts" do
it "should order null positions last" do
course
module_1 = @course.context_modules.create :name => 'one'
module_1.position = nil
module_1.save!
module_2 = @course.context_modules.create :name => 'two'
module_2.position = 1
module_2.save!
module_1.fix_position_conflicts
@course.context_modules.map{|m| [m.id, m.position]}.should eql [[module_2.id, 1], [module_1.id, 2]]
end
it "should break ties by object id" do
course
module_1 = @course.context_modules.create :name => 'one'
module_1.position = 1
module_1.save!
module_2 = @course.context_modules.create :name => 'two'
module_2.position = 1
module_2.save!
module_1.fix_position_conflicts
@course.context_modules.map{|m| [m.id, m.position]}.should eql [[module_1.id, 1], [module_2.id, 2]]
end
it "should leave gaps alone" do
course
module_1 = @course.context_modules.create :name => 'one'
module_1.position = 1
module_1.save!
module_2 = @course.context_modules.create :name => 'two'
module_2.position = 3
module_2.save!
module_1.fix_position_conflicts
@course.context_modules.map{|m| [m.id, m.position]}.should eql [[module_1.id, 1], [module_2.id, 3]]
end
end
end

105
spec/lib/cc/importer/common_cartridge_converter_spec.rb
View File

@ -15,7 +15,9 @@ describe "Standard Common Cartridge importing" do
@course = course
@migration = ContentMigration.create(:context => @course)
@migration.migration_settings[:migration_ids_to_import] = {:copy => {}}
@course.import_from_migration(@course_data, nil, @migration)
enable_cache do
@course.import_from_migration(@course_data, nil, @migration)
end
end
after(:all) do
@ -50,7 +52,8 @@ describe "Standard Common Cartridge importing" do
# This also tests the WebLinks, they are just content tags and don't have their own class
it "should import modules from organization" do
@course.context_modules.count.should == 3
@course.context_modules.map(&:position).should eql [1, 2, 3]
mod1 = @course.context_modules.find_by_migration_id("I_00000")
mod1.name.should == "Your Mom, Research, & You"
tag = mod1.content_tags[0]
@ -124,7 +127,7 @@ describe "Standard Common Cartridge importing" do
tag.content_id.should == @course.assignments.find_by_migration_id("I_00011_R").id
tag.indent.should == 0
end
it "should import external tools" do
@course.context_external_tools.count.should == 2
et = @course.context_external_tools.find_by_migration_id("I_00010_R")
@ -306,6 +309,102 @@ describe "Standard Common Cartridge importing" do
end
end
context "position conflicts" do
append_before do
@import_json =
{
"modules" => [
{
"title" => "monkeys",
"position" => 1,
"migration_id" => 'm_monkeys'
},
{
"title" => "ponies",
"position" => 2,
"migration_id" => 'm_ponies'
},
{
"title" => "last",
"position" => 3,
"migration_id" => "m_last"
}
],
"assignment_groups" => [
{
"title" => "monkeys",
"position" => 1,
"migration_id" => "ag_monkeys"
},
{
"title" => "ponies",
"position" => 2,
"migration_id" => "ag_ponies"
},
{
"title" => "last",
"position" => 3,
"migration_id" => "ag_last"
}
]
}
end
it "should fix position conflicts for modules" do
@course = course
mod1 = @course.context_modules.create :name => "ponies"
mod1.position = 1
mod1.migration_id = 'm_ponies'
mod1.save!
mod2 = @course.context_modules.create :name => "monsters"
mod2.migration_id = 'm_monsters'
mod2.position = 2
mod2.save!
@migration = ContentMigration.create(:context => @course)
@migration.migration_settings[:migration_ids_to_import] = {
:copy => {
"everything" => "0",
"all_context_modules" => "1"
}
}
@course.import_from_migration(@import_json, nil, @migration)
mods = @course.context_modules.to_a
mods.map(&:position).should eql [1, 2, 3, 4]
mods.map(&:name).should eql %w(monkeys ponies monsters last)
end
it "should fix position conflicts for assignment groups" do
@course = course
ag1 = @course.assignment_groups.create :name => "ponies"
ag1.position = 1
ag1.migration_id = 'ag_ponies'
ag1.save!
ag2 = @course.assignment_groups.create :name => "monsters"
ag2.position = 2
ag2.migration_id = 'ag_monsters'
ag2.save!
@migration = ContentMigration.create(:context => @course)
@migration.migration_settings[:migration_ids_to_import] = {
:copy => {
"everything" => "0",
"all_assignment_groups" => "1"
}
}
@course.import_from_migration(@import_json, nil, @migration)
ags = @course.assignment_groups.to_a
ags.map(&:position).should eql [1, 2, 3, 4]
ags.map(&:name).should eql %w(monkeys ponies monsters last)
end
end
end
describe "More Standard Common Cartridge importing" do