diff --git a/.rubocop.yml b/.rubocop.yml index 5add6e26a..f4187dbb5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,7 @@ require: - rubocop-performance - rubocop-rails - rubocop-rspec + - ./lib/rubocop/user_find_by.rb Layout/LineLength: Max: 150 @@ -140,6 +141,10 @@ RSpec/MultipleExpectations: RSpec/MultipleMemoizedHelpers: Max: 14 +# custom rules +UseFromEmail: + Enabled: true + AllCops: NewCops: enable Exclude: diff --git a/app/actions/contact_identify_action.rb b/app/actions/contact_identify_action.rb index 6b2f37433..a88d3535b 100644 --- a/app/actions/contact_identify_action.rb +++ b/app/actions/contact_identify_action.rb @@ -59,7 +59,7 @@ class ContactIdentifyAction def existing_email_contact return if params[:email].blank? - @existing_email_contact ||= account.contacts.find_by(email: params[:email]) + @existing_email_contact ||= account.contacts.from_email(params[:email]) end def existing_phone_number_contact diff --git a/app/builders/agent_builder.rb b/app/builders/agent_builder.rb index 6ea68821d..07b0e7345 100644 --- a/app/builders/agent_builder.rb +++ b/app/builders/agent_builder.rb @@ -27,7 +27,7 @@ class AgentBuilder # Finds a user by email or creates a new one with a temporary password. # @return [User] the found or created user. def find_or_create_user - user = User.find_by(email: email) + user = User.from_email(email) return user if user temp_password = "1!aA#{SecureRandom.alphanumeric(12)}" diff --git a/app/builders/contact_inbox_with_contact_builder.rb b/app/builders/contact_inbox_with_contact_builder.rb index d97f64cfe..6e913eda8 100644 --- a/app/builders/contact_inbox_with_contact_builder.rb +++ b/app/builders/contact_inbox_with_contact_builder.rb @@ -75,7 +75,7 @@ class ContactInboxWithContactBuilder def find_contact_by_email(email) return if email.blank? - account.contacts.find_by(email: email.downcase) + account.contacts.from_email(email) end def find_contact_by_phone_number(phone_number) diff --git a/app/controllers/devise_overrides/passwords_controller.rb b/app/controllers/devise_overrides/passwords_controller.rb index 26a9d4555..17dd32086 100644 --- a/app/controllers/devise_overrides/passwords_controller.rb +++ b/app/controllers/devise_overrides/passwords_controller.rb @@ -5,7 +5,7 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController skip_before_action :authenticate_user!, raise: false def create - @user = User.find_by(email: params[:email]) + @user = User.from_email(params[:email]) if @user @user.send_reset_password_instructions build_response(I18n.t('messages.reset_password_success'), 200) diff --git a/app/controllers/devise_overrides/sessions_controller.rb b/app/controllers/devise_overrides/sessions_controller.rb index 2659aeebf..e623e52f7 100644 --- a/app/controllers/devise_overrides/sessions_controller.rb +++ b/app/controllers/devise_overrides/sessions_controller.rb @@ -33,7 +33,7 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController def process_sso_auth_token return if params[:email].blank? - user = User.find_by(email: params[:email]) + user = User.from_email(params[:email]) @resource = user if user&.valid_sso_auth_token?(params[:sso_auth_token]) end end diff --git a/app/controllers/platform/api/v1/users_controller.rb b/app/controllers/platform/api/v1/users_controller.rb index 03ed55754..453e475b0 100644 --- a/app/controllers/platform/api/v1/users_controller.rb +++ b/app/controllers/platform/api/v1/users_controller.rb @@ -8,7 +8,7 @@ class Platform::Api::V1::UsersController < PlatformController def show; end def create - @resource = (User.find_by(email: user_params[:email]) || User.new(user_params)) + @resource = (User.from_email(user_params[:email]) || User.new(user_params)) @resource.skip_confirmation! @resource.save! @platform_app.platform_app_permissibles.find_or_create_by!(permissible: @resource) diff --git a/app/mailboxes/imap/imap_mailbox.rb b/app/mailboxes/imap/imap_mailbox.rb index 57d30253a..a8308b48a 100644 --- a/app/mailboxes/imap/imap_mailbox.rb +++ b/app/mailboxes/imap/imap_mailbox.rb @@ -91,7 +91,7 @@ class Imap::ImapMailbox end def find_or_create_contact - @contact = @inbox.contacts.find_by(email: @processed_mail.original_sender) + @contact = @inbox.contacts.from_email(@processed_mail.original_sender) if @contact.present? @contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact) else diff --git a/app/mailboxes/support_mailbox.rb b/app/mailboxes/support_mailbox.rb index 5a1f5ecf5..94404d8ed 100644 --- a/app/mailboxes/support_mailbox.rb +++ b/app/mailboxes/support_mailbox.rb @@ -86,7 +86,7 @@ class SupportMailbox < ApplicationMailbox end def find_or_create_contact - @contact = @inbox.contacts.find_by(email: original_sender_email) + @contact = @inbox.contacts.from_email(original_sender_email) if @contact.present? @contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact) else diff --git a/app/models/contact.rb b/app/models/contact.rb index af3f605a7..23eca55e6 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -165,6 +165,12 @@ class Contact < ApplicationRecord email_format end + def self.from_email(email) + # rubocop:disable UseFromEmail,Migration/DepartmentName + find_by(email: email.downcase) + # rubocop:enable UseFromEmail,Migration/DepartmentName + end + private def ip_lookup diff --git a/app/models/user.rb b/app/models/user.rb index ba638f637..b54f74a64 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -156,6 +156,12 @@ class User < ApplicationRecord mutations_from_database.changed?('email') end + def self.from_email(email) + # rubocop:disable UseFromEmail,Migration/DepartmentName + find_by(email: email.downcase) + # rubocop:enable UseFromEmail,Migration/DepartmentName + end + private def remove_macros diff --git a/app/services/data_import/contact_manager.rb b/app/services/data_import/contact_manager.rb index ab9a1ee70..460a83726 100644 --- a/app/services/data_import/contact_manager.rb +++ b/app/services/data_import/contact_manager.rb @@ -35,7 +35,7 @@ class DataImport::ContactManager def find_contact_by_email(params) return unless params[:email] - @account.contacts.find_by(email: params[:email]) + @account.contacts.from_email(params[:email]) end def find_contact_by_phone_number(params) diff --git a/lib/integrations/slack/slack_message_helper.rb b/lib/integrations/slack/slack_message_helper.rb index 9522112a8..762d488c5 100644 --- a/lib/integrations/slack/slack_message_helper.rb +++ b/lib/integrations/slack/slack_message_helper.rb @@ -69,7 +69,7 @@ module Integrations::Slack::SlackMessageHelper def sender user_email = slack_client.users_info(user: params[:event][:user])[:user][:profile][:email] - conversation.account.users.find_by(email: user_email) + conversation.account.users.from_email(user_email) end def private_note? diff --git a/lib/rubocop/user_find_by.rb b/lib/rubocop/user_find_by.rb new file mode 100644 index 000000000..0bc248c27 --- /dev/null +++ b/lib/rubocop/user_find_by.rb @@ -0,0 +1,16 @@ +require 'rubocop' + +# Enforces use of from_email for email attribute lookups +class UseFromEmail < RuboCop::Cop::Cop + MSG = 'Use `from_email` for email lookups to ensure case insensitivity.'.freeze + + def_node_matcher :find_by_email?, <<~PATTERN + (send _ :find_by (hash (pair (sym :email) _))) + PATTERN + + def on_send(node) + return unless find_by_email?(node) + + add_offense(node, message: MSG) + end +end diff --git a/lib/seeders/account_seeder.rb b/lib/seeders/account_seeder.rb index 9793ffd1d..802a02dbf 100644 --- a/lib/seeders/account_seeder.rb +++ b/lib/seeders/account_seeder.rb @@ -88,7 +88,7 @@ class Seeders::AccountSeeder end def create_conversation(contact_inbox:, conversation_data:) - assignee = User.find_by(email: conversation_data['assignee']) if conversation_data['assignee'].present? + assignee = User.from_email(conversation_data['assignee']) if conversation_data['assignee'].present? conversation = contact_inbox.conversations.create!(account: contact_inbox.inbox.account, contact: contact_inbox.contact, inbox: contact_inbox.inbox, assignee: assignee) create_messages(conversation: conversation, messages: conversation_data['messages']) @@ -111,7 +111,7 @@ class Seeders::AccountSeeder if message_data['message_type'] == 'incoming' conversation.contact elsif message_data['sender'].present? - User.find_by(email: message_data['sender']) + User.from_email(message_data['sender']) end end diff --git a/spec/controllers/platform/api/v1/users_controller_spec.rb b/spec/controllers/platform/api/v1/users_controller_spec.rb index 2ebb0d1a4..95d6cd452 100644 --- a/spec/controllers/platform/api/v1/users_controller_spec.rb +++ b/spec/controllers/platform/api/v1/users_controller_spec.rb @@ -115,11 +115,6 @@ RSpec.describe 'Platform Users API', type: :request do ) ) expect(platform_app.platform_app_permissibles.first.permissible_id).to eq data['id'] - - post '/platform/api/v1/users/', params: { name: 'test', email: 'TesT@test.com', password: 'Password1!' }, - headers: { api_access_token: platform_app.access_token.token }, as: :json - data = response.parsed_body - expect(data['message']).to eq('Email has already been taken') end it 'fetch existing user and creates permissible for the user' do diff --git a/spec/jobs/data_import_job_spec.rb b/spec/jobs/data_import_job_spec.rb index 5ece07e53..fb1057f4a 100644 --- a/spec/jobs/data_import_job_spec.rb +++ b/spec/jobs/data_import_job_spec.rb @@ -111,7 +111,7 @@ RSpec.describe DataImportJob do described_class.perform_now(existing_data_import) expect(existing_data_import.account.contacts.count).to eq(csv_length) - contact = Contact.find_by(email: csv_data[0]['email']) + contact = Contact.from_email(csv_data[0]['email']) expect(contact).to be_present expect(contact.phone_number).to eq("+#{csv_data[0]['phone_number']}") expect(contact.name).to eq((csv_data[0]['name']).to_s)