RuboCop: Style/MultilineIfModifier

[skip-stages=Flakey]

auto-corrected

Change-Id: Iaa83f08bec769fc687db68fe11a6bd4f5f0e3d4a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279271
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2021-11-22 13:53:34 -07:00
parent c302dd8bc1
commit 300ca66f2d
34 changed files with 447 additions and 333 deletions

View File

@ -324,6 +324,8 @@ Style/MethodCallWithoutArgsParentheses:
Severity: error
Style/MethodDefParentheses:
Severity: error
Style/MultilineIfModifier:
Severity: error
Style/MultilineIfThen:
Severity: error
Style/MultilineMemoization:

View File

@ -323,8 +323,10 @@ class ApplicationController < ActionController::Base
real_user: @real_current_user,
context: @context
)
rce_env_hash[:RICH_CONTENT_FILES_TAB_DISABLED] = !@context.grants_right?(@current_user, session, :read_as_admin) &&
!tab_enabled?(@context.class::TAB_FILES, :no_render => true) if @context.is_a?(Course)
if @context.is_a?(Course)
rce_env_hash[:RICH_CONTENT_FILES_TAB_DISABLED] = !@context.grants_right?(@current_user, session, :read_as_admin) &&
!tab_enabled?(@context.class::TAB_FILES, :no_render => true)
end
account = Context.get_account(@context)
rce_env_hash[:RICH_CONTENT_INST_RECORD_TAB_DISABLED] = account ? account.disable_rce_media_uploads? : false
js_env(rce_env_hash, true) # Allow overriding in case this gets called more than once

View File

@ -368,8 +368,10 @@ class ContextModulesController < ApplicationController
.having_submission.where(:assignment_id => assignment_ids).pluck(:assignment_id)
end
end
submitted_quiz_ids = @current_user.quiz_submissions.shard(@context.shard)
.completed.where(:quiz_id => quiz_ids).pluck(:quiz_id) if @current_user && quiz_ids.any?
if @current_user && quiz_ids.any?
submitted_quiz_ids = @current_user.quiz_submissions.shard(@context.shard)
.completed.where(:quiz_id => quiz_ids).pluck(:quiz_id)
end
submitted_assignment_ids ||= []
submitted_quiz_ids ||= []
all_tags.each do |tag|
@ -474,12 +476,14 @@ class ContextModulesController < ApplicationController
valid_previous_modules.reverse!
valid_previous_modules.each do |mod|
prog = mod.evaluate_for(@current_user)
next if prog.completed?
res[:modules] << {
:id => mod.id,
:name => mod.name,
:prerequisites => prerequisites_needing_finishing_for(mod, prog),
:locked => prog.locked?
} unless prog.completed?
}
end
elsif @module.require_sequential_progress && @progression.current_position && @tag && @tag.position && @progression.current_position < @tag.position
res[:locked] = true

View File

@ -2577,8 +2577,10 @@ class CoursesController < ApplicationController
enrollment_term_id =
params[:course].delete(:term_id).presence ||
params[:course].delete(:enrollment_term_id).presence
args[:enrollment_term] =
root_account.enrollment_terms.where(id: enrollment_term_id).first if enrollment_term_id
if enrollment_term_id
args[:enrollment_term] =
root_account.enrollment_terms.where(id: enrollment_term_id).first
end
end
# :manage will be false for teachers in concluded courses
args[:enrollment_term] ||= @context.enrollment_term if @context.grants_right?(@current_user, session, :manage)

View File

@ -68,22 +68,26 @@ module Lti
validate_access_token!
reg_key = access_token.reg_key
reg_info = RegistrationRequestService.retrieve_registration_password(context, reg_key) if reg_key
render_new_tool_proxy(
context: context,
tool_proxy_guid: reg_key,
dev_key: developer_key,
registration_url: reg_info[:registration_url]
) and return if reg_info.present?
if reg_info.present?
render_new_tool_proxy(
context: context,
tool_proxy_guid: reg_key,
dev_key: developer_key,
registration_url: reg_info[:registration_url]
) and return
end
rescue Lti::OAuth2::InvalidTokenError
render_unauthorized and return
end
elsif request.authorization.present?
secret = RegistrationRequestService.retrieve_registration_password(context, oauth_consumer_key)
render_new_tool_proxy(
context: context,
tool_proxy_guid: oauth_consumer_key,
registration_url: secret[:registration_url]
) and return if secret.present? && oauth_authenticated_request?(secret[:reg_password])
if secret.present? && oauth_authenticated_request?(secret[:reg_password])
render_new_tool_proxy(
context: context,
tool_proxy_guid: oauth_consumer_key,
registration_url: secret[:registration_url]
) and return
end
end
render_unauthorized
end

View File

@ -334,13 +334,17 @@ class OutcomesController < ApplicationController
def proficiency_roles_js_env
if @context.is_a?(Account) && @context.root_account.feature_enabled?(:account_level_mastery_scales)
proficiency_calculation_roles = []
@context.roles_with_enabled_permission(:manage_proficiency_calculations).each do |role|
proficiency_calculation_roles << role_json(@context, role, @current_user, session, skip_permissions: true)
end if @context.grants_right? @current_user, :manage_proficiency_calculations
if @context.grants_right? @current_user, :manage_proficiency_calculations
@context.roles_with_enabled_permission(:manage_proficiency_calculations).each do |role|
proficiency_calculation_roles << role_json(@context, role, @current_user, session, skip_permissions: true)
end
end
proficiency_scales_roles = []
@context.roles_with_enabled_permission(:manage_proficiency_scales).each do |role|
proficiency_scales_roles << role_json(@context, role, @current_user, session, skip_permissions: true)
end if @context.grants_right? @current_user, :manage_proficiency_scales
if @context.grants_right? @current_user, :manage_proficiency_scales
@context.roles_with_enabled_permission(:manage_proficiency_scales).each do |role|
proficiency_scales_roles << role_json(@context, role, @current_user, session, skip_permissions: true)
end
end
js_env(
PROFICIENCY_CALCULATION_METHOD_ENABLED_ROLES: proficiency_calculation_roles,
PROFICIENCY_SCALES_ENABLED_ROLES: proficiency_scales_roles

View File

@ -137,13 +137,15 @@ class TabsController < ApplicationController
tabs = context_tabs(@context, @current_user)
tab = (tabs.find { |t| t.with_indifferent_access[:css_class] == css_class }).with_indifferent_access
tab_config = @context.tab_configuration
tab_config = tabs.map do |t|
{
'id' => t.with_indifferent_access['id'],
'hidden' => t.with_indifferent_access['hidden'],
'position' => t.with_indifferent_access['position']
}
end if tab_config.blank? || tab_config.count != tabs.count
if tab_config.blank? || tab_config.count != tabs.count
tab_config = tabs.map do |t|
{
'id' => t.with_indifferent_access['id'],
'hidden' => t.with_indifferent_access['hidden'],
'position' => t.with_indifferent_access['position']
}
end
end
if [@context.class::TAB_HOME, @context.class::TAB_SETTINGS].include?(tab[:id])
render json: { error: t(:tab_unmanagable_error, "%{css_class} is not manageable", css_class: css_class) }, status: :bad_request
elsif new_pos && (new_pos <= 1 || new_pos >= tab_config.count + 1)

View File

@ -275,10 +275,12 @@ module ApplicationHelper
#
# preloading works similarily for window.deferredBundles only that their
# execution is delayed until the DOM is ready.
concat javascript_tag new_js_bundles.map { |(bundle, plugin, defer)|
container = defer ? 'window.deferredBundles' : 'window.bundles'
"(#{container} || (#{container} = [])).push('#{plugin ? "#{plugin}-" : ''}#{bundle}');"
}.join("\n") if new_js_bundles.present?
if new_js_bundles.present?
concat javascript_tag new_js_bundles.map { |(bundle, plugin, defer)|
container = defer ? 'window.deferredBundles' : 'window.bundles'
"(#{container} || (#{container} = [])).push('#{plugin ? "#{plugin}-" : ''}#{bundle}');"
}.join("\n")
end
end
end
@ -1017,10 +1019,12 @@ module ApplicationHelper
end
def custom_dashboard_url
url =
@domain_root_account.settings[
"#{ApplicationController.test_cluster_name}_dashboard_url".to_sym
] if ApplicationController.test_cluster_name
if ApplicationController.test_cluster_name
url =
@domain_root_account.settings[
"#{ApplicationController.test_cluster_name}_dashboard_url".to_sym
]
end
url ||= @domain_root_account.settings[:dashboard_url]
if url.present?
url += "?current_user_id=#{@current_user.id}" if @current_user

View File

@ -474,8 +474,10 @@ class Conversation < ActiveRecord::Base
def update_participants(message, options = {})
updated = false
conversation_participants.shard(self).activate do |conversation_participants|
conversation_participants = conversation_participants.where(:user_id =>
(options[:only_users]).map(&:id)) if options[:only_users]
if options[:only_users]
conversation_participants = conversation_participants.where(:user_id =>
(options[:only_users]).map(&:id))
end
skip_ids = options[:skip_users].try(:map, &:id) || [message.author_id]
update_for_skips = options[:update_for_skips] != false

View File

@ -745,9 +745,11 @@ class Course < ActiveRecord::Base
end
Course.clear_cache_keys(course_ids_to_update_user_account_associations, :account_associations)
user_ids_to_update_account_associations = Enrollment
.where("course_id IN (?) AND workflow_state<>'deleted'", course_ids_to_update_user_account_associations)
.group(:user_id).pluck(:user_id) unless course_ids_to_update_user_account_associations.empty?
unless course_ids_to_update_user_account_associations.empty?
user_ids_to_update_account_associations = Enrollment
.where("course_id IN (?) AND workflow_state<>'deleted'", course_ids_to_update_user_account_associations)
.group(:user_id).pluck(:user_id)
end
end
User.update_account_associations(user_ids_to_update_account_associations, :account_chain_cache => account_chain_cache) unless user_ids_to_update_account_associations.empty? || opts[:skip_user_account_associations]
user_ids_to_update_account_associations
@ -2301,10 +2303,12 @@ class Course < ActiveRecord::Base
if e.workflow_state == 'deleted'
e.sis_batch_id = nil
end
e.attributes = {
:course_section => section,
:workflow_state => e.is_a?(StudentViewEnrollment) ? 'active' : enrollment_state
} if e.completed? || e.rejected? || e.deleted? || e.workflow_state != enrollment_state
if e.completed? || e.rejected? || e.deleted? || e.workflow_state != enrollment_state
e.attributes = {
:course_section => section,
:workflow_state => e.is_a?(StudentViewEnrollment) ? 'active' : enrollment_state
}
end
end
# if we're reusing an enrollment and +limit_privileges_to_course_section+ was supplied, apply it
e.limit_privileges_to_course_section = limit_privileges_to_course_section if e

View File

@ -45,12 +45,18 @@ module Importers
def self.existing_migration_ids(context)
existing_ids = {}
existing_ids[:assessments] = context.quizzes
.where.not(quizzes: { migration_id: nil }).pluck(:migration_id) if context.respond_to?(:quizzes)
existing_ids[:assessment_questions] = context.assessment_questions
.where.not(assessment_questions: { migration_id: nil }).pluck(:migration_id) if context.respond_to?(:assessment_questions)
existing_ids[:assessment_question_banks] = context.assessment_question_banks
.where.not(assessment_question_banks: { migration_id: nil }).pluck(:migration_id) if context.respond_to?(:assessment_question_banks)
if context.respond_to?(:quizzes)
existing_ids[:assessments] = context.quizzes
.where.not(quizzes: { migration_id: nil }).pluck(:migration_id)
end
if context.respond_to?(:assessment_questions)
existing_ids[:assessment_questions] = context.assessment_questions
.where.not(assessment_questions: { migration_id: nil }).pluck(:migration_id)
end
if context.respond_to?(:assessment_question_banks)
existing_ids[:assessment_question_banks] = context.assessment_question_banks
.where.not(assessment_question_banks: { migration_id: nil }).pluck(:migration_id)
end
existing_ids
end
end

View File

@ -47,9 +47,11 @@ module Importers
def self.process_discussion_topics_migration(discussion_topics, migration)
topic_entries_to_import = migration.to_import('topic_entries')
discussion_topics.each do |topic|
context = Group.where(context_id: migration.context.id,
context_type: migration.context.class.to_s,
migration_id: topic['group_id']).first if topic['group_id']
if topic['group_id']
context = Group.where(context_id: migration.context.id,
context_type: migration.context.class.to_s,
migration_id: topic['group_id']).first
end
context ||= migration.context
next unless context && can_import_topic?(topic, migration)

View File

@ -54,18 +54,22 @@ module Importers
end
def self.find_or_initialize_from_migration(hash, context)
item = ExternalFeed.where(
context_id: context,
context_type: context.class.to_s,
migration_id: hash[:migration_id]
).first if hash[:migration_id]
item ||= ExternalFeed.where(
context_id: context,
context_type: context.class.to_s,
url: hash[:url],
header_match: hash[:header_match].presence,
verbosity: hash[:verbosity]
).first if hash[:url]
if hash[:migration_id]
item = ExternalFeed.where(
context_id: context,
context_type: context.class.to_s,
migration_id: hash[:migration_id]
).first
end
if hash[:url]
item ||= ExternalFeed.where(
context_id: context,
context_type: context.class.to_s,
url: hash[:url],
header_match: hash[:header_match].presence,
verbosity: hash[:verbosity]
).first
end
item ||= context.external_feeds.temp_record
item
end

View File

@ -44,10 +44,14 @@ module Importers
root_outcome_group = context.root_outcome_group
parent_group = hash[:parent_group] || root_outcome_group
item ||= LearningOutcomeGroup.where(context_id: context, context_type: context.class.to_s)
.where(migration_id: hash[:migration_id]).first if hash[:migration_id]
item ||= LearningOutcomeGroup.find_by(vendor_guid: hash[:vendor_guid],
context: context, learning_outcome_group: parent_group) if hash[:vendor_guid].present?
if hash[:migration_id]
item ||= LearningOutcomeGroup.where(context_id: context, context_type: context.class.to_s)
.where(migration_id: hash[:migration_id]).first
end
if hash[:vendor_guid].present?
item ||= LearningOutcomeGroup.find_by(vendor_guid: hash[:vendor_guid],
context: context, learning_outcome_group: parent_group)
end
# Don't migrate if we already have a folder with the same name inside the parent_group
item ||= LearningOutcomeGroup.active.where(
context: context,

View File

@ -100,8 +100,10 @@ module Importers
return
end
else
item ||= LearningOutcome.where(context_id: context, context_type: context.class.to_s)
.where(migration_id: hash[:migration_id]).first if hash[:migration_id]
if hash[:migration_id]
item ||= LearningOutcome.where(context_id: context, context_type: context.class.to_s)
.where(migration_id: hash[:migration_id]).first
end
item ||= context.created_learning_outcomes.temp_record
item.context = context
item.mark_as_importing!(migration)

View File

@ -250,8 +250,10 @@ class SplitUsers
max_position = restored_user.communication_channels.last&.position&.+(1) || 0
scope = source_user.communication_channels.where(id: cc_records.where(previous_user_id: restored_user).pluck(:context_id))
# passing the array to update_all so we can get postgres to add the position for us.
scope.update_all(["user_id=?, position=position+?, root_account_ids='{?}'",
restored_user.id, max_position, restored_user.root_account_ids]) unless scope.empty?
unless scope.empty?
scope.update_all(["user_id=?, position=position+?, root_account_ids='{?}'",
restored_user.id, max_position, restored_user.root_account_ids])
end
cc_records.where.not(previous_workflow_state: 'non existent').each do |cr|
CommunicationChannel.where(id: cr.context_id).update_all(workflow_state: cr.previous_workflow_state)

View File

@ -627,8 +627,10 @@ class User < ActiveRecord::Base
# probably a lot of dups, so more efficient to use a set than uniq an array
course_section_ids = Set.new
shard_enrollments.each { |e| course_section_ids << e.course_section_id }
data[:sections] += shard_sections = CourseSection.select(%i[id course_id nonxlist_course_id])
.where(:id => course_section_ids.to_a).to_a unless course_section_ids.empty?
unless course_section_ids.empty?
data[:sections] += shard_sections = CourseSection.select(%i[id course_id nonxlist_course_id])
.where(:id => course_section_ids.to_a).to_a
end
shard_sections ||= []
course_ids = Set.new
shard_sections.each do |s|

View File

@ -131,31 +131,33 @@ module AttachmentFu # :nodoc:
storage_mod = AttachmentFu::Backends.const_get("#{options[:storage].to_s.classify}Backend")
include storage_mod unless included_modules.include?(storage_mod)
case attachment_options[:processor]
when :none, nil
processors = AttachmentFu.default_processors.dup
begin
if processors.any?
attachment_options[:processor] = "#{processors.first}Processor"
processor_mod = AttachmentFu::Processors.const_get(attachment_options[:processor])
prepend processor_mod unless included_modules.include?(processor_mod)
unless parent_options[:processor]
case attachment_options[:processor]
when :none, nil
processors = AttachmentFu.default_processors.dup
begin
if processors.any?
attachment_options[:processor] = "#{processors.first}Processor"
processor_mod = AttachmentFu::Processors.const_get(attachment_options[:processor])
prepend processor_mod unless included_modules.include?(processor_mod)
end
rescue Object
raise unless load_related_exception?($!)
processors.shift
retry
end
rescue Object
raise unless load_related_exception?($!)
else
begin
processor_mod = AttachmentFu::Processors.const_get("#{attachment_options[:processor].to_s.classify}Processor")
include processor_mod unless included_modules.include?(processor_mod)
rescue Object
raise unless load_related_exception?($!)
processors.shift
retry
puts "Problems loading #{options[:processor]}Processor: #{$!}"
end
end
else
begin
processor_mod = AttachmentFu::Processors.const_get("#{attachment_options[:processor].to_s.classify}Processor")
include processor_mod unless included_modules.include?(processor_mod)
rescue Object
raise unless load_related_exception?($!)
puts "Problems loading #{options[:processor]}Processor: #{$!}"
end
end unless parent_options[:processor] # Don't let child override processor
end # Don't let child override processor
end
def load_related_exception?(e) # :nodoc: implementation specific

View File

@ -22,9 +22,11 @@ require "json"
module LuckySneaks
module Unidecoder
# Contains Unicode codepoints, loading as needed from JSON files
CODEPOINTS = Hash.new { |h, k|
h[k] = JSON.parse(File.read(File.join(File.dirname(__FILE__), "unidecoder_data", "#{k}.json")))
} unless defined?(CODEPOINTS)
unless defined?(CODEPOINTS)
CODEPOINTS = Hash.new { |h, k|
h[k] = JSON.parse(File.read(File.join(File.dirname(__FILE__), "unidecoder_data", "#{k}.json")))
}
end
class << self
# Returns string with its UTF-8 characters transliterated to ASCII ones

View File

@ -243,10 +243,12 @@ module Api::V1::AssignmentOverride
grouped = assignment_overrides_data.group_by { |o| o['assignment_id'] }
assignments = course.active_assignments.where(id: grouped.keys).preload(:assignment_overrides)
overrides = grouped.map do |assignment_id, overrides_data|
assignment = assignments.find { |a| a.id.to_s == assignment_id.to_s }
find_assignment_overrides(assignment, overrides_data.map { |o| o['id'] }) if assignment
end.flatten.compact if for_update
if for_update
overrides = grouped.map do |assignment_id, overrides_data|
assignment = assignments.find { |a| a.id.to_s == assignment_id.to_s }
find_assignment_overrides(assignment, overrides_data.map { |o| o['id'] }) if assignment
end.flatten.compact
end
interpreted = assignment_overrides_data.each_with_index.map do |override_data, i|
assignment = assignments.find { |a| a.id.to_s == override_data['assignment_id'].to_s }

View File

@ -227,13 +227,15 @@ module Api::V1::CalendarEvent
end
hash['participant_count'] = group.appointments_participants.count if include.include?('participant_count')
hash['reserved_times'] = group.reservations_for(user).map { |event|
{
:id => event.id,
:start_at => event.start_at,
:end_at => event.end_at
if include.include?('reserved_times')
hash['reserved_times'] = group.reservations_for(user).map { |event|
{
:id => event.id,
:start_at => event.start_at,
:end_at => event.end_at
}
}
} if include.include?('reserved_times')
end
hash['context_codes'] = group.context_codes_for_user(user)
hash['all_context_codes'] = group.context_codes if include.include?('all_context_codes') && group.grants_right?(user, session, :manage)
hash['requiring_action'] = group.requiring_action?(user)

View File

@ -87,9 +87,11 @@ module Api::V1::Outcome
hash['calculation_int'] = outcome.calculation_int
end
end
hash['ratings']&.each_with_index do |rating, i|
rating[:percent] = opts[:rating_percents][i] if i < opts[:rating_percents].length
end if opts[:rating_percents]
if opts[:rating_percents]
hash['ratings']&.each_with_index do |rating, i|
rating[:percent] = opts[:rating_percents][i] if i < opts[:rating_percents].length
end
end
hash['assessed'] = if opts[:assessed_outcomes] && outcome.context_type != "Account"
opts[:assessed_outcomes].include?(outcome.id)
else

View File

@ -211,20 +211,22 @@ module Api::V1::Submission
if other_fields.include?('attachments')
attachments = attempt.versioned_attachments.dup
attachments << attempt.attachment if attempt.attachment && attempt.attachment.context_type == 'Submission' && attempt.attachment.context_id == attempt.id
hash['attachments'] = attachments.filter_map do |attachment|
includes = includes.include?('canvadoc_document_id') ? ['preview_url', 'canvadoc_document_id'] : ['preview_url']
options = {
anonymous_instructor_annotations: assignment.anonymous_instructor_annotations?,
enable_annotations: true,
enrollment_type: user_type(context, user),
include: includes,
moderated_grading_allow_list: attempt.moderated_grading_allow_list(user),
skip_permission_checks: true,
submission_id: attempt.id
}
unless attachments.blank?
hash['attachments'] = attachments.filter_map do |attachment|
includes = includes.include?('canvadoc_document_id') ? ['preview_url', 'canvadoc_document_id'] : ['preview_url']
options = {
anonymous_instructor_annotations: assignment.anonymous_instructor_annotations?,
enable_annotations: true,
enrollment_type: user_type(context, user),
include: includes,
moderated_grading_allow_list: attempt.moderated_grading_allow_list(user),
skip_permission_checks: true,
submission_id: attempt.id
}
attachment_json(attachment, user, {}, options)
end unless attachments.blank?
attachment_json(attachment, user, {}, options)
end
end
end
# include the discussion topic entries

View File

@ -48,9 +48,11 @@ module Api::V1::SubmissionComment
)
end
sc_hash['attachments'] = submission_comment.attachments.map do |a|
attachment_json(a, user)
end unless submission_comment.attachments.blank?
unless submission_comment.attachments.blank?
sc_hash['attachments'] = submission_comment.attachments.map do |a|
attachment_json(a, user)
end
end
if @current_user && submission_comment.grants_right?(@current_user, :read_author)
sc_hash['author'] = user_display_json(submission_comment.author, submission_comment.context)
else

View File

@ -346,12 +346,14 @@ module BrandableCSS
file = APP_ROOT.join(CONFIG.dig('paths', 'rev_manifest'))
# in reality, if the rev-manifest.json file is missing you won't get this far, but let's be careful anyway
return(
@decorated_font_paths =
JSON.parse(file.read).each_with_object({}) do |(k, v), memo|
memo["/#{k}"] = "/dist/#{v}" if /^fonts.*woff2/.match?(k)
end.freeze
) if file.exist?
if file.exist?
return(
@decorated_font_paths =
JSON.parse(file.read).each_with_object({}) do |(k, v), memo|
memo["/#{k}"] = "/dist/#{v}" if /^fonts.*woff2/.match?(k)
end.freeze
)
end
# the file does not exist in production, we have a problem
if defined?(Rails) && Rails.env.production?

View File

@ -51,8 +51,10 @@ module PermissionsHelper
unpublished, published = sharded_courses.partition(&:unpublished?)
all_applicable_enrollments = []
enrollment_scope = Enrollment.not_inactive_by_date.for_user(self).select("enrollments.*, enrollment_states.state AS date_based_state_in_db")
all_applicable_enrollments += enrollment_scope.where(:course_id => unpublished)
.where(:type => %w[TeacherEnrollment TaEnrollment DesignerEnrollment StudentViewEnrollment]).to_a if unpublished.any?
if unpublished.any?
all_applicable_enrollments += enrollment_scope.where(:course_id => unpublished)
.where(:type => %w[TeacherEnrollment TaEnrollment DesignerEnrollment StudentViewEnrollment]).to_a
end
all_applicable_enrollments += enrollment_scope.where(:course_id => published).to_a if published.any?
grouped_enrollments = all_applicable_enrollments.group_by(&:course_id)

View File

@ -38,8 +38,10 @@ module SubmittablesGradingPeriodProtection
def grading_periods_allow_submittable_update?(submittable, submittable_params, flash_message: false)
return true unless submittable.graded?
submittable.only_visible_to_overrides =
submittable_params[:only_visible_to_overrides] if submittable_params.key?(:only_visible_to_overrides)
if submittable_params.key?(:only_visible_to_overrides)
submittable.only_visible_to_overrides =
submittable_params[:only_visible_to_overrides]
end
submittable.due_at = submittable_params[:due_at] if submittable_params.key?(:due_at)
return true unless submittable.only_visible_to_overrides_changed? || due_at_changed?(submittable)
return true unless constrained_by_grading_periods?

View File

@ -51,19 +51,25 @@ unless $canvas_tasks_loaded
('js:yarn_install' if npm_install)
].compact
task 'i18n:generate_js' => [
('js:yarn_install' if npm_install)
].compact if build_i18n
if build_i18n
task 'i18n:generate_js' => [
('js:yarn_install' if npm_install)
].compact
end
task 'js:webpack_development' => [
'js:gulp_rev',
('i18n:generate_js' if build_i18n),
] if build_js && build_dev_js
if build_js && build_dev_js
task 'js:webpack_development' => [
'js:gulp_rev',
('i18n:generate_js' if build_i18n),
]
end
task 'js:webpack_production' => [
'js:gulp_rev',
('i18n:generate_js' if build_i18n),
] if build_js && build_prod_js
if build_js && build_prod_js
task 'js:webpack_production' => [
'js:gulp_rev',
('i18n:generate_js' if build_i18n),
]
end
end
batch_times = []

View File

@ -177,88 +177,92 @@ class UserList
associated_shards = @addresses.map { |x| Pseudonym.associated_shards(x[:address].downcase) }.flatten.to_set
associated_shards << @root_account.shard
# Search for matching pseudonyms
Shard.partition_by_shard(all_account_ids) do |account_ids|
next if GlobalLookups.enabled? && !associated_shards.include?(Shard.current)
unless @addresses.empty?
Shard.partition_by_shard(all_account_ids) do |account_ids|
next if GlobalLookups.enabled? && !associated_shards.include?(Shard.current)
Pseudonym.active
.select("unique_id AS address, (SELECT name FROM #{User.quoted_table_name} WHERE users.id=user_id) AS name, user_id, account_id, sis_user_id")
.where("(LOWER(unique_id) IN (?) OR sis_user_id IN (?)) AND account_id IN (?)", @addresses.map { |x| x[:address].downcase }, @addresses.pluck(:address), account_ids)
.map { |pseudonym| pseudonym.attributes.symbolize_keys }.each do |login|
addresses = @addresses.select { |a|
a[:address].casecmp?(login[:address]) ||
(login[:sis_user_id] && (a[:address] == login[:sis_user_id] || a[:sis_user_id] == login[:sis_user_id]))
}
addresses.each do |address|
# already found a matching pseudonym
if address[:user_id]
# we already have the one from this-account, just go with it
next if address[:account_id] == @root_account.local_id && address[:shard] == @root_account.shard
Pseudonym.active
.select("unique_id AS address, (SELECT name FROM #{User.quoted_table_name} WHERE users.id=user_id) AS name, user_id, account_id, sis_user_id")
.where("(LOWER(unique_id) IN (?) OR sis_user_id IN (?)) AND account_id IN (?)", @addresses.map { |x| x[:address].downcase }, @addresses.pluck(:address), account_ids)
.map { |pseudonym| pseudonym.attributes.symbolize_keys }.each do |login|
addresses = @addresses.select { |a|
a[:address].casecmp?(login[:address]) ||
(login[:sis_user_id] && (a[:address] == login[:sis_user_id] || a[:sis_user_id] == login[:sis_user_id]))
}
addresses.each do |address|
# already found a matching pseudonym
if address[:user_id]
# we already have the one from this-account, just go with it
next if address[:account_id] == @root_account.local_id && address[:shard] == @root_account.shard
# neither is from this-account, flag an error
if (login[:account_id] != @root_account.local_id || Shard.current != @root_account.shard) &&
(login[:user_id] != address[:user_id] || Shard.current != address[:shard])
address[:type] = :pseudonym if address[:type] == :email
address[:user_id] = false
address[:details] = :non_unique
address.delete(:name)
address.delete(:shard)
next
# neither is from this-account, flag an error
if (login[:account_id] != @root_account.local_id || Shard.current != @root_account.shard) &&
(login[:user_id] != address[:user_id] || Shard.current != address[:shard])
address[:type] = :pseudonym if address[:type] == :email
address[:user_id] = false
address[:details] = :non_unique
address.delete(:name)
address.delete(:shard)
next
end
# allow this one to overrule, since it's from this-account
address.delete(:details)
end
# allow this one to overrule, since it's from this-account
address.delete(:details)
address.merge!(login)
address[:type] = :pseudonym
address[:shard] = Shard.current
end
address.merge!(login)
address[:type] = :pseudonym
address[:shard] = Shard.current
end
end
end unless @addresses.empty?
end
# Search for matching emails (only if not open registration; otherwise there's no point - we just
# create temporary users)
emails = @addresses.select { |a| a[:type] == :email } if @search_method != :open
associated_shards = @addresses.map { |x| CommunicationChannel.associated_shards(x[:address].downcase) }.flatten.to_set
associated_shards << @root_account.shard
Shard.partition_by_shard(all_account_ids) do |account_ids|
next if GlobalLookups.enabled? && !associated_shards.include?(Shard.current)
if @search_method != :open && !emails.empty?
Shard.partition_by_shard(all_account_ids) do |account_ids|
next if GlobalLookups.enabled? && !associated_shards.include?(Shard.current)
pseudos = Pseudonym.active
.select('path AS address, users.name AS name, communication_channels.user_id AS user_id, communication_channels.workflow_state AS workflow_state')
.joins(:user => :communication_channels)
.where("LOWER(path) IN (?) AND account_id IN (?)", emails.map { |x| x[:address].downcase }, account_ids)
pseudos = if @root_account.feature_enabled?(:allow_unconfirmed_users_in_user_list)
pseudos.merge(CommunicationChannel.unretired)
else
pseudos.merge(CommunicationChannel.active)
end
pseudos = Pseudonym.active
.select('path AS address, users.name AS name, communication_channels.user_id AS user_id, communication_channels.workflow_state AS workflow_state')
.joins(:user => :communication_channels)
.where("LOWER(path) IN (?) AND account_id IN (?)", emails.map { |x| x[:address].downcase }, account_ids)
pseudos = if @root_account.feature_enabled?(:allow_unconfirmed_users_in_user_list)
pseudos.merge(CommunicationChannel.unretired)
else
pseudos.merge(CommunicationChannel.active)
end
pseudos.map { |pseudonym| pseudonym.attributes.symbolize_keys }.each do |login|
addresses = emails.select { |a| a[:address].casecmp?(login[:address]) }
addresses.each do |address|
# if all we've seen is unconfirmed, and this one is active, we'll allow this one to overrule
if address[:workflow_state] == 'unconfirmed' && login[:workflow_state] == 'active'
address.delete(:user_id)
address.delete(:details)
address.delete(:shard)
end
# if we've seen an active, and this one is unconfirmed, skip it
next if address[:workflow_state] == 'active' && login[:workflow_state] == 'unconfirmed'
pseudos.map { |pseudonym| pseudonym.attributes.symbolize_keys }.each do |login|
addresses = emails.select { |a| a[:address].casecmp?(login[:address]) }
addresses.each do |address|
# if all we've seen is unconfirmed, and this one is active, we'll allow this one to overrule
if address[:workflow_state] == 'unconfirmed' && login[:workflow_state] == 'active'
address.delete(:user_id)
address.delete(:details)
address.delete(:shard)
end
# if we've seen an active, and this one is unconfirmed, skip it
next if address[:workflow_state] == 'active' && login[:workflow_state] == 'unconfirmed'
# ccs are not unique; just error out on duplicates
# we're in a bit of a pickle if open registration is disabled, and there are conflicting
# e-mails, but none of them are from a pseudonym
if address.key?(:user_id) && (address[:user_id] != login[:user_id] || address[:shard] != Shard.current)
address[:user_id] = false
address[:details] = :non_unique
address.delete(:name)
address.delete(:shard)
else
address.merge!(login)
address[:shard] = Shard.current
# ccs are not unique; just error out on duplicates
# we're in a bit of a pickle if open registration is disabled, and there are conflicting
# e-mails, but none of them are from a pseudonym
if address.key?(:user_id) && (address[:user_id] != login[:user_id] || address[:shard] != Shard.current)
address[:user_id] = false
address[:details] = :non_unique
address.delete(:name)
address.delete(:shard)
else
address.merge!(login)
address[:shard] = Shard.current
end
end
end
end
end if @search_method != :open && !emails.empty?
end
# Search for matching SMS
smses = @addresses.select { |a| a[:type] == :sms }
@ -268,30 +272,32 @@ class UserList
sms[:address] = "(#{number[0, 3]}) #{number[3, 3]}-#{number[6, 4]}"
end
sms_account_ids = @search_method == :closed ? all_account_ids : [@root_account]
Shard.partition_by_shard(sms_account_ids) do |account_ids|
sms_scope = @search_method == :closed ? Pseudonym.where(:account_id => account_ids) : Pseudonym
sms_scope.active
.select('path AS address, users.name AS name, communication_channels.user_id AS user_id')
.joins(:user => :communication_channels)
.where("communication_channels.workflow_state='active' AND (#{smses.map { |x| "path LIKE '#{x[:address].gsub(/[^\d]/, '')}%'" }.join(" OR ")})")
.map { |pseudonym| pseudonym.attributes.symbolize_keys }.each do |sms|
address = sms.delete(:address)[/\d+/]
addresses = smses.select { |a| a[:address].gsub(/[^\d]/, '') == address }
addresses.each do |a|
# ccs are not unique; just error out on duplicates
if a.key?(:user_id) && (a[:user_id] != login[:user_id] || a[:shard] != Shard.current)
a[:user_id] = false
a[:details] = :non_unique
a.delete(:name)
a.delete(:shard)
else
sms[:user_id] = sms[:user_id].to_i
a.merge!(sms)
a[:shard] = Shard.current
unless smses.empty?
Shard.partition_by_shard(sms_account_ids) do |account_ids|
sms_scope = @search_method == :closed ? Pseudonym.where(:account_id => account_ids) : Pseudonym
sms_scope.active
.select('path AS address, users.name AS name, communication_channels.user_id AS user_id')
.joins(:user => :communication_channels)
.where("communication_channels.workflow_state='active' AND (#{smses.map { |x| "path LIKE '#{x[:address].gsub(/[^\d]/, '')}%'" }.join(" OR ")})")
.map { |pseudonym| pseudonym.attributes.symbolize_keys }.each do |sms|
address = sms.delete(:address)[/\d+/]
addresses = smses.select { |a| a[:address].gsub(/[^\d]/, '') == address }
addresses.each do |a|
# ccs are not unique; just error out on duplicates
if a.key?(:user_id) && (a[:user_id] != login[:user_id] || a[:shard] != Shard.current)
a[:user_id] = false
a[:details] = :non_unique
a.delete(:name)
a.delete(:shard)
else
sms[:user_id] = sms[:user_id].to_i
a.merge!(sms)
a[:shard] = Shard.current
end
end
end
end
end unless smses.empty?
end
all_addresses = @addresses
@addresses = []

View File

@ -1621,22 +1621,28 @@ describe EnrollmentsApiController, type: :request do
}
}
# should display the user's own grades
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
} if e.student? && e.user_id == @user.id
if e.student? && e.user_id == @user.id
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
}
end
# should not display grades for other users.
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user)
} if e.student? && e.user_id != @user.id
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
) if e.user == @user
if e.student? && e.user_id != @user.id
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user)
}
end
if e.user == @user
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
)
end
h
end)
@ -1994,17 +2000,19 @@ describe EnrollmentsApiController, type: :request do
'sis_user_id' => nil,
'section_integration_id' => nil
}
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
"unposted_current_score" => nil,
"unposted_current_grade" => nil,
"unposted_final_score" => nil,
"unposted_final_grade" => nil
} if e.student?
if e.student?
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
"unposted_current_score" => nil,
"unposted_current_grade" => nil,
"unposted_final_score" => nil,
"unposted_final_grade" => nil
}
end
h
end)
end
@ -2314,18 +2322,22 @@ describe EnrollmentsApiController, type: :request do
'start_at' => nil,
'end_at' => nil,
}
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
} if e.student?
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
) if e.user == @user
if e.student?
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
}
end
if e.user == @user
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
)
end
h
end
@ -2377,18 +2389,22 @@ describe EnrollmentsApiController, type: :request do
'start_at' => nil,
'end_at' => nil,
}
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
} if e.student?
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
) if e.user == @user
if e.student?
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
}
end
if e.user == @user
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
)
end
h
end
link_header = response.headers['Link'].split(',')
@ -2775,18 +2791,22 @@ describe EnrollmentsApiController, type: :request do
'created_at' => e.user.created_at.iso8601
}
}
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
} if e.student?
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
) if e.user == @user
if e.student?
h['grades'] = {
'html_url' => course_student_grades_url(@course, e.user),
'final_score' => nil,
'current_score' => nil,
'final_grade' => nil,
'current_grade' => nil,
}
end
if e.user == @user
h.merge!(
'last_activity_at' => nil,
'last_attended_at' => nil,
'total_activity_time' => 0
)
end
h
})
end

View File

@ -82,28 +82,32 @@ RSpec.describe Mutations::UpdateNotificationPreferences do
user_id: nil
)
<<~GQL
#{"notificationPreferencesEnabled(
contextType: #{context_type},
#{"courseId: #{course_id}" if course_id}
#{"accountId: #{account_id}" if account_id}
)" if context_type && (course_id || account_id)}
#{if context_type && (course_id || account_id)
"notificationPreferencesEnabled(
contextType: #{context_type},
#{"courseId: #{course_id}" if course_id}
#{"accountId: #{account_id}" if account_id}
)"
end}
notificationPreferences {
sendScoresInEmails#{"(courseId: #{course_id})" if course_id}
sendObservedNamesInNotifications
readPrivacyNoticeDate
channels {
#{"notificationPolicyOverrides(
contextType: #{context_type},
#{"courseId: #{course_id}" if course_id}
#{"accountId: #{account_id}" if account_id}
#{if context_type && (course_id || account_id)
"notificationPolicyOverrides(
contextType: #{context_type},
#{"courseId: #{course_id}" if course_id}
#{"accountId: #{account_id}" if account_id}
) {
frequency
notification {
category
categoryDisplayName
name
}
}" if context_type && (course_id || account_id)}
frequency
notification {
category
categoryDisplayName
name
}
}"
end}
notificationPolicies#{"(contextType: #{context_type})" if context_type} {
frequency
notification {

View File

@ -27,8 +27,10 @@ def notification_set(opts = {})
notification_model({ :subject => "<%= t :subject, 'This is 5!' %>", :name => "Test Name" }.merge(notification_opts))
user_model({ :workflow_state => 'registered' }.merge(user_opts))
communication_channel_model.confirm!
notification_policy_model(:notification => @notification,
:communication_channel => @communication_channel) unless opts[:no_policy]
unless opts[:no_policy]
notification_policy_model(:notification => @notification,
:communication_channel => @communication_channel)
end
@notification.reload
end

View File

@ -60,11 +60,13 @@ module LtiSpecHelper
:consumer_key => "key",
:shared_secret => "secret")
tool.url = "http://www.example.com/basic_lti"
tool.resource_selection = {
:url => "http://example.com/selection_test",
:selection_width => 400,
:selection_height => 400
} if resource_selection
if resource_selection
tool.resource_selection = {
:url => "http://example.com/selection_test",
:selection_width => 400,
:selection_height => 400
}
end
tool.save!
tool
end

View File

@ -19,15 +19,17 @@
module GoogleDriveCommon
def setup_google_drive(add_user_service = true, authorized = true)
UserService.register(
:service => "google_drive",
:token => "token",
:secret => "secret",
:user => @user,
:service_domain => "drive.google.com",
:service_user_id => "service_user_id",
:service_user_name => "service_user_name"
) if add_user_service
if add_user_service
UserService.register(
:service => "google_drive",
:token => "token",
:secret => "secret",
:user => @user,
:service_domain => "drive.google.com",
:service_user_id => "service_user_id",
:service_user_name => "service_user_name"
)
end
allow_any_instance_of(GoogleDrive::Connection)
.to receive(:authorized?)