From 00dc65ef7319eeb6f0e85aa4238510a3918d8378 Mon Sep 17 00:00:00 2001 From: Tejaswini Chile Date: Mon, 18 Jul 2022 20:02:37 +0530 Subject: [PATCH] fix: Twilio validation over blank messaging_service_sid (#5055) --- .../channels/twilio_channels_controller.rb | 4 +- app/models/channel/twilio_sms.rb | 4 +- .../twilio_channels_controller_spec.rb | 52 ++++++++++++++++++- spec/models/channel/twilio_sms_spec.rb | 28 ++++++++++ 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb b/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb index 24a8ba7eb..55a9456cf 100644 --- a/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb +++ b/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb @@ -27,6 +27,8 @@ class Api::V1::Accounts::Channels::TwilioChannelsController < Api::V1::Accounts: end def phone_number + return if permitted_params[:phone_number].blank? + medium == 'sms' ? permitted_params[:phone_number] : "whatsapp:#{permitted_params[:phone_number]}" end @@ -38,7 +40,7 @@ class Api::V1::Accounts::Channels::TwilioChannelsController < Api::V1::Accounts: @twilio_channel = Current.account.twilio_sms.create!( account_sid: permitted_params[:account_sid], auth_token: permitted_params[:auth_token], - messaging_service_sid: permitted_params[:messaging_service_sid], + messaging_service_sid: permitted_params[:messaging_service_sid].presence, phone_number: phone_number, medium: medium ) diff --git a/app/models/channel/twilio_sms.rb b/app/models/channel/twilio_sms.rb index d6c9177fb..f575b75a0 100644 --- a/app/models/channel/twilio_sms.rb +++ b/app/models/channel/twilio_sms.rb @@ -28,8 +28,8 @@ class Channel::TwilioSms < ApplicationRecord validates :auth_token, presence: true # Must have _one_ of messaging_service_sid _or_ phone_number, and messaging_service_sid is preferred - validates :messaging_service_sid, uniqueness: true, presence: true, unless: :phone_number? - validates :phone_number, absence: true, if: :messaging_service_sid? + validates :messaging_service_sid, uniqueness: true, presence: true, unless: :phone_number.presence + validates :phone_number, absence: true, if: :messaging_service_sid.presence validates :phone_number, uniqueness: true, allow_nil: true enum medium: { sms: 0, whatsapp: 1 } diff --git a/spec/controllers/api/v1/accounts/channels/twilio_channels_controller_spec.rb b/spec/controllers/api/v1/accounts/channels/twilio_channels_controller_spec.rb index b8e41b9d1..7d8ca4c9c 100644 --- a/spec/controllers/api/v1/accounts/channels/twilio_channels_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/channels/twilio_channels_controller_spec.rb @@ -20,6 +20,7 @@ RSpec.describe '/api/v1/accounts/{account.id}/channels/twilio_channel', type: :r twilio_channel: { account_sid: 'sid', auth_token: 'token', + phone_number: '', messaging_service_sid: 'MGec8130512b5dd462cfe03095ec1342ed', name: 'SMS Channel', medium: 'sms' @@ -51,6 +52,30 @@ RSpec.describe '/api/v1/accounts/{account.id}/channels/twilio_channel', type: :r expect(json_response['messaging_service_sid']).to eq('MGec8130512b5dd462cfe03095ec1342ed') end + it 'creates inbox with blank phone number and returns inbox object' do + params = { + twilio_channel: { + account_sid: 'sid-1', + auth_token: 'token-1', + phone_number: '', + messaging_service_sid: 'MGec8130512b5dd462cfe03095ec1111ed', + name: 'SMS Channel', + medium: 'whatsapp' + } + } + allow(twilio_client).to receive(:messages).and_return(message_double) + allow(message_double).to receive(:list).and_return([]) + + post api_v1_account_channels_twilio_channel_path(account), + params: params, + headers: admin.create_new_auth_token + + expect(response).to have_http_status(:success) + json_response = JSON.parse(response.body) + + expect(json_response['messaging_service_sid']).to eq('MGec8130512b5dd462cfe03095ec1111ed') + end + context 'with a phone number' do # rubocop:disable RSpec/NestedGroups let(:params) do { @@ -58,13 +83,14 @@ RSpec.describe '/api/v1/accounts/{account.id}/channels/twilio_channel', type: :r account_sid: 'sid', auth_token: 'token', phone_number: '+1234567890', + messaging_service_sid: '', name: 'SMS Channel', medium: 'sms' } } end - it 'creates inbox and returns inbox object' do + it 'creates inbox with empty messaging service sid and returns inbox object' do allow(twilio_client).to receive(:messages).and_return(message_double) allow(message_double).to receive(:list).and_return([]) @@ -78,6 +104,30 @@ RSpec.describe '/api/v1/accounts/{account.id}/channels/twilio_channel', type: :r expect(json_response['name']).to eq('SMS Channel') expect(json_response['phone_number']).to eq('+1234567890') end + + it 'creates one more inbox with empty messaging service sid' do + params = { + twilio_channel: { + account_sid: 'sid-1', + auth_token: 'token-1', + phone_number: '+1224466880', + messaging_service_sid: '', + name: 'SMS Channel', + medium: 'whatsapp' + } + } + allow(twilio_client).to receive(:messages).and_return(message_double) + allow(message_double).to receive(:list).and_return([]) + + post api_v1_account_channels_twilio_channel_path(account), + params: params, + headers: admin.create_new_auth_token + + expect(response).to have_http_status(:success) + json_response = JSON.parse(response.body) + + expect(json_response['phone_number']).to eq('whatsapp:+1224466880') + end end it 'return error if Twilio tokens are incorrect' do diff --git a/spec/models/channel/twilio_sms_spec.rb b/spec/models/channel/twilio_sms_spec.rb index e77584935..ddaec7025 100644 --- a/spec/models/channel/twilio_sms_spec.rb +++ b/spec/models/channel/twilio_sms_spec.rb @@ -25,6 +25,34 @@ RSpec.describe Channel::TwilioSms do end end + describe '#validations' do + context 'with phone number blank' do + let!(:sms_channel) { create(:channel_twilio_sms, medium: :sms, phone_number: nil) } + + it 'allows channel to create with blank phone number' do + sms_channel_1 = create(:channel_twilio_sms, medium: :sms, phone_number: '') + + expect(sms_channel_1).to be_valid + expect(sms_channel_1.messaging_service_sid).to be_present + expect(sms_channel_1.phone_number).to be_blank + expect(sms_channel.phone_number).to be_nil + + sms_channel_1 = create(:channel_twilio_sms, medium: :sms, phone_number: nil) + expect(sms_channel_1.phone_number).to be_blank + expect(sms_channel_1.messaging_service_sid).to be_present + end + + it 'throws error for whatsapp channel' do + whatsapp_channel_1 = create(:channel_twilio_sms, medium: :sms, phone_number: '', messaging_service_sid: 'MGec8130512b5dd462cfe03095ec1111ed') + expect do + create(:channel_twilio_sms, medium: :whatsapp, phone_number: 'whatsapp', messaging_service_sid: 'MGec8130512b5dd462cfe03095ec1111ed') + end.to raise_error(ActiveRecord::RecordInvalid) { |error| expect(error.message).to eq 'Validation failed: Phone number must be blank' } + + expect(whatsapp_channel_1).to be_valid + end + end + end + describe '#send_message' do let(:channel) { create(:channel_twilio_sms) }