RuboCop: Rails/WhereEquals

auto-corrected, with post-review looking for cases where an unqualified
column would now be (incorrectly) qualified (only one found)

Change-Id: I62ef6d40ce9e7bc062db261d9c6fb9383ecd102e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278432
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

.rubocop.common.yml

@@ -118,6 +118,8 @@ Rails/SquishedSQLHeredocs:
   Severity: error
 Rails/Validation:
   Severity: error
+Rails/WhereEquals:
+  Severity: error
 Rails/WhereExists:
   EnforcedStyle: where
   Severity: error

app/controllers/enrollments_api_controller.rb

@@ -908,7 +908,7 @@ class EnrollmentsApiController < ApplicationController
       # enrollments without any extra checking.
       enrollments = if params[:state].present?
                       user.enrollments.where(enrollment_index_conditions(true)).joins(:enrollment_state)
-                          .where("enrollment_states.state IN (?)", enrollment_states_for_state_param)
+                          .where(enrollment_states: { state: enrollment_states_for_state_param })
                     else
                       user.enrollments.current_and_invited.where(enrollment_index_conditions)
                           .joins(:enrollment_state).where("enrollment_states.state<>'completed'")

app/controllers/outcomes_api_controller.rb

@@ -326,7 +326,7 @@ class OutcomesApiController < ApplicationController
                 .select(:title, :id, :assignment_id).preload(:quiz_questions)
                 .joins(assignment: :submissions)
                 .where(context: course)
-                .where("submissions.user_id = ?", student_id)
+                .where(submissions: { user_id: student_id })
                 .where("submissions.workflow_state <> 'deleted'")
       quiz_alignments = quizzes.map do |quiz|
         bank_ids = quiz.quiz_questions.filter_map { |qq| qq.assessment_question.try(:assessment_question_bank_id) }.uniq

app/models/assignment.rb

@@ -2834,7 +2834,7 @@ class Assignment < ActiveRecord::Base
   }
 
   scope :by_assignment_group_id, lambda { |group_id|
-    where('assignment_group_id = ?', group_id.to_s)
+    where(assignment_group_id: group_id.to_s)
   }
 
   # assignments only ever belong to courses, so we can reduce this to just IDs to simplify the db query

app/models/assignments/needs_grading_count_query.rb

@@ -78,15 +78,14 @@ module Assignments
 
       # ignore submissions this user has graded
       graded_sub_ids = assignment.submissions.joins(:provisional_grades)
-                                 .where("moderated_grading_provisional_grades.final = ?", false)
-                                 .where("moderated_grading_provisional_grades.scorer_id = ?", user.id)
+                                 .where(moderated_grading_provisional_grades: { final: false, scorer_id: user.id })
                                  .where("moderated_grading_provisional_grades.score IS NOT NULL").pluck(:id)
 
       moderation_set_student_ids = assignment.moderated_grading_selections.pluck(:student_id)
 
       # ignore submissions that don't need any more provisional grades
       pg_scope = assignment.submissions.joins(:provisional_grades)
-                           .where("moderated_grading_provisional_grades.final = ?", false)
+                           .where(moderated_grading_provisional_grades: { final: false })
                            .where("moderated_grading_provisional_grades.scorer_id <> ?", user.id)
                            .group("submissions.id", "submissions.user_id")
       pg_scope = pg_scope.where("submissions.id NOT IN (?)", graded_sub_ids) if graded_sub_ids.any?
@@ -130,7 +129,7 @@ module Assignments
     private
 
     def section_filtered_submissions
-      all_submissions.where('e.course_section_id in (?)', visible_section_ids)
+      all_submissions.where(e: { course_section_id: visible_section_ids })
     end
 
     def all_submissions

app/models/attachment.rb

@@ -724,7 +724,7 @@ class Attachment < ActiveRecord::Base
           if context.is_a?(User) || context.is_a?(Group)
             excluded_attachment_ids = []
             if context.is_a?(User)
-              excluded_attachment_ids += context.attachments.joins(:attachment_associations).where("attachment_associations.context_type = ?", "Submission").pluck(:id)
+              excluded_attachment_ids += context.attachments.joins(:attachment_associations).where(attachment_associations: { context_type: "Submission" }).pluck(:id)
             end
             excluded_attachment_ids += context.attachments.where(folder_id: context.submissions_folders).pluck(:id)
             attachment_scope = attachment_scope.where("id NOT IN (?)", excluded_attachment_ids) if excluded_attachment_ids.any?
@@ -795,7 +795,7 @@ class Attachment < ActiveRecord::Base
         Canvas::Errors.capture_exception(:attachment, e, :warn)
         # Failed to uniquely rename attachment, slapping on a UUID and moving on
         self.display_name = self.display_name + SecureRandom.uuid
-        Attachment.where("id = ?", self).limit(1).update_all(display_name: display_name)
+        Attachment.where(id: self).limit(1).update_all(display_name: display_name)
       end
     elsif method == :overwrite && atts.any?
       shard.activate do

app/models/auditors/grade_change.rb

@@ -223,7 +223,7 @@ class Auditors::GradeChange
 
     # If we *are* specifically searching for override grades, swap out the
     # placeholder ID for a real NULL check.
-    scope.unscope(where: :assignment_id).where("assignment_id IS NULL")
+    scope.unscope(where: :assignment_id).where(assignment_id: nil)
   end
 
   Stream = Audits.stream do

app/models/communication_channel.rb

@@ -404,7 +404,7 @@ class CommunicationChannel < ActiveRecord::Base
     # Add twitter and yo (in that order) if the user's account is setup for them.
     rank_order << TYPE_TWITTER if twitter_service
     rank_order << TYPE_SLACK if user.associated_root_accounts.any? { |a| a.settings[:encrypted_slack_key] }
-    self.unretired.where('communication_channels.path_type IN (?)', rank_order)
+    self.unretired.where(communication_channels: { path_type: rank_order })
         .order(Arel.sql("#{self.rank_sql(rank_order, 'communication_channels.path_type')} ASC, communication_channels.position asc")).to_a
   end
 

app/models/context_external_tool.rb

@@ -1010,7 +1010,7 @@ class ContextExternalTool < ActiveRecord::Base
 
   scope :having_setting, lambda { |setting|
                            setting ? joins(:context_external_tool_placements)
-                             .where("context_external_tool_placements.placement_type = ?", setting) : all
+                             .where(context_external_tool_placements: { placement_type: setting }) : all
                          }
 
   scope :placements, lambda { |*placements|

app/models/conversation.rb

@@ -495,7 +495,7 @@ class Conversation < ActiveRecord::Base
         maybe_update_timestamp('visible_last_authored_at', message.created_at, ["user_id = ?", message.author_id])
       ]
       updates << "workflow_state = CASE WHEN workflow_state = 'archived' THEN 'read' ELSE workflow_state END" if update_for_skips
-      conversation_participants.where("user_id IN (?)", skip_ids).update_all(updates.join(", "))
+      conversation_participants.where(user_id: skip_ids).update_all(updates.join(", "))
 
       if message.has_attachments?
         self.has_attachments = true

app/models/conversation_message_participant.rb

@@ -53,7 +53,7 @@ class ConversationMessageParticipant < ActiveRecord::Base
   def self.query_deleted(user_id, options = {})
     query = self.deleted.eager_load(:conversation_message).where(user_id: user_id).order(deleted_at: :desc)
 
-    query = query.where('conversation_messages.conversation_id = ?', options['conversation_id']) if options['conversation_id']
+    query = query.where(conversation_messages: { conversation_id: options['conversation_id'] }) if options['conversation_id']
     query = query.where('conversation_message_participants.deleted_at < ?', options['deleted_before']) if options['deleted_before']
     query = query.where('conversation_message_participants.deleted_at > ?', options['deleted_after']) if options['deleted_after']
 

app/models/course.rb

@@ -193,7 +193,7 @@ class Course < ActiveRecord::Base
   has_many :role_overrides, :as => :context, :inverse_of => :context
   has_many :content_migrations, :as => :context, :inverse_of => :context
   has_many :content_exports, :as => :context, :inverse_of => :context
-  has_many :epub_exports, -> { where("type IS NULL").order("created_at DESC") }
+  has_many :epub_exports, -> { where(type: nil).order("created_at DESC") }
 
   has_many :gradebook_filters, inverse_of: :course, dependent: :destroy
   attr_accessor :latest_epub_export
@@ -874,10 +874,10 @@ class Course < ActiveRecord::Base
   scope :deleted, -> { where(:workflow_state => 'deleted') }
 
   scope :master_courses, -> { joins(:master_course_templates).where.not(MasterCourses::MasterTemplate.table_name => { :workflow_state => 'deleted' }) }
-  scope :not_master_courses, -> { joins("LEFT OUTER JOIN #{MasterCourses::MasterTemplate.quoted_table_name} AS mct ON mct.course_id=courses.id AND mct.workflow_state<>'deleted'").where("mct IS NULL") }
+  scope :not_master_courses, -> { joins("LEFT OUTER JOIN #{MasterCourses::MasterTemplate.quoted_table_name} AS mct ON mct.course_id=courses.id AND mct.workflow_state<>'deleted'").where("mct IS NULL") } # rubocop:disable Rails/WhereEquals mct is a table, not a column
 
   scope :associated_courses, -> { joins(:master_course_subscriptions).where.not(MasterCourses::ChildSubscription.table_name => { :workflow_state => 'deleted' }) }
-  scope :not_associated_courses, -> { joins("LEFT OUTER JOIN #{MasterCourses::ChildSubscription.quoted_table_name} AS mcs ON mcs.child_course_id=courses.id AND mcs.workflow_state<>'deleted'").where("mcs IS NULL") }
+  scope :not_associated_courses, -> { joins("LEFT OUTER JOIN #{MasterCourses::ChildSubscription.quoted_table_name} AS mcs ON mcs.child_course_id=courses.id AND mcs.workflow_state<>'deleted'").where("mcs IS NULL") } # rubocop:disable Rails/WhereEquals mcs is a table, not a column
 
   scope :templates, -> { where(template: true) }
 

app/models/enrollment.rb

@@ -1283,23 +1283,21 @@ class Enrollment < ActiveRecord::Base
 
   scope :active_by_date, -> { joins(:enrollment_state).where("enrollment_states.state = 'active'") }
   scope :invited_by_date, -> {
-                            joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false)
+                            joins(:enrollment_state).where(enrollment_states: { restricted_access: false })
                                                     .where("enrollment_states.state IN ('invited', 'pending_invited')")
                           }
   scope :active_or_pending_by_date, -> {
-                                      joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false)
+                                      joins(:enrollment_state).where(enrollment_states: { restricted_access: false })
                                                               .where("enrollment_states.state IN ('active', 'invited', 'pending_invited', 'pending_active')")
                                     }
   scope :invited_or_pending_by_date, -> {
-                                       joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false)
+                                       joins(:enrollment_state).where(enrollment_states: { restricted_access: false })
                                                                .where("enrollment_states.state IN ('invited', 'pending_invited', 'pending_active')")
                                      }
-  scope :completed_by_date, -> {
-                              joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false)
-                                                      .where("enrollment_states.state = ?", "completed")
-                            }
+  scope :completed_by_date,
+        -> { joins(:enrollment_state).where(enrollment_states: { restricted_access: false, state: "completed" }) }
   scope :not_inactive_by_date, -> {
-                                 joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false)
+                                 joins(:enrollment_state).where(enrollment_states: { restricted_access: false })
                                                          .where("enrollment_states.state IN ('active', 'invited', 'completed', 'pending_invited', 'pending_active')")
                                }
 

app/models/group.rb

@@ -307,7 +307,7 @@ class Group < ActiveRecord::Base
 
   scope :active, -> { where("groups.workflow_state<>'deleted'") }
   scope :by_name, -> { order(Bookmarker.order_by) }
-  scope :uncategorized, -> { where("groups.group_category_id IS NULL") }
+  scope :uncategorized, -> { where(groups: { group_category_id: nil }) }
 
   def potential_collaborators
     if context.is_a?(Course)
@@ -386,9 +386,9 @@ class Group < ActiveRecord::Base
     return if users.empty?
 
     user_ids = users.map(&:id)
-    old_group_memberships = self.group_memberships.where("user_id IN (?)", user_ids).to_a
+    old_group_memberships = self.group_memberships.where(user_id: user_ids).to_a
     bulk_insert_group_memberships(users, options)
-    all_group_memberships = self.group_memberships.where("user_id IN (?)", user_ids)
+    all_group_memberships = self.group_memberships.where(user_id: user_ids)
     new_group_memberships = all_group_memberships - old_group_memberships
     new_group_memberships.sort_by!(&:user_id)
     users.sort_by!(&:id)

app/models/lti/tool_proxy.rb

@@ -31,7 +31,7 @@ module Lti
     after_save :manage_subscription
     before_destroy :delete_subscription
 
-    scope :active, -> { where("lti_tool_proxies.workflow_state = ?", 'active') }
+    scope :active, -> { where(lti_tool_proxies: { workflow_state: 'active' }) }
 
     serialize :raw_data
     serialize :update_payload
@@ -51,7 +51,7 @@ module Lti
     end
 
     def self.find_active_proxies_for_context(context)
-      find_all_proxies_for_context(context).where('lti_tool_proxies.workflow_state = ?', 'active')
+      find_all_proxies_for_context(context).where(lti_tool_proxies: { workflow_state: 'active' })
     end
 
     def self.find_installed_proxies_for_context(context)
@@ -94,9 +94,9 @@ module Lti
               # changed the order to go from the special ordering set up (to make sure we're going from the course to the
               # root account in order of parent accounts) and then takes the most recently installed tool
               order('ordering, lti_tool_proxies.id DESC')
-                  .where('lti_tool_proxies.workflow_state = ?', 'active')
+                  .where(lti_tool_proxies: { workflow_state: 'active' })
                   .where('lti_product_families.vendor_code = ? AND lti_product_families.product_code = ?', vendor_code, product_code)
-                  .where('lti_resource_handlers.resource_type_code = ?', resource_type_code)
+                  .where(lti_resource_handlers: { resource_type_code: resource_type_code })
       # You can disable a tool_binding somewhere in the account chain, and anything below that that reenables it should be
       # available, but nothing above it, so we're getting rid of anything that is disabled and above
       tools.split { |tool| !tool.binding_enabled }.first

app/models/microsoft_sync/user_mapping.rb

@@ -70,7 +70,7 @@ class MicrosoftSync::UserMapping < ActiveRecord::Base
           ON mappings.user_id=enrollments.user_id
           AND mappings.root_account_id=#{course.root_account_id.to_i}
         })
-        .where('mappings.id IS NULL')
+        .where(mappings: { id: nil })
         .select(:user_id).distinct.limit(MAX_ENROLLMENT_MEMBERS)
         .pluck(:user_id)
     end

app/models/notification_policy.rb

@@ -118,7 +118,7 @@ class NotificationPolicy < ActiveRecord::Base
 
   # Updates notification policies for a given category in a given communication channel
   def self.find_or_update_for_category(communication_channel, category, frequency = nil)
-    notifs = Notification.where("category = ?", category)
+    notifs = Notification.where(category: category)
     raise ActiveRecord::RecordNotFound unless notifs.exists?
 
     notifs.map do |notif|

app/models/quizzes/quiz.rb

@@ -1515,7 +1515,7 @@ class Quizzes::Quiz < ActiveRecord::Base
   # returns visible students for differentiated assignments
   def visible_students_with_da(context_students)
     quiz_students = context_students.joins(:quiz_student_visibilities)
-                                    .where('quiz_id = ?', self.id)
+                                    .where(quiz_student_visibilities: { quiz_id: id })
 
     # empty quiz_students means the quiz is for everyone
     return quiz_students if quiz_students.present?

app/models/role.rb

@@ -248,7 +248,7 @@ class Role < ActiveRecord::Base
   def self.custom_roles_and_counts_for_course(course, user, include_inactive = false)
     users_scope = course.users_visible_to(user)
     built_in_role_ids = Role.built_in_course_roles(root_account_id: course.root_account_id).map(&:id)
-    base_counts = users_scope.where('enrollments.role_id IN (?)', built_in_role_ids)
+    base_counts = users_scope.where(enrollments: { role_id: built_in_role_ids })
                              .group('enrollments.type').select('users.id').distinct.count
     role_counts = users_scope.where('enrollments.role_id NOT IN (?)', built_in_role_ids)
                              .group('enrollments.role_id').select('users.id').distinct.count

app/models/user.rb

@@ -1637,7 +1637,7 @@ class User < ActiveRecord::Base
     if preferences[:course_nicknames] == UserPreferenceValue::EXTERNAL
       self.shard.activate do
         scope = user_preference_values.where(:key => :course_nicknames)
-        scope = scope.where("sub_key IN (?)", courses.map(&:id).map(&:to_json)) if courses
+        scope = scope.where(sub_key: courses) if courses
         Hash[scope.pluck(:sub_key, :value)]
       end
     else
@@ -1884,7 +1884,7 @@ class User < ActiveRecord::Base
           end
 
           unless options[:include_completed_courses]
-            scope = scope.joins(:all_enrollments => :enrollment_state).where("enrollment_states.restricted_access = ?", false)
+            scope = scope.joins(:all_enrollments => :enrollment_state).where(enrollment_states: { restricted_access: false })
                          .where("enrollment_states.state IN ('active', 'invited', 'pending_invited', 'pending_active')")
           end
 

app/models/user_learning_object_scopes.rb

@@ -372,7 +372,7 @@ module UserLearningObjectScopes
       scope = assignment_scope.active
                               .expecting_submission
                               .where(final_grader: self, moderated_grading: true)
-                              .where("assignments.grades_published_at IS NULL")
+                              .where(assignments: { grades_published_at: nil })
                               .where(id: ModeratedGrading::ProvisionalGrade.joins(:submission)
           .where("submissions.assignment_id=assignments.id")
           .where(Submission.needs_grading_conditions).distinct.select(:assignment_id))

db/migrate/20180312165128_add_user_observer_foreign_key_again.rb

@@ -8,7 +8,7 @@ class AddUserObserverForeignKeyAgain < ActiveRecord::Migration[5.0]
     unless foreign_key_exists?(:user_observers, :column => :observer_id)
       UserObservationLink.where("observer_id > ?", Shard::IDS_PER_SHARD).find_in_batches do |uos|
         observer_ids = uos.map(&:observer_id).uniq
-        missing_ids = observer_ids - User.where("id IN (?)", observer_ids).pluck(:id)
+        missing_ids = observer_ids - User.where(id: observer_ids).pluck(:id)
         if missing_ids.any?
           uos.select { |uo| missing_ids.include?(uo.observer_id) }.each do |uo|
             uo.observer.associate_with_shard(Shard.current, :shadow)

gems/acts_as_list/spec/list_spec.rb

@@ -154,7 +154,7 @@ describe "ListTest" do
     it 'nil scope' do
       new1, new2, new3 = UnscopedListMixin.create, UnscopedListMixin.create, UnscopedListMixin.create
       new2.move_to_top
-      expect(UnscopedListMixin.where('parent_id IS NULL').order('pos').to_a).to eq [new2, new1, new3]
+      expect(UnscopedListMixin.where(parent_id: nil).order('pos').to_a).to eq [new2, new1, new3]
     end
 
     it 'remove_from_list should then fail in_list?' do

gems/plugins/account_reports/lib/account_reports/student_reports.rb

@@ -198,7 +198,7 @@ module AccountReports
             )
         )}, start_at)
       else
-        data = data.where("enrollments.last_activity_at IS NULL")
+        data = data.where(enrollments: { last_activity_at: nil })
         data = data.where(%{NOT EXISTS (
           SELECT 1 AS ONE
           FROM #{Enrollment.quoted_table_name} AS other_ens

lib/api/v1/stream_item.rb

@@ -181,7 +181,7 @@ module Api::V1::StreamItem
         if full_counts.keys.any? { |k| k[0] == 'DiscussionTopic' }
           ann_counts = base_scope.where(:stream_items => { :asset_type => "DiscussionTopic" })
                                  .joins("INNER JOIN #{DiscussionTopic.quoted_table_name} ON discussion_topics.id=stream_items.asset_id")
-                                 .where("discussion_topics.type = ?", "Announcement").except(:order).group('stream_item_instances.workflow_state').count
+                                 .where(discussion_topics: { type: "Announcement" }).except(:order).group('stream_item_instances.workflow_state').count
 
           ann_counts.each do |wf_state, ann_count|
             full_counts[['Announcement', nil, wf_state]] = ann_count
@@ -241,7 +241,7 @@ module Api::V1::StreamItem
       if total_counts.keys.any? { |k| k[0] == 'DiscussionTopic' }
         ann_scope = StreamItem.where(:stream_items => { :asset_type => "DiscussionTopic" })
                               .joins(:discussion_topic)
-                              .where("discussion_topics.type = ?", "Announcement")
+                              .where(discussion_topics: { type: "Announcement" })
         ann_total = ann_scope.where(:id => stream_item_ids).count
 
         if ann_total > 0

lib/data_fixup/backfill_moderation_graders.rb

@@ -27,7 +27,7 @@ module DataFixup::BackfillModerationGraders
 
     # Find all provisional graders for this batch of assignments
     graders = ModeratedGrading::ProvisionalGrade.joins(:submission)
-                                                .where("submissions.assignment_id IN (?)", assignments.select(:id))
+                                                .where(submissions: { assignment_id: assignments.select(:id) })
                                                 .pluck(Arel.sql("distinct submissions.assignment_id, moderated_grading_provisional_grades.scorer_id"))
     created_at = Time.zone.now
 

lib/user_merge.rb

@@ -477,7 +477,7 @@ class UserMerge
     # find all the modules progressions and delete the most restrictive
     # context_module_progressions
     ContextModuleProgression
-      .where("context_module_progressions.user_id = ?", from_user.id)
+      .where(context_module_progressions: { user_id: from_user.id })
       .where("EXISTS (SELECT *
                      FROM #{ContextModuleProgression.quoted_table_name} cmp2
                      WHERE context_module_progressions.context_module_id=cmp2.context_module_id

lib/user_search.rb

@@ -125,7 +125,7 @@ module UserSearch
                     end
                   end
                 end
-        users_scope = users_scope.where("role_id IN (?)", roles.map(&:id))
+        users_scope = users_scope.where(enrollments: { role_id: roles.map(&:id) })
       elsif enrollment_types
         enrollment_types = enrollment_types.map { |e| "#{e.camelize}Enrollment" }
         if enrollment_types.any? { |et| !Enrollment.readable_types.key?(et) }

spec/models/enrollment/query_builder_spec.rb

@@ -77,7 +77,7 @@ describe "Enrollment::QueryBuilder" do
     def enrollments(course_workflow_state = nil)
       scope = user.enrollments.joins(:course)
       if course_workflow_state
-        scope = scope.where("courses.workflow_state = ?", course_workflow_state)
+        scope = scope.where(courses: { workflow_state: course_workflow_state })
       end
       scope
     end