diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 10eec1cda8d..59a729793b1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -920,16 +920,17 @@ class ApplicationController < ActionController::Base end def content_tag_redirect(context, tag, error_redirect_symbol) + url_params = { :module_item_id => tag.id } if tag.content_type == 'Assignment' - redirect_to named_context_url(context, :context_assignment_url, tag.content_id) + redirect_to named_context_url(context, :context_assignment_url, tag.content_id, url_params) elsif tag.content_type == 'WikiPage' - redirect_to named_context_url(context, :context_wiki_page_url, tag.content.url) + redirect_to named_context_url(context, :context_wiki_page_url, tag.content.url, url_params) elsif tag.content_type == 'Attachment' - redirect_to named_context_url(context, :context_file_url, tag.content_id) + redirect_to named_context_url(context, :context_file_url, tag.content_id, url_params) elsif tag.content_type == 'Quiz' - redirect_to named_context_url(context, :context_quiz_url, tag.content_id) + redirect_to named_context_url(context, :context_quiz_url, tag.content_id, url_params) elsif tag.content_type == 'DiscussionTopic' - redirect_to named_context_url(context, :context_discussion_topic_url, tag.content_id) + redirect_to named_context_url(context, :context_discussion_topic_url, tag.content_id, url_params) elsif tag.content_type == 'ExternalUrl' @tag = tag @module = tag.context_module diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index bca67af71f1..2961c88ac50 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -67,7 +67,7 @@ class AssignmentsController < ApplicationController end @locked = @assignment.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true) @unlocked = !@locked || @assignment.grants_rights?(@current_user, session, :update)[:update] - @assignment_module = @assignment.context_module_tag + @assignment_module = ContextModuleItem.find_tag_with_preferred([@assignment], params[:module_item_id]) @assignment.context_module_action(@current_user, :read) if @unlocked && !@assignment.new_record? if @assignment.grants_right?(@current_user, session, :grade) visible_student_ids = @context.enrollments_visible_to(@current_user).find(:all, :select => 'user_id').map(&:user_id) diff --git a/app/controllers/context_modules_controller.rb b/app/controllers/context_modules_controller.rb index edf4dcf913c..b1c6d7431e4 100644 --- a/app/controllers/context_modules_controller.rb +++ b/app/controllers/context_modules_controller.rb @@ -269,16 +269,25 @@ class ContextModulesController < ApplicationController @modules = @context.context_modules.active @tags = @context.context_module_tags.active.sort_by{|t| t.position ||= 999} result = {} - result[:current_item] = @tags.detect{|t| t.content_type == type && t.content_id == id } - if !result[:current_item] - obj = @context.find_asset(params[:id], [:attachment, :discussion_topic, :assignment, :quiz, :wiki_page, :content_tag]) - - if obj.is_a?(ContentTag) - result[:current_item] = @tags.detect{|t| t.id == obj.id } - elsif obj.is_a?(DiscussionTopic) && obj.assignment_id - result[:current_item] = @tags.detect{|t| t.content_type == 'Assignment' && t.content_id == obj.assignment_id } - elsif obj.is_a?(Quiz) && obj.assignment_id - result[:current_item] = @tags.detect{|t| t.content_type == 'Assignment' && t.content_id == obj.assignment_id } + possible_tags = @tags.find_all {|t| t.content_type == type && t.content_id == id } + if possible_tags.size > 1 + # if there's more than one tag for the item, but the caller didn't + # specify which one they want, we don't want to return any information. + # this way the module item prev/next links won't appear with misleading navigation info. + if params[:module_item_id] + result[:current_item] = possible_tags.detect { |t| t.id == params[:module_item_id].to_i } + end + else + result[:current_item] = possible_tags.first + if !result[:current_item] + obj = @context.find_asset(params[:id], [:attachment, :discussion_topic, :assignment, :quiz, :wiki_page, :content_tag]) + if obj.is_a?(ContentTag) + result[:current_item] = @tags.detect{|t| t.id == obj.id } + elsif obj.is_a?(DiscussionTopic) && obj.assignment_id + result[:current_item] = @tags.detect{|t| t.content_type == 'Assignment' && t.content_id == obj.assignment_id } + elsif obj.is_a?(Quiz) && obj.assignment_id + result[:current_item] = @tags.detect{|t| t.content_type == 'Assignment' && t.content_id == obj.assignment_id } + end end end result[:current_item].evaluate_for(@current_user) rescue nil diff --git a/app/controllers/discussion_topics_controller.rb b/app/controllers/discussion_topics_controller.rb index 0a1bca4e981..5a1fa0f62e5 100644 --- a/app/controllers/discussion_topics_controller.rb +++ b/app/controllers/discussion_topics_controller.rb @@ -191,6 +191,8 @@ class DiscussionTopicsController < ApplicationController else format.html do + @context_module_tag = ContextModuleItem.find_tag_with_preferred([@topic, @topic.root_topic, @topic.assignment], params[:module_item_id]) + @sequence_asset = @context_module_tag.try(:content) env_hash = { :TOPIC => { :ID => @topic.id, diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb index 2be2bc73b68..af6ccf96b0f 100644 --- a/app/controllers/files_controller.rb +++ b/app/controllers/files_controller.rb @@ -124,8 +124,6 @@ class FilesController < ApplicationController if authorized_action(@attachment,@current_user,:read) if @attachment.grants_right?(@current_user, nil, :download) @headers = false - @tag = @attachment.context_module_tag - @module = @attachment.context_module_tag.context_module rescue nil render else show diff --git a/app/controllers/quizzes_controller.rb b/app/controllers/quizzes_controller.rb index ebbb4c42aeb..b86d8d70464 100644 --- a/app/controllers/quizzes_controller.rb +++ b/app/controllers/quizzes_controller.rb @@ -134,6 +134,8 @@ class QuizzesController < ApplicationController @locked_reason = @quiz.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true) @locked = @locked_reason && !@quiz.grants_right?(@current_user, session, :update) + @context_module_tag = ContextModuleItem.find_tag_with_preferred([@quiz, @quiz.assignment], params[:module_item_id]) + @sequence_asset = @context_module_tag.try(:content) @quiz.context_module_action(@current_user, :read) if !@locked @assignment = @quiz.assignment diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 2f9c7c76f49..b55cc20b8a4 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -24,6 +24,7 @@ class Assignment < ActiveRecord::Base include HasContentTags include CopyAuthorizedLinks include Mutable + include ContextModuleItem attr_accessible :title, :name, :description, :due_at, :points_possible, :min_score, :max_score, :mastery_score, :grading_type, :submission_types, @@ -40,7 +41,6 @@ class Assignment < ActiveRecord::Base has_one :quiz belongs_to :assignment_group has_one :discussion_topic, :conditions => ['discussion_topics.root_topic_id IS NULL'], :order => 'created_at' - has_one :context_module_tag, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND workflow_state != ?', 'context_module', 'deleted'], :include => {:context_module => [:context_module_progressions, :content_tags]} has_many :learning_outcome_tags, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND content_tags.workflow_state != ?', 'learning_outcome', 'deleted'], :include => :learning_outcome has_one :rubric_association, :as => :association, :conditions => ['rubric_associations.purpose = ?', "grading"], :order => :created_at, :include => :rubric has_one :rubric, :through => :rubric_association @@ -335,12 +335,13 @@ class Assignment < ActiveRecord::Base end def context_module_action(user, action, points=nil) - self.context_module_tag.context_module_action(user, action, points) if self.context_module_tag - if self.submission_types == 'discussion_topic' && self.discussion_topic && self.discussion_topic.context_module_tag - self.discussion_topic.context_module_tag.context_module_action(user, action, points) - elsif self.submission_types == 'online_quiz' && self.quiz && self.quiz.context_module_tag - self.quiz.context_module_tag.context_module_action(user, action, points) + tags_to_update = self.context_module_tags.to_a + if self.submission_types == 'discussion_topic' && self.discussion_topic + tags_to_update += self.discussion_topic.context_module_tags + elsif self.submission_types == 'online_quiz' && self.quiz + tags_to_update += self.quiz.context_module_tags end + tags_to_update.each { |tag| tag.context_module_action(user, action, points) } end set_broadcast_policy do |p| @@ -680,17 +681,16 @@ class Assignment < ActiveRecord::Base end def locked_for?(user=nil, opts={}) - @locks ||= {} locked = false return false if opts[:check_policies] && self.grants_right?(user, nil, :update) - @locks[user ? user.id : 0] ||= Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do + Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do locked = false if (self.unlock_at && self.unlock_at > Time.now) locked = {:asset_string => self.asset_string, :unlock_at => self.unlock_at} elsif (self.lock_at && self.lock_at <= Time.now) locked = {:asset_string => self.asset_string, :lock_at => self.lock_at} - elsif (self.could_be_locked && self.context_module_tag && self.context_module_tag.locked_for?(user, opts[:deep_check_if_needed])) - locked = {:asset_string => self.asset_string, :context_module => self.context_module_tag.context_module.attributes} + elsif self.could_be_locked && item = locked_by_module_item?(user, opts[:deep_check_if_needed]) + locked = {:asset_string => self.asset_string, :context_module => item.context_module.attributes} end locked end @@ -1237,10 +1237,6 @@ class Assignment < ActiveRecord::Base named_scope :no_graded_quizzes_or_topics, :conditions=>"submission_types NOT IN ('online_quiz', 'discussion_topic')" - named_scope :with_context_module_tags, lambda { - {:include => :context_module_tag } - } - named_scope :with_submissions, lambda { {:include => :submissions } } diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 19c23808277..5f04ed0209a 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -20,7 +20,8 @@ class Attachment < ActiveRecord::Base attr_accessible :context, :folder, :filename, :display_name, :user, :locked, :position, :lock_at, :unlock_at, :uploaded_data include HasContentTags - + include ContextModuleItem + belongs_to :context, :polymorphic => true belongs_to :cloned_item belongs_to :folder @@ -29,7 +30,6 @@ class Attachment < ActiveRecord::Base has_one :media_object has_many :submissions has_many :attachment_associations - has_one :context_module_tag, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND workflow_state != ?', 'context_module', 'deleted'], :include => {:context_module => :context_module_progressions} belongs_to :root_attachment, :class_name => 'Attachment' belongs_to :scribd_mime_type belongs_to :scribd_account @@ -990,17 +990,16 @@ class Attachment < ActiveRecord::Base end def locked_for?(user, opts={}) - @locks ||= {} return false if opts[:check_policies] && self.grants_right?(user, nil, :update) return {:manually_locked => true} if self.locked || (self.folder && self.folder.locked?) - @locks[user ? user.id : 0] ||= Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do + Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do locked = false if (self.unlock_at && Time.now < self.unlock_at) locked = {:asset_string => self.asset_string, :unlock_at => self.unlock_at} elsif (self.lock_at && Time.now > self.lock_at) locked = {:asset_string => self.asset_string, :lock_at => self.lock_at} - elsif (self.could_be_locked && self.context_module_tag && !self.context_module_tag.available_for?(user, opts[:deep_check_if_needed])) - locked = {:asset_string => self.asset_string, :context_module => self.context_module_tag.context_module.attributes} + elsif self.could_be_locked && item = locked_by_module_item?(user, opts[:deep_check_if_needed]) + locked = {:asset_string => self.asset_string, :context_module => item.context_module.attributes} end locked end @@ -1033,7 +1032,7 @@ class Attachment < ActiveRecord::Base end def context_module_action(user, action) - self.context_module_tag.context_module_action(user, action) if self.context_module_tag + self.context_module_tags.each { |tag| tag.context_module_action(user, action) } end include Workflow diff --git a/app/models/content_tag.rb b/app/models/content_tag.rb index 6aa78a3c2e7..21c99bdd186 100644 --- a/app/models/content_tag.rb +++ b/app/models/content_tag.rb @@ -33,7 +33,7 @@ class ContentTag < ActiveRecord::Base validates_presence_of :context, :unless => proc { |tag| tag.context_id && tag.context_type } validates_length_of :comments, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true before_save :default_values - after_save :enforce_unique_in_modules + after_save :update_could_be_locked after_save :touch_context_module after_save :touch_context_if_learning_outcome include CustomValidations @@ -86,23 +86,17 @@ class ContentTag < ActiveRecord::Base def context_name self.context.name rescue "" end - - def enforce_unique_in_modules - if self.workflow_state != 'deleted' && self.content_id && self.content_id > 0 && self.tag_type == 'context_module' && self.content_type != 'ContextExternalTool' - tags = ContentTag.find_all_by_content_id_and_content_type_and_tag_type_and_context_id_and_context_type(self.content_id, self.content_type, 'context_module', self.context_id, self.context_type) - tags.select{|t| t != self }.each do |tag| - tag.destroy - end - end + + def update_could_be_locked if self.content_id && self.content_type klass = self.content_type.constantize if klass.new.respond_to?(:could_be_locked=) - self.content_type.constantize.update_all({:could_be_locked => true}, {:id => self.content_id}) rescue nil + klass.update_all({:could_be_locked => true}, {:id => self.content_id}) end end true end - + def confirm_valid_module_requirements self.context_module && self.context_module.confirm_valid_requirements end diff --git a/app/models/context_module.rb b/app/models/context_module.rb index 7c1b8c76d07..50ae741b53c 100644 --- a/app/models/context_module.rb +++ b/app/models/context_module.rb @@ -303,7 +303,6 @@ class ContextModule < ActiveRecord::Base added_item else return nil unless item - added_item ||= ContentTag.find_by_content_id_and_content_type_and_context_id_and_context_type_and_tag_type(item.id, item.class.to_s, self.context_id, self.context_type, 'context_module') title = params[:title] || (item.title rescue item.name) added_item ||= self.content_tags.build(:context => context) added_item.attributes = { diff --git a/app/models/context_module_item.rb b/app/models/context_module_item.rb new file mode 100644 index 00000000000..224a47cdd79 --- /dev/null +++ b/app/models/context_module_item.rb @@ -0,0 +1,61 @@ +# +# Copyright (C) 2011 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# + +# This isn't a record on its own, but a module included in other records such +# as Attachment and Assignment. +# +# ContextModules contain items indirectly, through ContentTags that contain the +# information on position in the module, progression requirements, etc. +module ContextModuleItem + # set up the association for the AR class that included this module + def self.included(klass) + klass.has_many :context_module_tags, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND content_tags.workflow_state != ?', 'context_module', 'deleted'], :include => {:context_module => [:context_module_progressions, :content_tags]} + end + + # Check if this item is locked for the given user. + # If we are locked, this will return the module item (ContentTag) that is + # locking the item for the given user + def locked_by_module_item?(user, deep_check) + if self.context_module_tags.present? && self.context_module_tags.all? { |tag| tag.locked_for?(user, deep_check) } + item = self.context_module_tags.first + end + item || false + end + + # searches the ContextModuleItems in objs_to_search, in order, for the first + # context module tag -- returns the tag with id preferred_id if given and it + # exists + # + # If no preferred is found, but more than one tag exists for the same obj, we + # return nothing, since we can't know which tag is appropriate to return. + def self.find_tag_with_preferred(objs_to_search, preferred_id) + objs_to_search.each do |obj| + next unless obj.present? + tag = obj.context_module_tags.find_by_id(preferred_id) + return tag if tag + end + objs_to_search.each do |obj| + next unless obj.present? + tags = obj.context_module_tags.to_a + return nil if tags.size > 1 + tag = tags.first + return tag if tag + end + return nil + end +end diff --git a/app/models/discussion_topic.rb b/app/models/discussion_topic.rb index 36c983118ed..70900f57a08 100644 --- a/app/models/discussion_topic.rb +++ b/app/models/discussion_topic.rb @@ -23,6 +23,7 @@ class DiscussionTopic < ActiveRecord::Base include HasContentTags include CopyAuthorizedLinks include TextHelper + include ContextModuleItem attr_accessible :title, :message, :user, :delayed_post_at, :assignment, :plaintext_message, :podcast_enabled, :podcast_has_student_posts, @@ -39,7 +40,6 @@ class DiscussionTopic < ActiveRecord::Base has_many :discussion_entries, :order => :created_at, :dependent => :destroy has_many :root_discussion_entries, :class_name => 'DiscussionEntry', :include => [:user], :conditions => ['discussion_entries.parent_id IS NULL AND discussion_entries.workflow_state != ?', 'deleted'] - has_one :context_module_tag, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND workflow_state != ?', 'context_module', 'deleted'], :include => {:context_module => [:content_tags, :context_module_progressions]} has_one :external_feed_entry, :as => :asset belongs_to :external_feed belongs_to :context, :polymorphic => true @@ -135,8 +135,8 @@ class DiscussionTopic < ActiveRecord::Base attr_accessor :saved_by def update_assignment - if !self.assignment_id && @old_assignment_id && self.context_module_tag - self.context_module_tag.confirm_valid_module_requirements + if !self.assignment_id && @old_assignment_id + self.context_module_tags.each { |tag| tag.confirm_valid_module_requirements } end if @old_assignment_id Assignment.update_all({:workflow_state => 'deleted', :updated_at => Time.now.utc}, {:id => @old_assignment_id, :context_id => self.context_id, :context_type => self.context_type, :submission_types => 'discussion_topic'}) @@ -542,11 +542,12 @@ class DiscussionTopic < ActiveRecord::Base end def context_module_action(user, action, points=nil) - self.context_module_tag.context_module_action(user, action, points) if self.context_module_tag + tags_to_update = self.context_module_tags.to_a if self.for_assignment? - self.assignment.context_module_tag.context_module_action(user, action, points) if self.assignment.context_module_tag + tags_to_update += self.assignment.context_module_tags self.ensure_submission(user) if self.assignment.context.students.include?(user) && action == :contributed end + tags_to_update.each { |tag| tag.context_module_action(user, action, points) } end def ensure_submission(user) @@ -597,16 +598,15 @@ class DiscussionTopic < ActiveRecord::Base end def locked_for?(user=nil, opts={}) - @locks ||= {} return false if opts[:check_policies] && self.grants_right?(user, nil, :update) - @locks[user ? user.id : 0] ||= Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do + Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do locked = false if (self.delayed_post_at && self.delayed_post_at > Time.now) locked = {:asset_string => self.asset_string, :unlock_at => self.delayed_post_at} elsif (self.assignment && l = self.assignment.locked_for?(user, opts)) locked = l - elsif (self.could_be_locked && self.context_module_tag && !self.context_module_tag.available_for?(user, opts[:deep_check_if_needed])) - locked = {:asset_string => self.asset_string, :context_module => self.context_module_tag.context_module.attributes} + elsif self.could_be_locked && item = locked_by_module_item?(user, opts[:deep_check_if_needed]) + locked = {:asset_string => self.asset_string, :context_module => item.context_module.attributes} elsif (self.root_topic && l = self.root_topic.locked_for?(user, opts)) locked = l end diff --git a/app/models/quiz.rb b/app/models/quiz.rb index 9fbd0381c43..2501d057536 100644 --- a/app/models/quiz.rb +++ b/app/models/quiz.rb @@ -24,6 +24,8 @@ class Quiz < ActiveRecord::Base include CopyAuthorizedLinks include ActionView::Helpers::SanitizeHelper extend ActionView::Helpers::SanitizeHelper::ClassMethods + include ContextModuleItem + attr_accessible :title, :description, :points_possible, :assignment_id, :shuffle_answers, :show_correct_answers, :time_limit, :allowed_attempts, :scoring_policy, :quiz_type, :lock_at, :unlock_at, :due_at, :access_code, :anonymous_submissions, :assignment_group_id, @@ -36,7 +38,6 @@ class Quiz < ActiveRecord::Base has_many :quiz_questions, :dependent => :destroy, :order => 'position' has_many :quiz_submissions, :dependent => :destroy has_many :quiz_groups, :dependent => :destroy, :order => 'position' - has_one :context_module_tag, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND workflow_state != ?', 'context_module', 'deleted'], :include => {:context_module => [:context_module_progressions, :content_tags]} belongs_to :context, :polymorphic => true belongs_to :assignment belongs_to :cloned_item @@ -218,8 +219,8 @@ class Quiz < ActiveRecord::Base attr_accessor :saved_by def update_assignment send_later_if_production(:set_unpublished_question_count) if self.id - if !self.assignment_id && @old_assignment_id && self.context_module_tag - self.context_module_tag.confirm_valid_module_requirements + if !self.assignment_id && @old_assignment_id + self.context_module_tags.each { |tag| tag.confirm_valid_module_requirements } end if !self.graded? && (@old_assignment_id || self.last_assignment_id) Assignment.update_all({:workflow_state => 'deleted', :updated_at => Time.now.utc}, {:id => [@old_assignment_id, self.last_assignment_id].compact, :submission_types => 'online_quiz'}) @@ -588,9 +589,8 @@ class Quiz < ActiveRecord::Base alias_method :to_s, :quiz_title def locked_for?(user=nil, opts={}) - @locks ||= {} return false if opts[:check_policies] && self.grants_right?(user, nil, :update) - @locks[user ? user.id : 0] ||= Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do + Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do locked = false if (self.unlock_at && self.unlock_at > Time.now) sub = user && quiz_submissions.find_by_user_id(user.id) @@ -607,10 +607,10 @@ class Quiz < ActiveRecord::Base if !sub || !sub.manually_unlocked locked = l end - elsif (self.context_module_tag && !self.context_module_tag.available_for?(user, opts[:deep_check_if_needed])) + elsif item = locked_by_module_item?(user, opts[:deep_check_if_needed]) sub = user && quiz_submissions.find_by_user_id(user.id) if !sub || !sub.manually_unlocked - locked = {:asset_string => self.asset_string, :context_module => self.context_module_tag.context_module.attributes} + locked = {:asset_string => self.asset_string, :context_module => item.context_module.attributes} end end locked @@ -618,8 +618,11 @@ class Quiz < ActiveRecord::Base end def context_module_action(user, action, points=nil) - self.context_module_tag.context_module_action(user, action, points) if self.context_module_tag - self.assignment.context_module_tag.context_module_action(user, action, points) if self.assignment && self.assignment.context_module_tag + tags_to_update = self.context_module_tags.to_a + if self.assignment + tags_to_update += self.assignment.context_module_tags + end + tags_to_update.each { |tag| tag.context_module_action(user, action, points) } end # virtual attribute diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index d3d3e0f7b56..6f18e5745b5 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -24,12 +24,12 @@ class WikiPage < ActiveRecord::Base include Workflow include HasContentTags include CopyAuthorizedLinks + include ContextModuleItem belongs_to :wiki, :touch => true belongs_to :wiki_with_participants, :class_name => 'Wiki', :foreign_key => 'wiki_id', :include => {:wiki_namespaces => :context } belongs_to :cloned_item belongs_to :user - has_many :context_module_tags, :as => :content, :class_name => 'ContentTag', :conditions => ['content_tags.tag_type = ? AND workflow_state != ?', 'context_module', 'deleted'], :include => {:context_module => [:content_tags, :context_module_progressions]} has_many :wiki_page_comments, :order => "created_at DESC" acts_as_url :title, :scope => [:wiki_id, :not_deleted], :sync_url => true @@ -195,8 +195,7 @@ class WikiPage < ActiveRecord::Base def locked_for?(context, user, opts={}) return false unless self.could_be_locked - @locks ||= {} - @locks[user ? user.id : 0] ||= Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do + Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do m = context_module_tag_for(context, user).context_module rescue nil locked = false if (m && !m.available_for?(user)) diff --git a/app/views/assignments/show.html.erb b/app/views/assignments/show.html.erb index 0ecd9fc6b90..4354061b5c0 100644 --- a/app/views/assignments/show.html.erb +++ b/app/views/assignments/show.html.erb @@ -104,5 +104,5 @@ $(document).ready(function() { <%= render :partial => "shared/rubric_forms" %> <% end %> <%= render :partial => "shared/aligned_outcomes", :locals => {:asset => @assignment} %> - <%= render :partial => "shared/sequence_footer", :locals => {:asset => @assignment} if @assignment.context_module_tag %> + <%= render :partial => "shared/sequence_footer", :locals => {:asset => @assignment} if !@assignment.context_module_tags.empty? %> <% end %> diff --git a/app/views/context_modules/_module_item.html.erb b/app/views/context_modules/_module_item.html.erb index 5dbe2083a81..f2891b67a16 100644 --- a/app/views/context_modules/_module_item.html.erb +++ b/app/views/context_modules/_module_item.html.erb @@ -12,8 +12,10 @@ 'other' => image_tag("blank.png", :class => "image", :alt => '') } %> -<% criterion = completion_criteria && completion_criteria.find{|c| c[:id] == tag.id} %> -" class="context_module_item <%= module_item.content_type_class if module_item %> <%= 'also_assignment' if module_item && module_item.graded? %> indent_<%= tag.try_rescue(:indent) || '0' %> <%= 'progression_requirement' if criterion %> <%= criterion[:type] if criterion %>_requirement" style="<%= hidden unless module_item %>"> +<% criterion = completion_criteria && completion_criteria.find{|c| c[:id] == tag.id} + item_class = "#{module_item.content_type}_#{module_item.content_id}" if module_item +%> +
" class="context_module_item <%= module_item.content_type_class if module_item %> <%= 'also_assignment' if module_item && module_item.graded? %> indent_<%= tag.try_rescue(:indent) || '0' %> <%= 'progression_requirement' if criterion %> <%= criterion[:type] if criterion %>_requirement <%= item_class %>" style="<%= hidden unless module_item %>">
diff --git a/app/views/discussion_topics/show.html.erb b/app/views/discussion_topics/show.html.erb index fc9e4d36528..de3520aa53b 100644 --- a/app/views/discussion_topics/show.html.erb +++ b/app/views/discussion_topics/show.html.erb @@ -225,10 +225,7 @@
<%= - sequence_asset = @topic - sequence_asset = @topic.root_topic if @topic.root_topic && !@topic.context_module_tag && @topic.root_topic.context_module_tag - sequence_asset = @topic.assignment if @topic.assignment && !@topic.context_module_tag && @topic.assignment.context_module_tag - render :partial => "shared/sequence_footer", :locals => {:asset => sequence_asset, :context => sequence_asset.context} if sequence_asset.context_module_tag + render :partial => "shared/sequence_footer", :locals => {:asset => @sequence_asset, :context => @sequence_asset.context} if @sequence_asset %> <% end %> <% if @headers == false || @locked %> diff --git a/app/views/quizzes/show.html.erb b/app/views/quizzes/show.html.erb index 4e9dcbb0283..9feb14af7c7 100644 --- a/app/views/quizzes/show.html.erb +++ b/app/views/quizzes/show.html.erb @@ -220,8 +220,4 @@ <%= render :partial => "shared/message_students" %> <% end %> <% end %> -<% - sequence_asset = @quiz - sequence_asset = @quiz.assignment if @quiz.assignment && !@quiz.context_module_tag && @quiz.assignment.context_module_tag -%> -<%= render :partial => "shared/sequence_footer", :locals => {:asset => sequence_asset} if sequence_asset.context_module_tag %> +<%= render :partial => "shared/sequence_footer", :locals => {:asset => @sequence_asset} if @sequence_asset %> diff --git a/app/views/shared/_sequence_footer.html.erb b/app/views/shared/_sequence_footer.html.erb index cb9a7576ddf..b97657764c4 100644 --- a/app/views/shared/_sequence_footer.html.erb +++ b/app/views/shared/_sequence_footer.html.erb @@ -9,7 +9,7 @@ <%= image_tag "forward.png" %> <%= t(:next, "Next") %> - + " class="module_item_url" style="display: none;">  " class="module_url" style="display: none;"> 
diff --git a/public/javascripts/context_modules.js b/public/javascripts/context_modules.js index 0e9286bcd14..c3e2e0f5893 100644 --- a/public/javascripts/context_modules.js +++ b/public/javascripts/context_modules.js @@ -182,6 +182,14 @@ define([ }, function() { }); }, + itemClass: function(content_tag) { + return content_tag.content_type + "_" + content_tag.content_id; + }, + updateAllItemInstances: function(content_tag) { + $(".context_module_item."+modules.itemClass(content_tag)+" .title").each(function() { + $(this).text(content_tag.title); + }); + }, editModule: function($module) { var $form = $("#add_context_module_form"); $form.data('current_module', $module); @@ -279,6 +287,7 @@ define([ $item.removeClass('indent_' + idx); } $item.addClass('indent_' + (data.indent || 0)); + $item.addClass(modules.itemClass(data)); // don't just tack onto the bottom, put it in its correct position var $before = null; $module.find(".context_module_items").children().each(function() { @@ -663,6 +672,7 @@ define([ var $module = $("#context_module_" + data.content_tag.context_module_id); var $item = modules.addItemToModule($module, data.content_tag); $module.find(".context_module_items").sortable('refresh'); + modules.updateAllItemInstances(data.content_tag); modules.updateAssignmentData(); $(this).dialog('close'); }, diff --git a/spec/controllers/context_modules_controller_spec.rb b/spec/controllers/context_modules_controller_spec.rb index 2e36df79a07..3d5440f7f0a 100644 --- a/spec/controllers/context_modules_controller_spec.rb +++ b/spec/controllers/context_modules_controller_spec.rb @@ -55,10 +55,10 @@ describe ContextModulesController do header2 = @module.add_item :type => 'context_module_sub_header' get 'module_redirect', :course_id => @course.id, :context_module_id => @module.id, :first => 1 - response.should redirect_to course_assignment_url(@course.id, assignment1.id) + response.should redirect_to course_assignment_url(@course.id, assignment1.id, :module_item_id => assignmentTag1.id) get 'module_redirect', :course_id => @course.id, :context_module_id => @module.id, :last => 1 - response.should redirect_to course_assignment_url(@course.id, assignment2.id) + response.should redirect_to course_assignment_url(@course.id, assignment2.id, :module_item_id => assignmentTag2.id) assignmentTag1.destroy assignmentTag2.destroy @@ -152,7 +152,7 @@ describe ContextModulesController do get 'item_redirect', :course_id => @course.id, :id => assignmentTag1.id response.should be_redirect - response.should redirect_to course_assignment_url(@course, assignment1) + response.should redirect_to course_assignment_url(@course, assignment1, :module_item_id => assignmentTag1.id) end it "should redirect to a discussion page" do @@ -165,7 +165,7 @@ describe ContextModulesController do get 'item_redirect', :course_id => @course.id, :id => topicTag.id response.should be_redirect - response.should redirect_to course_discussion_topic_url(@course, topic) + response.should redirect_to course_discussion_topic_url(@course, topic, :module_item_id => topicTag.id) end it "should redirect to a quiz page" do @@ -178,7 +178,7 @@ describe ContextModulesController do get 'item_redirect', :course_id => @course.id, :id => tag.id response.should be_redirect - response.should redirect_to course_quiz_url(@course, quiz) + response.should redirect_to course_quiz_url(@course, quiz, :module_item_id => tag.id) end end @@ -217,4 +217,4 @@ describe ContextModulesController do ct1.context_module.should == m1 end end -end \ No newline at end of file +end diff --git a/spec/integration/context_module_spec.rb b/spec/integration/context_module_spec.rb index 905a7fc7ab9..6b1ae11b668 100644 --- a/spec/integration/context_module_spec.rb +++ b/spec/integration/context_module_spec.rb @@ -132,7 +132,7 @@ describe ContextModule do get next_link response.should be_redirect - response.location.ends_with?(@test_url).should be_true + response.location.ends_with?(@test_url + "?module_item_id=#{@tag2.id}").should be_true get @test_url response.should be_success diff --git a/spec/models/context_module_spec.rb b/spec/models/context_module_spec.rb index 9ac6ef68419..250007b339f 100644 --- a/spec/models/context_module_spec.rb +++ b/spec/models/context_module_spec.rb @@ -136,6 +136,22 @@ describe ContextModule do @tag.content.should eql(@file) @module.content_tags.should be_include(@tag) end + + it "should allow adding items more than once" do + course_module + @assignment = @course.assignments.create!(:title => "some assignment") + @tag1 = @module.add_item(:id => @assignment.id, :type => "assignment") + @tag2 = @module.add_item(:id => @assignment.id, :type => "assignment") + @tag1.should_not == @tag2 + @module.content_tags.should be_include(@tag1) + @module.content_tags.should be_include(@tag2) + + @mod2 = @course.context_modules.create!(:name => "mod2") + @tag3 = @mod2.add_item(:id => @assignment.id, :type => "assignment") + @tag3.should_not == @tag1 + @tag3.should_not == @tag2 + @mod2.content_tags.should == [@tag3] + end end describe "completion_requirements=" do @@ -249,6 +265,32 @@ describe ContextModule do @progression.should be_locked end + describe "multi-items" do + it "should be locked if all tags are locked" do + course_module + @user = User.create!(:name => "some name") + @course.enroll_student(@user) + @a1 = @course.assignments.create!(:title => "some assignment") + @tag1 = @module.add_item({:id => @a1.id, :type => 'assignment'}) + @module.require_sequential_progress = true + @module.completion_requirements = {@tag1.id => {:type => 'must_submit'}} + @module.save! + @a2 = @course.assignments.create!(:title => "locked assignment") + @a2.locked_for?(@user).should be_false + @tag2 = @module.add_item({:id => @a2.id, :type => 'assignment'}) + @a2.reload.locked_for?(@user).should be_true + + @mod2 = @course.context_modules.create!(:name => "mod2") + @tag3 = @mod2.add_item({:id => @a2.id, :type => 'assignment'}) + # not locked, because the second tag allows access + @a2.reload.locked_for?(@user).should be_false + @mod2.prerequisites = "module_#{@module.id}" + @mod2.save! + # now locked, because mod2 is locked + @a2.reload.locked_for?(@user).should be_true + end + end + it "should not be available if previous module is incomplete" do course_module @assignment = @course.assignments.create!(:title => "some assignment") @@ -448,7 +490,7 @@ describe ContextModule do @progression.current_position.should eql(@tag2.position) @assignment.reload; @assignment2.reload @assignment.locked_for?(@user).should eql(false) - @assignment2.locked_for?(@user).should_not eql(false) + @assignment2.locked_for?(@user).should eql(false) @module.completion_requirements = {@tag.id => {:type => 'min_score', :min_score => 5}} @module.save diff --git a/spec/selenium/context_modules_spec.rb b/spec/selenium/context_modules_spec.rb index fed29dea86f..6a2a886cf80 100644 --- a/spec/selenium/context_modules_spec.rb +++ b/spec/selenium/context_modules_spec.rb @@ -85,6 +85,15 @@ describe "context_modules" do @module = @course.context_modules.create!(:name => "some module") end + def edit_module_item(module_item) + driver.execute_script("$(arguments[0]).addClass('context_module_item_hover')", module_item) + module_item.find_element(:css, '.edit_item_link').click + edit_form = driver.find_element(:id, 'edit_item_form') + yield edit_form + submit_form(edit_form) + wait_for_ajaximations + end + context "context modules as a teacher" do before (:each) do @@ -255,12 +264,9 @@ describe "context_modules" do item_edit_text = "Assignment Edit 1" module_item = add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) tag = ContentTag.last - driver.execute_script("$('#context_module_item_#{tag.id}').addClass('context_module_item_hover')") - module_item.find_element(:css, '.edit_item_link').click - edit_form = driver.find_element(:id, 'edit_item_form') - replace_content(edit_form.find_element(:id, 'content_tag_title'), item_edit_text) - submit_form(edit_form) - wait_for_ajaximations + edit_module_item(module_item) do |edit_form| + replace_content(edit_form.find_element(:id, 'content_tag_title'), item_edit_text) + end module_item = driver.find_element(:id, "context_module_item_#{tag.id}") module_item.should include_text(item_edit_text) @@ -275,6 +281,41 @@ describe "context_modules" do add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) end + it "should allow adding an item twice" do + item1 = add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) + item2 = add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) + item1.should_not == item2 + @assignment.reload.context_module_tags.size.should == 2 + end + + it "should rename all instances of an item" do + item1 = add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) + item2 = add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) + edit_module_item(item2) do |edit_form| + replace_content(edit_form.find_element(:id, 'content_tag_title'), "renamed assignment") + end + all_items = ff(".context_module_item.Assignment_#{@assignment.id}") + all_items.size.should == 2 + all_items.each { |i| i.find_element(:css, '.title').text.should == 'renamed assignment' } + @assignment.reload.title.should == 'renamed assignment' + run_jobs + @assignment.context_module_tags.each { |tag| tag.title.should == 'renamed assignment' } + + # reload the page and renaming should still work on existing items + get "/courses/#{@course.id}/modules" + wait_for_ajaximations + item3 = add_existing_module_item('#assignments_select', 'Assignment', @assignment.title) + edit_module_item(item3) do |edit_form| + replace_content(edit_form.find_element(:id, 'content_tag_title'), "again") + end + all_items = ff(".context_module_item.Assignment_#{@assignment.id}") + all_items.size.should == 3 + all_items.each { |i| i.find_element(:css, '.title').text.should == 'again' } + @assignment.reload.title.should == 'again' + run_jobs + @assignment.context_module_tags.each { |tag| tag.title.should == 'again' } + end + it "should add a quiz to a module" do add_existing_module_item('#quizs_select', 'Quiz', @quiz.title) end @@ -338,7 +379,7 @@ describe "context_modules" do driver.find_element(:css, '.ui-dialog .add_prerequisite_link').click wait_for_animations #have to do it this way because the select has no css attributes on it - click_option(':input:visible.eq(3)', first_module_name) + click_option('.criterion select', "the module, #{first_module_name}") submit_form(add_form) wait_for_ajaximations db_module = ContextModule.last @@ -347,7 +388,7 @@ describe "context_modules" do driver.find_element(:css, "#context_module_#{db_module.id} .edit_module_link").click driver.find_element(:css, '.ui-dialog').should be_displayed wait_for_ajaximations - prereq_select = find_all_with_jquery(':input:visible')[3] + prereq_select = find_with_jquery('.criterion select') option = first_selected_option(prereq_select) option.text.should == 'the module, ' + first_module_name end @@ -627,6 +668,48 @@ describe "context_modules" do end describe "sequence footer" do + it "should show the right nav when an item is in modules multiple times" do + @assignment = @course.assignments.create!(:title => "some assignment") + @atag1 = @module_1.add_item(:id => @assignment.id, :type => "assignment") + @after1 = @module_1.add_item(:type => "external_url", :title => "url1", :url => "http://example.com/1") + @atag2 = @module_2.add_item(:id => @assignment.id, :type => "assignment") + @after2 = @module_2.add_item(:type => "external_url", :title => "url2", :url => "http://example.com/2") + get "/courses/#{@course.id}/modules/items/#{@atag1.id}" + wait_for_ajaximations + prev = driver.find_element(:css, '#sequence_footer a.prev') + URI.parse(prev.attribute('href')).path.should == "/courses/#{@course.id}/modules/items/#{@tag_1.id}" + nxt = driver.find_element(:css, '#sequence_footer a.next') + URI.parse(nxt.attribute('href')).path.should == "/courses/#{@course.id}/modules/items/#{@after1.id}" + + get "/courses/#{@course.id}/modules/items/#{@atag2.id}" + wait_for_ajaximations + prev = driver.find_element(:css, '#sequence_footer a.prev') + URI.parse(prev.attribute('href')).path.should == "/courses/#{@course.id}/modules/items/#{@tag_2.id}" + nxt = driver.find_element(:css, '#sequence_footer a.next') + URI.parse(nxt.attribute('href')).path.should == "/courses/#{@course.id}/modules/items/#{@after2.id}" + + # if the user didn't get here from a module link, we show no nav, + # because we can't know which nav to show + get "/courses/#{@course.id}/assignments/#{@assignment.id}" + wait_for_ajaximations + prev = driver.find_element(:css, '#sequence_footer a.prev') + prev.should_not be_displayed + nxt = driver.find_element(:css, '#sequence_footer a.next') + nxt.should_not be_displayed + end + + it "should show the nav when going straight to the item if there's only one tag" do + @assignment = @course.assignments.create!(:title => "some assignment") + @atag1 = @module_1.add_item(:id => @assignment.id, :type => "assignment") + @after1 = @module_1.add_item(:type => "external_url", :title => "url1", :url => "http://example.com/1") + get "/courses/#{@course.id}/assignments/#{@assignment.id}" + wait_for_ajaximations + prev = driver.find_element(:css, '#sequence_footer a.prev') + URI.parse(prev.attribute('href')).path.should == "/courses/#{@course.id}/modules/items/#{@tag_1.id}" + nxt = driver.find_element(:css, '#sequence_footer a.next') + URI.parse(nxt.attribute('href')).path.should == "/courses/#{@course.id}/modules/items/#{@after1.id}" + end + it "should show module navigation for group assignment discussions" do group_assignment_discussion(:course => @course) @group.users << @student