fix more find_by_id cases where we could pass in an empty string
Change-Id: I6bd72dd78f739edac6377f863f298fa14283be38 Reviewed-on: https://gerrit.instructure.com/3299 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Zach Wily <zach@instructure.com>
This commit is contained in:
parent
23e71e669a
commit
20092fb2c9
|
@ -136,7 +136,7 @@ class AccountsController < ApplicationController
|
|||
def confirm_delete_user
|
||||
if authorized_action(@account, @current_user, :manage_admin_users)
|
||||
@context = @account
|
||||
@user = @account.all_users.find_by_id(params[:user_id])
|
||||
@user = @account.all_users.find_by_id(params[:user_id]) if params[:user_id].present?
|
||||
if !@user
|
||||
flash[:error] = "No user found with that id"
|
||||
redirect_to account_url(@account)
|
||||
|
|
|
@ -24,7 +24,7 @@ class AssessmentQuestionsController < ApplicationController
|
|||
params[:assessment_question][:form_question_data] ||= params[:question]
|
||||
question_bank_id = params[:assessment_question].delete(:assessment_question_bank_id)
|
||||
@question = @context.assessment_questions.build(params[:assessment_question])
|
||||
if question_bank_id
|
||||
if question_bank_id.present?
|
||||
@bank = @context.assessment_question_banks.active.find_by_id(question_bank_id)
|
||||
@question.assessment_question_bank = @bank
|
||||
end
|
||||
|
|
|
@ -91,7 +91,7 @@ class AssignmentGroupsController < ApplicationController
|
|||
ids = []
|
||||
order.each_index do |idx|
|
||||
id = order[idx]
|
||||
group = @context.assignment_groups.active.find_by_id(id)
|
||||
group = @context.assignment_groups.active.find_by_id(id) if id.present?
|
||||
if group
|
||||
group.move_to_bottom
|
||||
ids << group.id
|
||||
|
|
|
@ -141,7 +141,7 @@ class AssignmentsController < ApplicationController
|
|||
def remind_peer_review
|
||||
@assignment = @context.assignments.active.find(params[:assignment_id])
|
||||
if authorized_action(@assignment, @current_user, :grade)
|
||||
@request = AssessmentRequest.find_by_id(params[:id])
|
||||
@request = AssessmentRequest.find_by_id(params[:id]) if params[:id].present?
|
||||
respond_to do |format|
|
||||
if @request.asset.assignment == @assignment && @request.send_reminder!
|
||||
format.html { redirect_to named_context_url(@context, :context_assignment_peer_reviews_url) }
|
||||
|
@ -157,7 +157,7 @@ class AssignmentsController < ApplicationController
|
|||
def delete_peer_review
|
||||
@assignment = @context.assignments.active.find(params[:assignment_id])
|
||||
if authorized_action(@assignment, @current_user, :grade)
|
||||
@request = AssessmentRequest.find_by_id(params[:id])
|
||||
@request = AssessmentRequest.find_by_id(params[:id]) if params[:id].present?
|
||||
respond_to do |format|
|
||||
if @request.asset.assignment == @assignment && @request.destroy
|
||||
format.html { redirect_to named_context_url(@context, :context_assignment_peer_reviews_url) }
|
||||
|
@ -209,7 +209,7 @@ class AssignmentsController < ApplicationController
|
|||
return
|
||||
end
|
||||
@assignment ||= @context.assignments.build
|
||||
if params[:model_key] && session["assignment_#{params[:model_key]}"]
|
||||
if params[:model_key] && session["assignment_#{params[:model_key]}"].present?
|
||||
@assignment = @context.assignments.find_by_id(session["assignment_#{params[:model_key]}"])
|
||||
else
|
||||
@assignment.title = params[:title]
|
||||
|
@ -232,7 +232,7 @@ class AssignmentsController < ApplicationController
|
|||
def create
|
||||
params[:assignment][:time_zone_edited] = Time.zone.name if params[:assignment]
|
||||
group = get_assignment_group(params[:assignment])
|
||||
if params[:model_key] && session["assignment_#{params[:model_key]}"]
|
||||
if params[:model_key] && session["assignment_#{params[:model_key]}"].present?
|
||||
@assignment = @context.assignments.find_by_id(session["assignment_#{params[:model_key]}"])
|
||||
@assignment.attributes = params[:assignment] if @assignment
|
||||
end
|
||||
|
|
|
@ -58,7 +58,7 @@ class CommunicationChannelsController < ApplicationController
|
|||
def confirm
|
||||
id = params[:communication_channel_id]
|
||||
nonce = params[:nonce]
|
||||
cc = @current_user.communication_channels.find_by_id_and_confirmation_code(id, nonce)
|
||||
cc = @current_user.communication_channels.find_by_id_and_confirmation_code(id, nonce) if id.present?
|
||||
# cc = nil if cc && cc.confirmation_code != nonce
|
||||
if cc
|
||||
@communication_channel = cc
|
||||
|
@ -102,7 +102,9 @@ class CommunicationChannelsController < ApplicationController
|
|||
end
|
||||
|
||||
def merge
|
||||
@cc = CommunicationChannel.find_by_id_and_confirmation_code_and_path_type(params[:communication_channel_id], params[:code], 'email')
|
||||
@cc = if params[:communication_channel_id].present?
|
||||
CommunicationChannel.find_by_id_and_confirmation_code_and_path_type(params[:communication_channel_id], params[:code], 'email')
|
||||
end
|
||||
if @cc.user_id == @current_user.id
|
||||
flash[:notice] = "You have already claimed that email address"
|
||||
redirect_to profile_url
|
||||
|
@ -141,7 +143,7 @@ class CommunicationChannelsController < ApplicationController
|
|||
end
|
||||
|
||||
def destroy
|
||||
@cc = @current_user.communication_channels.find_by_id(params[:id])
|
||||
@cc = @current_user.communication_channels.find_by_id(params[:id]) if params[:id]
|
||||
if !@cc || @cc.destroy
|
||||
render :json => @cc.to_json
|
||||
else
|
||||
|
|
|
@ -32,7 +32,7 @@ class ContentExportsController < ApplicationController
|
|||
|
||||
def show
|
||||
if authorized_action(@context, @current_user, :manage)
|
||||
if export = @context.content_exports.find_by_id(params[:id])
|
||||
if params[:id].present? && export = @context.content_exports.find_by_id(params[:id])
|
||||
render_export(export)
|
||||
else
|
||||
render :json => {:errors => {:base => "Export does not exist"}}.to_json, :status => :bad_request
|
||||
|
@ -65,7 +65,7 @@ class ContentExportsController < ApplicationController
|
|||
|
||||
def destroy
|
||||
if authorized_action(@context, @current_user, :manage)
|
||||
if export = @context.content_exports.find_by_id(params[:id])
|
||||
if params[:id].present? && export = @context.content_exports.find_by_id(params[:id])
|
||||
export.destroy
|
||||
render :json => {:success=>'true'}.to_json
|
||||
else
|
||||
|
|
|
@ -46,7 +46,7 @@ class ContentImportsController < ApplicationController
|
|||
params[:migration_settings][:question_bank_name] = params[:new_question_bank_name]
|
||||
end
|
||||
|
||||
if !params[:content_migration_id].blank?
|
||||
if params[:content_migration_id].present?
|
||||
@migration = ContentMigration.for_context(@context).find_by_id(params[:content_migration_id])
|
||||
end
|
||||
@migration ||= ContentMigration.new
|
||||
|
@ -138,7 +138,7 @@ class ContentImportsController < ApplicationController
|
|||
course_id = params[:copy][:autocomplete_course_id] if params[:copy] && params[:copy][:autocomplete_course_id] && !params[:copy][:autocomplete_course_id].empty?
|
||||
@copy_context = @possible_courses.find{|c| c.id == course_id.to_i } if course_id
|
||||
if !@copy_context
|
||||
@copy_context ||= Course.find_by_id(course_id)
|
||||
@copy_context ||= Course.find_by_id(course_id) if course_id.present?
|
||||
@copy_context = nil if @copy_context && !@copy_context.grants_rights?(@current_user, session, :manage)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -134,7 +134,7 @@ class ContextController < ApplicationController
|
|||
if data && data['root_account_id'] && data['context_code']
|
||||
context = Context.find_by_asset_string(data['context_code'])
|
||||
context = nil unless context.respond_to?(:is_a_context?) && context.is_a_context?
|
||||
user = User.find_by_id(data['puser_id'] && data['puser_id'].split("_").first)
|
||||
user = User.find_by_id(data['puser_id'].split("_").first) if data['puser_id'].present?
|
||||
mo.context ||= context
|
||||
mo.user ||= user
|
||||
mo.save!
|
||||
|
@ -167,7 +167,7 @@ class ContextController < ApplicationController
|
|||
@headers = false
|
||||
@show_left_side = false
|
||||
@padless = true
|
||||
if params[:user_id] && params[:ts] && params[:verifier]
|
||||
if params[:user_id].present? && params[:ts] && params[:verifier]
|
||||
@user = User.find_by_id(params[:user_id])
|
||||
@user = nil unless @user && @user.valid_access_verifier?(params[:ts], params[:verifier])
|
||||
end
|
||||
|
@ -196,7 +196,7 @@ class ContextController < ApplicationController
|
|||
end
|
||||
|
||||
def inbox_item
|
||||
@item = @current_user.inbox_items.find_by_id(params[:id])
|
||||
@item = @current_user.inbox_items.find_by_id(params[:id]) if params[:id].present?
|
||||
if !@item
|
||||
flash[:error] = "The message you were trying to view has been removed"
|
||||
redirect_to inbox_url
|
||||
|
@ -237,7 +237,7 @@ class ContextController < ApplicationController
|
|||
end
|
||||
|
||||
def destroy_inbox_item
|
||||
@item = @current_user.inbox_items.find_by_id(params[:id])
|
||||
@item = @current_user.inbox_items.find_by_id(params[:id]) if params[:id].present?
|
||||
@asset = @item && @item.asset
|
||||
@item && @item.destroy
|
||||
render :json => @item.to_json
|
||||
|
|
|
@ -746,7 +746,9 @@ class CoursesController < ApplicationController
|
|||
if account.grants_right?(@current_user, session, :manage_courses)
|
||||
args = params[:course].slice(:name, :start_at, :conclude_at)
|
||||
root_account = account.root_account || account
|
||||
args[:enrollment_term] = root_account.enrollment_terms.find_by_id(params[:course][:enrollment_term_id])
|
||||
args[:enrollment_term] = if params[:course][:enrollment_term_id].present?
|
||||
root_account.enrollment_terms.find_by_id(params[:course][:enrollment_term_id])
|
||||
end
|
||||
end
|
||||
args[:enrollment_term] ||= @context.enrollment_term
|
||||
args[:abstract_course] = @context.abstract_course
|
||||
|
|
|
@ -115,7 +115,7 @@ class EportfoliosController < ApplicationController
|
|||
if authorized_action(@portfolio, @current_user, :update)
|
||||
order = params[:order].split(",")
|
||||
order.each do |id|
|
||||
category = @portfolio.eportfolio_categories.find_by_id(id)
|
||||
category = @portfolio.eportfolio_categories.find_by_id(id) if id.present?
|
||||
category.move_to_bottom if category
|
||||
end
|
||||
respond_to do |format|
|
||||
|
@ -130,7 +130,7 @@ class EportfoliosController < ApplicationController
|
|||
@category = @portfolio.eportfolio_categories.find(params[:eportfolio_category_id])
|
||||
order = params[:order].split(",")
|
||||
order.each do |id|
|
||||
entry = @category.eportfolio_entries.find_by_id(id)
|
||||
entry = @category.eportfolio_entries.find_by_id(id) if id.present?
|
||||
entry.move_to_bottom if entry
|
||||
end
|
||||
respond_to do |format|
|
||||
|
|
|
@ -32,7 +32,7 @@ class FacebookController < ApplicationController
|
|||
end
|
||||
|
||||
def authorize_user
|
||||
@oauth_request ||= OauthRequest.find_by_id(params[:oauth_request_id]) if params[:oauth_request_id]
|
||||
@oauth_request ||= OauthRequest.find_by_id(params[:oauth_request_id]) if params[:oauth_request_id].present?
|
||||
if @oauth_request && @oauth_request.original_host_with_port != request.host_with_port
|
||||
@original_host_with_port = @oauth_request.original_host_with_port
|
||||
redirect_to install_url
|
||||
|
|
|
@ -42,7 +42,7 @@ class FilesController < ApplicationController
|
|||
|
||||
def check_file_access_flags
|
||||
if params[:user_id] && params[:ts] && params[:verifier]
|
||||
user = User.find_by_id(params[:user_id])
|
||||
user = User.find_by_id(params[:user_id]) if params[:user_id].present?
|
||||
if user && user.valid_access_verifier?(params[:ts], params[:verifier])
|
||||
# attachment.rb checks for this session attribute when determining
|
||||
# permissions, but it should be ignored by the rest of the models'
|
||||
|
@ -264,7 +264,7 @@ class FilesController < ApplicationController
|
|||
path = params[:file_path]
|
||||
|
||||
#if the relative path matches the given file id use that file
|
||||
if @attachment = @context.attachments.find_by_id(params[:file_id])
|
||||
if params[:file_id].present? && @attachment = @context.attachments.find_by_id(params[:file_id])
|
||||
if @attachment.matches_full_display_path?(path) || @attachment.matches_full_path?(path)
|
||||
params[:id] = params[:file_id]
|
||||
else
|
||||
|
@ -412,7 +412,9 @@ class FilesController < ApplicationController
|
|||
@attachment.file_state = 'deleted'
|
||||
@attachment.workflow_state = workflow_state
|
||||
if @context.respond_to?(:folders)
|
||||
@folder = @context.folders.active.find_by_id(params[:attachment][:folder_id])
|
||||
if params[:attachment][:folder_id].present?
|
||||
@folder = @context.folders.active.find_by_id(params[:attachment][:folder_id])
|
||||
end
|
||||
@folder ||= Folder.unfiled_folder(@context)
|
||||
@attachment.folder_id = @folder.id
|
||||
end
|
||||
|
@ -494,7 +496,9 @@ class FilesController < ApplicationController
|
|||
end
|
||||
|
||||
def s3_success
|
||||
@attachment = Attachment.find_by_id_and_workflow_state_and_uuid(params[:id], 'unattached', params[:uuid])
|
||||
if params[:id].present?
|
||||
@attachment = Attachment.find_by_id_and_workflow_state_and_uuid(params[:id], 'unattached', params[:uuid])
|
||||
end
|
||||
details = AWS::S3::S3Object.about(@attachment.full_filename, @attachment.bucket_name) rescue nil
|
||||
if @attachment && details
|
||||
unless @attachment.workflow_state == 'unattached_temporary'
|
||||
|
@ -527,15 +531,19 @@ class FilesController < ApplicationController
|
|||
# POST /files
|
||||
# POST /files.xml
|
||||
def create
|
||||
@folder = @context.folders.active.find_by_id(params[:attachment].delete(:folder_id))
|
||||
if (folder_id = params[:attachment].delete(:folder_id)) && folder_id.present?
|
||||
@folder = @context.folders.active.find_by_id(folder_id)
|
||||
end
|
||||
@folder ||= Folder.unfiled_folder(@context)
|
||||
params[:attachment][:uploaded_data] ||= params[:attachment_uploaded_data]
|
||||
params[:attachment][:uploaded_data] ||= params[:file]
|
||||
params[:attachment][:user] = @current_user
|
||||
params[:attachment].delete :context_id
|
||||
params[:attachment].delete :context_type
|
||||
@attachment = @context.attachments.find_by_id_and_workflow_state(params[:attachment].delete(:unattached_attachment_id), 'unattached')
|
||||
@attachment ||= @context.attachments.new #(params[:attachment])
|
||||
if (unattached_attachment_id = params[:attachment].delete(:unattached_attachment_id)) && unattached_attachment_id.present?
|
||||
@attachment = @context.attachments.find_by_id_and_workflow_state(unattached_attachment_id, 'unattached')
|
||||
end
|
||||
@attachment ||= @context.attachments.new
|
||||
if authorized_action(@attachment, @current_user, :create)
|
||||
get_quota
|
||||
return if quota_exceeded(named_context_url(@context, :context_files_url))
|
||||
|
@ -645,7 +653,7 @@ class FilesController < ApplicationController
|
|||
def image_thumbnail
|
||||
cancel_cache_buster
|
||||
url = Rails.cache.fetch(['thumbnail_url', params[:uuid]].cache_key, :expires_in => 30.minutes) do
|
||||
attachment = Attachment.find_by_id_and_uuid(params[:id], params[:uuid])
|
||||
attachment = Attachment.find_by_id_and_uuid(params[:id], params[:uuid]) if params[:id].present?
|
||||
url = attachment.thumbnail_url rescue nil
|
||||
url ||= '/images/no_pic.gif'
|
||||
url
|
||||
|
|
|
@ -136,7 +136,7 @@ class FoldersController < ApplicationController
|
|||
if !@folder.parent_folder_id || !@context.folders.find_by_id(@folder.parent_folder_id)
|
||||
@folder.parent_folder_id = Folder.unfiled_folder(@context).id
|
||||
end
|
||||
if source_folder_id && (source_folder = Folder.find_by_id(source_folder_id)) && source_folder.grants_right?(@current_user, session, :read)
|
||||
if source_folder_id.present? && (source_folder = Folder.find_by_id(source_folder_id)) && source_folder.grants_right?(@current_user, session, :read)
|
||||
@folder = source_folder.clone_for(@context, @folder, {:everything => true})
|
||||
end
|
||||
respond_to do |format|
|
||||
|
|
|
@ -34,7 +34,7 @@ class InfoController < ApplicationController
|
|||
def avatar_image_url
|
||||
cancel_cache_buster
|
||||
url = Rails.cache.fetch(['avatar_img', params[:user_id]].cache_key, :expires_in => 30.minutes) do
|
||||
user = User.find_by_id(params[:user_id])
|
||||
user = User.find_by_id(params[:user_id]) if params[:user_id].present?
|
||||
if user && service_enabled?(:avatars)
|
||||
url = user.avatar_url(nil, @domain_root_account && @domain_root_account.settings[:avatars])
|
||||
end
|
||||
|
@ -71,7 +71,8 @@ class InfoController < ApplicationController
|
|||
|
||||
def record_error
|
||||
error = params[:error] || {}
|
||||
if @current_user && params[:feedback_type] == 'teacher' && params[:course_id] && @course = @current_user.courses.find_by_id(params[:course_id])
|
||||
if @current_user && params[:feedback_type] == 'teacher' && params[:course_id].present? &&
|
||||
@course = @current_user.courses.find_by_id(params[:course_id])
|
||||
return if record_error_for_teacher
|
||||
end
|
||||
# error = {:error => error} unless error.is_a?(Hash)
|
||||
|
@ -79,8 +80,8 @@ class InfoController < ApplicationController
|
|||
error[:user_agent] = request.headers['User-Agent']
|
||||
begin
|
||||
report_id = error.delete(:id)
|
||||
@report = ErrorReport.find_by_id(report_id) if report_id
|
||||
@report ||= ErrorReport.find_by_id(session[:last_error_id])
|
||||
@report = ErrorReport.find_by_id(report_id) if report_id.present?
|
||||
@report ||= ErrorReport.find_by_id(session[:last_error_id]) if session[:last_error_id].present?
|
||||
@report ||= ErrorReport.create()
|
||||
@report.user = @current_user
|
||||
@report.account ||= @domain_root_account
|
||||
|
|
|
@ -236,7 +236,9 @@ class OutcomesController < ApplicationController
|
|||
|
||||
def create
|
||||
if authorized_action(@context, @current_user, :manage_outcomes)
|
||||
@outcome_group = @context.learning_outcome_groups.find_by_id(params[:learning_outcome_group_id])
|
||||
if params[:learning_outcome_group_id].present?
|
||||
@outcome_group = @context.learning_outcome_groups.find_by_id(params[:learning_outcome_group_id])
|
||||
end
|
||||
@outcome_group ||= LearningOutcomeGroup.default_for(@context)
|
||||
@outcome = @context.created_learning_outcomes.build(params[:learning_outcome])
|
||||
respond_to do |format|
|
||||
|
@ -274,8 +276,8 @@ class OutcomesController < ApplicationController
|
|||
|
||||
def destroy
|
||||
if authorized_action(@context, @current_user, :manage_outcomes)
|
||||
@outcome = @context.learning_outcomes.find_by_id(params[:id])
|
||||
@outcome ||= @context.learning_outcome_tags.find_by_learning_outcome_id(params[:id])
|
||||
@outcome = @context.learning_outcomes.find_by_id(params[:id]) if params[:id].present?
|
||||
@outcome ||= @context.learning_outcome_tags.find_by_learning_outcome_id(params[:id]) if params[:id].present?
|
||||
respond_to do |format|
|
||||
if @outcome
|
||||
if @outcome.context_code == @context.asset_string
|
||||
|
|
|
@ -129,7 +129,7 @@ class ProfileController < ApplicationController
|
|||
respond_to do |format|
|
||||
if @user.update_attributes(params[:user])
|
||||
pseudonymed = false
|
||||
if params[:default_email_id]
|
||||
if params[:default_email_id].present?
|
||||
@user.communication_channels.each_with_index{|cc, idx| cc.insert_at(idx + 1) }
|
||||
@email_channel = @user.communication_channels.find_by_id(params[:default_email_id])
|
||||
@email_channel.move_to_top if @email_channel
|
||||
|
@ -156,7 +156,7 @@ class ProfileController < ApplicationController
|
|||
format.json { render :json => pseudonym_to_update.errors.to_json, :status => :bad_request }
|
||||
end
|
||||
end
|
||||
if params[:default_communication_channel_id]
|
||||
if params[:default_communication_channel_id].present?
|
||||
cc = @user.communication_channels.each_with_index{|cc, idx| cc.insert_at(idx + 1) }
|
||||
cc = @user.communication_channels.find_by_id_and_path_type(params[:default_communication_channel_id], 'email')
|
||||
cc.insert_at(1) if cc
|
||||
|
|
|
@ -47,7 +47,7 @@ class QuizQuestionsController < ApplicationController
|
|||
@bank = AssessmentQuestionBank.find(params[:assessment_question_bank_id])
|
||||
if authorized_action(@bank, @current_user, :read)
|
||||
@assessment_questions = @bank.assessment_questions.active.find_all_by_id(params[:assessment_questions_ids].split(",")).compact
|
||||
@group = @quiz.quiz_groups.find_by_id(params[:quiz_group_id])
|
||||
@group = @quiz.quiz_groups.find_by_id(params[:quiz_group_id]) if params[:quiz_group_id].present?
|
||||
@questions = @quiz.add_assessment_questions(@assessment_questions, @group)
|
||||
render :json => @questions.to_json
|
||||
end
|
||||
|
|
|
@ -333,7 +333,9 @@ class QuizzesController < ApplicationController
|
|||
params[:quiz][:access_code] = nil if params[:quiz][:access_code] == ""
|
||||
if params[:quiz][:quiz_type] == 'assignment' || params[:quiz][:quiz_type] == 'graded_survey'
|
||||
params[:quiz][:assignment_group_id] ||= @context.assignment_groups.first.id
|
||||
@assignment_group = @context.assignment_groups.active.find_by_id(params[:quiz].delete(:assignment_group_id))
|
||||
if (assignment_group_id = params[:quiz].delete(:assignment_group_id)) && assignment_group_id.present?
|
||||
@assignment_group = @context.assignment_groups.active.find_by_id(assignment_group_id)
|
||||
end
|
||||
if @assignment_group
|
||||
@assignment = @context.assignments.build(:title => params[:quiz][:title], :due_at => params[:quiz][:lock_at], :submission_types => 'online_quiz')
|
||||
@assignment.assignment_group = @assignment_group
|
||||
|
@ -372,7 +374,9 @@ class QuizzesController < ApplicationController
|
|||
params[:quiz].delete(:points_possible) unless params[:quiz][:quiz_type] == 'graded_survey'
|
||||
params[:quiz][:access_code] = nil if params[:quiz][:access_code] == ""
|
||||
if params[:quiz][:quiz_type] == 'assignment' || params[:quiz][:quiz_type] == 'graded_survey' #'new' && params[:quiz][:assignment_group_id]
|
||||
@assignment_group = @context.assignment_groups.active.find_by_id(params[:quiz].delete(:assignment_group_id)) if params[:quiz][:assignment_group_id].present?
|
||||
if (assignment_group_id = params[:quiz].delete(:assignment_group_id)) && assignment_group_id.present?
|
||||
@assignment_group = @context.assignment_groups.active.find_by_id(assignment_group_id)
|
||||
end
|
||||
@assignment_group ||= @context.assignment_groups.first
|
||||
# The code to build an assignment for a quiz used to be here, but it's
|
||||
# been moved to the model quiz.rb instead. See Quiz:build_assignment.
|
||||
|
|
|
@ -64,7 +64,9 @@ class RubricsController < ApplicationController
|
|||
@association_object = RubricAssociation.get_association_object(params[:rubric_association])
|
||||
params[:rubric][:user] = @current_user if params[:rubric]
|
||||
if (!@association_object || authorized_action(@association_object, @current_user, :read)) && authorized_action(@context, @current_user, :manage_grades)
|
||||
@association = @context.rubric_associations.find_by_id(params[:rubric_association_id])
|
||||
if params[:rubric_association_id].present?
|
||||
@association = @context.rubric_associations.find_by_id(params[:rubric_association_id])
|
||||
end
|
||||
@association_object ||= @association.association if @association
|
||||
params[:rubric_association][:association] = @association_object
|
||||
params[:rubric_association][:update_if_existing] = params[:action] == 'update'
|
||||
|
|
|
@ -40,7 +40,7 @@ class SectionsController < ApplicationController
|
|||
@section = @context.course_sections.find(params[:section_id])
|
||||
course_id = params[:new_course_id]
|
||||
# cross-listing should only be allowed within the same root account
|
||||
@new_course = @section.root_account.all_courses.find_by_id(course_id)
|
||||
@new_course = @section.root_account.all_courses.find_by_id(course_id) if course_id.present?
|
||||
@new_course ||= @section.root_account.all_courses.find_by_sis_source_id(course_id)
|
||||
allowed = @new_course && @section.grants_right?(@current_user, session, :update) && @new_course.grants_right?(@current_user, session, :manage_admin_users)
|
||||
res = {:allowed => !!allowed}
|
||||
|
@ -56,7 +56,7 @@ class SectionsController < ApplicationController
|
|||
def crosslist
|
||||
@section = @context.course_sections.find(params[:section_id])
|
||||
course_id = params[:new_course_id]
|
||||
@new_course = Course.find_by_id(course_id)
|
||||
@new_course = Course.find_by_id(course_id) if course_id.present?
|
||||
if authorized_action(@section, @current_user, :update) && authorized_action(@new_course, @current_user, :manage_admin_users)
|
||||
@section.crosslist_to_course @new_course
|
||||
respond_to do |format|
|
||||
|
|
|
@ -75,7 +75,7 @@ class SubmissionsController < ApplicationController
|
|||
elsif params[:download]
|
||||
@attachment = @submission.attachment if @submission.attachment_id == params[:download].to_i
|
||||
@attachment ||= Attachment.find_by_id(@submission.submission_history.map(&:attachment_id).find{|a| a == params[:download].to_i })
|
||||
@attachment ||= @submission.attachments.find_by_id(params[:download])
|
||||
@attachment ||= @submission.attachments.find_by_id(params[:download]) if params[:download].present?
|
||||
@attachment ||= @submission.submission_history.map(&:versioned_attachments).flatten.find{|a| a.id == params[:download].to_i }
|
||||
@attachment ||= @submission.attachment if @submission.attachment_id == params[:download].to_i
|
||||
@attachment ||= Attachment.find(0)
|
||||
|
|
|
@ -48,7 +48,7 @@ class UsersController < ApplicationController
|
|||
end
|
||||
|
||||
def grades
|
||||
@user = User.find_by_id(params[:user_id])
|
||||
@user = User.find_by_id(params[:user_id]) if params[:user_id].present?
|
||||
@user ||= @current_user
|
||||
if authorized_action(@user, @current_user, :read)
|
||||
@current_enrollments = @user.current_enrollments
|
||||
|
@ -362,7 +362,7 @@ class UsersController < ApplicationController
|
|||
|
||||
def registered
|
||||
@pseudonym_session.destroy if @pseudonym_session
|
||||
@pseudonym = Pseudonym.find_by_id(flash[:pseudonym_id])
|
||||
@pseudonym = Pseudonym.find_by_id(flash[:pseudonym_id]) if flash[:pseudonym_id].present?
|
||||
if flash[:user_id] && (@user = User.find(flash[:user_id]))
|
||||
@email_address = @pseudonym && @pseudonym.communication_channel && @pseudonym.communication_channel.path
|
||||
@email_address ||= @user.email
|
||||
|
@ -506,9 +506,10 @@ class UsersController < ApplicationController
|
|||
|
||||
def admin_merge
|
||||
@user = User.find(params[:user_id])
|
||||
@pending_other_user = User.find_by_id(params[:pending_user_id] || session[:pending_user_id])
|
||||
pending_user_id = params[:pending_user_id] || session[:pending_user_id]
|
||||
@pending_other_user = User.find_by_id(pending_user_id) if pending_user_id.present?
|
||||
@pending_other_user = nil if @pending_other_user == @user
|
||||
@other_user = User.find_by_id(params[:new_user_id])
|
||||
@other_user = User.find_by_id(params[:new_user_id]) if params[:new_user_id].present?
|
||||
if authorized_action(@user, @current_user, :manage_logins)
|
||||
if @user && (params[:clear] || !@pending_other_user)
|
||||
session[:pending_user_id] = @user.id
|
||||
|
|
|
@ -614,7 +614,9 @@ class Account < ActiveRecord::Base
|
|||
|
||||
account = @special_accounts[special_account_type]
|
||||
return account if account
|
||||
account = Account.find_by_id(Setting.get("#{special_account_type}_account_id", nil))
|
||||
if (account_id = Setting.get("#{special_account_type}_account_id", nil)) && account_id.present?
|
||||
account = Account.find_by_id(account_id)
|
||||
end
|
||||
return @special_accounts[special_account_type] = account if account
|
||||
account = Account.create!(:name => default_account_name)
|
||||
Setting.set("#{special_account_type}_account_id", account.id)
|
||||
|
@ -685,7 +687,9 @@ class Account < ActiveRecord::Base
|
|||
memoize :sub_account_count
|
||||
|
||||
def current_sis_batch
|
||||
self.sis_batches.find_by_id(self.read_attribute(:current_sis_batch_id))
|
||||
if (current_sis_batch_id = self.read_attribute(:current_sis_batch_id)) && current_sis_batch_id.present?
|
||||
self.sis_batches.find_by_id(current_sis_batch_id)
|
||||
end
|
||||
end
|
||||
|
||||
def turnitin_settings
|
||||
|
|
|
@ -210,7 +210,7 @@ class Assignment < ActiveRecord::Base
|
|||
attr_accessor :updated_submissions
|
||||
def update_submissions_later
|
||||
if @old_assignment_group_id != self.assignment_group_id
|
||||
AssignmentGroup.find_by_id(@old_assignment_group_id).touch rescue nil
|
||||
AssignmentGroup.find_by_id(@old_assignment_group_id).try(:touch) if @old_assignment_group_id.present?
|
||||
end
|
||||
self.assignment_group.touch if self.assignment_group
|
||||
if @notify_graded_students_of_grading
|
||||
|
@ -624,7 +624,7 @@ class Assignment < ActiveRecord::Base
|
|||
end
|
||||
|
||||
set_policy do
|
||||
given { |user, session| self.cached_context_grants_right?(user, session, :read) }#students.find_by_id(user) }
|
||||
given { |user, session| self.cached_context_grants_right?(user, session, :read) }
|
||||
set { can :read and can :read_own_submission }
|
||||
|
||||
given { |user, session| self.submittable_type? &&
|
||||
|
@ -639,10 +639,10 @@ class Assignment < ActiveRecord::Base
|
|||
}
|
||||
set { can :update_content }
|
||||
|
||||
given { |user, session| self.cached_context_grants_right?(user, session, :manage_grades) }#self.context.admins.find_by_id(user) }
|
||||
given { |user, session| self.cached_context_grants_right?(user, session, :manage_grades) }
|
||||
set { can :update and can :update_content and can :grade and can :delete and can :create and can :read and can :attach_submission_comment_files }
|
||||
|
||||
given { |user, session| self.cached_context_grants_right?(user, session, :manage_assignments) }#self.context.admins.find_by_id(user) }
|
||||
given { |user, session| self.cached_context_grants_right?(user, session, :manage_assignments) }
|
||||
set { can :update and can :update_content and can :delete and can :create and can :read and can :attach_submission_comment_files }
|
||||
end
|
||||
|
||||
|
|
|
@ -640,16 +640,18 @@ class Attachment < ActiveRecord::Base
|
|||
given { |user, session| self.context_type == 'Submission' && self.context.grant_rights?(user, session, :comment) }
|
||||
set { can :create }
|
||||
|
||||
given { |user, session|
|
||||
u = session && User.find_by_id(session['file_access_user_id'])
|
||||
u && self.cached_context_grants_right?(u, session, :read) &&
|
||||
given { |user, session|
|
||||
session && session['file_access_user_id'].present? &&
|
||||
(u = User.find_by_id(session['file_access_user_id'])) &&
|
||||
self.cached_context_grants_right?(u, session, :read) &&
|
||||
session['file_access_expiration'] && session['file_access_expiration'].to_i > Time.now.to_i
|
||||
}
|
||||
set { can :read }
|
||||
|
||||
given { |user, session|
|
||||
u = session && User.find_by_id(session['file_access_user_id'])
|
||||
u && self.cached_context_grants_right?(u, session, :read) &&
|
||||
given { |user, session|
|
||||
session && session['file_access_user_id'].present? &&
|
||||
(u = User.find_by_id(session['file_access_user_id'])) &&
|
||||
self.cached_context_grants_right?(u, session, :read) &&
|
||||
(self.cached_context_grants_right?(u, session, :manage_files) || !self.locked_for?(u)) &&
|
||||
session['file_access_expiration'] && session['file_access_expiration'].to_i > Time.now.to_i
|
||||
}
|
||||
|
|
|
@ -50,7 +50,7 @@ class ContextModule < ActiveRecord::Base
|
|||
prereqs = []
|
||||
(self.prerequisites || []).each do |pre|
|
||||
if pre[:type] == 'context_module'
|
||||
position = ContextModule.module_positions(self.context)[pre[:id].to_i] || 0 #self.context.context_modules.active.find_by_id(pre[:id])
|
||||
position = ContextModule.module_positions(self.context)[pre[:id].to_i] || 0
|
||||
prereqs << pre if position && position < (self.position || 0)
|
||||
else
|
||||
prereqs << pre
|
||||
|
@ -367,7 +367,7 @@ class ContextModule < ActiveRecord::Base
|
|||
if pre[:type] == 'context_module'
|
||||
prog = user.module_progression_for(pre[:id])
|
||||
if !prog
|
||||
prereq = self.context.context_modules.active.find_by_id(pre[:id]) if !prog
|
||||
prereq = self.context.context_modules.active.find_by_id(pre[:id]) if !prog && pre[:id].present?
|
||||
prog = prereq.evaluate_for(user, true) if prereq
|
||||
end
|
||||
prog.completed? rescue false
|
||||
|
@ -506,7 +506,7 @@ class ContextModule < ActiveRecord::Base
|
|||
if progression.unlocked? || progression.started?
|
||||
orig_reqs = (progression.requirements_met || []).map{|r| "#{r[:id]}_#{r[:type]}" }.sort
|
||||
completes = (self.completion_requirements || []).map do |req|
|
||||
tag = tags.detect{|t| t.id == req[:id].to_i} #ContentTag.find_by_id(req[:id])
|
||||
tag = tags.detect{|t| t.id == req[:id].to_i}
|
||||
if !tag
|
||||
res = true
|
||||
elsif ['min_score', 'max_score', 'must_submit'].include?(req[:type]) && !tag.scoreable?
|
||||
|
@ -618,7 +618,7 @@ class ContextModule < ActiveRecord::Base
|
|||
if req[:type] == 'context_module'
|
||||
id = context.merge_mapped_id("context_module_#{req[:id]}")
|
||||
if !id
|
||||
cm = self.context.context_modules.find_by_id(req[:id])
|
||||
cm = self.context.context_modules.find_by_id(req[:id]) if req[:id].present?
|
||||
clone_id = cm.cloned_item_id if cm
|
||||
obj = ContextModule.find_by_cloned_item_id_and_context_id_and_context_type(clone_id, context.id, context.class.to_s) if clone_id
|
||||
id = obj.id if obj
|
||||
|
@ -718,7 +718,7 @@ class ContextModule < ActiveRecord::Base
|
|||
hash[:migration_id] ||= hash[:linked_resource_id]
|
||||
hash[:migration_id] ||= Digest::MD5.hexdigest(hash[:title]) if hash[:title]
|
||||
item = nil
|
||||
existing_item = content_tags.find_by_id(hash[:id])
|
||||
existing_item = content_tags.find_by_id(hash[:id]) if hash[:id].present?
|
||||
existing_item ||= content_tags.find_by_migration_id(hash[:migration_id]) if hash[:migration_id]
|
||||
existing_item ||= content_tags.new(:context => context)
|
||||
context.imported_migration_items << existing_item if context.imported_migration_items && existing_item.new_record?
|
||||
|
|
|
@ -74,7 +74,9 @@ class EportfolioEntry < ActiveRecord::Base
|
|||
def attachments
|
||||
res = []
|
||||
content_sections.each do |section|
|
||||
res << (self.eportfolio.user.all_attachments.find_by_id(section["attachment_id"]) rescue nil) if section["section_type"] == "attachment"
|
||||
if section["attachment_id"].present? && section["section_type"] == "attachment"
|
||||
res << (self.eportfolio.user.all_attachments.find_by_id(section["attachment_id"]) rescue nil)
|
||||
end
|
||||
end
|
||||
res.compact
|
||||
end
|
||||
|
@ -82,7 +84,9 @@ class EportfolioEntry < ActiveRecord::Base
|
|||
def submissions
|
||||
res = []
|
||||
content_sections.each do |section|
|
||||
res << (self.eportfolio.user.submissions.find_by_id(section["submission_id"]) rescue nil) if section["section_type"] == "submission"
|
||||
if section["submission_id"].present? && section["section_type"] == "submission"
|
||||
res << (self.eportfolio.user.submissions.find_by_id(section["submission_id"]) rescue nil)
|
||||
end
|
||||
end
|
||||
res.compact
|
||||
end
|
||||
|
@ -98,14 +102,14 @@ class EportfolioEntry < ActiveRecord::Base
|
|||
new_obj[:content] = Sanitize.clean(obj[:content] || '', config).strip
|
||||
new_obj = nil if new_obj[:content].empty?
|
||||
elsif obj[:section_type] == 'submission'
|
||||
submission = eportfolio.user.submissions.find_by_id(obj[:submission_id])
|
||||
submission = eportfolio.user.submissions.find_by_id(obj[:submission_id]) if obj[:submission_id].present?
|
||||
if submission
|
||||
new_obj[:submission_id] = submission.id
|
||||
else
|
||||
new_obj = nil
|
||||
end
|
||||
elsif obj[:section_type] == 'attachment'
|
||||
attachment = eportfolio.user.attachments.active.find_by_id(obj[:attachment_id])
|
||||
attachment = eportfolio.user.attachments.active.find_by_id(obj[:attachment_id]) if obj[:attachment_id].present?
|
||||
if attachment
|
||||
new_obj[:attachment_id] = attachment.id
|
||||
else
|
||||
|
|
|
@ -116,7 +116,7 @@ class MediaObject < ActiveRecord::Base
|
|||
def self.build_media_objects(data, root_account_id)
|
||||
root_account = Account.find_by_id(root_account_id)
|
||||
data[:entries].each do |entry|
|
||||
attachment = Attachment.find_by_id(entry[:originalId])
|
||||
attachment = Attachment.find_by_id(entry[:originalId]) if entry[:originalId].present?
|
||||
mo = MediaObject.find_or_initialize_by_media_id(entry[:entryId])
|
||||
mo.root_account ||= root_account || Account.default
|
||||
mo.title ||= entry[:name]
|
||||
|
|
|
@ -431,7 +431,7 @@ class Quiz < ActiveRecord::Base
|
|||
@submission_questions.each do |q|
|
||||
if q[:pick_count] #QuizGroup
|
||||
if q[:assessment_question_bank_id]
|
||||
bank = AssessmentQuestionBank.find_by_id(q[:assessment_question_bank_id])
|
||||
bank = AssessmentQuestionBank.find_by_id(q[:assessment_question_bank_id]) if q[:assessment_question_bank_id].present?
|
||||
if bank
|
||||
questions = bank.select_for_submission(q[:pick_count])
|
||||
questions = questions.map{|aq| aq.data}
|
||||
|
|
|
@ -220,7 +220,7 @@ class Rubric < ActiveRecord::Base
|
|||
criterion[:id] = unique_item_id(criterion_data[:id])
|
||||
ratings = []
|
||||
points = 0
|
||||
if criterion_data[:learning_outcome_id]
|
||||
if criterion_data[:learning_outcome_id].present?
|
||||
outcome = LearningOutcome.find_by_id(criterion_data[:learning_outcome_id])
|
||||
if outcome
|
||||
@outcomes_changed = true
|
||||
|
|
|
@ -60,7 +60,7 @@ class RubricAssociation < ActiveRecord::Base
|
|||
return @context if a_type == @context.class.to_s && a_id == @context.id
|
||||
klass = ValidAssociationModels[a_type]
|
||||
return nil unless klass
|
||||
klass.find_by_id(a_id) # authorization is checked in the calling method
|
||||
klass.find_by_id(a_id) if a_id.present? # authorization is checked in the calling method
|
||||
end
|
||||
|
||||
set_broadcast_policy do |p|
|
||||
|
@ -205,7 +205,9 @@ class RubricAssociation < ActiveRecord::Base
|
|||
def self.generate_with_invitees(current_user, rubric, context, params, invitees=nil)
|
||||
raise "context required" unless context
|
||||
association_object = params.delete :association
|
||||
association = RubricAssociation.find_by_id(params.delete(:id))
|
||||
if (association_id = params.delete(:id)) && association_id.present?
|
||||
association = RubricAssociation.find_by_id(association_id)
|
||||
end
|
||||
association = nil unless association && association.context == context && association.association == association_object
|
||||
raise "association required" unless association || association_object
|
||||
# Update/create the association -- this is what ties the rubric to an entity
|
||||
|
|
|
@ -28,7 +28,7 @@
|
|||
<% end %>
|
||||
</div>
|
||||
<% elsif page_section["section_type"] == "submission" %>
|
||||
<% submission = @portfolio.user.submissions.find_by_id(page_section["submission_id"]) %>
|
||||
<% submission = @portfolio.user.submissions.find_by_id(page_section["submission_id"]) if page_section["submission_id"].present? %>
|
||||
<% if submission %>
|
||||
<iframe class="submission_preview" src="<%= eportfolio_entry_preview_submission_path(@portfolio, @page, submission.id) %>" style="border: 1px solid #aaa; width: 100%; height: 300px; display: block; margin-left: auto; margin-right: auto;">
|
||||
</iframe>
|
||||
|
|
|
@ -69,7 +69,7 @@ module CC
|
|||
c_node.ignore_for_scoring criterion[:ignore_for_scoring] unless criterion[:ignore_for_scoring].nil?
|
||||
c_node.description criterion[:description]
|
||||
c_node.long_description criterion[:long_description] unless criterion[:long_description].blank?
|
||||
if criterion[:learning_outcome_id]
|
||||
if criterion[:learning_outcome_id].present?
|
||||
if lo = @course.learning_outcomes.find_by_id(criterion[:learning_outcome_id])
|
||||
c_node.learning_outcome_identifierref CCHelper.create_key(lo)
|
||||
end
|
||||
|
|
|
@ -76,7 +76,7 @@ class GradebookImporter
|
|||
|
||||
row.map do |name_and_id|
|
||||
title, id = Assignment.title_and_id(name_and_id)
|
||||
assignment = @context.assignments.active.gradeable.find_by_id(id) if id
|
||||
assignment = @context.assignments.active.gradeable.find_by_id(id) if id.present?
|
||||
assignment ||= @context.assignments.active.gradeable.find_by_title(name_and_id) #backward compat
|
||||
assignment ||= Assignment.new(:title => title || name_and_id)
|
||||
assignment.original_id = assignment.id
|
||||
|
@ -94,7 +94,8 @@ class GradebookImporter
|
|||
def process_student(row)
|
||||
unsorted_name = to_unsorted_name(row[0])
|
||||
student_id = row[1] # the second column in the csv should have the student_id for each row
|
||||
student = @context.students.find_by_id(student_id) || @context.students.find_by_name(unsorted_name)
|
||||
student = @context.students.find_by_id(student_id) if student_id.present?
|
||||
student ||= @context.students.find_by_name(unsorted_name) if unsorted_name.present?
|
||||
student ||= User.new(:name => unsorted_name)
|
||||
student.original_id = student.id
|
||||
student.id ||= NegativeId.generate
|
||||
|
|
Loading…
Reference in New Issue