hide description from locked assignments on the calendar; fixes #5215

Fixing this required some minor refactoring:

 * A model can now define "filter_hash_for_user" to filter serialized data out
   of the model based on user. In this case, we were sending the assignment to
   the browser in json, and we wanted to strip out the description if the
   assignment was locked for the user.
 * The lock_explanation generator was not i18n'd before. That was fixed, and
   similar code in javascript was also refactored so it can be called by
   anybody. (In this case, by the assignments in the calendar.)

Change-Id: Ia606be2a16df9bd87222306445f548b3a7a78801
Reviewed-on: https://gerrit.instructure.com/5051
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Zach Wily 2011-08-11 13:49:40 -06:00
parent 6a65bf5b4e
commit 50ac28ff9c
9 changed files with 191 additions and 60 deletions

View File

@ -78,6 +78,9 @@ class CalendarsController < ApplicationController
format = request.format.to_sym.to_s
if format == 'json'
args << { :user_content => %w(description) }
if @current_user
args.last[:permissions] = { :user => @current_user, :session => session }
end
end
@events.concat(@undated_events).send("to_#{format}", *args)
end

View File

@ -68,26 +68,85 @@ module ApplicationHelper
end
def lock_explanation(hash, type, context=nil)
res = "This #{type} "
hash ||= {}
# Any additions to this function should also be made in javascripts/content_locks.js
if hash[:lock_at]
res += "was locked #{datetime_string(hash[:lock_at])}"
case type
when "quiz"
return I18n.t('messages.quiz_locked_at', "This quiz was locked %{at}.", :at => datetime_string(hash[:lock_at]))
when "assignment"
return I18n.t('messages.assignment_locked_at', "This assignment was locked %{at}.", :at => datetime_string(hash[:lock_at]))
when "topic"
return I18n.t('messages.topic_locked_at', "This topic was locked %{at}.", :at => datetime_string(hash[:lock_at]))
when "file"
return I18n.t('messages.file_locked_at', "This file was locked %{at}.", :at => datetime_string(hash[:lock_at]))
when "page"
return I18n.t('messages.page_locked_at', "This page was locked %{at}.", :at => datetime_string(hash[:lock_at]))
else
return I18n.t('messages.content_locked_at', "This content was locked %{at}.", :at => datetime_string(hash[:lock_at]))
end
elsif hash[:unlock_at]
res += "is locked until #{datetime_string(hash[:unlock_at])}"
case type
when "quiz"
return I18n.t('messages.quiz_locked_until', "This quiz is locked until %{date}.", :date => datetime_string(hash[:unlock_at]))
when "assignment"
return I18n.t('messages.assignment_locked_until', "This assignment is locked until %{date}.", :date => datetime_string(hash[:unlock_at]))
when "topic"
return I18n.t('messages.topic_locked_until', "This topic is locked until %{date}.", :date => datetime_string(hash[:unlock_at]))
when "file"
return I18n.t('messages.file_locked_until', "This file is locked until %{date}.", :date => datetime_string(hash[:unlock_at]))
when "page"
return I18n.t('messages.page_locked_until', "This page is locked until %{date}.", :date => datetime_string(hash[:unlock_at]))
else
return I18n.t('messages.content_locked_until', "This content is locked until %{date}.", :date => datetime_string(hash[:unlock_at]))
end
elsif hash[:context_module]
obj = hash[:context_module].is_a?(ContextModule) ? hash[:context_module] : OpenObject.new(hash[:context_module])
res += "hasn't been unlocked yet. It is part of the module <b>#{obj.name}</b>."
html = case type
when "quiz"
I18n.t('messages.quiz_locked_module', "This quiz is part of the module *%{module}* and hasn't been unlocked yet.",
:module => TextHelper.escape_html(obj.name), :wrapper => '<b>\1</b>')
when "assignment"
I18n.t('messages.assignment_locked_module', "This assignment is part of the module *%{module}* and hasn't been unlocked yet.",
:module => TextHelper.escape_html(obj.name), :wrapper => '<b>\1</b>')
when "topic"
I18n.t('messages.topic_locked_module', "This topic is part of the module *%{module}* and hasn't been unlocked yet.",
:module => TextHelper.escape_html(obj.name), :wrapper => '<b>\1</b>')
when "file"
I18n.t('messages.file_locked_module', "This file is part of the module *%{module}* and hasn't been unlocked yet.",
:module => TextHelper.escape_html(obj.name), :wrapper => '<b>\1</b>')
when "page"
I18n.t('messages.page_locked_module', "This page is part of the module *%{module}* and hasn't been unlocked yet.",
:module => TextHelper.escape_html(obj.name), :wrapper => '<b>\1</b>')
else
I18n.t('messages.content_locked_module', "This content is part of the module *%{module}* and hasn't been unlocked yet.",
:module => TextHelper.escape_html(obj.name), :wrapper => '<b>\1</b>')
end
if context
res += "<br/><a href='#{context_url(context, :context_context_modules_url)}'>Visit the #{context.class.to_s.downcase} modules page for information on how to unlock this content.</a>"
res += "<a href='#{context_url(context, :context_context_module_prerequisites_needing_finishing_url, obj.id, hash[:asset_string])}' style='display: none;' id='module_prerequisites_lookup_link'>&nbsp;</a>"
html << "<br/>".html_safe
html << I18n.t('messages.visit_modules_page', "*Visit the course modules page for information on how to unlock this content.*",
:wrapper => "<a href='#{context_url(context, :context_context_modules_url)}'>\\1</a>")
html << "<a href='#{context_url(context, :context_context_module_prerequisites_needing_finishing_url, obj.id, hash[:asset_string])}' style='display: none;' id='module_prerequisites_lookup_link'>&nbsp;</a>".html_safe
jammit_js :prerequisites_lookup
end
return html
else
res += "is currently locked"
case type
when "quiz"
return I18n.t('messages.quiz_locked', "This quiz is currently locked.")
when "assignment"
return I18n.t('messages.assignment_locked', "This assignment is currently locked.")
when "topic"
return I18n.t('messages.topic_locked', "This topic is currently locked.")
when "file"
return I18n.t('messages.file_locked', "This file is currently locked.")
when "page"
return I18n.t('messages.page_locked', "This page is currently locked.")
else
return I18n.t('messages.content_locked', "This quiz is currently locked.")
end
end
raw(res)
end
def avatar_image(user_id, height=50)
if session["reported_#{user_id}"]
image_tag "no_pic.gif"

View File

@ -662,6 +662,13 @@ class Assignment < ActiveRecord::Base
can :update and can :update_content and can :delete and can :create and can :read and can :attach_submission_comment_files
end
def filter_attributes_for_user(hash, user, session)
if lock_info = self.locked_for?(user)
hash.delete('description')
hash['lock_info'] = lock_info
end
end
def self.search(query)
find(:all, :conditions => wildcard('title', 'description', query))
end
@ -1140,7 +1147,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'] - ['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'] - ['cloned_item_id', 'migration_id']).join(", ") }
}
named_scope :due_between, lambda { |start, ending|

View File

@ -138,6 +138,7 @@
<div class="details_time time_string">&nbsp;</div>
<div class="details_title title">&nbsp;</div>
<div class="details_description description user_content" style="min-height: 20px; max-height: 150px; overflow: auto;"></div>
<div class="details_description lock_explanation"></div>
<!--a href="#" class="assignment_link" style="display: none;">View Assignment Details</a-->
<div class="links">
<a href="/{{ context_type }}s/{{ context_id }}/assignments/{{ id }}" class="assignment_url" style="display: none;">&nbsp;</a>

View File

@ -96,6 +96,7 @@ javascripts:
calendar:
- public/javascripts/calendar.js
- public/javascripts/calendar_move.js
- public/javascripts/content_locks.js
collaborations:
- public/javascripts/collaborations.js
context_messages:

View File

@ -212,12 +212,12 @@ class ActiveRecord::Base
hash = Serializer.new(self, options).serializable_record
if options[:permissions]
permissions_hash = self.grants_rights?(options[:permissions][:user], options[:permissions][:session], *options[:permissions][:policies])
if options[:include_root]
hash[self.class.base_ar_class.model_name.element]["permissions"] = permissions_hash
else
hash["permissions"] = permissions_hash
obj_hash = options[:include_root] ? hash[self.class.base_ar_class.model_name.element] : hash
if self.respond_to?(:filter_attributes_for_user)
self.filter_attributes_for_user(obj_hash, options[:permissions][:user], options[:permissions][:session])
end
permissions_hash = self.grants_rights?(options[:permissions][:user], options[:permissions][:session], *options[:permissions][:policies])
obj_hash["permissions"] = permissions_hash
end
self.revert_from_serialization_options if self.respond_to?(:revert_from_serialization_options)

View File

@ -670,6 +670,9 @@ I18n.scoped('calendars', function(I18n) {
}
data.description = $event.data('description');
$box.fillTemplateData({data: data, htmlValues: ['description']});
if (data.lock_info) {
$box.find(".lock_explanation").html(INST.lockExplanation(data.lock_info, 'assignment'));
}
$("#edit_event").dialog('close');
$("#event_details").dialog('close');
$("#event_details").find(".description").css("max-height", Math.max($(window).height() - 200, 150));

View File

@ -17,6 +17,86 @@
*/
I18n.scoped('content_locks', function(I18n) {
INST.lockExplanation = function(data, type) {
// Any additions to this function should also be added to similar logic in ApplicationController.rb
if(data.lock_at) {
switch (type) {
case "quiz":
return I18n.t('messages.quiz_locked_at', "This quiz was locked %{at}.", {at: $.parseFromISO(data.lock_at).datetime_formatted});
case "assignment":
return I18n.t('messages.assignment_locked_at', "This assignment was locked %{at}.", {at: $.parseFromISO(data.lock_at).datetime_formatted});
case "topic":
return I18n.t('messages.topic_locked_at', "This topic was locked %{at}.", {at: $.parseFromISO(data.lock_at).datetime_formatted});
case "file":
return I18n.t('messages.file_locked_at', "This file was locked %{at}.", {at: $.parseFromISO(data.lock_at).datetime_formatted});
case "page":
return I18n.t('messages.page_locked_at', "This page was locked %{at}.", {at: $.parseFromISO(data.lock_at).datetime_formatted});
default:
return I18n.t('messages.content_locked_at', "This content was locked %{at}.", {at: $.parseFromISO(data.lock_at).datetime_formatted});
}
} else if(data.unlock_at) {
switch (type) {
case "quiz":
return I18n.t('messages.quiz_locked_until', "This quiz is locked until %{date}.", {date: $.parseFromISO(data.unlock_at).datetime_formatted});
case "assignment":
return I18n.t('messages.assignment_locked_until', "This assignment is locked until %{date}.", {date: $.parseFromISO(data.unlock_at).datetime_formatted});
case "topic":
return I18n.t('messages.topic_locked_until', "This topic is locked until %{date}.", {date: $.parseFromISO(data.unlock_at).datetime_formatted});
case "file":
return I18n.t('messages.file_locked_until', "This file is locked until %{date}.", {date: $.parseFromISO(data.unlock_at).datetime_formatted});
case "page":
return I18n.t('messages.page_locked_until', "This page is locked until %{date}.", {date: $.parseFromISO(data.unlock_at).datetime_formatted});
default:
return I18n.t('messages.content_locked_until', "This content is locked until %{date}.", {date: $.parseFromISO(data.unlock_at).datetime_formatted});
}
} else if(data.context_module) {
var html = '';
switch (type) {
case "quiz":
html += I18n.t('messages.quiz_locked_module', "This quiz is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'});
break;
case "assignment":
html += I18n.t('messages.assignment_locked_module', "This assignment is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'});
break;
case "topic":
html += I18n.t('messages.topic_locked_module', "This topic is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'});
break;
case "file":
html += I18n.t('messages.file_locked_module', "This file is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'});
break;
case "page":
html += I18n.t('messages.page_locked_module', "This page is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'});
break;
default:
html += I18n.t('messages.content_locked_module', "This content is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'});
break;
}
if ($("#context_modules_url").length > 0) {
html += "<br/>";
html += "<a href='" + $("#context_modules_url").attr('href') + "'>";
html += I18n.t('messages.visit_modules_page_for_details', "Visit the modules page for information on how to unlock this content.");
html += "</a>";
return html;
}
}
else {
switch(type) {
case "quiz":
return I18n.t('messages.quiz_locked_no_reason', "This quiz is locked. No other reason has been provided.");
case "assignment":
return I18n.t('messages.assignment_locked_no_reason', "This assignment is locked. No other reason has been provided.");
case "topic":
return I18n.t('messages.topic_locked_no_reason', "This topic is locked. No other reason has been provided.");
case "file":
return I18n.t('messages.file_locked_no_reason', "This file is locked. No other reason has been provided.");
case "page":
return I18n.t('messages.page_locked_no_reason', "This page is locked. No other reason has been provided.");
default:
return I18n.t('messages.content_locked_no_reason', "This content is locked. No other reason has been provided.");
}
}
}
$(document).ready(function() {
$(".content_lock_icon").live('click', function(event) {
if($(this).data('lock_reason')) {
@ -24,50 +104,7 @@ $(document).ready(function() {
var data = $(this).data('lock_reason');
var type = data.type;
var $reason = $("<div/>");
// Note: CourseController#locks is capable of returning locks for quiz, assignments, and discussion_topics,
// this is currently only ever used for quizzes (in quiz_index.js)
switch(type) {
case "quiz":
$reason.text(I18n.t('messages.quiz_locked_no_reason', "This quiz is locked. No other reason has been provided."));
break;
default:
$reason.text(I18n.t('messages.content_locked_no_reason', "This content is locked. No other reason has been provided."));
break;
}
if(data.lock_at) {
switch (type) {
case "quiz":
$reason.text(I18n.t('messages.quiz_locked_at', "This quiz was locked %{at}", {at: $.parseFromISO(data.lock_at).datetime_formatted}));
break;
default:
$reason.text(I18n.t('messages.content_locked_at', "This content was locked %{at}", {at: $.parseFromISO(data.lock_at).datetime_formatted}));
}
} else if(data.unlock_at) {
switch (type) {
case "quiz":
$reason.text(I18n.t('messages.quiz_locked_until', "This quiz is locked until %{date}", {date: $.parseFromISO(data.unlock_at).datetime_formatted}));
break;
default:
$reason.text(I18n.t('messages.quiz_locked_until', "This quiz is locked until %{date}", {date: $.parseFromISO(data.unlock_at).datetime_formatted}));
break;
}
} else if(data.context_module) {
switch (type) {
case "quiz":
$reason.html(I18n.t('messages.quiz_locked_module', "This quiz is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'}));
break;
default:
$reason.html(I18n.t('messages.content_locked_module', "This content is part of the module *%{module}* and hasn't been unlocked yet.", {module: $.htmlEscape(data.context_module.name), wrapper: '<b>$1</b>'}));
break;
}
if($("#context_modules_url").length > 0) {
$reason.append("<br/>");
var $link = $("<a/>");
$link.attr('href', $("#context_modules_url").attr('href'));
$link.text(I18n.t('messages.visit_modules_page_for_details', "Visit the modules page for information on how to unlock this content."));
$reason.append($link);
}
}
$reason.html(INST.lockExplanation(data, type));
var $dialog = $("#lock_reason_dialog");
if($dialog.length === 0) {
$dialog = $("<div/>").attr('id', 'lock_reason_dialog');

View File

@ -0,0 +1,20 @@
require File.expand_path(File.dirname(__FILE__) + '/common')
describe "calendar selenium tests" do
it_should_behave_like "in-process server selenium tests"
it "should not show students the description of an assignment that is locked" do
course_with_student_logged_in
assignment = @course.assignments.create(:name => "locked assignment", :description => "this is secret", :due_at => Time.now + 2.days, :unlock_at => Time.now + 1.day)
assignment.locked_for?(@user).should_not be_nil
get "/calendar"
div = keep_trying_until { driver.find_element(:id, "event_assignment_#{assignment.id}") }
div.click
keep_trying_until { driver.find_element(:id, "event_details").displayed? }
details = driver.find_element(:id, "event_details")
details.find_element(:css, ".description").text.should_not match /secret/
details.find_element(:css, ".lock_explanation").text.should match /is locked until/
end
end