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/controllers/public/api/v1/inboxes/conversations_controller.rb b/app/controllers/public/api/v1/inboxes/conversations_controller.rb index 671863bdc..4e3b5dca9 100644 --- a/app/controllers/public/api/v1/inboxes/conversations_controller.rb +++ b/app/controllers/public/api/v1/inboxes/conversations_controller.rb @@ -1,15 +1,31 @@ class Public::Api::V1::Inboxes::ConversationsController < Public::Api::V1::InboxesController include Events::Types - before_action :set_conversation, only: [:toggle_typing, :update_last_seen] + before_action :set_conversation, only: [:toggle_typing, :update_last_seen, :show, :toggle_status] def index @conversations = @contact_inbox.hmac_verified? ? @contact.conversations : @contact_inbox.conversations end + def show; end + def create @conversation = create_conversation end + def toggle_status + # Check if the conversation is already resolved to prevent redundant operations + return if @conversation.resolved? + + # Assign the conversation's contact as the resolver + # This step attributes the resolution action to the contact involved in the conversation + # If this assignment is not made, the system implicitly becomes the resolver by default + Current.contact = @conversation.contact + + # Update the conversation's status to 'resolved' to reflect its closure + @conversation.status = :resolved + @conversation.save! + end + def toggle_typing case params[:typing_status] when 'on' @@ -30,7 +46,11 @@ class Public::Api::V1::Inboxes::ConversationsController < Public::Api::V1::Inbox private def set_conversation - @conversation = @contact_inbox.contact.conversations.find_by!(display_id: params[:id]) + @conversation = if @contact_inbox.hmac_verified? + @contact_inbox.contact.conversations.find_by!(display_id: params[:id]) + else + @contact_inbox.conversations.find_by!(display_id: params[:id]) + end end def create_conversation 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/app/views/public/api/v1/inboxes/conversations/show.json.jbuilder b/app/views/public/api/v1/inboxes/conversations/show.json.jbuilder new file mode 100644 index 000000000..1b9588465 --- /dev/null +++ b/app/views/public/api/v1/inboxes/conversations/show.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'public/api/v1/models/conversation', formats: [:json], resource: @conversation diff --git a/app/views/public/api/v1/inboxes/conversations/toggle_status.json.jbuilder b/app/views/public/api/v1/inboxes/conversations/toggle_status.json.jbuilder new file mode 100644 index 000000000..1b9588465 --- /dev/null +++ b/app/views/public/api/v1/inboxes/conversations/toggle_status.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'public/api/v1/models/conversation', formats: [:json], resource: @conversation diff --git a/config/routes.rb b/config/routes.rb index 8c4d4124b..1ee74145f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -366,8 +366,9 @@ Rails.application.routes.draw do resources :inboxes do scope module: :inboxes do resources :contacts, only: [:create, :show, :update] do - resources :conversations, only: [:index, :create] do + resources :conversations, only: [:index, :create, :show] do member do + post :toggle_status post :toggle_typing post :update_last_seen end 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/controllers/public/api/v1/inbox/conversations_controller_spec.rb b/spec/controllers/public/api/v1/inbox/conversations_controller_spec.rb index d54f26272..2b2bdefc3 100644 --- a/spec/controllers/public/api/v1/inbox/conversations_controller_spec.rb +++ b/spec/controllers/public/api/v1/inbox/conversations_controller_spec.rb @@ -36,6 +36,45 @@ RSpec.describe 'Public Inbox Contact Conversations API', type: :request do end end + describe 'GET /public/api/v1/inboxes/{identifier}/contact/{source_id}/conversations/{conversation_id}' do + it 'returns the conversation that the contact has access to' do + conversation = create(:conversation, contact_inbox: contact_inbox) + create(:message, account: conversation.account, inbox: conversation.inbox, conversation: conversation, content: 'message-1') + create(:message, account: conversation.account, inbox: conversation.inbox, conversation: conversation, content: 'message-2') + + get "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{contact_inbox.source_id}/conversations/#{conversation.display_id}" + + expect(response).to have_http_status(:success) + data = response.parsed_body + expect(data['id']).to eq(conversation.display_id) + expect(data['messages']).to be_a(Array) + expect(data['messages'].length).to eq(conversation.messages.count) + expect(data['messages'].pluck('content')).to include(conversation.messages.first.content) + end + end + + describe 'POST /public/api/v1/inboxes/{identifier}/contact/{source_id}/conversations/{conversation_id}/toggle_status' do + it 'resolves the conversation' do + conversation = create(:conversation, contact_inbox: contact_inbox) + display_id = conversation.display_id + + post "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{contact_inbox.source_id}/conversations/#{display_id}/toggle_status" + + expect(response).to have_http_status(:success) + expect(conversation.reload).to be_resolved + end + + it 'does not resolve a conversation that is already resolved' do + conversation = create(:conversation, contact_inbox: contact_inbox, status: :resolved) + display_id = conversation.display_id + + post "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{contact_inbox.source_id}/conversations/#{display_id}/toggle_status" + + expect(response).to have_http_status(:success) + expect(Conversation.where(id: conversation.id, status: :resolved).count).to eq(1) + end + end + describe 'POST /public/api/v1/inboxes/{identifier}/contact/{source_id}/conversations' do it 'creates a conversation for that contact' do post "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{contact_inbox.source_id}/conversations" 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) diff --git a/swagger/paths/index.yml b/swagger/paths/index.yml index d58439647..2c6dd3f87 100644 --- a/swagger/paths/index.yml +++ b/swagger/paths/index.yml @@ -94,7 +94,6 @@ patch: $ref: ./public/inboxes/contacts/update.yml - /public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations: parameters: - $ref: '#/parameters/public_inbox_identifier' @@ -104,6 +103,38 @@ get: $ref: ./public/inboxes/conversations/index.yml +/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}: + parameters: + - $ref: '#/parameters/public_inbox_identifier' + - $ref: '#/parameters/public_contact_identifier' + - $ref: '#/parameters/conversation_id' + get: + $ref: ./public/inboxes/conversations/show.yml + +/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/toggle_status: + parameters: + - $ref: '#/parameters/public_inbox_identifier' + - $ref: '#/parameters/public_contact_identifier' + - $ref: '#/parameters/conversation_id' + post: + $ref: ./public/inboxes/conversations/toggle_status.yml + +/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/toggle_typing: + parameters: + - $ref: '#/parameters/public_inbox_identifier' + - $ref: '#/parameters/public_contact_identifier' + - $ref: '#/parameters/conversation_id' + post: + $ref: ./public/inboxes/conversations/toggle_typing.yml + +/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/update_last_seen: + parameters: + - $ref: '#/parameters/public_inbox_identifier' + - $ref: '#/parameters/public_contact_identifier' + - $ref: '#/parameters/conversation_id' + post: + $ref: ./public/inboxes/conversations/update_last_seen.yml + /public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/messages: parameters: - $ref: '#/parameters/public_inbox_identifier' diff --git a/swagger/paths/public/inboxes/conversations/show.yml b/swagger/paths/public/inboxes/conversations/show.yml new file mode 100644 index 000000000..2ec6caa63 --- /dev/null +++ b/swagger/paths/public/inboxes/conversations/show.yml @@ -0,0 +1,14 @@ +tags: + - Conversations API +operationId: get-single-conversation +summary: Get a single conversation +description: Retrieves the details of a specific conversation +responses: + 200: + description: Success + schema: + $ref: '#/definitions/public_conversation' + 401: + description: Unauthorized + 404: + description: Conversation not found diff --git a/swagger/paths/public/inboxes/conversations/toggle_status.yml b/swagger/paths/public/inboxes/conversations/toggle_status.yml new file mode 100644 index 000000000..cac1c3d8d --- /dev/null +++ b/swagger/paths/public/inboxes/conversations/toggle_status.yml @@ -0,0 +1,14 @@ +tags: + - Conversations API +operationId: resolve-conversation +summary: Resolve a conversation +description: Marks a conversation as resolved +responses: + 200: + description: Conversation resolved successfully + schema: + $ref: '#/definitions/public_conversation' + 401: + description: Unauthorized + 404: + description: Conversation not found diff --git a/swagger/paths/public/inboxes/conversations/toggle_typing.yml b/swagger/paths/public/inboxes/conversations/toggle_typing.yml new file mode 100644 index 000000000..af01c77f6 --- /dev/null +++ b/swagger/paths/public/inboxes/conversations/toggle_typing.yml @@ -0,0 +1,18 @@ +tags: + - Conversations API +operationId: toggle-typing-status +summary: Toggle typing status +description: Toggles the typing status in a conversation +parameters: + - name: typing_status + in: query + required: true + type: string + description: Typing status, either 'on' or 'off' +responses: + 200: + description: Typing status toggled successfully + 401: + description: Unauthorized + 404: + description: Conversation not found diff --git a/swagger/paths/public/inboxes/conversations/update_last_seen.yml b/swagger/paths/public/inboxes/conversations/update_last_seen.yml new file mode 100644 index 000000000..4e56aa11d --- /dev/null +++ b/swagger/paths/public/inboxes/conversations/update_last_seen.yml @@ -0,0 +1,12 @@ +tags: + - Conversations API +operationId: update-last-seen +summary: Update last seen +description: Updates the last seen time of the contact in a conversation +responses: + 200: + description: Last seen updated successfully + 401: + description: Unauthorized + 404: + description: Conversation not found diff --git a/swagger/swagger.json b/swagger/swagger.json index 4859a785f..dde7c5f01 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -894,6 +894,149 @@ } } }, + "/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}": { + "parameters": [ + { + "$ref": "#/parameters/public_inbox_identifier" + }, + { + "$ref": "#/parameters/public_contact_identifier" + }, + { + "$ref": "#/parameters/conversation_id" + } + ], + "get": { + "tags": [ + "Conversations API" + ], + "operationId": "get-single-conversation", + "summary": "Get a single conversation", + "description": "Retrieves the details of a specific conversation", + "responses": { + "200": { + "description": "Success", + "schema": { + "$ref": "#/definitions/public_conversation" + } + }, + "401": { + "description": "Unauthorized" + }, + "404": { + "description": "Conversation not found" + } + } + } + }, + "/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/toggle_status": { + "parameters": [ + { + "$ref": "#/parameters/public_inbox_identifier" + }, + { + "$ref": "#/parameters/public_contact_identifier" + }, + { + "$ref": "#/parameters/conversation_id" + } + ], + "post": { + "tags": [ + "Conversations API" + ], + "operationId": "resolve-conversation", + "summary": "Resolve a conversation", + "description": "Marks a conversation as resolved", + "responses": { + "200": { + "description": "Conversation resolved successfully", + "schema": { + "$ref": "#/definitions/public_conversation" + } + }, + "401": { + "description": "Unauthorized" + }, + "404": { + "description": "Conversation not found" + } + } + } + }, + "/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/toggle_typing": { + "parameters": [ + { + "$ref": "#/parameters/public_inbox_identifier" + }, + { + "$ref": "#/parameters/public_contact_identifier" + }, + { + "$ref": "#/parameters/conversation_id" + } + ], + "post": { + "tags": [ + "Conversations API" + ], + "operationId": "toggle-typing-status", + "summary": "Toggle typing status", + "description": "Toggles the typing status in a conversation", + "parameters": [ + { + "name": "typing_status", + "in": "query", + "required": true, + "type": "string", + "description": "Typing status, either 'on' or 'off'" + } + ], + "responses": { + "200": { + "description": "Typing status toggled successfully" + }, + "401": { + "description": "Unauthorized" + }, + "404": { + "description": "Conversation not found" + } + } + } + }, + "/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/update_last_seen": { + "parameters": [ + { + "$ref": "#/parameters/public_inbox_identifier" + }, + { + "$ref": "#/parameters/public_contact_identifier" + }, + { + "$ref": "#/parameters/conversation_id" + } + ], + "post": { + "tags": [ + "Conversations API" + ], + "operationId": "update-last-seen", + "summary": "Update last seen", + "description": "Updates the last seen time of the contact in a conversation", + "responses": { + "200": { + "description": "Last seen updated successfully" + }, + "401": { + "description": "Unauthorized" + }, + "404": { + "description": "Conversation not found" + } + } + } + }, "/public/api/v1/inboxes/{inbox_identifier}/contacts/{contact_identifier}/conversations/{conversation_id}/messages": { "parameters": [ {