From 98fe5ee5c011f9e8611c4dc6930f65fd2e8bfb5a Mon Sep 17 00:00:00 2001 From: Bracken Mosbacker Date: Thu, 26 Apr 2012 08:32:54 -0600 Subject: [PATCH] enable assignment properties to be locked this creates a "plugin" that allows an account to choose assignment properties that can be locked when a course is copied Test Plan: * enable the plugin for an account * create an assignment and set it to be locked on copy * copy the course to another course * as an admin of the account you should still be able to edit the assignment * as a non-admin teacher of the course you should not be able to edit the frozen properties refs #7931 Change-Id: I61d5dbfdf10f8f7519f8db06449b14ef5e7b8454 Reviewed-on: https://gerrit.instructure.com/10190 Tested-by: Jenkins Reviewed-by: Cody Cutrer --- app/controllers/assignments_api_controller.rb | 22 +- app/controllers/assignments_controller.rb | 1 + app/models/assignment.rb | 44 ++- app/models/role_override.rb | 5 + .../_assignment_freezer_settings.html.erb | 19 + app/views/shared/_assignment.html.erb | 20 +- app/views/shared/_full_assignment.html.erb | 347 ++++++++++++------ app/views/shared/_topic.html.erb | 3 + .../20120417133444_add_assignment_lock.rb | 13 + lib/canvas/plugins/default_plugins.rb | 10 + lib/cc/assignment_resources.rb | 2 +- .../importer/canvas/assignment_converter.rb | 2 +- lib/cc/xsd/cccv1p0.xsd | 1 + public/javascripts/assignments.js | 2 +- public/javascripts/calendar.js | 4 +- public/javascripts/full_assignment.js | 21 +- public/javascripts/topics.js | 12 +- spec/apis/v1/assignments_api_spec.rb | 25 ++ .../canvas_cartridge_converter_spec.rb | 3 + spec/models/assignment_spec.rb | 123 +++++++ spec/selenium/assignments_spec.rb | 67 ++++ 21 files changed, 596 insertions(+), 150 deletions(-) create mode 100644 app/views/plugins/_assignment_freezer_settings.html.erb create mode 100644 db/migrate/20120417133444_add_assignment_lock.rb diff --git a/app/controllers/assignments_api_controller.rb b/app/controllers/assignments_api_controller.rb index 23260f56358..36bab96c1b1 100644 --- a/app/controllers/assignments_api_controller.rb +++ b/app/controllers/assignments_api_controller.rb @@ -185,16 +185,20 @@ class AssignmentsApiController < ApplicationController @assignment = @context.assignments.find(params[:id]) if authorized_action(@assignment, @current_user, :update_content) - if custom_vals = params[:assignment][:set_custom_field_values] - @assignment.set_custom_field_values = custom_vals - end - - if @assignment.update_attributes(assignment_params) - render :json => assignment_json(@assignment, @current_user, session, [], @context.user_is_teacher?(@current_user)).to_json, :status => 201 + if @assignment.frozen? + render :json => {:message => t('errors.no_edit_frozen', "You cannot edit a frozen assignment.")}.to_json, :status => 400 else - # TODO: we don't really have a strategy in the API yet for returning - # errors. - render :json => 'error'.to_json, :status => 400 + if custom_vals = params[:assignment][:set_custom_field_values] + @assignment.set_custom_field_values = custom_vals + end + + if @assignment.update_attributes(assignment_params) + render :json => assignment_json(@assignment, @current_user, session, [], @context.user_is_teacher?(@current_user)).to_json, :status => 201 + else + # TODO: we don't really have a strategy in the API yet for returning + # errors. + render :json => 'error'.to_json, :status => 400 + end end end end diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index 9aff85d40e8..ec3b9c6663b 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -281,6 +281,7 @@ class AssignmentsController < ApplicationController params[:assignment] = p else params[:assignment] ||= {} + @assignment.updating_user = @current_user if params[:assignment][:default_grade] params[:assignment][:overwrite_existing_grades] = (params[:assignment][:overwrite_existing_grades] == "1") @assignment.set_default_grade(params[:assignment]) diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 1ce481ba2b1..dd2bbc9c282 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -32,8 +32,8 @@ class Assignment < ActiveRecord::Base :peer_reviews, :automatic_peer_reviews, :grade_group_students_individually, :notify_of_update, :time_zone_edited, :turnitin_enabled, :turnitin_settings, :set_custom_field_values, :context, :position, :allowed_extensions, - :external_tool_tag_attributes - attr_accessor :original_id + :external_tool_tag_attributes, :freeze_on_copy + attr_accessor :original_id, :updating_user has_many :submissions, :class_name => 'Submission', :dependent => :destroy has_many :attachments, :as => :context, :dependent => :destroy @@ -77,6 +77,7 @@ class Assignment < ActiveRecord::Base validates_presence_of :context_type validates_length_of :title, :maximum => maximum_string_length, :allow_nil => true validates_length_of :description, :maximum => maximum_long_text_length, :allow_nil => true, :allow_blank => true + validate :frozen_atts_not_altered, :if => :frozen?, :on => :update acts_as_list :scope => :assignment_group_id has_a_broadcast_policy @@ -1261,7 +1262,7 @@ class Assignment < ActiveRecord::Base named_scope :only_graded, :conditions => "submission_types != 'not_graded'" named_scope :with_just_calendar_attributes, lambda { - { :select => ((Assignment.column_names & CalendarEvent.column_names) + ['due_at', 'assignment_group_id', 'could_be_locked', 'unlock_at', 'lock_at', 'submission_types'] - ['cloned_item_id', 'migration_id']).join(", ") } + { :select => ((Assignment.column_names & CalendarEvent.column_names) + ['due_at', 'assignment_group_id', 'could_be_locked', 'unlock_at', 'lock_at', 'submission_types', '(freeze_on_copy AND copied) AS frozen'] - ['cloned_item_id', 'migration_id']).join(", ") } } named_scope :due_between, lambda { |start, ending| @@ -1520,6 +1521,7 @@ class Assignment < ActiveRecord::Base description += hash[:instructions_in_html] == false ? ImportedHtmlConverter.convert_text(hash[:instructions] || "", context) : ImportedHtmlConverter.convert(hash[:instructions] || "", context) description += Attachment.attachment_list_from_migration(context, hash[:attachment_ids]) item.description = description + item.copied = true if !hash[:submission_types].blank? item.submission_types = hash[:submission_types] elsif ['discussion_topic'].include?(hash[:submission_format]) @@ -1595,7 +1597,7 @@ class Assignment < ActiveRecord::Base [:all_day, :turnitin_enabled, :peer_reviews_assigned, :peer_reviews, :automatic_peer_reviews, :anonymous_peer_reviews, :grade_group_students_individually, :allowed_extensions, :min_score, - :max_score, :mastery_score, :position, :peer_review_count + :max_score, :mastery_score, :position, :peer_review_count, :freeze_on_copy ].each do |prop| item.send("#{prop}=", hash[prop]) unless hash[prop].nil? end @@ -1687,4 +1689,38 @@ class Assignment < ActiveRecord::Base end protected :infer_comment_context_from_filename + FREEZABLE_ATTRIBUTES = %w{title description lock_at points_possible grading_type + submission_types assignment_group_id allowed_extensions + group_category_id notify_of_update peer_reviews workflow_state} + def frozen? + !!(self.freeze_on_copy && self.copied && PluginSetting.settings_for_plugin(:assignment_freezer)) + end + + def frozen_for_user?(user) + frozen? && !self.context.grants_right?(user, :manage_frozen_assignments) + end + + def att_frozen?(att, user=nil) + return false unless frozen? + if settings = PluginSetting.settings_for_plugin(:assignment_freezer) + if Canvas::Plugin.value_to_boolean(settings[att.to_s]) + if user + return !self.context.grants_right?(user, :manage_frozen_assignments) + else + return true + end + end + end + + false + end + + def frozen_atts_not_altered + FREEZABLE_ATTRIBUTES.each do |att| + if self.changes[att] && att_frozen?(att, @updating_user) + self.errors.add(att, t('errors.cannot_save_att', "You don't have permission to edit the locked attribute %{att_name}", :att_name => att)) + end + end + end + end diff --git a/app/models/role_override.rb b/app/models/role_override.rb index 626b91d1233..7a3e93714f7 100644 --- a/app/models/role_override.rb +++ b/app/models/role_override.rb @@ -602,6 +602,11 @@ class RoleOverride < ActiveRecord::Base :label => lambda { t('permissions.manage_sections', "Manage (create / edit / delete) course sections") }, :true_for => %w(AccountAdmin TeacherEnrollment DesignerEnrollment), :available_to => %w(AccountAdmin AccountMembership TeacherEnrollment TaEnrollment DesignerEnrollment), + }, + :manage_frozen_assignments => { + :label => lambda { t('permissions.manage_frozen_assignment', "Manage (edit / delete) frozen assignments") }, + :true_for => %w(AccountAdmin), + :available_to => %w(AccountAdmin AccountMembership), } }) diff --git a/app/views/plugins/_assignment_freezer_settings.html.erb b/app/views/plugins/_assignment_freezer_settings.html.erb new file mode 100644 index 00000000000..ebe35223dfd --- /dev/null +++ b/app/views/plugins/_assignment_freezer_settings.html.erb @@ -0,0 +1,19 @@ +<% fields_for :settings, OpenObject.new(settings) do |f| %> + + + + + <% Assignment::FREEZABLE_ATTRIBUTES.sort.each do |att| %> + + + + <% end %> +
+

<%= mt(:description, <<-TEXT) +This plugin allows admins to check a property on assignments that makes the configured +properties un-editable once the assignment is copied into another course. +TEXT + %>

+
<%= f.check_box att, {}, 'yes', 'no' %> + <%= f.label att, att %>
+<% end %> diff --git a/app/views/shared/_assignment.html.erb b/app/views/shared/_assignment.html.erb index 63c6cee1c3a..bf7712d2c40 100644 --- a/app/views/shared/_assignment.html.erb +++ b/app/views/shared/_assignment.html.erb @@ -8,10 +8,12 @@ <% cache(['assignment_render', assignment || context.asset_string, Time.zone.utc_offset].cache_key, :expires_in => (tomorrow_at_midnight - Time.zone.now).to_i) do %> <% assignment_url = context_url(context, :context_assignment_url, assignment_id) %> <% submission_url = context_url(context, :context_assignment_submission_url, "{{ assignment_id }}", "{{ user_id }}") %> -
<%= "muted" if assignment && assignment.muted? %>" id="assignment_<%= assignment ? assignment.id : "blank" %>" style="<%= hidden unless assignment %>"> +
<%= "muted" if assignment && assignment.muted? %>" id="assignment_<%= assignment ? assignment.id : "blank" %>" style="<%= hidden unless assignment %>">
- <%= image_tag 'move.png', :alt => t('alts.move', "Move"), :class => "move_icon", :title => t('titles.sort_or_move', "Sort Assignments or Move to Another Group"), :style => "margin-top: 3px;" %> + <% if assignment && !assignment.att_frozen?(:assignment_group_id, @current_user) %> + <%= image_tag 'move.png', :alt => t('alts.move', "Move"), :class => "move_icon", :title => t('titles.sort_or_move', "Sort Assignments or Move to Another Group"), :style => "margin-top: 3px;" %> + <% end %>
<%= assignment.try_rescue(:title) %> <%= image_tag("sound_mute.png", :title => t(:student_mute_notification, "Instructor is working on grades")) if assignment && assignment.muted? %> @@ -47,12 +49,14 @@ <%= image_tag 'preview.png', :alt => "Preview", :title => t('links.preview_assignment', "Preview Assignment"), :style => "display: none;" %> - - <%= image_tag 'edit.png', :alt => "Edit", :title => t('links.edit_assignment', "Edit Assignment") %> - - - <%= image_tag 'delete.png', :alt => "Delete", :title => t('links.delete_assignment', "Delete Assignment") %> - + <% if assignment && !assignment.frozen_for_user?(@current_user) %> + + <%= image_tag 'edit.png', :alt => "Edit", :title => t('links.edit_assignment', "Edit Assignment") %> + + + <%= image_tag 'delete.png', :alt => "Delete", :title => t('links.delete_assignment', "Delete Assignment") %> + + <% end %>
diff --git a/app/views/shared/_full_assignment.html.erb b/app/views/shared/_full_assignment.html.erb index afcbfaf844e..8e3864d5194 100644 --- a/app/views/shared/_full_assignment.html.erb +++ b/app/views/shared/_full_assignment.html.erb @@ -9,6 +9,7 @@ end is_discussion_topic = !!(assignment && assignment.submission_types == "discussion_topic" && assignment.discussion_topic) is_external_tool = !!(assignment.try(:submission_types) == "external_tool") + js_env :HIDE_DESCRIPTION => assignment.att_frozen?(:description, @current_user) %> <% js_block do %>