From 836b317b8a8688c093c523f58ecb4ff33251a2b1 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Thu, 13 May 2021 15:03:25 +0530 Subject: [PATCH] feat: Remove restrictions on API channel webhook_url (#2261) --- .../dashboard/settings/inbox/channels/Api.vue | 5 +-- app/listeners/webhook_listener.rb | 6 ++-- app/models/channel/api.rb | 2 +- ...e_not_null_from_webhook_url_channel_api.rb | 5 +++ db/schema.rb | 4 +-- spec/listeners/webhook_listener_spec.rb | 36 ++++++++++++++++++- 6 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20210513083044_remove_not_null_from_webhook_url_channel_api.rb diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Api.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Api.vue index 8c19892d6..2bbfcf568 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Api.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Api.vue @@ -56,7 +56,8 @@ import { required } from 'vuelidate/lib/validators'; import router from '../../../../index'; import PageHeader from '../../SettingsSubPageHeader'; -const shouldBeWebhookUrl = (value = '') => value.startsWith('http'); +const shouldBeWebhookUrl = (value = '') => + value ? value.startsWith('http') : true; export default { components: { @@ -76,7 +77,7 @@ export default { }, validations: { channelName: { required }, - webhookUrl: { required, shouldBeWebhookUrl }, + webhookUrl: { shouldBeWebhookUrl }, }, methods: { async createChannel() { diff --git a/app/listeners/webhook_listener.rb b/app/listeners/webhook_listener.rb index dd270eb32..f688d7763 100644 --- a/app/listeners/webhook_listener.rb +++ b/app/listeners/webhook_listener.rb @@ -59,7 +59,9 @@ class WebhookListener < BaseListener WebhookJob.perform_later(webhook.url, payload) end - # Deliver for API Inbox - WebhookJob.perform_later(inbox.channel.webhook_url, payload) if inbox.channel_type == 'Channel::Api' + return unless inbox.channel_type == 'Channel::Api' + return if inbox.channel.webhook_url.blank? + + WebhookJob.perform_later(inbox.channel.webhook_url, payload) end end diff --git a/app/models/channel/api.rb b/app/models/channel/api.rb index baa6015d3..672d8f383 100644 --- a/app/models/channel/api.rb +++ b/app/models/channel/api.rb @@ -3,7 +3,7 @@ # Table name: channel_api # # id :bigint not null, primary key -# webhook_url :string not null +# webhook_url :string # created_at :datetime not null # updated_at :datetime not null # account_id :integer not null diff --git a/db/migrate/20210513083044_remove_not_null_from_webhook_url_channel_api.rb b/db/migrate/20210513083044_remove_not_null_from_webhook_url_channel_api.rb new file mode 100644 index 000000000..790c52ec5 --- /dev/null +++ b/db/migrate/20210513083044_remove_not_null_from_webhook_url_channel_api.rb @@ -0,0 +1,5 @@ +class RemoveNotNullFromWebhookUrlChannelApi < ActiveRecord::Migration[6.0] + def change + change_column :channel_api, :webhook_url, :string, null: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 415559101..60f080d5c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_30_100138) do +ActiveRecord::Schema.define(version: 2021_05_13_083044) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -139,7 +139,7 @@ ActiveRecord::Schema.define(version: 2021_04_30_100138) do create_table "channel_api", force: :cascade do |t| t.integer "account_id", null: false - t.string "webhook_url", null: false + t.string "webhook_url" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false end diff --git a/spec/listeners/webhook_listener_spec.rb b/spec/listeners/webhook_listener_spec.rb index ed581dc6f..cb06dd6c0 100644 --- a/spec/listeners/webhook_listener_spec.rb +++ b/spec/listeners/webhook_listener_spec.rb @@ -13,7 +13,7 @@ describe WebhookListener do let!(:event) { Events::Base.new(event_name, Time.zone.now, message: message) } describe '#message_created' do - let(:event_name) { :'conversation.created' } + let(:event_name) { :'message.created' } context 'when webhook is not configured' do it 'does not trigger webhook' do @@ -29,5 +29,39 @@ describe WebhookListener do listener.message_created(event) end end + + context 'when inbox is an API Channel' do + it 'triggers webhook if webhook_url is present' do + channel_api = create(:channel_api, account: account) + api_inbox = channel_api.inbox + api_conversation = create(:conversation, account: account, inbox: api_inbox, assignee: user) + api_message = create( + :message, + message_type: 'outgoing', + account: account, + inbox: api_inbox, + conversation: api_conversation + ) + api_event = Events::Base.new(event_name, Time.zone.now, message: api_message) + expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url, api_message.webhook_data.merge(event: 'message_created')).once + listener.message_created(api_event) + end + + it 'does not trigger webhook if webhook_url is not present' do + channel_api = create(:channel_api, webhook_url: nil, account: account) + api_inbox = channel_api.inbox + api_conversation = create(:conversation, account: account, inbox: api_inbox, assignee: user) + api_message = create( + :message, + message_type: 'outgoing', + account: account, + inbox: channel_api.inbox, + conversation: api_conversation + ) + api_event = Events::Base.new(event_name, Time.zone.now, message: api_message) + expect(WebhookJob).not_to receive(:perform_later) + listener.message_created(api_event) + end + end end end