diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 57840cd7bb865..8a5bc3ad919e9 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -740,6 +740,12 @@ def locked_for?(user, opts={}) end end + def clear_locked_cache(user) + super + Rails.cache.delete(discussion_topic.locked_cache_key(user)) if self.submission_types == 'discussion_topic' && discussion_topic + Rails.cache.delete(quiz.locked_cache_key(user)) if self.submission_types == 'online_quiz' && quiz + end + def submission_types_array (self.submission_types || "").split(",") end diff --git a/app/models/content_tag.rb b/app/models/content_tag.rb index a16b0afb4340c..9adb6ac10650b 100644 --- a/app/models/content_tag.rb +++ b/app/models/content_tag.rb @@ -191,43 +191,72 @@ def asset_safe_title(column) name end + def self.asset_workflow_state(asset) + if asset.respond_to?(:published?) + if asset.respond_to?(:deleted?) && asset.deleted? + 'deleted' + elsif asset.published? + 'active' + else + 'unpublished' + end + else + if asset.respond_to?(:workflow_state) + workflow_state = asset.workflow_state.to_s + if ['active', 'available', 'published'].include?(workflow_state) + 'active' + elsif ['unpublished', 'deleted'].include?(workflow_state) + workflow_state + end + else + nil + end + end + end + + def asset_workflow_state + ContentTag.asset_workflow_state(self.content) + end + + def asset_context_matches? + self.content && self.content.respond_to?(:context) && self.content.context == context + end + def update_asset_name! return unless self.sync_title_to_asset_title? - correct_context = content && content.respond_to?(:context) && content.context == context - if correct_context - # Assignment proxies name= and name to title= and title, which breaks the asset_safe_title logic - if content.respond_to?("name=") && content.respond_to?("name") && !content.is_a?(Assignment) - content.name = asset_safe_title('name') - elsif content.respond_to?("title=") - content.title = asset_safe_title('title') - elsif content.respond_to?("display_name=") - content.display_name = asset_safe_title('display_name') - end - content.save if content.changed? + return unless self.asset_context_matches? + + # Assignment proxies name= and name to title= and title, which breaks the asset_safe_title logic + if content.respond_to?("name=") && content.respond_to?("name") && !content.is_a?(Assignment) + content.name = asset_safe_title('name') + elsif content.respond_to?("title=") + content.title = asset_safe_title('title') + elsif content.respond_to?("display_name=") + content.display_name = asset_safe_title('display_name') end + content.save if content.changed? end def update_asset_workflow_state! return unless self.sync_workflow_state_to_asset? - correct_context = self.content && self.content.respond_to?(:context) && self.content.context == self.context - if correct_context - asset_workflow_state = nil - if self.unpublished? && self.content.respond_to?(:unpublished?) - asset_workflow_state = 'unpublished' - elsif self.active? - if self.content.respond_to?(:active?) - asset_workflow_state = 'active' - elsif self.content.respond_to?(:available?) - asset_workflow_state = 'available' - elsif self.content.respond_to?(:published?) - asset_workflow_state = 'published' - end - end - if asset_workflow_state - self.content.update_attribute(:workflow_state, asset_workflow_state) - self.class.update_for(self.content) + return unless self.asset_context_matches? + + new_asset_workflow_state = nil + if self.unpublished? && self.content.respond_to?(:unpublished?) + new_asset_workflow_state = 'unpublished' + elsif self.active? + if self.content.respond_to?(:active?) + new_asset_workflow_state = 'active' + elsif self.content.respond_to?(:available?) + new_asset_workflow_state = 'available' + elsif self.content.respond_to?(:published?) + new_asset_workflow_state = 'published' end end + if new_asset_workflow_state + self.content.update_attribute(:workflow_state, new_asset_workflow_state) + self.class.update_for(self.content) + end end def self.delete_for(asset) @@ -296,15 +325,12 @@ def self.update_for(asset) # update workflow_state tag_ids = tags.select{|t| t.sync_workflow_state_to_asset? }.map(&:id) attr_hash = {:updated_at => Time.now.utc} - if asset.respond_to?(:workflow_state) - if ['active', 'available', 'published'].include?(asset.workflow_state) - attr_hash[:workflow_state] = 'active' - elsif ['unpublished', 'deleted'].include?(asset.workflow_state) - attr_hash[:workflow_state] = asset.workflow_state - end - end + + workflow_state = asset_workflow_state(asset) + attr_hash[:workflow_state] = workflow_state if workflow_state ContentTag.where(:id => tag_ids).update_all(attr_hash) if attr_hash[:workflow_state] && !tag_ids.empty? + # update the module timestamp ContentTag.touch_context_modules(module_ids) end diff --git a/app/models/context_module.rb b/app/models/context_module.rb index 43ca39464da78..3732f6dfe2aa5 100644 --- a/app/models/context_module.rb +++ b/app/models/context_module.rb @@ -249,7 +249,7 @@ def content_tags_visible_to(user) def add_item(params, added_item=nil, opts={}) params[:type] = params[:type].underscore if params[:type] - position = opts[:position] || (self.content_tags.active.maximum(:position) || 0) + 1 + position = opts[:position] || (self.content_tags.not_deleted.maximum(:position) || 0) + 1 if params[:type] == "wiki_page" || params[:type] == "page" item = opts[:wiki_page] || self.context.wiki.wiki_pages.find_by_id(params[:id]) elsif params[:type] == "attachment" || params[:type] == "file" @@ -261,7 +261,7 @@ def add_item(params, added_item=nil, opts={}) elsif params[:type] == "quiz" item = opts[:quiz] || self.context.quizzes.active.find_by_id(params[:id]) end - workflow_state = item.workflow_state if item && item.respond_to?(:workflow_state) && ['active', 'unpublished'].include?(item.workflow_state) + workflow_state = ContentTag.asset_workflow_state(item) if item workflow_state ||= 'active' if params[:type] == 'external_url' title = params[:title] diff --git a/app/models/discussion_topic.rb b/app/models/discussion_topic.rb index e5a0f4e13401d..40695f3f94674 100644 --- a/app/models/discussion_topic.rb +++ b/app/models/discussion_topic.rb @@ -901,6 +901,12 @@ def locked_for?(user, opts={}) end end + def clear_locked_cache(user) + super + Rails.cache.delete(assignment.locked_cache_key(user)) if assignment + Rails.cache.delete(root_topic.locked_cache_key(user)) if root_topic + end + def self.process_migration(data, migration) process_announcements_migration(Array(data['announcements']), migration) process_discussion_topics_migration(Array(data['discussion_topics']), migration) diff --git a/app/models/quizzes/quiz.rb b/app/models/quizzes/quiz.rb index f6d05be2194b3..10e2344fc9023 100644 --- a/app/models/quizzes/quiz.rb +++ b/app/models/quizzes/quiz.rb @@ -771,6 +771,11 @@ def locked_for?(user, opts={}) end end + def clear_locked_cache(user) + super + Rails.cache.delete(assignment.locked_cache_key(user)) if self.for_assignment? + end + def context_module_action(user, action, points=nil) tags_to_update = self.context_module_tags.to_a if self.assignment diff --git a/lib/features/draft_state.rb b/lib/features/draft_state.rb index 9cc200fb91560..6f00824dddce0 100644 --- a/lib/features/draft_state.rb +++ b/lib/features/draft_state.rb @@ -24,8 +24,16 @@ def perform course.quizzes.where(workflow_state: 'unpublished').update_all(workflow_state: 'edited') course.assignments.where(workflow_state: 'unpublished').update_all(workflow_state: 'published') course.context_modules.where(workflow_state: 'unpublished').update_all(workflow_state: 'active') - course.context_module_tags.where(workflow_state: 'unpublished').update_all(workflow_state: 'active') course.discussion_topics.where(workflow_state: 'unpublished').update_all(workflow_state: 'active') + + # content tags referencing wiki pages or quizzes do not need to be updated + # wiki pages and quizzes handle unpublished values correctly in non-draft state + course.context_module_tags.where(workflow_state: 'unpublished') + .where("content_type NOT IN ('WikiPage', 'Quiz', 'Quizzes::Quiz')") + .update_all(workflow_state: 'active') + + # invalidate cache for modules + course.context_modules.where("workflow_state<>'deleted'").update_all(updated_at: Time.now.utc) end end end diff --git a/spec/apis/v1/context_module_items_api_spec.rb b/spec/apis/v1/context_module_items_api_spec.rb index 94fa2c73332ff..08daf047d729d 100644 --- a/spec/apis/v1/context_module_items_api_spec.rb +++ b/spec/apis/v1/context_module_items_api_spec.rb @@ -25,6 +25,7 @@ @assignment = @course.assignments.create!(:name => "pls submit", :submission_types => ["online_text_entry"], :points_possible => 20) @assignment_tag = @module1.add_item(:id => @assignment.id, :type => 'assignment') @quiz = @course.quizzes.create!(:title => "score 10") + @quiz.publish! @quiz_tag = @module1.add_item(:id => @quiz.id, :type => 'quiz') @topic = @course.discussion_topics.create!(:message => 'pls contribute') @topic_tag = @module1.add_item(:id => @topic.id, :type => 'discussion_topic') diff --git a/spec/controllers/context_modules_controller_spec.rb b/spec/controllers/context_modules_controller_spec.rb index e1ab1de6262aa..0387f12df70e3 100644 --- a/spec/controllers/context_modules_controller_spec.rb +++ b/spec/controllers/context_modules_controller_spec.rb @@ -233,6 +233,7 @@ @module = @course.context_modules.create! quiz = @course.quizzes.create! + quiz.publish! tag = @module.add_item :type => 'quiz', :id => quiz.id diff --git a/spec/integration/context_module_spec.rb b/spec/integration/context_module_spec.rb index 2bcbb46dbbe6b..932e5dbb83c2c 100644 --- a/spec/integration/context_module_spec.rb +++ b/spec/integration/context_module_spec.rb @@ -112,7 +112,8 @@ def progression_testing(progress_by_item_link) @is_attachment = false course_with_student_logged_in(:active_all => true) @quiz = @course.quizzes.create!(:title => "new quiz", :shuffle_answers => true) - + @quiz.publish! + # separate timestamps so touch_context will actually invalidate caches Timecop.freeze(4.seconds.ago) do @mod1 = @course.context_modules.create!(:name => "some module") @@ -129,33 +130,44 @@ def progression_testing(progress_by_item_link) @mod2.save! end + # all modules, tags, etc need to be published + @mod1.should be_published + @mod2.should be_published + @quiz.should be_published + @tag1.should be_published + yield '
yay!
' - + @tag2.should be_published + + # verify the second item is locked (doesn't display) get @test_url response.should be_success html = Nokogiri::HTML(response.body) html.css('#test_content').length.should == (@test_content_length || 0) - - p1 = @mod1.evaluate_for(@user, true, true) - - @quiz_submission = @quiz.generate_submission(@user) + + # complete first module's requirements + p1 = @mod1.evaluate_for(@student, true, true) + p1.workflow_state.should == 'unlocked' + + @quiz_submission = @quiz.generate_submission(@student) @quiz_submission.grade_submission @quiz_submission.workflow_state = 'completed' @quiz_submission.kept_score = 1 @quiz_submission.save! - + #emulate settings on progression if the user took the quiz but background jobs haven't run yet p1.requirements_met = [{:type=>"min_score", :min_score=>"1", :max_score=>nil, :id=>@quiz.id}] p1.save! - + + # navigate to the second item (forcing update to progression) next_link = progress_by_item_link ? "/courses/#{@course.id}/modules/items/#{@tag2.id}" : "/courses/#{@course.id}/modules/#{@mod2.id}/items/first" - get next_link response.should be_redirect response.location.ends_with?(@test_url + "?module_item_id=#{@tag2.id}").should be_true - + + # verify the second item is no accessible get @test_url response.should be_success html = Nokogiri::HTML(response.body) @@ -173,6 +185,7 @@ def progression_testing(progress_by_item_link) asmnt = @course.assignments.create!(:title => 'assignment', :description => content) @test_url = "/courses/#{@course.id}/assignments/#{asmnt.id}" @tag2 = @mod2.add_item(:type => 'assignment', :id => asmnt.id) + @tag2.should be_published end end end @@ -183,6 +196,7 @@ def progression_testing(progress_by_item_link) discussion = @course.discussion_topics.create!(:title => "topic", :message => content) @test_url = "/courses/#{@course.id}/discussion_topics/#{discussion.id}" @tag2 = @mod2.add_item(:type => 'discussion_topic', :id => discussion.id) + @tag2.should be_published @test_content_length = 1 end end @@ -192,8 +206,10 @@ def progression_testing(progress_by_item_link) [true, false].each do |progress_type| progression_testing(progress_type) do |content| quiz = @course.quizzes.create!(:title => "quiz", :description => content) + quiz.publish! @test_url = "/courses/#{@course.id}/quizzes/#{quiz.id}" @tag2 = @mod2.add_item(:type => 'quiz', :id => quiz.id) + @tag2.should be_published end end end @@ -204,6 +220,7 @@ def progression_testing(progress_by_item_link) page = @course.wiki.wiki_pages.create!(:title => "wiki", :body => content) @test_url = "/courses/#{@course.id}/wiki/#{page.url}" @tag2 = @mod2.add_item(:type => 'wiki_page', :id => page.id) + @tag2.should be_published end end end @@ -215,6 +232,7 @@ def progression_testing(progress_by_item_link) att = Attachment.create!(:filename => 'test.html', :display_name => "test.html", :uploaded_data => StringIO.new(content), :folder => Folder.unfiled_folder(@course), :context => @course) @test_url = "/courses/#{@course.id}/files/#{att.id}" @tag2 = @mod2.add_item(:type => 'attachment', :id => att.id) + @tag2.should be_published end end end diff --git a/spec/lib/draft_state_spec.rb b/spec/lib/draft_state_spec.rb index 55162bb25da03..b78eefb4004ce 100644 --- a/spec/lib/draft_state_spec.rb +++ b/spec/lib/draft_state_spec.rb @@ -33,29 +33,37 @@ end it "should kick off a publish job in state change callback" do - quiz = t_course.quizzes.create! title: 'a quiz' - quiz.workflow_state = 'unpublished' - quiz.save! - - page = t_course.wiki.wiki_pages.create title: 'some page' - page.workflow_state = 'unpublished' - page.save! + mod = t_course.context_modules.create name: 'blargh' + mod.workflow_state = 'unpublished' + mod.save! topic = t_course.discussion_topics.create title: 'a topic' topic.workflow_state = 'unpublished' topic.save! + topic_item = mod.add_item type: 'discussion_topic', id: topic.id + topic_item.workflow_state = 'unpublished' + topic_item.save! assignment = t_course.assignments.create name: 'do this' assignment.workflow_state = 'unpublished' assignment.save! + assignment_item = mod.add_item type: 'assignment', id: assignment.id + assignment_item.workflow_state = 'unpublished' + assignment_item.save! - mod = t_course.context_modules.create name: 'blargh' - mod.workflow_state = 'unpublished' - mod.save! + quiz = t_course.quizzes.create! title: 'a quiz' + quiz.workflow_state = 'unpublished' + quiz.save! + quiz_item = mod.add_item type: 'quiz', id: quiz.id + quiz_item.workflow_state = 'unpublished' + quiz_item.save! - item = mod.add_item type: 'wiki_page', id: page.id - item.workflow_state = 'unpublished' - item.save! + page = t_course.wiki.wiki_pages.create title: 'some page' + page.workflow_state = 'unpublished' + page.save! + page_item = mod.add_item type: 'wiki_page', id: page.id + page_item.workflow_state = 'unpublished' + page_item.save! t_course.disable_feature!(:draft_state) expect { @@ -64,12 +72,21 @@ run_jobs - quiz.reload.should be_edited - page.reload.hide_from_students.should be_true - topic.reload.should be_active - assignment.reload.should be_published - mod.reload.should be_active - item.reload.should be_active + # make sure everything published properly + mod.reload.should be_published + + topic.reload.should be_published + topic_item.reload.should be_published + assignment.reload.should be_published + assignment_item.reload.should be_published + + # quizzes can be unpublished in non-draft-state, thus should remain unchanged + quiz.reload.should be_unpublished + quiz_item.reload.should be_unpublished + + # pages can be unpublished (hidden from students) in non-draft-state, thus should remain unchanged + page.reload.should be_unpublished + page_item.reload.should be_unpublished end end diff --git a/spec/models/content_tag_spec.rb b/spec/models/content_tag_spec.rb index a2616528e518c..785e7a8e52a88 100644 --- a/spec/models/content_tag_spec.rb +++ b/spec/models/content_tag_spec.rb @@ -20,6 +20,76 @@ require File.expand_path(File.dirname(__FILE__) + '/../lib/validates_as_url.rb') describe ContentTag do + + describe "::asset_workflow_state" do + context "respond_to?(:published?)" do + mock_asset = Class.new do + def initialize(opts={}) + opts = {published: true, deleted: false}.merge(opts) + @published = opts[:published] + @deleted = opts[:deleted] + end + + def published?; !!@published; end + def unpublished?; !@published; end + def deleted?; @deleted; end + end + + it "returns 'deleted' for deleted assets" do + a = mock_asset.new(deleted: true) + ContentTag.asset_workflow_state(a).should == 'deleted' + end + + it "returns 'active' for published assets" do + a = mock_asset.new(published: true) + ContentTag.asset_workflow_state(a).should == 'active' + end + + it "returns 'unpublished' for unpublished assets" do + a = mock_asset.new(published: false) + ContentTag.asset_workflow_state(a).should == 'unpublished' + end + end + + context "respond_to?(:workflow_state)" do + mock_asset = Class.new do + attr_reader :workflow_state + def initialize(workflow_state) + @workflow_state = workflow_state + end + end + + it "returns 'active' for 'active' workflow_state" do + a = mock_asset.new('active') + ContentTag.asset_workflow_state(a).should == 'active' + end + + it "returns 'active' for 'available' workflow_state" do + a = mock_asset.new('available') + ContentTag.asset_workflow_state(a).should == 'active' + end + + it "returns 'active' for 'published' workflow_state" do + a = mock_asset.new('published') + ContentTag.asset_workflow_state(a).should == 'active' + end + + it "returns 'unpublished' for 'unpublished' workflow_state" do + a = mock_asset.new('unpublished') + ContentTag.asset_workflow_state(a).should == 'unpublished' + end + + it "returns 'deleted' for 'deleted' workflow_state" do + a = mock_asset.new('deleted') + ContentTag.asset_workflow_state(a).should == 'deleted' + end + + it "returns nil for other workflow_state" do + a = mock_asset.new('terrified') + ContentTag.asset_workflow_state(a).should == nil + end + end + end describe "#sync_workflow_state_to_asset?" do it "true when content_type is Quiz" do diff --git a/spec/selenium/context_modules_student_spec.rb b/spec/selenium/context_modules_student_spec.rb index 103363d812256..7bc0ec67acf4e 100644 --- a/spec/selenium/context_modules_student_spec.rb +++ b/spec/selenium/context_modules_student_spec.rb @@ -45,6 +45,7 @@ def navigate_to_module_item(module_num, link_text) @module_3 = create_context_module('Module Three') @quiz_1 = @course.quizzes.create!(:title => "some quiz") + @quiz_1.publish! @tag_3 = @module_3.add_item({:id => @quiz_1.id, :type => 'quiz'}) @module_3.completion_requirements = {@tag_3.id => {:type => 'must_view'}} @module_3.prerequisites = "module_#{@module_2.id}"