Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: Ongoing campaign URL validation (#3890)
  • Loading branch information
muhsin-k committed Feb 4, 2022
1 parent a7987d4 commit a737f89
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 13 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/accounts/campaigns_controller.rb
Expand Up @@ -18,7 +18,7 @@ def destroy
def show; end

def update
@campaign.update(campaign_params)
@campaign.update!(campaign_params)
end

private
Expand Down
17 changes: 17 additions & 0 deletions app/models/campaign.rb
Expand Up @@ -33,12 +33,14 @@
# fk_rails_... (account_id => accounts.id) ON DELETE => cascade
# fk_rails_... (inbox_id => inboxes.id) ON DELETE => cascade
#
require 'uri'
class Campaign < ApplicationRecord
validates :account_id, presence: true
validates :inbox_id, presence: true
validates :title, presence: true
validates :message, presence: true
validate :validate_campaign_inbox
validate :validate_url
validate :prevent_completed_campaign_from_update, on: :update
belongs_to :account
belongs_to :inbox
Expand Down Expand Up @@ -86,6 +88,21 @@ def ensure_correct_campaign_attributes
end
end

def validate_url
return unless trigger_rules['url']

errors.add(:url, 'invalid') if inbox.inbox_type == 'Website' && !url_valid?(trigger_rules['url'])
end

def url_valid?(url)
url = begin
URI.parse(url)
rescue StandardError
false
end
url.is_a?(URI::HTTP) || url.is_a?(URI::HTTPS)
end

def prevent_completed_campaign_from_update
errors.add :status, 'The campaign is already completed' if !campaign_status_changed? && completed?
end
Expand Down
Expand Up @@ -5,7 +5,7 @@
let(:inbox) { create(:inbox, account: account) }
let(:contact) { create(:contact, account: account, identifier: '123') }
let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: inbox) }
let(:campaign) { create(:campaign, inbox: inbox, account: account) }
let(:campaign) { create(:campaign, inbox: inbox, account: account, trigger_rules: { url: 'https://test.com' }) }

describe '#perform' do
it 'creates a conversation with campaign id and message with campaign message' do
Expand Down
27 changes: 23 additions & 4 deletions spec/controllers/api/v1/accounts/campaigns_controller_spec.rb
Expand Up @@ -15,7 +15,7 @@
context 'when it is an authenticated user' do
let(:agent) { create(:user, account: account, role: :agent) }
let(:administrator) { create(:user, account: account, role: :administrator) }
let!(:campaign) { create(:campaign, account: account) }
let!(:campaign) { create(:campaign, account: account, trigger_rules: { url: 'https://test.com' }) }

it 'returns unauthorized for agents' do
get "/api/v1/accounts/#{account.id}/campaigns",
Expand All @@ -38,7 +38,7 @@
end

describe 'GET /api/v1/accounts/{account.id}/campaigns/:id' do
let(:campaign) { create(:campaign, account: account) }
let(:campaign) { create(:campaign, account: account, trigger_rules: { url: 'https://test.com' }) }

context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
Expand Down Expand Up @@ -107,6 +107,25 @@
expect(JSON.parse(response.body, symbolize_names: true)[:title]).to eq('test')
end

it 'creates a new ongoing campaign' do
post "/api/v1/accounts/#{account.id}/campaigns",
params: { inbox_id: inbox.id, title: 'test', message: 'test message', trigger_rules: { url: 'https://test.com' } },
headers: administrator.create_new_auth_token,
as: :json

expect(response).to have_http_status(:success)
expect(JSON.parse(response.body, symbolize_names: true)[:title]).to eq('test')
end

it 'throws error when invalid url provided for ongoing campaign' do
post "/api/v1/accounts/#{account.id}/campaigns",
params: { inbox_id: inbox.id, title: 'test', message: 'test message', trigger_rules: { url: 'javascript' } },
headers: administrator.create_new_auth_token,
as: :json

expect(response).to have_http_status(:unprocessable_entity)
end

it 'creates a new oneoff campaign' do
twilio_sms = create(:channel_twilio_sms, account: account)
twilio_inbox = create(:inbox, channel: twilio_sms)
Expand All @@ -133,7 +152,7 @@

describe 'PATCH /api/v1/accounts/{account.id}/campaigns/:id' do
let(:inbox) { create(:inbox, account: account) }
let!(:campaign) { create(:campaign, account: account) }
let!(:campaign) { create(:campaign, account: account, trigger_rules: { url: 'https://test.com' }) }

context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
Expand Down Expand Up @@ -172,7 +191,7 @@

describe 'DELETE /api/v1/accounts/{account.id}/campaigns/:id' do
let(:inbox) { create(:inbox, account: account) }
let!(:campaign) { create(:campaign, account: account) }
let!(:campaign) { create(:campaign, account: account, trigger_rules: { url: 'https://test.com' }) }

context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/api/v1/accounts/inboxes_controller_spec.rb
Expand Up @@ -133,7 +133,7 @@
let(:agent) { create(:user, account: account, role: :agent) }
let(:administrator) { create(:user, account: account, role: :administrator) }

let!(:campaign) { create(:campaign, account: account, inbox: inbox) }
let!(:campaign) { create(:campaign, account: account, inbox: inbox, trigger_rules: { url: 'https://test.com' }) }

it 'returns unauthorized for agents' do
get "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}/campaigns",
Expand All @@ -145,7 +145,7 @@

it 'returns all campaigns belonging to the inbox to administrators' do
# create a random campaign
create(:campaign, account: account)
create(:campaign, account: account, trigger_rules: { url: 'https://test.com' })
get "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}/campaigns",
headers: administrator.create_new_auth_token,
as: :json
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/api/v1/widget/campaigns_controller_spec.rb
Expand Up @@ -3,8 +3,8 @@
RSpec.describe '/api/v1/widget/campaigns', type: :request do
let(:account) { create(:account) }
let(:web_widget) { create(:channel_widget, account: account) }
let!(:campaign_1) { create(:campaign, inbox: web_widget.inbox, enabled: true, account: account) }
let!(:campaign_2) { create(:campaign, inbox: web_widget.inbox, enabled: false, account: account) }
let!(:campaign_1) { create(:campaign, inbox: web_widget.inbox, enabled: true, account: account, trigger_rules: { url: 'https://test.com' }) }
let!(:campaign_2) { create(:campaign, inbox: web_widget.inbox, enabled: false, account: account, trigger_rules: { url: 'https://test.com' }) }

describe 'GET /api/v1/widget/campaigns' do
let(:params) { { website_token: web_widget.website_token } }
Expand Down
2 changes: 1 addition & 1 deletion spec/listeners/campaign_listener_spec.rb
Expand Up @@ -5,7 +5,7 @@
let(:inbox) { create(:inbox, account: account) }
let(:contact) { create(:contact, account: account, identifier: '123') }
let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: inbox) }
let(:campaign) { create(:campaign, inbox: inbox, account: account) }
let(:campaign) { create(:campaign, inbox: inbox, account: account, trigger_rules: { url: 'https://test.com' }) }

let!(:event) do
Events::Base.new('campaign_triggered', Time.zone.now,
Expand Down
9 changes: 7 additions & 2 deletions spec/models/campaign_spec.rb
Expand Up @@ -9,7 +9,10 @@
end

describe '.before_create' do
let(:campaign) { build(:campaign, display_id: nil) }
let(:account) { create(:account) }
let(:website_channel) { create(:channel_widget, account: account) }
let(:website_inbox) { create(:inbox, channel: website_channel, account: account) }
let(:campaign) { build(:campaign, inbox: website_inbox, display_id: nil, trigger_rules: { url: 'https://test.com' }) }

before do
campaign.save
Expand Down Expand Up @@ -37,7 +40,9 @@
end

context 'when a campaign is completed' do
let!(:campaign) { create(:campaign, campaign_status: :completed) }
let(:account) { create(:account) }
let(:web_widget) { create(:channel_widget, account: account) }
let!(:campaign) { create(:campaign, inbox: web_widget.inbox, campaign_status: :completed, trigger_rules: { url: 'https://test.com' }) }

it 'would prevent further updates' do
campaign.title = 'new name'
Expand Down

0 comments on commit a737f89

Please sign in to comment.