master courses: create master content tags
gives a place for us to keep track of changed items for now (and soon the restrictions for content after copy) closes #MC-7 Change-Id: I7bbd6e371528a06ea93181b0f4d91f451ae99d52 Reviewed-on: https://gerrit.instructure.com/95502 Reviewed-by: Jeremy Stanley <jeremy@instructure.com> Tested-by: Jenkins Product-Review: James Williams <jamesw@instructure.com> QA-Review: James Williams <jamesw@instructure.com>
This commit is contained in:
parent
154cc7e35d
commit
f22e8b3f47
|
@ -2,4 +2,10 @@ module MasterCourses
|
|||
def self.table_name_prefix
|
||||
'master_courses_'
|
||||
end
|
||||
|
||||
# probably not be a comprehensive list but oh well
|
||||
ALLOWED_CONTENT_TYPES = %w{
|
||||
Announcement AssessmentQuestionBank Assignment Attachment CalendarEvent
|
||||
DiscussionTopic ContextExternalTool ContextModule LearningOutcome Quizzes::Quiz Rubric WikiPage
|
||||
}.freeze
|
||||
end
|
||||
|
|
|
@ -0,0 +1,9 @@
|
|||
class MasterCourses::MasterContentTag < ActiveRecord::Base
|
||||
# i want to get off content tag's wild ride
|
||||
|
||||
belongs_to :master_template, :class_name => "MasterCourses::MasterTemplate"
|
||||
belongs_to :content, :polymorphic => true
|
||||
validates_inclusion_of :content_type, :allow_nil => false, :in => MasterCourses::ALLOWED_CONTENT_TYPES
|
||||
|
||||
strong_params
|
||||
end
|
|
@ -3,6 +3,7 @@ class MasterCourses::MasterTemplate < ActiveRecord::Base
|
|||
# instead of the entire course, but for now that's what we'll roll with
|
||||
|
||||
belongs_to :course
|
||||
has_many :master_content_tags, :class_name => "MasterCourses::MasterContentTag", :inverse_of => :master_template
|
||||
|
||||
strong_params
|
||||
|
||||
|
@ -19,4 +20,31 @@ class MasterCourses::MasterTemplate < ActiveRecord::Base
|
|||
def self.full_template_for(course)
|
||||
course.master_course_templates.active.for_full_course.first
|
||||
end
|
||||
|
||||
# load all the tags into a nested index for fast searching in content_tag_for
|
||||
def load_tags!
|
||||
@content_tag_index = {}
|
||||
self.master_content_tags.to_a.group_by(&:content_type).each do |content_type, typed_tags|
|
||||
@content_tag_index[content_type] = typed_tags.index_by(&:content_id)
|
||||
end
|
||||
true
|
||||
end
|
||||
|
||||
def content_tag_for(content)
|
||||
if @content_tag_index
|
||||
(@content_tag_index[content.class.base_class.name] || {})[content.id] || create_content_tag_for!(content)
|
||||
else
|
||||
self.master_content_tags.polymorphic_where(:content => content).first || create_content_tag_for!(content)
|
||||
end
|
||||
end
|
||||
|
||||
def create_content_tag_for!(content)
|
||||
self.class.unique_constraint_retry do |retry_count|
|
||||
tag = nil
|
||||
tag = self.master_content_tags.polymorphic_where(:content => content).first if retry_count > 0
|
||||
tag ||= self.master_content_tags.create!(:content => content)
|
||||
tag
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
|
|
|
@ -780,7 +780,7 @@ ActiveRecord::Relation.class_eval do
|
|||
raise ArgumentError unless args.length == 1
|
||||
|
||||
column = args.first.first
|
||||
values = args.first.last
|
||||
values = Array(args.first.last)
|
||||
original_length = values.length
|
||||
values = values.compact
|
||||
|
||||
|
|
|
@ -0,0 +1,25 @@
|
|||
class CreateMasterContentTags < ActiveRecord::Migration[4.2]
|
||||
tag :predeploy
|
||||
|
||||
def change
|
||||
create_table :master_courses_master_content_tags do |t|
|
||||
t.integer :master_template_id, limit: 8, null: false
|
||||
|
||||
# should we add a workflow state and make this soft-deletable? meh methinks
|
||||
# maybe someday if we decide to use these to define the template content aets
|
||||
|
||||
t.string :content_type, null: false
|
||||
t.integer :content_id, limit: 8, null: false
|
||||
|
||||
# here i was originally going to have a boolean column to keep track whether any changes had been made
|
||||
# since last export but i think i want to tie it directly to a master course 'migration'
|
||||
# to make it more robust against failure (i.e. we'll know that we'll need to re-export if the export failed)
|
||||
# so i'm going to add a column later when i have a new table
|
||||
end
|
||||
|
||||
add_foreign_key :master_courses_master_content_tags, :master_courses_master_templates, column: "master_template_id"
|
||||
add_index :master_courses_master_content_tags, :master_template_id
|
||||
add_index :master_courses_master_content_tags, [:master_template_id, :content_type, :content_id], :unique => true,
|
||||
:name => "index_master_content_tags_on_template_id_and_content"
|
||||
end
|
||||
end
|
|
@ -26,4 +26,42 @@ describe MasterCourses::MasterTemplate do
|
|||
expect(template2).to be_active
|
||||
end
|
||||
end
|
||||
|
||||
describe "content_tag_for" do
|
||||
before :once do
|
||||
@template = MasterCourses::MasterTemplate.set_as_master_course(@course)
|
||||
@assignment = @course.assignments.create!
|
||||
end
|
||||
|
||||
it "should create tags for course content" do
|
||||
tag = @template.content_tag_for(@assignment)
|
||||
expect(tag.reload.content).to eq @assignment
|
||||
end
|
||||
|
||||
it "should find tags" do
|
||||
tag = @template.create_content_tag_for!(@assignment)
|
||||
@template.expects(:create_content_tag_for!).never # don't try to recreate
|
||||
expect(@template.content_tag_for(@assignment)).to eq tag
|
||||
end
|
||||
|
||||
it "should not fail on double-create" do
|
||||
tag = @template.create_content_tag_for!(@assignment)
|
||||
expect(@template.create_content_tag_for!(@assignment)).to eq tag # should retry on unique constraint failure
|
||||
end
|
||||
|
||||
it "should be able to load tags for fast searching" do
|
||||
tag = @template.create_content_tag_for!(@assignment)
|
||||
@template.load_tags!
|
||||
|
||||
old_tag_id = tag.id
|
||||
tag.destroy! # delete in the db - proves that we cached them
|
||||
|
||||
expect(@template.content_tag_for(@assignment).id).to eq old_tag_id
|
||||
|
||||
# should still create a tag even if it's not found in the index
|
||||
@page = @course.wiki.wiki_pages.create!(:title => "title")
|
||||
page_tag = @template.content_tag_for(@page)
|
||||
expect(page_tag.reload.content).to eq @page
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue