RuboCop: Lint/AssignmentInCondition

[skip-stages=Flakey]

(manual)

Change-Id: I855b13a7888f4926df42fe6306cc248e611efdf6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274712
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

.rubocop.common.yml

@@ -37,6 +37,8 @@ Lint/AmbiguousOperatorPrecedence:
   Severity: error
 Lint/AmbiguousRegexpLiteral:
   Severity: error
+Lint/AssignmentInCondition:
+  Severity: error
 Lint/Debugger:
   Severity: error
 Lint/DeprecatedClassMethods:

gems/plugins/moodle_importer/lib/moodle_importer/converter.rb

@@ -66,7 +66,7 @@ module Moodle
 
     def add_warnings_to_map(warning_map)
       warning_map.values.each do |warnings|
-        if hashes = warnings['multiple_dropdowns_question']
+        if (hashes = warnings['multiple_dropdowns_question'])
           if hashes.count > 2
             q_hash = hashes.first
             q_hash['import_warnings'] ||= []
@@ -82,7 +82,7 @@ module Moodle
           end
         end
 
-        if hashes = warnings['calculated_question']
+        if (hashes = warnings['calculated_question'])
           if hashes.count > 2
             q_hash = hashes.first
             q_hash['import_warnings'] ||= []

gems/plugins/qti_exporter/lib/canvas/migration/worker/qti_worker.rb

@@ -62,7 +62,7 @@ module Canvas::Migration
           cm.update_conversion_progress(100)
 
           cm.migration_settings[:migration_ids_to_import] = { :copy => { :everything => true } }.merge(cm.migration_settings[:migration_ids_to_import] || {})
-          if path = converter.course[:files_import_root_path]
+          if (path = converter.course[:files_import_root_path])
             cm.migration_settings[:files_import_root_path] = path
           end
           cm.save

gems/plugins/qti_exporter/lib/qti.rb

@@ -39,7 +39,7 @@ module Qti
   end
 
   def self.qti_enabled?
-    if plugin = Canvas::Plugin.find(:qti_converter)
+    if (plugin = Canvas::Plugin.find(:qti_converter))
       return plugin.settings[:enabled].to_s == 'true'
     end
 
@@ -54,7 +54,7 @@ module Qti
   end
 
   def self.convert_questions(manifest_path, opts = {})
-    if path_map = opts[:file_path_map]
+    if (path_map = opts[:file_path_map])
       # used when searching for matching file paths to help find the best matching path
       sorted_paths = path_map.keys.sort_by { |v| v.length }
     else

gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb

@@ -40,7 +40,7 @@ module Qti
       @doc = nil
       @flavor = opts[:flavor]
       @opts = opts
-      if @path_map = opts[:file_path_map]
+      if (@path_map = opts[:file_path_map])
         @sorted_paths = opts[:sorted_file_paths]
         @sorted_paths ||= @path_map.keys.sort_by { |v| v.length }
       end
@@ -49,7 +49,8 @@ module Qti
         @package_root = PackageRoot.new(opts[:base_dir])
         @identifier = @manifest_node['identifier']
         @href = @package_root.item_path(@manifest_node['href'])
-        if title = @manifest_node.at_css('title langstring') || title = @manifest_node.at_css('xmlns|title xmlns|langstring', 'xmlns' => Qti::Converter::IMS_MD)
+        if (title = @manifest_node.at_css('title langstring') ||
+           @manifest_node.at_css('xmlns|title xmlns|langstring', 'xmlns' => Qti::Converter::IMS_MD))
           @title = title.text
         end
       else
@@ -133,10 +134,13 @@ module Qti
           end
         elsif @doc.at_css('itemBody associateInteraction prompt')
           @question[:question_text] = "" # apparently they deliberately had a blank question?
-        elsif text = @doc.at_css('itemBody div:first-child') || @doc.at_css('itemBody p:first-child') || @doc.at_css('itemBody div') || @doc.at_css('itemBody p')
+        elsif (text = @doc.at_css('itemBody div:first-child') ||
+           @doc.at_css('itemBody p:first-child') ||
+            @doc.at_css('itemBody div') ||
+             @doc.at_css('itemBody p'))
           @question[:question_text] = sanitize_html!(text)
         elsif @doc.at_css('itemBody')
-          if text = @doc.at_css('itemBody').children.find { |c| c.text.strip != '' }
+          if (text = @doc.at_css('itemBody').children.find { |c| c.text.strip != '' })
             @question[:question_text] = sanitize_html_string(text.text)
           end
         end
@@ -171,32 +175,32 @@ module Qti
     }
 
     def parse_instructure_metadata
-      if meta = @doc.at_css('instructureMetadata')
+      if (meta = @doc.at_css('instructureMetadata'))
-        if bank =  get_node_att(meta, 'instructureField[name=question_bank]', 'value')
+        if (bank = get_node_att(meta, 'instructureField[name=question_bank]', 'value'))
           @question[:question_bank_name] = bank
         end
-        if bank = get_node_att(meta, 'instructureField[name=question_bank_iden]', 'value')
+        if (bank = get_node_att(meta, 'instructureField[name=question_bank_iden]', 'value'))
           @question[:question_bank_id] = bank
-          if bb_bank = get_node_att(meta, 'instructureField[name=bb_question_bank_iden]', 'value')
+          if (bb_bank = get_node_att(meta, 'instructureField[name=bb_question_bank_iden]', 'value'))
             @question[:bb_question_bank_id] = bb_bank
           end
         end
-        if score = get_node_att(meta, 'instructureField[name=max_score]', 'value')
+        if (score = get_node_att(meta, 'instructureField[name=max_score]', 'value'))
           @question[:points_possible] = [score.to_f, 0.0].max
         end
-        if score = get_node_att(meta, 'instructureField[name=points_possible]', 'value')
+        if (score = get_node_att(meta, 'instructureField[name=points_possible]', 'value'))
           @question[:points_possible] = [score.to_f, 0.0].max
         end
-        if ref = get_node_att(meta, 'instructureField[name=assessment_question_identifierref]', 'value')
+        if (ref = get_node_att(meta, 'instructureField[name=assessment_question_identifierref]', 'value'))
           @question[:assessment_question_migration_id] = ref
         end
-        if ref = get_node_att(meta, 'instructureField[name=original_answer_ids]', 'value')
+        if (ref = get_node_att(meta, 'instructureField[name=original_answer_ids]', 'value'))
           @original_answer_ids = ref.split(",")
         end
         if get_node_att(meta, 'instructureField[name=cc_profile]', 'value') == 'cc.pattern_match.v0p1'
           @question[:is_cc_pattern_match] = true
         end
-        if type =  get_node_att(meta, 'instructureField[name=bb_question_type]', 'value')
+        if (type = get_node_att(meta, 'instructureField[name=bb_question_type]', 'value'))
           @migration_type = type
           case @migration_type
           when 'True/False'
@@ -218,7 +222,7 @@ module Qti
           when 'Essay'
             @question[:question_type] = 'essay_question'
           end
-        elsif type = get_node_att(meta, 'instructureField[name=question_type]', 'value')
+        elsif (type = get_node_att(meta, 'instructureField[name=question_type]', 'value'))
           @migration_type = type
           QUESTION_TYPE_MAPPING.each do |k, v|
             @migration_type = v if k === @migration_type
@@ -273,7 +277,7 @@ module Qti
         elsif (@flavor == Qti::Flavors::D2L && f.text.present?) || id =~ /general_|_all/i
           extract_feedback!(@question, :neutral_comments, f)
         elsif id =~ /feedback_(\d*)_fb/i
-          if answer = @question[:answers].find { |a| a[:migration_id] == "RESPONSE_#{$1}" }
+          if (answer = @question[:answers].find { |a| a[:migration_id] == "RESPONSE_#{$1}" })
             extract_feedback!(answer, :comments, f)
           end
         end
@@ -309,13 +313,13 @@ module Qti
       manifest_node = opts[:manifest_node]
 
       if manifest_node
-        if type = get_interaction_type(manifest_node)
+        if (type = get_interaction_type(manifest_node))
           opts[:interaction_type] ||= type.text.downcase
         end
-        if type = get_node_att(manifest_node, 'instructureMetadata instructureField[name=bb_question_type]', 'value')
+        if (type = get_node_att(manifest_node, 'instructureMetadata instructureField[name=bb_question_type]', 'value'))
           opts[:custom_type] ||= type.downcase
         end
-        if type = get_node_att(manifest_node, 'instructureMetadata instructureField[name=question_type]', 'value')
+        if (type = get_node_att(manifest_node, 'instructureMetadata instructureField[name=question_type]', 'value'))
           type = type.downcase
           opts[:custom_type] ||= type
           if type == 'matching_question'
@@ -396,9 +400,9 @@ module Qti
     def get_feedback_id(cond)
       id = nil
 
-      if feedback = cond.at_css('setOutcomeValue[identifier=FEEDBACK]')
+      if (feedback = cond.at_css('setOutcomeValue[identifier=FEEDBACK]'))
         if feedback.at_css('variable[identifier=FEEDBACK]')
-          if feedback = feedback.at_css('baseValue[baseType=identifier]')
+          if (feedback = feedback.at_css('baseValue[baseType=identifier]'))
             id = feedback.text.strip
           end
         end

gems/plugins/qti_exporter/lib/qti/assessment_test_converter.rb

@@ -45,11 +45,12 @@ module Qti
     def create_instructure_quiz
       begin
         # Get manifest data
-        if md = @manifest_node.at_css("instructureMetadata")
+        if (md = @manifest_node.at_css("instructureMetadata"))
-          if item = get_node_att(md, 'instructureField[name=show_score]', 'value')
+          if (item = get_node_att(md, 'instructureField[name=show_score]', 'value'))
             @quiz[:show_score] = item =~ /true/i ? true : false
           end
-          if item = get_node_att(md, 'instructureField[name=quiz_type]', 'value') || item = get_node_att(md, 'instructureField[name=bb8_assessment_type]', 'value')
+          if (item = get_node_att(md, 'instructureField[name=quiz_type]', 'value') ||
+             get_node_att(md, 'instructureField[name=bb8_assessment_type]', 'value'))
             # known possible values: Self-assessment, Survey, Examination (practice is instructure default)
             # BB8: Test, Pool
             @quiz[:quiz_type] = "assignment" if item =~ /examination|test|quiz/i
@@ -58,14 +59,14 @@ module Qti
               return nil
             end
           end
-          if item = get_node_att(md, 'instructureField[name=which_attempt_to_keep]', 'value')
+          if (item = get_node_att(md, 'instructureField[name=which_attempt_to_keep]', 'value'))
             # known possible values: Highest, First, Last (highest is instructure default)
             @quiz[:which_attempt_to_keep] = "keep_latest" if item =~ /last/i
           end
-          if item = get_node_att(md, 'instructureField[name=max_score]', 'value')
+          if (item = get_node_att(md, 'instructureField[name=max_score]', 'value'))
             @quiz[:points_possible] = item
           end
-          if item = get_node_att(md, 'instructureField[name=bb8_object_id]', 'value')
+          if (item = get_node_att(md, 'instructureField[name=bb8_object_id]', 'value'))
             @quiz[:alternate_migration_id] = item
           end
         end
@@ -94,23 +95,22 @@ module Qti
     end
 
     def parse_instructure_metadata(doc)
-      if meta = doc.at_css('instructureMetadata')
+      if (meta = doc.at_css('instructureMetadata'))
-        if password = get_node_att(meta, 'instructureField[name=password]', 'value')
+        if (password = get_node_att(meta, 'instructureField[name=password]', 'value'))
           @quiz[:access_code] = password
         end
-        if id = get_node_att(meta, 'instructureField[name=assignment_identifierref]', 'value')
+        if (id = get_node_att(meta, 'instructureField[name=assignment_identifierref]', 'value'))
           @quiz[:assignment_migration_id] = id
         end
 
         if @opts[:flavor] == Qti::Flavors::D2L
-          if intro = get_node_att(meta, 'instructureField[name=d2l_intro_message]', 'value')
+          if (intro = get_node_att(meta, 'instructureField[name=d2l_intro_message]', 'value'))
             @quiz[:questions].unshift({ :question_type => 'text_only_question', :question_text => intro, :migration_id => unique_local_id })
           end
-          if html = get_node_att(meta, 'instructureField[name=assessment_rubric_html]', 'value')
+          if (html = get_node_att(meta, 'instructureField[name=assessment_rubric_html]', 'value')) &&
-            if node = (Nokogiri::HTML5.fragment(html) rescue nil)
+             (node = (Nokogiri::HTML5.fragment(html) rescue nil))
-              description = sanitize_html_string(node.text)
+            description = sanitize_html_string(node.text)
-              @quiz[:description] = description if description.present?
+            @quiz[:description] = description if description.present?
-            end
           end
         end
       end
@@ -120,20 +120,19 @@ module Qti
       @quiz[:title] = @title || get_node_att(doc, 'assessmentTest', 'title')
       @quiz[:quiz_name] = @quiz[:title]
       @quiz[:migration_id] = get_node_att(doc, 'assessmentTest', 'identifier')
-      if limit = doc.at_css('timeLimits')
+      if (limit = doc.at_css('timeLimits'))
         @quiz[:time_limit] = AssessmentTestConverter.parse_time_limit(limit['maxTime'])
       end
-      if part = doc.at_css('testPart[identifier=BaseTestPart]') || doc.at_css('testPart')
+      if (part = doc.at_css('testPart[identifier=BaseTestPart]') || doc.at_css('testPart'))
-        if control = part.at_css('itemSessionControl')
+        if (control = part.at_css('itemSessionControl'))
-          if max = control['maxAttempts']
+          if (max = control['maxAttempts'])
             max = -1 if max =~ /unlimited/i
             max = max.to_i
             # -1 means no limit in instructure, 0 means no limit in QTI
             @quiz[:allowed_attempts] = max >= 1 ? max : -1
           end
-          if show = control['showSolution']
+          if (show = control['showSolution'])
-            show = show
+            @quiz[:show_correct_answers] = show.downcase == "true"
-            @quiz[:show_correct_answers] = show.downcase == "true" ? true : false
           end
         end
 
@@ -173,15 +172,15 @@ module Qti
       group = nil
       questions_list = @quiz[:questions]
 
-      if shuffle = get_node_att(section, 'ordering', 'shuffle')
+      if (shuffle = get_node_att(section, 'ordering', 'shuffle'))
         @quiz[:shuffle_answers] = true if shuffle =~ /true/i
       end
-      if select = AssessmentTestConverter.parse_pick_count(section)
+      if (select = AssessmentTestConverter.parse_pick_count(section))
         group = { :questions => [], :pick_count => select, :question_type => 'question_group', :title => section['title'] }
-        if weight = get_node_att(section, 'weight', 'value')
+        if (weight = get_node_att(section, 'weight', 'value'))
           group[:question_points] = convert_weight_to_points(weight)
         end
-        if val = get_float_val(section, 'points_per_item')
+        if (val = get_float_val(section, 'points_per_item'))
           group[:question_points] = val
         end
         bank_refs = section.css('sourcebank_ref')
@@ -191,17 +190,17 @@ module Qti
         elsif bank_refs.count == 1
           group[:question_bank_migration_id] = translate_bank_id(bank_refs.first.text)
         end
-        if val = get_node_val(section, 'sourcebank_context')
+        if (val = get_node_val(section, 'sourcebank_context'))
           group[:question_bank_context] = val
         end
-        if val = get_bool_val(section, 'sourcebank_is_external')
+        if (val = get_bool_val(section, 'sourcebank_is_external'))
           group[:question_bank_is_external] = val
         end
         group[:migration_id] = section['identifier'] && section['identifier'] != "" ? section['identifier'] : unique_local_id
         questions_list = group[:questions]
       end
       if section['visible'] and section['visible'] =~ /true/i
-        if title = section['title']
+        if (title = section['title'])
           # Create an empty question with a title in it
           @quiz[:questions] << { :question_type => 'text_only_question', :question_text => title, :migration_id => unique_local_id }
         end
@@ -265,7 +264,7 @@ module Qti
       if @opts[:flavor] == Qti::Flavors::D2L && item_ref['label'].present?
         question[:migration_id] = item_ref['label']
       end
-      if weight = get_node_att(item_ref, 'weight', 'value')
+      if (weight = get_node_att(item_ref, 'weight', 'value'))
         question[:points_possible] = convert_weight_to_points(weight)
       end
     end

gems/plugins/qti_exporter/lib/qti/associate_interaction.rb

@@ -35,13 +35,13 @@ module Qti
       if @doc.at_css('associateInteraction')
         match_map = {}
         get_all_matches_with_interaction(match_map)
-        if pair_node = @doc.at_css('responseDeclaration[baseType=pair][cardinality=multiple]')
+        if (pair_node = @doc.at_css('responseDeclaration[baseType=pair][cardinality=multiple]'))
           get_answers_from_matching_pairs(pair_node, match_map)
         else
           get_all_answers_with_interaction(match_map)
           check_for_meta_matches
         end
-      elsif node = @doc.at_css('matchInteraction')
+      elsif (node = @doc.at_css('matchInteraction'))
         get_all_match_interaction(node)
       elsif @custom_type == 'respondus_matching'
         get_respondus_answers
@@ -71,10 +71,10 @@ module Qti
       @question[:answers].each do |answer|
         answer[:left] = answer[:text] if answer[:text].present?
         answer[:left_html] = answer[:html] if answer[:html].present?
-        if answer[:match_id]
+        if answer[:match_id] &&
-          if @question[:matches] && match = @question[:matches].find { |m| m[:match_id] == answer[:match_id] }
+           @question[:matches] &&
-            answer[:right] = match[:text]
+           (match = @question[:matches].find { |m| m[:match_id] == answer[:match_id] })
-          end
+          answer[:right] = match[:text]
         end
       end
 
@@ -110,18 +110,16 @@ module Qti
     end
 
     def get_canvas_matches(match_map)
-      if ci = @doc.at_css('choiceInteraction')
+      @doc.at_css('choiceInteraction')&.css('simpleChoice')&.each do |sc|
-        ci.css('simpleChoice').each do |sc|
+        match = {}
-          match = {}
+        @question[:matches] << match
-          @question[:matches] << match
+        match_map[sc['identifier']] = match
-          match_map[sc['identifier']] = match
+        if sc['identifier'] =~ /(\d+)/
-          if sc['identifier'] =~ /(\d+)/
+          match[:match_id] = $1.to_i
-            match[:match_id] = $1.to_i
+        else
-          else
+          match[:match_id] = unique_local_id
-            match[:match_id] = unique_local_id
-          end
-          match[:text] = sc.text.strip
         end
+        match[:text] = sc.text.strip
       end
     end
 
@@ -139,13 +137,13 @@ module Qti
       @doc.css('responseIf, responseElseIf').each do |r_if|
         answer_mig_id = nil
         match_mig_id = nil
-        if match = r_if.at_css('match')
+        if (match = r_if.at_css('match'))
           answer_mig_id = get_node_att(match, 'variable', 'identifier')
           match_mig_id = match.at_css('baseValue[baseType=identifier]').text rescue nil
         end
-        if answer = answer_map[answer_mig_id]
+        if (answer = answer_map[answer_mig_id])
           answer[:feedback_id] = get_feedback_id(r_if)
-          if r_if.at_css('setOutcomeValue[identifier=SCORE] sum') && match = match_map[match_mig_id]
+          if r_if.at_css('setOutcomeValue[identifier=SCORE] sum') && (match = match_map[match_mig_id])
             answer[:match_id] = match[:match_id]
           end
         end
@@ -190,7 +188,7 @@ module Qti
     end
 
     def get_all_matches_from_body
-      if matches = @doc.at_css('div.RIGHT_MATCH_BLOCK')
+      if (matches = @doc.at_css('div.RIGHT_MATCH_BLOCK'))
         matches.css('div').each do |m|
           match = {}
           @question[:matches] << match
@@ -260,7 +258,7 @@ module Qti
           match = {}
           extract_answer!(match, m)
 
-          if other_match = @question[:matches].detect { |om| match[:text].to_s.strip == om[:text].to_s.strip && match[:html].to_s.strip == om[:html].to_s.strip }
+          if (other_match = @question[:matches].detect { |om| match[:text].to_s.strip == om[:text].to_s.strip && match[:html].to_s.strip == om[:html].to_s.strip })
             match_map[m['identifier']] = other_match[:match_id]
           else
             @question[:matches] << match
@@ -281,9 +279,9 @@ module Qti
         answer[:id] = unique_local_id
         answer[:comments] = ""
 
-        if option = a.at_css('simpleAssociableChoice[identifier^=MATCH]')
+        if (option = a.at_css('simpleAssociableChoice[identifier^=MATCH]'))
           answer[:match_id] = match_map[option.text.strip]
-        elsif resp_id = a['responseIdentifier']
+        elsif (resp_id = a['responseIdentifier'])
           @doc.css("match variable[identifier=#{resp_id}]").each do |variable|
             match = variable.parent
             response_if = match.parent
@@ -314,7 +312,7 @@ module Qti
 
     # Replaces the matches with their full-text instead of a,b,c/1,2,3/etc.
     def check_for_meta_matches
-      if long_matches = @doc.search('instructureMetadata matchingMatch')
+      if (long_matches = @doc.search('instructureMetadata matchingMatch'))
         @question[:matches].each_with_index do |match, i|
           match[:text] = long_matches[i].text.strip.gsub(/ +/, " ") if long_matches[i]
         end
@@ -348,7 +346,7 @@ module Qti
       # Check if there are correct answers explicitly specified
       @doc.css('correctResponse > value').each do |match|
         answer_id, match_id = match.text.split
-        if answer = answer_map[answer_id.strip] and m = match_map[match_id.strip]
+        if (answer = answer_map[answer_id.strip]) && (m = match_map[match_id.strip])
           answer[:match_id] = m[:match_id]
         end
       end

gems/plugins/qti_exporter/lib/qti/calculated_interaction.rb

@@ -50,10 +50,12 @@ module Qti
       get_feedback()
       get_formulas()
 
-      if !@question[:answer_tolerance] && tolerance = get_node_att(@doc, 'instructureMetadata instructureField[name=formula_tolerance]', 'value')
+      if !@question[:answer_tolerance] &&
+         (tolerance = get_node_att(@doc, 'instructureMetadata instructureField[name=formula_tolerance]', 'value'))
         @question[:answer_tolerance] = tolerance
       end
-      if !@question[:formula_decimal_places] && precision = get_node_att(@doc, 'instructureMetadata instructureField[name=formula_precision]', 'value')
+      if !@question[:formula_decimal_places] &&
+         (precision = get_node_att(@doc, 'instructureMetadata instructureField[name=formula_precision]', 'value'))
         @question[:formula_decimal_places] = precision.to_i
       end
 
@@ -101,7 +103,7 @@ module Qti
 
     def get_formulas
       @question[:formulas] = []
-      if formulas_node = @doc.at_css('formulas')
+      if (formulas_node = @doc.at_css('formulas'))
         @question[:formula_decimal_places] = formulas_node['decimal_places'].to_i
         formulas_node.css('formula').each do |f_node|
           formula = {}

gems/plugins/qti_exporter/lib/qti/choice_interaction.rb

@@ -113,7 +113,7 @@ module Qti
           answer[:migration_id] = choice['identifier']
           answer[:id] = get_or_generate_answer_id(answer[:migration_id])
 
-          if feedback = choice.at_css('feedbackInline')
+          if (feedback = choice.at_css('feedbackInline'))
             # weird Angel feedback
             answer[:text] = choice.children.first.text.strip
             answer[:comments] = feedback.text.strip
@@ -238,7 +238,7 @@ module Qti
       # Check if there are correct answers explicitly specified
       @doc.css('correctResponse > value, correctResponse > Value').each do |correct_id|
         correct_id = correct_id.text if correct_id
-        if correct_id && answer = answers_hash[correct_id]
+        if correct_id && (answer = answers_hash[correct_id])
           answer[:weight] = DEFAULT_CORRECT_WEIGHT
         end
       end
@@ -248,16 +248,12 @@ module Qti
     def get_response_weight(cond)
       weight = AssessmentItemConverter::DEFAULT_INCORRECT_WEIGHT
 
-      if sum = cond.at_css('setOutcomeValue[identifier=SCORE] sum baseValue[baseType]')
+      if (base = cond.at_css('setOutcomeValue[identifier=SCORE] sum baseValue[baseType]')) ||
+         (base = cond.at_css('setOutcomeValue[identifier=D2L_CORRECT] sum baseValue[baseType]')) ||
+         (base = cond.at_css('setOutcomeValue[identifier=SCORE] > baseValue[baseType]')) ||
+         (base = cond.at_css('setOutcomeValue[identifier^=SCORE] baseValue[baseType]')) ||
+         (base = cond.at_css('setOutcomeValue[identifier$=SCORE] baseValue[baseType]'))
         # it'll only be true if the score is a sum > 0
-        weight = get_base_value(sum)
-      elsif sum = cond.at_css('setOutcomeValue[identifier=D2L_CORRECT] sum baseValue[baseType]')
-        weight = get_base_value(sum)
-      elsif base = cond.at_css('setOutcomeValue[identifier=SCORE] > baseValue[baseType]')
-        weight = get_base_value(base)
-      elsif base = cond.at_css('setOutcomeValue[identifier^=SCORE] baseValue[baseType]')
-        weight = get_base_value(base)
-      elsif base = cond.at_css('setOutcomeValue[identifier$=SCORE] baseValue[baseType]')
         weight = get_base_value(base)
       end
 

gems/plugins/qti_exporter/lib/qti/extended_text_interaction.rb

@@ -62,7 +62,7 @@ module Qti
         # the python tool "fixes" IDs that aren't quite legal QTI (e.g., "1a" becomes "RESPONSE_1a")
         # but does not update the question text, breaking fill-in-multiple-blanks questions.
         # fortunately it records what it does in an XML comment at the top of the doc, so we can undo it.
-        if comment = @doc.children.find { |el| el.class == Nokogiri::XML::Comment }
+        if (comment = @doc.children.find { |el| el.class == Nokogiri::XML::Comment })
           regex = /Warning: replacing bad NMTOKEN "([^"]+)" with "([^"]+)"/
           match_data = regex.match(comment.text)
           while match_data
@@ -77,18 +77,18 @@ module Qti
           text = get_node_val(match, 'baseValue[baseType=string]')
           text ||= get_node_val(match, 'baseValue[baseType=identifier]')
           existing = false
-          if @question[:question_type] != 'fill_in_multiple_blanks_question' and answer = @question[:answers].find { |a| a[:text] == text }
+          if @question[:question_type] != 'fill_in_multiple_blanks_question' &&
+             (answer = @question[:answers].find { |a| a[:text] == text })
             existing = true
           else
             answer = {}
           end
           answer[:text] ||= text
-          unless answer[:feedback_id]
+          if !answer[:feedback_id] && (f_id = get_feedback_id(cond))
-            if f_id = get_feedback_id(cond)
+            answer[:feedback_id] = f_id
-              answer[:feedback_id] = f_id
-            end
           end
-          if @question[:question_type] == 'fill_in_multiple_blanks_question' and id = get_node_att(match, 'variable', 'identifier')
+          if @question[:question_type] == 'fill_in_multiple_blanks_question' &&
+             (id = get_node_att(match, 'variable', 'identifier'))
             id = id.strip
             answer[:blank_id] = fib_map[id] || id
             # strip illegal characters from blank ids

gems/plugins/qti_exporter/lib/qti/fill_in_the_blank.rb

@@ -81,7 +81,7 @@ module Qti
     def process_canvas
       answer_hash = {}
       @doc.css('choiceInteraction').each do |ci|
-        if blank_id = ci['responseIdentifier']
+        if (blank_id = ci['responseIdentifier'])
           blank_id.gsub!(/^response_/, '')
         end
         ci.search('simpleChoice').each do |choice|
@@ -100,7 +100,7 @@ module Qti
         @doc.css('responseProcessing responseCondition responseIf,responseElseIf').each do |if_node|
           if if_node.at_css('setOutcomeValue[identifier=SCORE] sum')
             id = if_node.at_css('match baseValue[baseType=identifier]').text
-            if answer = answer_hash[id]
+            if (answer = answer_hash[id])
               answer[:weight] = AssessmentItemConverter::DEFAULT_CORRECT_WEIGHT
             end
           end
@@ -118,7 +118,7 @@ module Qti
       @doc.css('responseDeclaration').each do |res_node|
         res_id = res_node['identifier']
         res_node.css('correctResponse value').each do |correct_id|
-          if answer = (answer_hash[res_id] && answer_hash[res_id][correct_id.text])
+          if (answer = (answer_hash[res_id] && answer_hash[res_id][correct_id.text]))
             answer[:weight] = AssessmentItemConverter::DEFAULT_CORRECT_WEIGHT
           end
         end
@@ -151,7 +151,7 @@ module Qti
 
     def process_d2l
       @question[:question_text] = ''
-      if body = @doc.at_css('itemBody')
+      if (body = @doc.at_css('itemBody'))
         body.children.each do |node|
           next if node.name == 'text'
 
@@ -167,7 +167,7 @@ module Qti
       end
 
       @doc.css('responseCondition stringMatch').each do |match|
-        if blank_id = get_node_att(match, 'variable', 'identifier')
+        if (blank_id = get_node_att(match, 'variable', 'identifier'))
           text = get_node_val(match, 'baseValue')
           answer = { :id => unique_local_id, :weight => AssessmentItemConverter::DEFAULT_CORRECT_WEIGHT }
           answer[:migration_id] = blank_id
@@ -180,7 +180,7 @@ module Qti
 
     def process_respondus
       @doc.css('responseCondition stringMatch baseValue[baseType=string]').each do |val_node|
-        if blank_id = val_node['identifier']
+        if (blank_id = val_node['identifier'])
           blank_id = blank_id.sub(%r{^RESPONSE_-([^-]*)-}, '\1')
           @question[:answers] << {
             :weight => AssessmentItemConverter::DEFAULT_CORRECT_WEIGHT,

gems/plugins/qti_exporter/lib/qti/html_helper.rb

@@ -54,7 +54,7 @@ module Qti
                 # So check for the file starting with the full relative path, going down to just the file name
                 paths = val.split("/")
                 paths.length.times do |i|
-                  if mig_id = find_best_path_match(paths[i..-1].join('/'))
+                  if (mig_id = find_best_path_match(paths[i..-1].join('/')))
                     subnode[attr] = "#{CC::CCHelper::OBJECT_TOKEN}/attachments/#{mig_id}"
                     break
                   end
@@ -62,7 +62,7 @@ module Qti
               else
                 val.gsub!(/\$[A-Z_]*\$/, '') # remove any path tokens like $TOKEN_EH$
                 # try to find the file by exact path match. If not found, try to find best match
-                if mig_id = find_best_path_match(val)
+                if (mig_id = find_best_path_match(val))
                   subnode[attr] = "#{CC::CCHelper::OBJECT_TOKEN}/attachments/#{mig_id}"
                 end
               end
@@ -140,7 +140,7 @@ module Qti
     # returns a tuple of [text, html]
     # html is null if it's not an html blob
     def detect_html(node)
-      if text_node = node.at_css('div.text')
+      if (text_node = node.at_css('div.text'))
         return [text_node.text.strip, nil]
       end
 

gems/plugins/qti_exporter/lib/qti/numeric_interaction.rb

@@ -37,17 +37,17 @@ module Qti
 
     def get_answer_values
       answer = { :weight => 100, :comments => "", :id => unique_local_id }
-      if gte = @doc.at_css('responseCondition gte baseValue')
+      if (gte = @doc.at_css('responseCondition gte baseValue'))
         answer[:start] = gte.text.to_f
       end
-      if lte = @doc.at_css('responseCondition lte baseValue')
+      if (lte = @doc.at_css('responseCondition lte baseValue'))
         answer[:end] = lte.text.to_f
       end
 
       if (answer[:start] && answer[:end])
         answer[:numerical_answer_type] = "range_answer"
         @question[:answers] << answer
-      elsif equal = @doc.at_css('responseCondition equal baseValue')
+      elsif (equal = @doc.at_css('responseCondition equal baseValue'))
         answer[:exact] = equal.text.to_f
         answer[:numerical_answer_type] = "exact_answer"
         @question[:answers] << answer
@@ -61,7 +61,7 @@ module Qti
         answer[:id] = get_or_generate_answer_id(bv && bv['identifier'])
         answer[:feedback_id] = get_feedback_id(r_if)
 
-        if or_node = r_if.at_css('or')
+        if (or_node = r_if.at_css('or'))
           # exact answer
           exact_node = or_node.at_css('stringMatch baseValue')
           next unless exact_node
@@ -91,7 +91,7 @@ module Qti
           unless is_precision
             answer[:numerical_answer_type] = 'exact_answer'
             answer[:exact] = exact.to_f
-            if upper = or_node.at_css('and customOperator[class=varlte] baseValue')
+            if (upper = or_node.at_css('and customOperator[class=varlte] baseValue'))
               # do margin computation with BigDecimal to avoid rounding errors
               # (this is also used when _scoring_ numeric range questions)
               margin = BigDecimal(upper.text) - BigDecimal(exact) rescue "0.0"
@@ -99,13 +99,13 @@ module Qti
             end
           end
           @question[:answers] << answer
-        elsif and_node = r_if.at_css('and')
+        elsif (and_node = r_if.at_css('and'))
           # range answer
           answer[:numerical_answer_type] = 'range_answer'
-          if lower = and_node.at_css('customOperator[class=vargte] baseValue')
+          if (lower = and_node.at_css('customOperator[class=vargte] baseValue'))
             answer[:start] = lower.text.to_f rescue 0.0
           end
-          if upper = and_node.at_css('customOperator[class=varlte] baseValue')
+          if (upper = and_node.at_css('customOperator[class=varlte] baseValue'))
             answer[:end] = upper.text.to_f rescue 0.0
           end
           if upper || lower

gems/plugins/qti_exporter/lib/qti/order_interaction.rb

@@ -28,7 +28,7 @@ module Qti
     def parse_question_data
       match_map = {}
       get_all_matches(match_map)
-      if node = @doc.at_css('correctResponse')
+      if (node = @doc.at_css('correctResponse'))
         get_correct_responses(match_map)
       else
         get_all_answers(match_map)
@@ -38,7 +38,7 @@ module Qti
     end
 
     def get_all_matches(match_map)
-      if matches = @doc.at_css('orderInteraction')
+      if (matches = @doc.at_css('orderInteraction'))
         matches.css('simpleChoice').each do |sc|
           match = {}
           @question[:matches] << match
@@ -57,7 +57,7 @@ module Qti
         answer[:id] = unique_local_id
         answer[:comments] = ""
 
-        if option = a.at_css('baseValue')
+        if (option = a.at_css('baseValue'))
           answer[:match_id] = match_map[option.text.strip]
         end
       end
@@ -72,7 +72,7 @@ module Qti
         answer[:comments] = ""
 
         match_id = answ.text.strip
-        if m = match_map[match_id]
+        if (m = match_map[match_id])
           answer[:match_id] = m
         end
       end

gems/plugins/qti_exporter/lib/qti/respondus_settings.rb

@@ -75,7 +75,7 @@ module Qti
     protected
 
     def apply_if_set(assessment, key, setting_name, &block)
-      if setting = read_setting(setting_name)
+      if (setting = read_setting(setting_name))
         assessment[key] = block ? block.call(setting) : setting
       end
     end

gems/plugins/qti_exporter/spec_canvas/lib/qti/vista_questions_spec.rb

@@ -60,16 +60,16 @@ if Qti.migration_executable
       expect(manifest_node.at_css("instructureMetadata")).to eq manifest_node
       expect(manifest_node['identifier']).to eq nil
       expect(manifest_node['href']).to eq 'multiple_choice.xml'
-      if title = manifest_node.at_css('title langstring')
+      if (title = manifest_node.at_css('title langstring'))
         expect(title.text).to eq nil
       end
-      if type = manifest_node.at_css('interactiontype')
+      if (type = manifest_node.at_css('interactiontype'))
         expect(type.text.downcase).to eq 'extendedtextinteraction'
       end
-      if type = manifest_node.at_css('instructureMetadata instructureField[name=quiz_type]')
+      if (type = manifest_node.at_css('instructureMetadata instructureField[name=quiz_type]'))
         expect(type['value'].downcase).to eq 'calculated'
       end
-      if type = manifest_node.at_css('instructureField[name=bb8_assessment_type]')
+      if (type = manifest_node.at_css('instructureField[name=bb8_assessment_type]'))
         expect(type['value'].downcase).to eq 'calculated'
       end
     end

gems/rubocop-canvas/lib/rubocop_canvas/cops/specs/deterministic_described_classes.rb

@@ -129,7 +129,7 @@ module RuboCop
           mod_name_parts = mod_name.to_a
           result = [mod_name_parts[1]]
           top_level = false
-          if parent = mod_name_parts[0]
+          if (parent = mod_name_parts[0])
             if parent.cbase_type?
               top_level = true
             elsif parent.const_type?

lib/api/v1/account.rb

@@ -28,7 +28,7 @@ module Api::V1::Account
   # which should act similarly to the account_json method, but include a parameter 'hash'
   # which will have the current account json (to which the method is expected to change and return)
   def self.register_extension(extension)
-    if result = extension.respond_to?(:extend_account_json)
+    if (result = extension.respond_to?(:extend_account_json))
       @@extensions << extension
     end
     result

lib/api/v1/assignment.rb

@@ -285,7 +285,8 @@ module Api::V1::Assignment
             rating_hash["long_description"] = c[:long_description] || ""
             rating_hash
           end
-          if row[:learning_outcome_id] && outcome = LearningOutcome.where(id: row[:learning_outcome_id]).first
+          if row[:learning_outcome_id] &&
+             (outcome = LearningOutcome.where(id: row[:learning_outcome_id]).first)
             row_hash["outcome_id"] = outcome.id
             row_hash["vendor_guid"] = outcome.vendor_guid
           end
@@ -357,7 +358,7 @@ module Api::V1::Assignment
       hash['assignment_visibility'] = (opts[:assignment_visibilities] || assignment.students_with_visibility.pluck(:id).uniq).map(&:to_s)
     end
 
-    if submission = opts[:submission]
+    if (submission = opts[:submission])
       should_show_statistics = opts[:include_score_statistics] && assignment.can_view_score_statistics?(user)
 
       if submission.is_a?(Array)

lib/api/v1/assignment_override.rb

@@ -204,7 +204,7 @@ module Api::V1::AssignmentOverride
         if data[field].blank?
           # override value of nil/'' is meaningful
           override_data[field] = nil
-        elsif value = Time.zone.parse(data[field].to_s)
+        elsif (value = Time.zone.parse(data[field].to_s))
           override_data[field] = value
         else
           errors << "invalid #{field} #{data[field].inspect}"

lib/api/v1/calendar_event.rb

@@ -133,7 +133,7 @@ module Api::V1::CalendarEvent
       else
         hash['reserve_url'] = api_v1_calendar_event_reserve_url(event, '{{ id }}')
       end
-      if participant_limit = event.participants_per_appointment
+      if (participant_limit = event.participants_per_appointment)
         hash["available_slots"] = [participant_limit - event.child_events.size, 0].max
         hash["participants_per_appointment"] = participant_limit
       end

lib/api/v1/content_migration.rb

@@ -51,7 +51,8 @@ module Api::V1::ContentMigration
     end
 
     if migration.for_course_copy?
-      if source = migration.source_course || (migration.migration_settings[:source_course_id] && Course.find(migration.migration_settings[:source_course_id]))
+      if (source = migration.source_course ||
+         (migration.migration_settings[:source_course_id] && Course.find(migration.migration_settings[:source_course_id])))
         json[:settings] = {}
         json[:settings][:source_course_id] = source.id
         json[:settings][:source_course_name] = source.name
@@ -62,7 +63,7 @@ module Api::V1::ContentMigration
     if migration.job_progress
       json['progress_url'] = polymorphic_url([:api_v1, migration.job_progress])
     end
-    if plugin = Canvas::Plugin.find(migration.migration_type)
+    if (plugin = Canvas::Plugin.find(migration.migration_type))
       if plugin.meta[:display_name] && plugin.meta[:display_name].respond_to?(:call)
         json['migration_type_title'] = plugin.meta[:display_name].call
       elsif plugin.meta[:name] && plugin.meta[:name].respond_to?(:call)

lib/api/v1/context_module.rb

@@ -142,7 +142,7 @@ module Api::V1::ContextModule
     end
 
     # add completion requirements
-    if criterion = context_module.completion_requirements && context_module.completion_requirements.detect { |r| r[:id] == content_tag.id }
+    if (criterion = context_module.completion_requirements&.detect { |r| r[:id] == content_tag.id })
       ch = { 'type' => criterion[:type] }
       ch['min_score'] = criterion[:min_score] if criterion[:type] == 'min_score'
       ch['completed'] = !!(progression.requirements_met.present? && progression.requirements_met.detect { |r| r[:type] == criterion[:type] && r[:id] == content_tag.id }) if progression
@@ -175,7 +175,7 @@ module Api::V1::ContextModule
     attrs = [:usage_rights, :locked, :hidden, :lock_explanation, :display_name, :due_at, :unlock_at, :lock_at, :points_possible]
 
     attrs.each do |attr|
-      if item.respond_to?(attr) && val = item.try(attr)
+      if (val = item.try(attr))
         details[attr] = val
       end
     end

lib/api/v1/course.rb

@@ -218,7 +218,7 @@ module Api::V1::Course
       teacher_counts = Enrollment.from("(#{scope.to_sql}) AS t").group("t.course_id").count
       to_preload = []
       courses.each do |course|
-        next unless count = teacher_counts[course.id]
+        next unless (count = teacher_counts[course.id])
 
         if count > threshold
           course.teacher_count = count

lib/api/v1/course_json.rb

@@ -64,7 +64,7 @@ module Api::V1
       @hash['workflow_state'] = @course.api_state
       @hash['course_format'] = @course.course_format if @course.course_format.present?
       @hash['restrict_enrollments_to_course_dates'] = !!@course.restrict_enrollments_to_course_dates
-      if visibility = @course.overridden_course_visibility
+      if (visibility = @course.overridden_course_visibility)
         @hash['overridden_course_visibility'] = visibility
       end
       if @includes.include?(:current_grading_period_scores)

lib/api/v1/discussion_topics.rb

@@ -113,7 +113,7 @@ module Api::V1::DiscussionTopics
 
     json.merge!(serialize_additional_topic_fields(topic, context, user, opts))
 
-    if hold = topic.subscription_hold(user, @context_enrollment, session)
+    if (hold = topic.subscription_hold(user, @context_enrollment, session))
       json[:subscription_hold] = hold
     end
 

lib/api/v1/feature_flag.rb

@@ -61,7 +61,7 @@ module Api::V1::FeatureFlag
   private
 
   def add_localized_attr(hash, feature, attr_name)
-    if attr = feature.instance_variable_get("@#{attr_name}")
+    if (attr = feature.instance_variable_get("@#{attr_name}"))
       hash[attr_name] = attr.is_a?(Proc) ? attr.call : attr.to_s
     end
   end

lib/api/v1/gradebook_history.rb

@@ -149,11 +149,11 @@ module Api::V1
         collection = collection.where("graded_at IS NOT NULL")
       end
 
-      if assignment_id = options[:assignment_id]
+      if (assignment_id = options[:assignment_id])
         collection = collection.where(assignment_id: assignment_id)
       end
 
-      if grader_id = options[:grader_id]
+      if (grader_id = options[:grader_id])
         if grader_id.to_s == '0'
           # yes, this is crazy.  autograded submissions have the grader_id of (quiz_id x -1)
           collection = collection.where("submissions.grader_id<=0")

lib/assignment_override_applicator.rb

@@ -366,7 +366,7 @@ module AssignmentOverrideApplicator
     applicable_overrides = overrides.select(&:due_at_overridden)
     if applicable_overrides.empty?
       assignment_or_quiz
-    elsif override = applicable_overrides.detect { |o| o.due_at.nil? }
+    elsif (override = applicable_overrides.detect { |o| o.due_at.nil? })
       override
     else
       applicable_overrides.sort_by(&:due_at).last

lib/basic_lti/basic_outcomes.rb

@@ -281,7 +281,7 @@ module BasicLTI
           self.description = error_message
         elsif assignment.grading_type != "pass_fail" && (assignment.points_possible.nil?)
 
-          unless submission = existing_submission
+          unless (submission = existing_submission)
             submission = Submission.create!(submission_hash.merge(:user => user,
                                                                   :assignment => assignment))
           end
@@ -410,7 +410,7 @@ module BasicLTI
         def to_xml
           xml = LtiResponse::Legacy.envelope.dup
           xml.at_css('message_response > statusinfo > codemajor').content = code_major.capitalize
-          if score = submission_score
+          if (score = submission_score)
             xml.at_css('message_response > result > sourcedid').content = sourcedid
             xml.at_css('message_response > result > resultscore > textstring').content = score
           else

lib/brand_config_regenerator.rb

@@ -119,7 +119,7 @@ class BrandConfigRegenerator
     total += five_percent
     @progress.calculate_completion!(five_percent, total)
     # take things off the queue from front-to-back
-    while thing = things_left_to_process.shift
+    while (thing = things_left_to_process.shift)
       # if for some reason this one isn't ready (it _should_ be by default,
       # because we get higher tiers first) put it back on the queue to try
       # again later

lib/canvas/migration/external_content/migrator.rb

@@ -39,7 +39,7 @@ module Canvas::Migration::ExternalContent
         self.registered_services.each do |key, service|
           if service.applies_to_course?(course)
             begin
-              if export = service.begin_export(course, opts)
+              if (export = service.begin_export(course, opts))
                 pending_exports[key] = export
               end
             rescue => e
@@ -125,7 +125,7 @@ module Canvas::Migration::ExternalContent
           service = import_service_for(key)
           if service
             begin
-              if import = service.send_imported_content(migration.context, migration, content)
+              if (import = service.send_imported_content(migration.context, migration, content))
                 pending_imports[key] = import
               end
             rescue => e

lib/canvas/migration/helpers/selective_content_formatter.rb

@@ -95,7 +95,7 @@ module Canvas::Migration::Helpers
                             @migration.context.root_account.feature_enabled?(:selectable_outcomes_in_course_copy)
       content_list = []
       if type
-        if match_data = type.match(/submodules_(.*)/)
+        if (match_data = type.match(/submodules_(.*)/))
           (submodule_data(course_data['context_modules'], match_data[1]) || []).each do |item|
             content_list << item_hash('context_modules', item)
           end
@@ -220,9 +220,11 @@ module Canvas::Migration::Helpers
         hash[:title] = item['file_name']
       when 'assessment_question_banks'
         if hash[:title].blank? && @migration && @migration.context.respond_to?(:assessment_question_banks)
-          if hash[:migration_id] && bank = @migration.context.assessment_question_banks.where(migration_id: hash[:migration_id]).first
+          if hash[:migration_id] &&
+             (bank = @migration.context.assessment_question_banks.where(migration_id: hash[:migration_id]).first)
             hash[:title] = bank.title
-          elsif @migration.question_bank_id && default_bank = @migration.context.assessment_question_banks.where(id: @migration.question_bank_id).first
+          elsif @migration.question_bank_id &&
+                (default_bank = @migration.context.assessment_question_banks.where(id: @migration.question_bank_id).first)
             hash[:title] = default_bank.title
           end
           hash[:title] ||= @migration.question_bank_name || AssessmentQuestionBank.default_imported_title
@@ -241,14 +243,15 @@ module Canvas::Migration::Helpers
 
     def add_linked_resource(type, item, hash)
       if type == 'assignments'
-        if mig_id = item['quiz_migration_id']
+        if (mig_id = item['quiz_migration_id'])
           hash[:linked_resource] = { :type => 'quizzes', :migration_id => mig_id }
-        elsif mig_id = item['topic_migration_id']
+        elsif (mig_id = item['topic_migration_id'])
           hash[:linked_resource] = { :type => 'discussion_topics', :migration_id => mig_id }
-        elsif mig_id = item['page_migration_id']
+        elsif (mig_id = item['page_migration_id'])
           hash[:linked_resource] = { :type => 'wiki_pages', :migration_id => mig_id }
         end
-      elsif ['discussion_topics', 'quizzes', 'wiki_pages'].include?(type) && mig_id = item['assignment_migration_id']
+      elsif ['discussion_topics', 'quizzes', 'wiki_pages'].include?(type) &&
+            (mig_id = item['assignment_migration_id'])
         hash[:linked_resource] = { :type => 'assignments', :migration_id => mig_id }
       end
       hash
@@ -449,7 +452,7 @@ module Canvas::Migration::Helpers
     end
 
     def submodule_data(modules, parent_mig_id)
-      if mod = modules.detect { |m| m['migration_id'] == parent_mig_id }
+      if (mod = modules.detect { |m| m['migration_id'] == parent_mig_id })
         mod['submodules']
       else
         modules.each do |mod|

lib/canvas/migration/migrator.rb

@@ -130,7 +130,7 @@ module Canvas::Migration
         r_node.css('dependency').each do |d_node|
           resource[:dependencies] << d_node[:identifierref]
         end
-        if variant = r_node.at_css('variant')
+        if (variant = r_node.at_css('variant'))
           resource[:preferred_resource_id] = variant['identifierref']
         end
         @resources[id] = resource
@@ -158,20 +158,20 @@ module Canvas::Migration
       doc = open_rel_path(resource[:href])
       if !doc && resource[:files]
         resource[:files].each do |file|
-          break if doc = open_rel_path(file[:href])
+          break if (doc = open_rel_path(file[:href]))
         end
       end
 
-      if !doc && node = @resource_nodes_for_flat_manifest[resource[:migration_id]]
+      if !doc && (node = @resource_nodes_for_flat_manifest[resource[:migration_id]])
         # check for in-line node
-        if node_name
+        doc = if node_name
-          doc = node.children.find { |c| c.name == node_name }
+                node.children.find { |c| c.name == node_name }
-        else
+              else
-          doc = node
+                node
-        end
+              end
       end
 
-      doc.remove_namespaces! if doc && doc.respond_to?('remove_namespaces!')
+      doc.remove_namespaces! if doc.respond_to?('remove_namespaces!')
       doc
     end
   end

lib/canvas/migration/migrator_helper.rb

@@ -71,7 +71,7 @@ module Canvas::Migration
 
     def unique_quiz_dir
       if content_migration
-        if a = content_migration.attachment
+        if (a = content_migration.attachment)
           key = "#{a.filename.gsub(/\..*/, '')}_#{content_migration.id}"
         else
           key = content_migration.id.to_s
@@ -186,7 +186,7 @@ module Canvas::Migration
       assessments.each do |a|
         if a[:migration_id].present? && should_prepend?(:assessments, a[:migration_id], existing_ids)
           a[:migration_id] = prepend_id(a[:migration_id], prepend_value)
-          if h = a[:assignment]
+          if (h = a[:assignment])
             h[:migration_id] = prepend_id(h[:migration_id], prepend_value)
           end
         end
@@ -274,7 +274,7 @@ module Canvas::Migration
       @overview[:start_timestamp] = nil
       @overview[:end_timestamp] = nil
       dates = []
-      if @overview[:course] = @course[:course]
+      if (@overview[:course] = @course[:course])
         @overview[:start_timestamp] = @course[:course][:start_timestamp] || @course[:course][:start_at]
         @overview[:end_timestamp] = @course[:course][:end_timestamp] || @course[:course][:conclude_at]
       end
@@ -398,7 +398,7 @@ module Canvas::Migration
           topic[:title] = t[:title]
           topic[:migration_id] = t[:migration_id]
           topic[:error_message] = t[:error_message] if t[:error_message]
-          if t[:assignment] && a_mig_id = t[:assignment][:migration_id]
+          if t[:assignment] && (a_mig_id = t[:assignment][:migration_id])
             topic[:assignment_migration_id] = a_mig_id
             ensure_linked_assignment(t[:assignment], topic_migration_id: t[:migration_id])
           end
@@ -431,7 +431,7 @@ module Canvas::Migration
           @overview[:wikis] << wiki
           wiki[:migration_id] = w[:migration_id]
           wiki[:title] = w[:title]
-          if w[:assignment] && a_mig_id = w[:assignment][:migration_id]
+          if w[:assignment] && (a_mig_id = w[:assignment][:migration_id])
             wiki[:assignment_migration_id] = a_mig_id
             ensure_linked_assignment(w[:assignment], page_migration_id: w[:migration_id])
           end

lib/canvas/migration/package_identifier.rb

@@ -116,7 +116,7 @@ module Canvas::Migration
     end
 
     def find_converter
-      if plugin = Canvas::Plugin.all_for_tag(:export_system).find { |p| p.settings[:provides] && p.settings[:provides][@type] }
+      if (plugin = Canvas::Plugin.all_for_tag(:export_system).find { |p| p.settings[:provides] && p.settings[:provides][@type] })
         return plugin.settings[:provides][@type]
       end
 

lib/canvas/migration/xml_helper.rb

@@ -29,7 +29,7 @@ module Canvas::Migration
     end
 
     def get_node_att(node, selector, attribute, default = nil)
-      if node = node.at_css(selector)
+      if (node = node.at_css(selector))
         return node[attribute]
       end
 
@@ -44,7 +44,7 @@ module Canvas::Migration
     # descendants of the current node, so you have to iterate
     # over the children and see if it's there.
     def get_val_if_child(node, name)
-      if child = node.children.find { |c| c.name == name }
+      if (child = node.children.find { |c| c.name == name })
         return child.text
       end
 

lib/canvas/plugins/ticketing_system/custom_error.rb

@@ -64,7 +64,7 @@ module Canvas::Plugins::TicketingSystem
     end
 
     def sub_account_tag(asset_manager = ::Context, expected_type = Course)
-      if context_string = self.data['context_asset_string']
+      if (context_string = self.data['context_asset_string'])
         context = asset_manager.find_by_asset_string(context_string)
         if context.is_a? expected_type
           "subaccount_#{context.account_id}"

lib/canvas/plugins/validators/google_drive_validator.rb

@@ -29,7 +29,7 @@ module Canvas::Plugins::Validators::GoogleDriveValidator
         :token_uri => '',
         :client_secret_json => ''
       }
-    elsif res = check_json(settings)
+    elsif (res = check_json(settings))
       plugin_setting.errors.add(:base, res)
       false
     else

lib/cc/assignment_resources.rb

@@ -142,7 +142,7 @@ module CC
       end
       node.submission_formats do |fmt|
         assignment.submission_types.split(',').each do |st|
-          if cc_type = SUBMISSION_TYPE_MAP[st]
+          if (cc_type = SUBMISSION_TYPE_MAP[st])
             fmt.format(:type => cc_type)
           end
         end

lib/cc/canvas_resource.rb

@@ -139,7 +139,7 @@ module CC
             if tab['id'].is_a?(String)
               # it's an external tool, so translate the id to a migration_id
               tool_id = tab['id'].sub('context_external_tool_', '')
-              if tool = ContextExternalTool.find_for(tool_id, @course, :course_navigation, false)
+              if (tool = ContextExternalTool.find_for(tool_id, @course, :course_navigation, false))
                 tab['id'] = "context_external_tool_#{create_key(tool)}"
               end
             end
@@ -156,7 +156,7 @@ module CC
         if @course.image_url.present?
           atts << :image_url
         elsif @course.image_id.present?
-          if image_att = @course.attachments.active.where(id: @course.image_id).first
+          if (image_att = @course.attachments.active.where(id: @course.image_id).first)
             c.image_identifier_ref(create_key(image_att))
           end
         end

lib/cc/cc_helper.rb

@@ -143,7 +143,7 @@ module CC
     def get_html_title_and_body_and_meta_fields(doc)
       meta_fields = {}
       doc.css('html head meta').each do |meta_node|
-        if key = meta_node['name']
+        if (key = meta_node['name'])
           meta_fields[key] = meta_node['content']
         end
       end
@@ -232,7 +232,7 @@ module CC
         end
         @rewriter.set_handler('files') do |match|
           if match.obj_id.nil?
-            if match_data = match.url.match(%r{/files/folder/(.*)})
+            if (match_data = match.url.match(%r{/files/folder/(.*)}))
               # this might not be the best idea but let's keep going and see what happens
               "#{COURSE_TOKEN}/files/folder/#{match_data[1]}"
             elsif match.prefix.present?
@@ -348,7 +348,7 @@ module CC
 
           media_id = anchor['id'].gsub(/^media_comment_/, '')
           obj = MediaObject.active.by_media_id(media_id).first
-          if obj && migration_id = @key_generator.create_key(obj)
+          if obj && (migration_id = @key_generator.create_key(obj))
             @used_media_objects << obj
             info = CCHelper.media_object_info(obj, course: @course, flavor: media_object_flavor)
             @media_object_infos[obj.id] = info
@@ -360,7 +360,7 @@ module CC
         doc.css('iframe[data-media-id]').each do |iframe|
           media_id = iframe['data-media-id']
           obj = MediaObject.active.by_media_id(media_id).take
-          if obj && migration_id = @key_generator.create_key(obj)
+          if obj && (migration_id = @key_generator.create_key(obj))
             @used_media_objects << obj
             info = CCHelper.media_object_info(obj, course: @course, flavor: media_object_flavor)
             @media_object_infos[obj.id] = info

lib/cc/exporter/epub/converters/topic_epub_converter.rb

@@ -63,7 +63,7 @@ module CC::Exporter::Epub::Converters
         topic[:identifier] = get_node_val(meta_doc, 'topic_id')
         topic[:href] = "topics.xhtml##{topic[:identifier]}"
 
-        if asmnt_node = meta_doc.at_css('assignment')
+        if (asmnt_node = meta_doc.at_css('assignment'))
           topic[:assignment] = assignment_data(asmnt_node)
         end
       end

lib/cc/importer/blti_converter.rb

@@ -91,7 +91,7 @@ module CC::Importer
           tool[:consumer_key] = ext[:custom_fields].delete 'consumer_key'
           tool[:shared_secret] = ext[:custom_fields].delete 'shared_secret'
           tool[:tool_id] = ext[:custom_fields].delete 'tool_id'
-          if tool[:assignment_points_possible] = ext[:custom_fields].delete('outcome')
+          if (tool[:assignment_points_possible] = ext[:custom_fields].delete('outcome'))
             tool[:assignment_points_possible] = tool[:assignment_points_possible].to_f
           end
           tool[:settings] = ext[:custom_fields]

lib/cc/importer/canvas/assignment_converter.rb

@@ -26,7 +26,7 @@ module CC::Importer::Canvas
       assignments = convert_cc_assignments
 
       @manifest.css('resource[type$=learning-application-resource]').each do |res|
-        if meta_path = res.at_css('file[href$="assignment_settings.xml"]')
+        if (meta_path = res.at_css('file[href$="assignment_settings.xml"]'))
           meta_path = @package_root.item_path meta_path['href']
           html_path = @package_root.item_path res.at_css('file[href$="html"]')['href']
 

lib/cc/importer/canvas/course_settings.rb

@@ -39,7 +39,7 @@ module CC::Importer::Canvas
 
     def convert_all_course_settings
       @course[:course] = convert_course_settings(settings_doc(COURSE_SETTINGS))
-      if doc = settings_doc(SYLLABUS, true)
+      if (doc = settings_doc(SYLLABUS, true))
         @course[:course][:syllabus_body] = convert_syllabus(doc)
       end
       @course[:assignment_groups] = convert_assignment_groups(settings_doc(ASSIGNMENT_GROUPS))
@@ -87,11 +87,11 @@ module CC::Importer::Canvas
         course[date_type] = val
       end
       ['grading_standard_id', 'home_page_announcement_limit'].each do |int_val|
-        if val = get_int_val(doc, int_val)
+        if (val = get_int_val(doc, int_val))
           course[int_val] = val
         end
       end
-      if nav = get_node_val(doc, 'tab_configuration')
+      if (nav = get_node_val(doc, 'tab_configuration'))
         begin
           nav = JSON.parse(nav)
           # Validate the format a little bit

lib/cc/importer/canvas/media_track_converter.rb

@@ -25,7 +25,7 @@ module CC::Importer::Canvas
       track_map = {}
       return track_map unless doc
 
-      if media_tracks = doc.at_css('media_tracks')
+      if (media_tracks = doc.at_css('media_tracks'))
         media_tracks.css('media').each do |media|
           file_migration_id = media['identifierref']
           tracks = []

lib/cc/importer/canvas/quiz_metadata_converter.rb

@@ -30,7 +30,7 @@ module CC::Importer::Canvas
       @manifest.css('resource[type$="learning-application-resource"]').each do |res|
         res.css('file').select { |f| f['href'].to_s.end_with?(ASSESSMENT_META) }.each do |file|
           meta_path = file['href']
-          if quiz = quiz_map[meta_path]
+          if (quiz = quiz_map[meta_path])
             doc = open_file_xml(@package_root.item_path(meta_path))
             get_quiz_meta(doc, quiz)
           end
@@ -74,7 +74,7 @@ module CC::Importer::Canvas
         quiz[bool_val] = val unless val.nil?
       end
 
-      if asmnt_node = doc.at_css('assignment')
+      if (asmnt_node = doc.at_css('assignment'))
         quiz['assignment'] = parse_canvas_assignment_data(asmnt_node)
       end
 

lib/cc/importer/canvas/topic_converter.rb

@@ -29,7 +29,7 @@ module CC::Importer::Canvas
         cc_path = @package_root.item_path res.at_css('file')['href']
         cc_id = res['identifier']
         canvas_id = get_node_att(res, 'dependency', 'identifierref')
-        if canvas_id && meta_res = @manifest.at_css(%{resource[identifier="#{canvas_id}"]})
+        if canvas_id && (meta_res = @manifest.at_css(%{resource[identifier="#{canvas_id}"]}))
           canvas_path = @package_root.item_path meta_res.at_css('file')['href']
           meta_node = open_file_xml(canvas_path)
         else
@@ -73,7 +73,7 @@ module CC::Importer::Canvas
         %w(has_group_category allow_rating only_graders_can_rate sort_by_rating locked).each do |setting|
           get_bool_val(meta_doc, setting).tap { |val| topic[setting] = val unless val.nil? }
         end
-        if asmnt_node = meta_doc.at_css('assignment')
+        if (asmnt_node = meta_doc.at_css('assignment'))
           topic['assignment'] = parse_canvas_assignment_data(asmnt_node)
         end
       end

lib/cc/importer/canvas/webcontent_converter.rb

@@ -45,7 +45,7 @@ module CC::Importer::Canvas
 
       doc = open_file_xml path
 
-      if folders = doc.at_css('folders')
+      if (folders = doc.at_css('folders'))
         @course[:hidden_folders] = []
         @course[:locked_folders] = []
         folders.css('folder').each do |folder|
@@ -54,29 +54,29 @@ module CC::Importer::Canvas
         end
       end
 
-      if files = doc.at_css('files')
+      if (files = doc.at_css('files'))
         files.css('file').each do |file|
           id = file['identifier']
           if file_map[id]
             file_map[id][:hidden] = true if get_bool_val(file, 'hidden', false)
             file_map[id][:locked] = true if get_bool_val(file, 'locked', false)
 
-            if unlock_at = get_time_val(file, 'unlock_at')
+            if (unlock_at = get_time_val(file, 'unlock_at'))
               file_map[id][:unlock_at] = unlock_at
             end
-            if lock_at = get_time_val(file, 'lock_at')
+            if (lock_at = get_time_val(file, 'lock_at'))
               file_map[id][:lock_at] = lock_at
             end
 
-            if display_name = file.at_css("display_name")
+            if (display_name = file.at_css("display_name"))
               file_map[id][:display_name] = display_name.text
             end
-            if usage_rights = file.at_css("usage_rights")
+            if (usage_rights = file.at_css("usage_rights"))
               rights_hash = { :use_justification => usage_rights.attr('use_justification') }
-              if legal_copyright = usage_rights.at_css('legal_copyright')
+              if (legal_copyright = usage_rights.at_css('legal_copyright'))
                 rights_hash.merge!(:legal_copyright => legal_copyright.text)
               end
-              if license = usage_rights.at_css('license')
+              if (license = usage_rights.at_css('license'))
                 rights_hash.merge!(:license => license.text)
               end
               file_map[id][:usage_rights] = rights_hash

lib/cc/importer/canvas/wiki_converter.rb

@@ -51,7 +51,7 @@ module CC::Importer::Canvas
       wiki[:url_name] = wiki_name
       wiki[:assignment] = nil
       wiki[:todo_date] = meta['todo_date']
-      if asg_id = meta['assignment_identifier']
+      if (asg_id = meta['assignment_identifier'])
         wiki[:assignment] = {
           migration_id: asg_id,
           assignment_overrides: [],

lib/cc/importer/standard/assignment_converter.rb

@@ -23,7 +23,7 @@ module CC::Importer::Standard
 
     def convert_cc_assignments(asmnts = [])
       resources_by_type("assignment", "assignment_xmlv1p0").each do |res|
-        if doc = get_node_or_open_file(res, 'assignment')
+        if (doc = get_node_or_open_file(res, 'assignment'))
           path = res[:href] || (res[:files] && res[:files].first && res[:files].first[:href])
           resource_dir = File.dirname(path) if path
 
@@ -35,7 +35,7 @@ module CC::Importer::Standard
 
           # FIXME check the XML namespace to make sure it's actually a canvas assignment
           # (blocked by remove_namespaces! in lib/canvas/migration/migrator.rb)
-          if assgn_node = doc.at_css('extensions > assignment')
+          if (assgn_node = doc.at_css('extensions > assignment'))
             parse_canvas_assignment_data(assgn_node, nil, asmnt)
           end
 
@@ -52,7 +52,7 @@ module CC::Importer::Standard
       asmnt[:instructor_description] = get_node_val(doc, 'instructor_text')
       asmnt[:title] = get_node_val(doc, 'title')
       asmnt[:gradable] = get_bool_val(doc, 'gradable')
-      if points_possible = get_node_att(doc, 'gradable', 'points_possible')
+      if (points_possible = get_node_att(doc, 'gradable', 'points_possible'))
         asmnt[:grading_type] = 'points'
         asmnt[:points_possible] = points_possible.to_f
       end

lib/cc/importer/standard/converter.rb

@@ -118,7 +118,7 @@ module CC::Importer::Standard
 
     def get_canvas_att_replacement_url(path, resource_dir = nil)
       if path.start_with?('../')
-        if url = get_canvas_att_replacement_url(path.sub('../', ''), resource_dir)
+        if (url = get_canvas_att_replacement_url(path.sub('../', ''), resource_dir))
           return url
         end
       end
@@ -131,7 +131,7 @@ module CC::Importer::Standard
 
       unless mig_id
         path = path.gsub(%r{\$[^$]*\$|\.\./}, '')
-        if key = @file_path_migration_id.keys.detect { |k| k.end_with?(path) }
+        if (key = @file_path_migration_id.keys.detect { |k| k.end_with?(path) })
           mig_id = @file_path_migration_id[key]
         end
       end
@@ -170,7 +170,7 @@ module CC::Importer::Standard
             begin
               if val =~ FILEBASE_REGEX
                 val.gsub!(FILEBASE_REGEX, '')
-                if new_url = get_canvas_att_replacement_url(val, resource_dir)
+                if (new_url = get_canvas_att_replacement_url(val, resource_dir))
                   node[attr] = URI::escape(new_url)
 
                   if node.text.strip.blank? && !node.at_css("img") # add in the filename if the link is blank and doesn't have something visible like an image
@@ -179,7 +179,7 @@ module CC::Importer::Standard
                 end
               else
                 if ImportedHtmlConverter.relative_url?(val)
-                  if new_url = get_canvas_att_replacement_url(val)
+                  if (new_url = get_canvas_att_replacement_url(val))
                     node[attr] = URI::escape(new_url)
                   end
                 end
@@ -200,7 +200,7 @@ module CC::Importer::Standard
     def convert_blti_links_with_flat(lti_converter)
       tools = []
       resources_by_type("imsbasiclti").each do |res|
-        if doc = get_node_or_open_file(res)
+        if (doc = get_node_or_open_file(res))
           tool = lti_converter.convert_blti_link(doc)
           tool[:migration_id] = res[:migration_id]
           res[:url] = tool[:url] # for the organization item to reference

lib/cc/importer/standard/discussion_converter.rb

@@ -28,7 +28,7 @@ module CC::Importer::Standard
       resources_by_type("imsdt").each do |res|
         path = res[:href] || (res[:files] && res[:files].first && res[:files].first[:href])
         resource_dir = File.dirname(path) if path
-        if doc = get_node_or_open_file(res, TOPIC_NODE)
+        if (doc = get_node_or_open_file(res, TOPIC_NODE))
           topic = { :migration_id => res[:migration_id] }
           topic[:description] = get_node_val(doc, 'text')
           topic[:description] = replace_urls(topic[:description])
@@ -45,9 +45,9 @@ module CC::Importer::Standard
               topic[:description] += "\n<li><a href=\"#{get_canvas_att_replacement_url(att_path, resource_dir) || att_path}\">#{File.basename att_path}</a>"
             end
             topic[:description] += "\n</ul>"
-          elsif att_node = doc.at_css('attachment')
+          elsif (att_node = doc.at_css('attachment'))
             path = att_node['href']
-            if id = find_file_migration_id(path)
+            if (id = find_file_migration_id(path))
               topic[:attachment_migration_id] = id
             end
           end

lib/cc/importer/standard/org_converter.rb

@@ -31,7 +31,7 @@ module CC::Importer::Standard
       doc.css('organizations organization > item > item').each do |item_node|
         if item_node['identifierref']
           # item points to a single item
-          if item = process_item(item_node, 0)
+          if (item = process_item(item_node, 0))
             if !misc_module
               misc_module = { :title => "Misc Module", :migration_id => "misc_module_top_level_items", :items => [] }
               modules << misc_module
@@ -69,7 +69,7 @@ module CC::Importer::Standard
             else
               add_children(item_node, mod, indent + 1)
             end
-          elsif item = process_item(item_node, indent)
+          elsif (item = process_item(item_node, indent))
             mod[:items] << item
           end
         end
@@ -78,7 +78,7 @@ module CC::Importer::Standard
 
     def process_item(item_node, indent)
       item = nil
-      if resource = @resources[item_node['identifierref']]
+      if (resource = @resources[item_node['identifierref']])
         case resource[:type]
         when /assessment\z/
           item = {
@@ -101,7 +101,7 @@ module CC::Importer::Standard
           item[:linked_resource_title] ||= title
           item = nil if item[:url].blank?
         when /\Aimsbasiclti/
-          if asmnt = find_assignment(resource[:migration_id])
+          if (asmnt = find_assignment(resource[:migration_id]))
             item = {
               :indent => indent,
               :linked_resource_type => 'ASSIGNMENT',

lib/cc/importer/standard/quiz_converter.rb

@@ -33,7 +33,7 @@ module CC::Importer::Standard
         id = res[:migration_id]
 
         if path.nil? # inline qti
-          next unless res_node = @resource_nodes_for_flat_manifest[id]
+          next unless (res_node = @resource_nodes_for_flat_manifest[id])
 
           qti_node = res_node.elements.first
           path = "#{id}_qti.xml"
@@ -45,10 +45,10 @@ module CC::Importer::Standard
           qti_converted_dir = File.join(conversion_dir, id)
           if run_qti_converter(full_path, qti_converted_dir, id)
             # get quizzes/questions
-            if q_list = convert_questions(qti_converted_dir, id)
+            if (q_list = convert_questions(qti_converted_dir, id))
               questions += q_list
             end
-            if quiz = convert_assessment(qti_converted_dir, id)
+            if (quiz = convert_assessment(qti_converted_dir, id))
               quizzes << quiz
             end
           end
@@ -102,7 +102,7 @@ module CC::Importer::Standard
         manifest_file = File.join(out_folder, Qti::Converter::MANIFEST_FILE)
         quizzes = Qti.convert_assessments(manifest_file, :flavor => Qti::Flavors::COMMON_CARTRIDGE)
         ::Canvas::Migration::MigratorHelper.prepend_id_to_assessments(quizzes, resource_id)
-        if quiz = quizzes.first
+        if (quiz = quizzes.first)
           quiz[:migration_id] = resource_id
         end
       rescue

lib/cc/importer/standard/weblink_converter.rb

@@ -34,7 +34,7 @@ module CC::Importer::Standard
           title = get_node_val(doc, 'webLink title')
           url = get_node_att(doc, 'webLink url', 'href')
         end
-      elsif doc = get_node_or_open_file(resource, 'webLink')
+      elsif (doc = get_node_or_open_file(resource, 'webLink'))
         title = get_node_val(doc, 'title')
         url = get_node_att(doc, 'url', 'href')
       end

lib/cc/learning_outcomes.rb

@@ -133,7 +133,7 @@ module CC
           end
         end
 
-        if item.data && criterion = item.data[:rubric_criterion]
+        if item.data && (criterion = item.data[:rubric_criterion])
           out_node.points_possible criterion[:points_possible] if criterion[:points_possible]
           out_node.mastery_points criterion[:mastery_points] if criterion[:mastery_points]
           if criterion[:ratings] && criterion[:ratings].length > 0

lib/cc/qti/qti_generator.rb

@@ -323,7 +323,7 @@ module CC
         pick_count = group['pick_count'].to_i
         chosen = 0
         if group[:assessment_question_bank_id]
-          if bank = @course.assessment_question_banks.where(id: group[:assessment_question_bank_id]).first
+          if (bank = @course.assessment_question_banks.where(id: group[:assessment_question_bank_id]).first)
             bank.assessment_questions.each do |question|
               # try adding questions until the pick count is reached
               chosen += 1 if add_cc_question(node, question)
@@ -351,9 +351,9 @@ module CC
               bank = nil
 
               if group[:assessment_question_bank_id]
-                if bank = @course.assessment_question_banks.where(id: group[:assessment_question_bank_id]).first
+                if (bank = @course.assessment_question_banks.where(id: group[:assessment_question_bank_id]).first)
                   sel_node.sourcebank_ref create_key(bank)
-                elsif bank = AssessmentQuestionBank.where(id: group[:assessment_question_bank_id]).first
+                elsif (bank = AssessmentQuestionBank.where(id: group[:assessment_question_bank_id]).first)
                   sel_node.sourcebank_ref bank.id
                   is_external = true
                 end

lib/cc/qti/qti_items.rb

@@ -45,7 +45,7 @@ module CC
       def add_ref_or_question(node, question)
         aq = nil
         unless question[:assessment_question_id].blank?
-          if aq = AssessmentQuestion.where(id: question[:assessment_question_id]).first
+          if (aq = AssessmentQuestion.where(id: question[:assessment_question_id]).first)
             if aq.deleted? ||
                !aq.assessment_question_bank ||
                aq.assessment_question_bank.deleted? ||
@@ -440,7 +440,7 @@ module CC
         correct_points = "%.2f" % correct_points
 
         groups.each_pair do |id, answers|
-          if answer = answers.find { |a| a['weight'].to_i > 0 }
+          if (answer = answers.find { |a| a['weight'].to_i > 0 })
             node.respcondition do |r_node|
               r_node.conditionvar do |c_node|
                 c_node.varequal(answer['id'], :respident => "response_#{id}")

lib/cc/rubrics.rb

@@ -94,7 +94,7 @@ module CC
         c_node.long_description criterion[:long_description] if criterion[:long_description].present?
         c_node.criterion_use_range criterion[:criterion_use_range] if criterion[:criterion_use_range].present?
         if criterion[:learning_outcome_id].present?
-          if lo = @course.available_outcome(criterion[:learning_outcome_id])
+          if (lo = @course.available_outcome(criterion[:learning_outcome_id]))
             if lo.context_type == "Course" && lo.context_id == @course.id
               c_node.learning_outcome_identifierref create_key(lo)
             else

lib/cc/web_resources.rb

@@ -234,7 +234,7 @@ module CC
           # download from kaltura if the file wasn't already exported here in add_course_files
           if !@added_attachments || @added_attachments[obj.attachment_id] != path
             unless CanvasKaltura::ClientV3::ASSET_STATUSES[info[:asset][:status]] == :READY &&
-                   url = (client.flavorAssetGetPlaylistUrl(obj.media_id, info[:asset][:id]) || client.flavorAssetGetDownloadUrl(info[:asset][:id]))
+                   (url = (client.flavorAssetGetPlaylistUrl(obj.media_id, info[:asset][:id]) || client.flavorAssetGetDownloadUrl(info[:asset][:id])))
               add_error(I18n.t('course_exports.errors.media_file', "A media file failed to export"))
               next
             end

lib/course_link_validator.rb

@@ -214,7 +214,7 @@ class CourseLinkValidator
   def find_invalid_link(url)
     return if url.start_with?('mailto:')
 
-    unless result = self.visited_urls[url]
+    unless (result = self.visited_urls[url])
       begin
         if ImportedHtmlConverter.relative_url?(url) || (self.domain_regex && url.match(self.domain_regex))
           if valid_route?(url)

lib/cuty_capt.rb

@@ -119,7 +119,7 @@ class CutyCapt
   end
 
   def self.snapshot_url(url, &block)
-    return nil unless config = self.config
+    return nil unless (config = self.config)
     return nil unless self.verify_url(url)
 
     format = "png"

lib/dates_overridable.rb

@@ -227,7 +227,7 @@ module DatesOverridable
       hash.merge!({ :all_day => assignment.all_day, :all_day_date => assignment.all_day_date })
     end
 
-    if @applied_overrides && override = @applied_overrides.find { |o| o.due_at == due_at }
+    if @applied_overrides && (override = @applied_overrides.find { |o| o.due_at == due_at })
       hash[:override] = override
       hash[:title] = override.title
       hash[:set_type] = override.set_type
@@ -255,7 +255,7 @@ module DatesOverridable
       elsif self.multiple_due_dates_apply_to?(user)
         hash[:vdd_tooltip] = OverrideTooltipPresenter.new(self, user).as_json
       else
-        if due_date = self.overridden_for(user).due_at
+        if (due_date = self.overridden_for(user).due_at)
           hash[:due_date] = due_date
         elsif user_is_admin && (due_date = all_due_dates.dig(0, :due_at))
           hash[:due_date] = due_date

lib/gradebook_importer.rb

@@ -231,6 +231,7 @@ class GradebookImporter
       @assignments.each_with_index do |assignment, idx|
         next if assignment.changed? && !readonly_assignment?(assignment)
 
+        # rubocop:disable Lint/AssignmentInCondition https://github.com/rubocop/rubocop/issues/10136
         indexes_to_delete << idx if readonly_assignment?(assignment) || @students.all? do |student|
           submission = @gradebook_importer_assignments[student.id][idx]
 
@@ -243,6 +244,7 @@ class GradebookImporter
 
           no_change || !submission['gradeable']
         end
+        # rubocop:enable Lint/AssignmentInCondition
       end
 
       custom_column_ids_to_skip_on_import = []

lib/locale_selection.rb

@@ -41,7 +41,7 @@ module LocaleSelection
       -> { Account.recursive_default_locale_for_id(context.id) if context.try(:is_a?, Account) },
       -> { root_account.try(:default_locale) },
       -> {
-        if accept_language && locale = infer_browser_locale(accept_language, LocaleSelection.locales_with_aliases)
+        if accept_language && (locale = infer_browser_locale(accept_language, LocaleSelection.locales_with_aliases))
           GuardRail.activate(:primary) do
             user.update_attribute(:browser_locale, locale) if user && user.browser_locale != locale
           end
@@ -84,7 +84,7 @@ module LocaleSelection
     #                           en-US range is a longer match, so it loses)
 
     best_locales = supported_locales.inject([]) { |ary, locale|
-      if best_range = ranges.detect { |r, q| r + '-' == (locale.downcase + '-')[0..r.size] || r == '*' }
+      if (best_range = ranges.detect { |r, q| r + '-' == (locale.downcase + '-')[0..r.size] || r == '*' })
         ary << [locale, best_range.last, ranges.index(best_range)] unless best_range.last == 0
       end
       ary

lib/lti/api_service_helper.rb

@@ -42,7 +42,7 @@ module Lti
     end
 
     def authenticate_body_hash
-      if body_hash = OAuth::Helper.parse_header(request.authorization)['oauth_body_hash']
+      if (body_hash = OAuth::Helper.parse_header(request.authorization)['oauth_body_hash'])
         request.body.rewind
         generated_hash = Digest::SHA1.base64digest(request.body.read)
         request.body.rewind # Be Kind Rewind

lib/messageable_user.rb

@@ -126,25 +126,21 @@ class MessageableUser < User
   # common_groups
   def populate_common_contexts
     @global_common_courses = {}
-    if common_courses = read_attribute(:common_courses)
+    read_attribute(:common_courses)&.to_s&.split(',')&.each do |common_course|
-      common_courses.to_s.split(',').each do |common_course|
+      course_id, role = common_course.split(':')
-        course_id, role = common_course.split(':')
+      course_id = course_id.to_i
-        course_id = course_id.to_i
+      # a course id of 0 indicates admin visibility without an actual shared
-        # a course id of 0 indicates admin visibility without an actual shared
+      # course; don't "globalize" it
-        # course; don't "globalize" it
+      course_id = Shard.global_id_for(course_id) unless course_id.zero?
-        course_id = Shard.global_id_for(course_id) unless course_id.zero?
+      @global_common_courses[course_id] ||= []
-        @global_common_courses[course_id] ||= []
+      @global_common_courses[course_id] << role
-        @global_common_courses[course_id] << role
-      end
     end
 
     @global_common_groups = {}
-    if common_groups = read_attribute(:common_groups)
+    read_attribute(:common_groups)&.to_s&.split(',')&.each do |group_id|
-      common_groups.to_s.split(',').each do |group_id|
+      group_id = Shard.global_id_for(group_id.to_i)
-        group_id = Shard.global_id_for(group_id.to_i)
+      @global_common_groups[group_id] ||= []
-        @global_common_groups[group_id] ||= []
+      @global_common_groups[group_id] << 'Member'
-        @global_common_groups[group_id] << 'Member'
-      end
     end
   end
   after_find :populate_common_contexts

lib/messageable_user/calculator.rb

@@ -295,7 +295,7 @@ class MessageableUser
 
       # skipping messageability constraints, do I see the user in that specific
       # course, and if so with which enrollment type(s)?
-      if include_course_id && course = Course.where(id: include_course_id).first
+      if include_course_id && (course = Course.where(id: include_course_id).first)
         missing_users = users.reject { |user| user.global_common_courses.keys.include?(course.global_id) }
         if missing_users.present?
           course.shard.activate do
@@ -348,7 +348,7 @@ class MessageableUser
 
       # skipping messageability constraints, do I see the user in that specific
       # group?
-      if include_group_id && group = Group.where(id: include_group_id).first
+      if include_group_id && (group = Group.where(id: include_group_id).first)
         missing_users = users.reject { |user| user.global_common_groups.keys.include?(group.global_id) }
         if missing_users.present?
           group.shard.activate do
@@ -427,7 +427,7 @@ class MessageableUser
 
       course.shard.activate do
         # make sure the course is recognized
-        return unless options[:admin_context] || course = course_index[course.id]
+        return unless options[:admin_context] || (course = course_index[course.id])
 
         scope = enrollment_scope(options.merge(
                                    :include_concluded_students => false,

lib/model_cache.rb

@@ -68,7 +68,7 @@ module ModelCache
 
   module InstanceMethods
     def add_to_caches
-      return unless cache = ModelCache[self.class.name.underscore.pluralize.to_sym]
+      return unless (cache = ModelCache[self.class.name.underscore.pluralize.to_sym])
 
       cache.keys.each do |key|
         cache[key][send(key)] = self
@@ -76,7 +76,7 @@ module ModelCache
     end
 
     def update_in_caches
-      return unless cache = ModelCache[self.class.name.underscore.pluralize.to_sym]
+      return unless (cache = ModelCache[self.class.name.underscore.pluralize.to_sym])
 
       cache.keys.each do |key|
         if saved_change_to_attribute?(key)

lib/reporting/counts_report.rb

@@ -94,11 +94,11 @@ module Reporting
     private
 
     def save_detailed_progressive(account, data)
-      if snapshot = account.report_snapshots.progressive.last
+      progressive = if (snapshot = account.report_snapshots.progressive.last)
-        progressive = snapshot.data.with_indifferent_access
+                      snapshot.data.with_indifferent_access
-      else
+                    else
-        progressive = new_progressive_hash.with_indifferent_access
+                      new_progressive_hash.with_indifferent_access
-      end
+                    end
       progressive[:generated_at] = @timestamp
       create_progressive_hashes(progressive, data)
 

lib/simple_tags.rb

@@ -20,11 +20,7 @@
 module SimpleTags
   module ReaderInstanceMethods
     def tags
-      @tag_array ||= if tags = read_attribute(:tags)
+      @tag_array ||= read_attribute(:tags)&.split(',') || []
-                       tags.split(',')
-                     else
-                       []
-                     end
     end
 
     def serialized_tags(tags = self.tags)

lib/tasks/brand_configs.rake

@@ -4,7 +4,7 @@ namespace :brand_configs do
   desc "Writes the .css (css variables), .js & .json files that are used to load the theme editor variables for each brand " +
        "Set BRAND_CONFIG_MD5=<whatever> to save just that one, otherwise writes a file for each BrandConfig in db."
   task :write => :environment do
-    if md5 = ENV['BRAND_CONFIG_MD5']
+    if (md5 = ENV['BRAND_CONFIG_MD5'])
       BrandConfig.find(md5).save_all_files!
     else
       BrandConfig.clean_unused_from_db!

lib/tasks/canvas/quizzes.rake

@@ -27,7 +27,7 @@ namespace :canvas do
       require 'json'
       require 'benchmark'
 
-      unless out_path = args[:out]
+      unless (out_path = args[:out])
         raise "Missing path to output file."
       end
 

lib/unzip_attachment.rb

@@ -207,11 +207,11 @@ class UnzipAttachment
     # For every directory in the path...
     # (-2 means all entries but the last, which should be a filename)
     list[0..-2].each do |dir|
-      if new_dir = current.sub_folders.where(name: dir).first
+      current = if (new_dir = current.sub_folders.where(name: dir).first)
-        current = new_dir
+                  new_dir
-      else
+                else
-        current = assert_folder(current, dir)
+                  assert_folder(current, dir)
-      end
+                end
     end
     current
   end

lib/user_content.rb

@@ -226,7 +226,7 @@ module UserContent
           end
         end
 
-        if module_item = rest.try(:match, %r{/items/(\d+)})
+        if (module_item = rest.try(:match, %r{/items/(\d+)}))
           type   = 'items'
           obj_id = module_item[1].to_i
         end

lib/user_list_v2.rb

@@ -137,7 +137,7 @@ class UserListV2
     @addresses.each do |a|
       address = @lowercase ? a[:address].downcase : a[:address]
       unless grouped_results.key?(address)
-        if name = a.delete(:name)
+        if (name = a.delete(:name))
           a[:user_name] = name
         end
         @missing_results << a

lib/user_merge.rb

@@ -237,7 +237,7 @@ class UserMerge
         end
       end
 
-      if existing_record = target_values[[key, sub_key]]
+      if (existing_record = target_values[[key, sub_key]])
         existing_record_updates[existing_record] = value
       else
         new_record_hashes << { :user_id => target_user.id, :key => key, :sub_key => sub_key&.to_json, :value => value.to_yaml }
@@ -389,7 +389,7 @@ class UserMerge
       if from_user.shard != target_user.shard
         User.clone_communication_channel(source_cc, target_user, max_position)
       end
-    elsif to_retire = source_cc
+    elsif (to_retire = source_cc)
       # retired, retired
     end
     to_retire

spec/apis/v1/authentication_audit_api_spec.rb

@@ -62,23 +62,23 @@ describe "AuthenticationAudit API", type: :request do
     end
 
     def fetch_for_context(context, options = {})
-      type = context.class.to_s.downcase unless type = options.delete(:type)
+      type = context.class.to_s.downcase unless (type = options.delete(:type))
       id = context.id.to_s
 
       arguments = { controller: 'authentication_audit_api', action: "for_#{type}", :"#{type}_id" => id, format: 'json' }
       query_string = []
 
-      if per_page = options.delete(:per_page)
+      if (per_page = options.delete(:per_page))
         arguments[:per_page] = per_page.to_s
         query_string << "per_page=#{arguments[:per_page]}"
       end
 
-      if start_time = options.delete(:start_time)
+      if (start_time = options.delete(:start_time))
         arguments[:start_time] = start_time.iso8601
         query_string << "start_time=#{arguments[:start_time]}"
       end
 
-      if end_time = options.delete(:end_time)
+      if (end_time = options.delete(:end_time))
         arguments[:end_time] = end_time.iso8601
         query_string << "end_time=#{arguments[:end_time]}"
       end

spec/apis/v1/course_audit_api_spec.rb

@@ -56,28 +56,28 @@ describe "CourseAudit API", type: :request do
     end
 
     def fetch_for_context(context, id: nil, **options)
-      type = context.class.to_s.downcase unless type = options.delete(:type)
+      type = context.class.to_s.downcase unless (type = options.delete(:type))
       id ||= context.id.to_s
 
       arguments = { controller: 'course_audit_api', action: "for_#{type}", :"#{type}_id" => id, format: 'json' }
       query_string = []
 
-      if per_page = options.delete(:per_page)
+      if (per_page = options.delete(:per_page))
         arguments[:per_page] = per_page.to_s
         query_string << "per_page=#{arguments[:per_page]}"
       end
 
-      if start_time = options.delete(:start_time)
+      if (start_time = options.delete(:start_time))
         arguments[:start_time] = start_time.iso8601
         query_string << "start_time=#{arguments[:start_time]}"
       end
 
-      if end_time = options.delete(:end_time)
+      if (end_time = options.delete(:end_time))
         arguments[:end_time] = end_time.iso8601
         query_string << "end_time=#{arguments[:end_time]}"
       end
 
-      if account = options.delete(:account)
+      if (account = options.delete(:account))
         arguments[:account_id] = Shard.global_id_for(account).to_s
         query_string << "account_id=#{arguments[:account_id]}"
       end

spec/apis/v1/grade_change_audit_api_spec.rb

@@ -56,29 +56,29 @@ describe "GradeChangeAudit API", type: :request do
     end
 
     def fetch_for_context(context, options = {})
-      type = context.class.to_s.downcase unless type = options.delete(:type)
+      type = context.class.to_s.downcase unless (type = options.delete(:type))
       user = options.delete(:user) || @viewing_user
       id = Shard.global_id_for(context).to_s
 
       arguments = { controller: :grade_change_audit_api, action: "for_#{type}", "#{type}_id": id, format: :json }
       query_string = []
 
-      if per_page = options.delete(:per_page)
+      if (per_page = options.delete(:per_page))
         arguments[:per_page] = per_page.to_s
         query_string << "per_page=#{arguments[:per_page]}"
       end
 
-      if start_time = options.delete(:start_time)
+      if (start_time = options.delete(:start_time))
         arguments[:start_time] = start_time.iso8601
         query_string << "start_time=#{arguments[:start_time]}"
       end
 
-      if end_time = options.delete(:end_time)
+      if (end_time = options.delete(:end_time))
         arguments[:end_time] = end_time.iso8601
         query_string << "end_time=#{arguments[:end_time]}"
       end
 
-      if account = options.delete(:account)
+      if (account = options.delete(:account))
         arguments[:account_id] = Shard.global_id_for(account).to_s
         query_string << "account_id=#{arguments[:account_id]}"
       end

spec/apis/v1/outcomes_api_spec.rb

@@ -73,7 +73,7 @@ describe "Outcomes API", type: :request do
       retval["points_possible"] = presets[:points_possible] || proficiency.points_possible
       retval["mastery_points"]  = presets[:mastery_points]  || proficiency.mastery_points
       retval["ratings"]         = presets[:ratings]         || proficiency.ratings_hash.map(&:stringify_keys)
-    elsif criterion = (outcome.data && outcome.data[:rubric_criterion])
+    elsif (criterion = outcome.data && outcome.data[:rubric_criterion])
       retval["points_possible"] = presets[:points_possible] || criterion[:points_possible].to_i
       retval["mastery_points"]  = presets[:mastery_points]  || criterion[:mastery_points].to_i
       retval["ratings"]         = presets[:ratings]         || criterion[:ratings].map(&:stringify_keys)

spec/factories/account_factory.rb

@@ -93,7 +93,7 @@ module Factories
     if opts[:role_changes]
       opts[:role_changes].each_pair do |permission, enabled|
         role = opts[:role] || admin_role
-        if ro = account.role_overrides.where(:permission => permission.to_s, :role_id => role.id).first
+        if (ro = account.role_overrides.where(:permission => permission.to_s, :role_id => role.id).first)
           ro.update_attribute(:enabled, enabled)
         else
           account.role_overrides.create(:permission => permission.to_s, :enabled => enabled, :role => role)

spec/factories/calendar_event_factory.rb

@@ -50,7 +50,7 @@ module Factories
 
   def appointment_group_model(opts = {})
     @course ||= opts.delete(:course) || course_model
-    if sub_context = opts.delete(:sub_context)
+    if (sub_context = opts.delete(:sub_context))
       opts[:sub_context_codes] = [sub_context.asset_string]
     end
     @appointment_group = AppointmentGroup.create!(valid_appointment_group_attributes.merge(opts))

spec/factories/course_factory.rb

@@ -158,7 +158,7 @@ module Factories
     if options[:account_associations]
       create_records(CourseAccountAssociation, course_ids.map { |id| { account_id: account.id, course_id: id, depth: 0, root_account_id: account.resolved_root_account_id, created_at: now, updated_at: now } })
     end
-    if user = options[:enroll_user]
+    if (user = options[:enroll_user])
       section_ids = create_records(CourseSection, course_ids.map { |id| { course_id: id, root_account_id: account.id, name: "Default Section", default_section: true, created_at: now, updated_at: now } })
       type = options[:enrollment_type] || "TeacherEnrollment"
       role_id = Role.get_built_in_role(type, root_account_id: account.resolved_root_account_id).id

spec/rspec_mock_extensions.rb

@@ -43,7 +43,7 @@ module RspecMockAnyInstantiation
     end
 
     def instantiate(record, column_types = {}, &block)
-      if obj = @@any_instantiation[[base_class, record['id'].to_i]]
+      if (obj = @@any_instantiation[[base_class, record['id'].to_i]])
         obj
       else
         super
@@ -51,7 +51,7 @@ module RspecMockAnyInstantiation
     end
 
     def instantiate_instance_of(klass, record, column_types = {}, &block)
-      if obj = @@any_instantiation[[klass, record['id'].to_i]]
+      if (obj = @@any_instantiation[[klass, record['id'].to_i]])
         obj
       else
         super

spec/selenium/helpers/conversations_common.rb

@@ -203,7 +203,7 @@ module ConversationsCommon
   end
 
   def run_progress_job
-    return unless progress = Progress.where(tag: 'conversation_batch_update').first
+    return unless (progress = Progress.where(tag: 'conversation_batch_update').first)
 
     job = Delayed::Job.find(progress.delayed_job_id)
     job.invoke_job

spec/selenium/users_spec.rb

@@ -260,9 +260,7 @@ describe "users" do
     end
 
     it "requires terms if configured to do so" do
-      if terms = Account.default.terms_of_service
+      Account.default.terms_of_service&.update(passive: false)
-        terms.update(passive: false)
-      end
 
       get "/register"
 
@@ -279,9 +277,7 @@ describe "users" do
     end
 
     it "registers a student with a join code" do
-      if terms = Account.default.terms_of_service
+      Account.default.terms_of_service&.update(passive: false)
-        terms.update(passive: false)
-      end
 
       Account.default.allow_self_enrollment!
       course_factory(active_all: true)
@@ -305,9 +301,7 @@ describe "users" do
     end
 
     it "registers a teacher" do
-      if terms = Account.default.terms_of_service
+      Account.default.terms_of_service&.update(passive: false)
-        terms.update(passive: false)
-      end
 
       get '/register'
       f('#signup_teacher').click
@@ -338,9 +332,7 @@ describe "users" do
     end
 
     it "registers an observer" do
-      if terms = Account.default.terms_of_service
+      Account.default.terms_of_service&.update(passive: false)
-        terms.update(passive: false)
-      end
 
       user = user_with_pseudonym(:active_all => true, :password => 'lolwut12')
       pairing_code = user.generate_observer_pairing_code

spec/spec_helper.rb

@@ -662,7 +662,7 @@ RSpec.configure do |config|
 
     def as(ancestor)
       @__as ||= {}
-      unless r = @__as[ancestor]
+      unless (r = @__as[ancestor])
         r = (@__as[ancestor] = As.new(self, ancestor))
       end
       r
@@ -723,12 +723,12 @@ RSpec.configure do |config|
   end
 
   def run_jobs
-    while job = Delayed::Job.get_and_lock_next_available(
+    while (job = Delayed::Job.get_and_lock_next_available(
       'spec run_jobs',
       Delayed::Settings.queue,
       0,
       Delayed::MAX_PRIORITY
-    )
+    ))
       run_job(job)
     end
   end