diff --git a/app/models/campaign.rb b/app/models/campaign.rb index 1103e8fb2..f937b1d71 100644 --- a/app/models/campaign.rb +++ b/app/models/campaign.rb @@ -38,6 +38,7 @@ class Campaign < ApplicationRecord validate :validate_url validate :prevent_completed_campaign_from_update, on: :update validate :sender_must_belong_to_account + validate :inbox_must_belong_to_account belongs_to :account belongs_to :inbox @@ -92,6 +93,14 @@ class Campaign < ApplicationRecord errors.add(:url, 'invalid') if inbox.inbox_type == 'Website' && !use_http_protocol end + def inbox_must_belong_to_account + return unless inbox + + return if inbox.account_id == account_id + + errors.add(:inbox_id, 'must belong to the same account as the campaign') + end + def sender_must_belong_to_account return unless sender diff --git a/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb b/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb index 1d9908652..f33a66d0a 100644 --- a/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb @@ -15,7 +15,8 @@ RSpec.describe 'Campaigns API', type: :request do 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, trigger_rules: { url: 'https://test.com' }) } + let(:inbox) { create(:inbox, account: account) } + 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}/campaigns", @@ -128,7 +129,7 @@ RSpec.describe 'Campaigns API', type: :request do it 'creates a new oneoff campaign' do twilio_sms = create(:channel_twilio_sms, account: account) - twilio_inbox = create(:inbox, channel: twilio_sms) + twilio_inbox = create(:inbox, channel: twilio_sms, account: account) label1 = create(:label, account: account) label2 = create(:label, account: account) scheduled_at = 2.days.from_now diff --git a/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb b/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb index dc8823efc..ab89f2bb5 100644 --- a/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb +++ b/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb @@ -2,12 +2,14 @@ require 'rails_helper' RSpec.describe Campaigns::TriggerOneoffCampaignJob do let(:account) { create(:account) } - let!(:twilio_sms) { create(:channel_twilio_sms) } - let!(:twilio_inbox) { create(:inbox, channel: twilio_sms) } + let!(:twilio_sms) { create(:channel_twilio_sms, account: account) } + let!(:twilio_inbox) { create(:inbox, channel: twilio_sms, account: account) } let(:label1) { create(:label, account: account) } let(:label2) { create(:label, account: account) } - let!(:campaign) { create(:campaign, inbox: twilio_inbox, audience: [{ type: 'Label', id: label1.id }, { type: 'Label', id: label2.id }]) } + let!(:campaign) do + create(:campaign, inbox: twilio_inbox, account: account, audience: [{ type: 'Label', id: label1.id }, { type: 'Label', id: label2.id }]) + end it 'enqueues the job' do expect { described_class.perform_later(campaign) }.to have_enqueued_job(described_class) diff --git a/spec/jobs/trigger_scheduled_items_job_spec.rb b/spec/jobs/trigger_scheduled_items_job_spec.rb index ec1f72015..4409e7ee0 100644 --- a/spec/jobs/trigger_scheduled_items_job_spec.rb +++ b/spec/jobs/trigger_scheduled_items_job_spec.rb @@ -36,12 +36,12 @@ RSpec.describe TriggerScheduledItemsJob do end context 'when unexecuted Scheduled campaign jobs' do - let!(:twilio_sms) { create(:channel_twilio_sms) } - let!(:twilio_inbox) { create(:inbox, channel: twilio_sms) } + let!(:twilio_sms) { create(:channel_twilio_sms, account: account) } + let!(:twilio_inbox) { create(:inbox, channel: twilio_sms, account: account) } it 'triggers Campaigns::TriggerOneoffCampaignJob' do - campaign = create(:campaign, inbox: twilio_inbox) - create(:campaign, inbox: twilio_inbox, scheduled_at: 10.days.after) + campaign = create(:campaign, inbox: twilio_inbox, account: account) + create(:campaign, inbox: twilio_inbox, account: account, scheduled_at: 10.days.after) expect(Campaigns::TriggerOneoffCampaignJob).to receive(:perform_later).with(campaign).once described_class.perform_now end diff --git a/spec/models/campaign_spec.rb b/spec/models/campaign_spec.rb index bb03b4520..480f187f3 100644 --- a/spec/models/campaign_spec.rb +++ b/spec/models/campaign_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Campaign do 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' }) } + let(:campaign) { build(:campaign, account: account, inbox: website_inbox, display_id: nil, trigger_rules: { url: 'https://test.com' }) } before do campaign.save! @@ -29,9 +29,10 @@ RSpec.describe Campaign do stub_request(:post, /graph.facebook.com/) end - let!(:facebook_channel) { create(:channel_facebook_page) } - let!(:facebook_inbox) { create(:inbox, channel: facebook_channel) } - let(:campaign) { build(:campaign, inbox: facebook_inbox) } + let(:account) { create(:account) } + let!(:facebook_channel) { create(:channel_facebook_page, account: account) } + let!(:facebook_inbox) { create(:inbox, channel: facebook_channel, account: account) } + let(:campaign) { build(:campaign, inbox: facebook_inbox, account: account) } it 'would not save the campaigns' do expect(campaign.save).to be false @@ -42,7 +43,7 @@ RSpec.describe Campaign do context 'when a campaign is completed' do 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' }) } + let!(:campaign) { create(:campaign, account: account, inbox: web_widget.inbox, campaign_status: :completed, trigger_rules: { url: 'https://test.com' }) } it 'would prevent further updates' do campaign.title = 'new name' @@ -63,9 +64,10 @@ RSpec.describe Campaign do describe 'ensure_correct_campaign_attributes' do context 'when Twilio SMS campaign' do - let!(:twilio_sms) { create(:channel_twilio_sms) } - let!(:twilio_inbox) { create(:inbox, channel: twilio_sms) } - let(:campaign) { build(:campaign, inbox: twilio_inbox) } + let(:account) { create(:account) } + let!(:twilio_sms) { create(:channel_twilio_sms, account: account) } + let!(:twilio_inbox) { create(:inbox, channel: twilio_sms, account: account) } + let(:campaign) { build(:campaign, account: account, inbox: twilio_inbox) } it 'only saves campaign type as oneoff and wont leave scheduled_at empty' do campaign.campaign_type = 'ongoing' @@ -84,9 +86,10 @@ RSpec.describe Campaign do end context 'when SMS campaign' do - let!(:sms_channel) { create(:channel_sms) } - let!(:sms_inbox) { create(:inbox, channel: sms_channel) } - let(:campaign) { build(:campaign, inbox: sms_inbox) } + let(:account) { create(:account) } + let!(:sms_channel) { create(:channel_sms, account: account) } + let!(:sms_inbox) { create(:inbox, channel: sms_channel, account: account) } + let(:campaign) { build(:campaign, account: account, inbox: sms_inbox) } it 'only saves campaign type as oneoff and wont leave scheduled_at empty' do campaign.campaign_type = 'ongoing' @@ -136,4 +139,25 @@ RSpec.describe Campaign do ) end end + + context 'when validating inbox' do + let(:account) { create(:account) } + let(:other_account) { create(:account) } + let(:web_widget) { create(:channel_widget, account: account) } + let(:inbox) { create(:inbox, channel: web_widget, account: account) } + let(:other_account_inbox) { create(:inbox, account: other_account) } + + it 'allows inbox from the same account' do + campaign = build(:campaign, inbox: inbox, account: account) + expect(campaign).to be_valid + end + + it 'does not allow inbox from different account' do + campaign = build(:campaign, inbox: other_account_inbox, account: account) + expect(campaign).not_to be_valid + expect(campaign.errors[:inbox_id]).to include( + 'must belong to the same account as the campaign' + ) + end + end end diff --git a/spec/services/sms/oneoff_sms_campaign_service_spec.rb b/spec/services/sms/oneoff_sms_campaign_service_spec.rb index 27492f83b..2f7e5bbf4 100644 --- a/spec/services/sms/oneoff_sms_campaign_service_spec.rb +++ b/spec/services/sms/oneoff_sms_campaign_service_spec.rb @@ -4,8 +4,8 @@ describe Sms::OneoffSmsCampaignService do subject(:sms_campaign_service) { described_class.new(campaign: campaign) } let(:account) { create(:account) } - let!(:sms_channel) { create(:channel_sms) } - let!(:sms_inbox) { create(:inbox, channel: sms_channel) } + let!(:sms_channel) { create(:channel_sms, account: account) } + let!(:sms_inbox) { create(:inbox, channel: sms_channel, account: account) } let(:label1) { create(:label, account: account) } let(:label2) { create(:label, account: account) } let!(:campaign) do diff --git a/spec/services/twilio/oneoff_sms_campaign_service_spec.rb b/spec/services/twilio/oneoff_sms_campaign_service_spec.rb index f85199c6e..f8858f174 100644 --- a/spec/services/twilio/oneoff_sms_campaign_service_spec.rb +++ b/spec/services/twilio/oneoff_sms_campaign_service_spec.rb @@ -4,8 +4,8 @@ describe Twilio::OneoffSmsCampaignService do subject(:sms_campaign_service) { described_class.new(campaign: campaign) } let(:account) { create(:account) } - let!(:twilio_sms) { create(:channel_twilio_sms) } - let!(:twilio_inbox) { create(:inbox, channel: twilio_sms) } + let!(:twilio_sms) { create(:channel_twilio_sms, account: account) } + let!(:twilio_inbox) { create(:inbox, channel: twilio_sms, account: account) } let(:label1) { create(:label, account: account) } let(:label2) { create(:label, account: account) } let!(:campaign) do