From dfddf9cacc83922cf455e88ca80cc5f77f16100a Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 14 Jul 2021 12:24:09 +0530 Subject: [PATCH] chore: one off SMS campaign APIs (#2589) --- .devcontainer/devcontainer.json | 14 ++-- .../api/v1/accounts/campaigns_controller.rb | 3 +- .../campaigns/trigger_oneoff_campaign_job.rb | 7 ++ app/jobs/trigger_scheduled_items_job.rb | 10 +++ app/models/campaign.rb | 72 +++++++++++++++---- .../twilio/oneoff_sms_campaign_service.rb | 40 +++++++++++ .../api/v1/models/_campaign.json.jbuilder | 6 ++ config/schedule.yml | 6 ++ ...07142801_add_campaign_type_to_campaigns.rb | 10 +++ db/schema.rb | 7 ++ .../v1/accounts/campaigns_controller_spec.rb | 22 ++++++ .../trigger_oneoff_campaign_job_spec.rb | 23 ++++++ spec/jobs/trigger_scheduled_items_job_spec.rb | 24 +++++++ spec/models/campaign_spec.rb | 64 +++++++++++++++++ .../oneoff_sms_campaign_service_spec.rb | 52 ++++++++++++++ 15 files changed, 337 insertions(+), 23 deletions(-) create mode 100644 app/jobs/campaigns/trigger_oneoff_campaign_job.rb create mode 100644 app/jobs/trigger_scheduled_items_job.rb create mode 100644 app/services/twilio/oneoff_sms_campaign_service.rb create mode 100644 db/migrate/20210707142801_add_campaign_type_to_campaigns.rb create mode 100644 spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb create mode 100644 spec/jobs/trigger_scheduled_items_job_spec.rb create mode 100644 spec/services/twilio/oneoff_sms_campaign_service_spec.rb diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index ec85426d7..c0301d87b 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -12,23 +12,21 @@ "extensions": [ "rebornix.Ruby", "misogi.ruby-rubocop", - "wingrunr21.vscode-ruby" + "wingrunr21.vscode-ruby", + "davidpallinder.rails-test-runner", + "eamodio.gitlens", + "github.copilot", + "mrmlnc.vscode-duplicate" ], - // TODO: figure whether we can get all this ports work properly - - // 3000 rails - // 3035 webpacker // 5432 postgres // 6379 redis // 1025,8025 mailhog - "forwardPorts": [5432, 6379, 1025, 8025], + "forwardPorts": [8025], //your application may need to listen on all interfaces (0.0.0.0) not just localhost for it to be available externally. Defaults to [] "appPort": [3000, 3035], - // Use 'postCreateCommand' to run commands after the container is created. - // #TODO: can we move logic of copy env file into dockerfile ? "postCreateCommand": ".devcontainer/scripts/setup.sh && bundle exec rake db:chatwoot_prepare && yarn", "portsAttributes": { "3000": { diff --git a/app/controllers/api/v1/accounts/campaigns_controller.rb b/app/controllers/api/v1/accounts/campaigns_controller.rb index 5c58d3f51..229127df6 100644 --- a/app/controllers/api/v1/accounts/campaigns_controller.rb +++ b/app/controllers/api/v1/accounts/campaigns_controller.rb @@ -28,6 +28,7 @@ class Api::V1::Accounts::CampaignsController < Api::V1::Accounts::BaseController end def campaign_params - params.require(:campaign).permit(:title, :description, :message, :enabled, :inbox_id, :sender_id, trigger_rules: {}) + params.require(:campaign).permit(:title, :description, :message, :enabled, :inbox_id, :sender_id, + :scheduled_at, audience: [:type, :id], trigger_rules: {}) end end diff --git a/app/jobs/campaigns/trigger_oneoff_campaign_job.rb b/app/jobs/campaigns/trigger_oneoff_campaign_job.rb new file mode 100644 index 000000000..158ee0aa0 --- /dev/null +++ b/app/jobs/campaigns/trigger_oneoff_campaign_job.rb @@ -0,0 +1,7 @@ +class Campaigns::TriggerOneoffCampaignJob < ApplicationJob + queue_as :low + + def perform(campaign) + campaign.trigger! + end +end diff --git a/app/jobs/trigger_scheduled_items_job.rb b/app/jobs/trigger_scheduled_items_job.rb new file mode 100644 index 000000000..6bf85e1e4 --- /dev/null +++ b/app/jobs/trigger_scheduled_items_job.rb @@ -0,0 +1,10 @@ +class TriggerScheduledItemsJob < ApplicationJob + queue_as :scheduled_jobs + + def perform + # trigger the scheduled campaign jobs + Campaign.where(campaign_type: :one_off, campaign_status: :active).where(scheduled_at: 3.days.ago..Time.current).all.each do |campaign| + Campaigns::TriggerOneoffCampaignJob.perform_later(campaign) + end + end +end diff --git a/app/models/campaign.rb b/app/models/campaign.rb index bde63ce10..5257c2c2a 100644 --- a/app/models/campaign.rb +++ b/app/models/campaign.rb @@ -2,23 +2,30 @@ # # Table name: campaigns # -# id :bigint not null, primary key -# description :text -# enabled :boolean default(TRUE) -# message :text not null -# title :string not null -# trigger_rules :jsonb -# created_at :datetime not null -# updated_at :datetime not null -# account_id :bigint not null -# display_id :integer not null -# inbox_id :bigint not null -# sender_id :integer +# id :bigint not null, primary key +# audience :jsonb +# campaign_status :integer default("active"), not null +# campaign_type :integer default("ongoing"), not null +# description :text +# enabled :boolean default(TRUE) +# message :text not null +# scheduled_at :datetime +# title :string not null +# trigger_rules :jsonb +# created_at :datetime not null +# updated_at :datetime not null +# account_id :bigint not null +# display_id :integer not null +# inbox_id :bigint not null +# sender_id :integer # # Indexes # -# index_campaigns_on_account_id (account_id) -# index_campaigns_on_inbox_id (inbox_id) +# index_campaigns_on_account_id (account_id) +# index_campaigns_on_campaign_status (campaign_status) +# index_campaigns_on_campaign_type (campaign_type) +# index_campaigns_on_inbox_id (inbox_id) +# index_campaigns_on_scheduled_at (scheduled_at) # # Foreign Keys # @@ -30,20 +37,57 @@ class Campaign < ApplicationRecord validates :inbox_id, presence: true validates :title, presence: true validates :message, presence: true + validate :validate_campaign_inbox + validate :prevent_completed_campaign_from_update, on: :update belongs_to :account belongs_to :inbox belongs_to :sender, class_name: 'User', optional: true + enum campaign_type: { ongoing: 0, one_off: 1 } + # TODO : enabled attribute is unneccessary . lets move that to the campaign status with additional statuses like draft, disabled etc. + enum campaign_status: { active: 0, completed: 1 } + has_many :conversations, dependent: :nullify, autosave: true + before_validation :ensure_correct_campaign_attributes after_commit :set_display_id, unless: :display_id? + def trigger! + return unless one_off? + return if completed? + + Twilio::OneoffSmsCampaignService.new(campaign: self).perform if inbox.inbox_type == 'Twilio SMS' + end + private def set_display_id reload end + def validate_campaign_inbox + return unless inbox + + errors.add :inbox, 'Unsupported Inbox type' unless ['Website', 'Twilio SMS'].include? inbox.inbox_type + end + + # TO-DO we clean up with better validations when campaigns evolve into more inboxes + def ensure_correct_campaign_attributes + return if inbox.blank? + + if inbox.inbox_type == 'Twilio SMS' + self.campaign_type = 'one_off' + self.scheduled_at ||= Time.now.utc + else + self.campaign_type = 'ongoing' + self.scheduled_at = nil + end + end + + def prevent_completed_campaign_from_update + errors.add :status, 'The campaign is already completed' if !campaign_status_changed? && completed? + end + # creating db triggers trigger.before(:insert).for_each(:row) do "NEW.display_id := nextval('camp_dpid_seq_' || NEW.account_id);" diff --git a/app/services/twilio/oneoff_sms_campaign_service.rb b/app/services/twilio/oneoff_sms_campaign_service.rb new file mode 100644 index 000000000..a3ee9d339 --- /dev/null +++ b/app/services/twilio/oneoff_sms_campaign_service.rb @@ -0,0 +1,40 @@ +class Twilio::OneoffSmsCampaignService + pattr_initialize [:campaign!] + + def perform + raise "Invalid campaign #{campaign.id}" if campaign.inbox.inbox_type != 'Twilio SMS' || !campaign.one_off? + raise 'Completed Campaign' if campaign.completed? + + # marks campaign completed so that other jobs won't pick it up + campaign.completed! + + audience_label_ids = campaign.audience.select { |audience| audience['type'] == 'Label' }.pluck('id') + audience_labels = campaign.account.labels.where(id: audience_label_ids).pluck(:title) + process_audience(audience_labels) + end + + private + + delegate :inbox, to: :campaign + delegate :channel, to: :inbox + + def process_audience(audience_labels) + campaign.account.contacts.tagged_with(audience_labels, any: true).each do |contact| + next if contact.phone_number.blank? + + send_message(to: contact.phone_number, from: channel.phone_number, content: campaign.message) + end + end + + def send_message(to:, from:, content:) + client.messages.create({ + body: content, + from: from, + to: to + }) + end + + def client + ::Twilio::REST::Client.new(channel.account_sid, channel.auth_token) + end +end diff --git a/app/views/api/v1/models/_campaign.json.jbuilder b/app/views/api/v1/models/_campaign.json.jbuilder index ecadb7b9e..2ecce8bbd 100644 --- a/app/views/api/v1/models/_campaign.json.jbuilder +++ b/app/views/api/v1/models/_campaign.json.jbuilder @@ -9,7 +9,13 @@ json.sender do json.partial! 'api/v1/models/agent.json.jbuilder', resource: resource.sender if resource.sender.present? end json.message resource.message +json.campaign_status resource.campaign_status json.enabled resource.enabled +json.campaign_type resource.campaign_type +if resource.campaign_type == 'one_off' + json.scheduled_at resource.scheduled_at + json.audience resource.audience +end json.trigger_rules resource.trigger_rules json.created_at resource.created_at json.updated_at resource.updated_at diff --git a/config/schedule.yml b/config/schedule.yml index b5ae49733..6859713fa 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -6,3 +6,9 @@ internal_check_new_versions_job: cron: "0 12 */1 * *" class: "Internal::CheckNewVersionsJob" queue: scheduled_jobs + +# executed At every 5th minute.. +internal_check_new_versions_job: + cron: "*/5 * * * *" + class: "TriggerScheduledItemsJob" + queue: scheduled_jobs \ No newline at end of file diff --git a/db/migrate/20210707142801_add_campaign_type_to_campaigns.rb b/db/migrate/20210707142801_add_campaign_type_to_campaigns.rb new file mode 100644 index 000000000..c89cecea5 --- /dev/null +++ b/db/migrate/20210707142801_add_campaign_type_to_campaigns.rb @@ -0,0 +1,10 @@ +class AddCampaignTypeToCampaigns < ActiveRecord::Migration[6.0] + def change + change_table :campaigns, bulk: true do |t| + t.integer :campaign_type, default: 0, null: false, index: true + t.integer :campaign_status, default: 0, null: false, index: true + t.jsonb :audience, default: [] + t.datetime :scheduled_at, index: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 694639ff1..399b27a3d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -126,8 +126,15 @@ ActiveRecord::Schema.define(version: 2021_07_08_140842) do t.jsonb "trigger_rules", default: {} t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false + t.integer "campaign_type", default: 0, null: false + t.integer "campaign_status", default: 0, null: false + t.jsonb "audience", default: [] + t.datetime "scheduled_at" t.index ["account_id"], name: "index_campaigns_on_account_id" + t.index ["campaign_status"], name: "index_campaigns_on_campaign_status" + t.index ["campaign_type"], name: "index_campaigns_on_campaign_type" t.index ["inbox_id"], name: "index_campaigns_on_inbox_id" + t.index ["scheduled_at"], name: "index_campaigns_on_scheduled_at" end create_table "canned_responses", id: :serial, force: :cascade do |t| diff --git a/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb b/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb index 5371eb535..4a9cfece0 100644 --- a/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/campaigns_controller_spec.rb @@ -106,6 +106,28 @@ RSpec.describe 'Campaigns API', type: :request do expect(response).to have_http_status(:success) expect(JSON.parse(response.body, symbolize_names: true)[:title]).to eq('test') end + + it 'creates a new oneoff campaign' do + twilio_sms = create(:channel_twilio_sms, account: account) + twilio_inbox = create(:inbox, channel: twilio_sms) + label1 = create(:label, account: account) + label2 = create(:label, account: account) + + post "/api/v1/accounts/#{account.id}/campaigns", + params: { + inbox_id: twilio_inbox.id, title: 'test', message: 'test message', + scheduled_at: 2.days.from_now, + audience: [{ type: 'Label', id: label1.id }, { type: 'Label', id: label2.id }] + }, + headers: administrator.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + expect(response_data[:campaign_type]).to eq('one_off') + expect(response_data[:scheduled_at].present?).to eq true + expect(response_data[:audience].pluck(:id)).to include(label1.id, label2.id) + end end end diff --git a/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb b/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb new file mode 100644 index 000000000..6c56896ac --- /dev/null +++ b/spec/jobs/campaigns/trigger_oneoff_campaign_job_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +RSpec.describe Campaigns::TriggerOneoffCampaignJob, type: :job do + let(:account) { create(:account) } + let!(:twilio_sms) { create(:channel_twilio_sms) } + let!(:twilio_inbox) { create(:inbox, channel: twilio_sms) } + 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 }]) } + + it 'enqueues the job' do + expect { described_class.perform_later(campaign) }.to have_enqueued_job(described_class) + .on_queue('low') + end + + context 'when called with a campaign' do + it 'triggers the campaign' do + expect(campaign).to receive(:trigger!) + described_class.perform_now(campaign) + end + end +end diff --git a/spec/jobs/trigger_scheduled_items_job_spec.rb b/spec/jobs/trigger_scheduled_items_job_spec.rb new file mode 100644 index 000000000..4ec0d331e --- /dev/null +++ b/spec/jobs/trigger_scheduled_items_job_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe TriggerScheduledItemsJob, type: :job do + subject(:job) { described_class.perform_later } + + let(:account) { create(:account) } + + it 'enqueues the job' do + expect { job }.to have_enqueued_job(described_class) + .on_queue('scheduled_jobs') + end + + context 'when unexecuted Scheduled campaign jobs' do + let!(:twilio_sms) { create(:channel_twilio_sms) } + let!(:twilio_inbox) { create(:inbox, channel: twilio_sms) } + + it 'triggers Campaigns::TriggerOneoffCampaignJob' do + campaign = create(:campaign, inbox: twilio_inbox) + create(:campaign, inbox: twilio_inbox, scheduled_at: 10.days.after) + expect(Campaigns::TriggerOneoffCampaignJob).to receive(:perform_later).with(campaign).once + described_class.perform_now + end + end +end diff --git a/spec/models/campaign_spec.rb b/spec/models/campaign_spec.rb index 30fa1a43d..05ff8cdc7 100644 --- a/spec/models/campaign_spec.rb +++ b/spec/models/campaign_spec.rb @@ -20,4 +20,68 @@ RSpec.describe Campaign, type: :model do expect(campaign.display_id).to eq(1) end end + + context 'when Inbox other then Website or Twilio SMS' do + let!(:facebook_channel) { create(:channel_facebook_page) } + let!(:facebook_inbox) { create(:inbox, channel: facebook_channel) } + let(:campaign) { build(:campaign, inbox: facebook_inbox) } + + it 'would not save the campaigns' do + expect(campaign.save).to eq false + expect(campaign.errors.full_messages.first).to eq 'Inbox Unsupported Inbox type' + end + end + + context 'when a campaign is completed' do + let!(:campaign) { create(:campaign, campaign_status: :completed) } + + it 'would prevent further updates' do + campaign.title = 'new name' + expect(campaign.save).to eq false + expect(campaign.errors.full_messages.first).to eq 'Status The campaign is already completed' + end + + it 'can be deleted' do + campaign.destroy! + expect(described_class.exists?(campaign.id)).to eq false + end + + it 'cant be triggered' do + expect(Twilio::OneoffSmsCampaignService).not_to receive(:new).with(campaign: campaign) + expect(campaign.trigger!).to eq nil + end + end + + 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) } + + it 'only saves campaign type as oneoff and wont leave scheduled_at empty' do + campaign.campaign_type = 'ongoing' + campaign.save! + expect(campaign.reload.campaign_type).to eq 'one_off' + expect(campaign.scheduled_at.present?).to eq true + end + + it 'calls twilio service on trigger!' do + sms_service = double + expect(Twilio::OneoffSmsCampaignService).to receive(:new).with(campaign: campaign).and_return(sms_service) + expect(sms_service).to receive(:perform) + campaign.save! + campaign.trigger! + end + end + + context 'when Website campaign' do + let(:campaign) { build(:campaign) } + + it 'only saves campaign type as ongoing' do + campaign.campaign_type = 'one_off' + campaign.save! + expect(campaign.reload.campaign_type).to eq 'ongoing' + end + end + end end diff --git a/spec/services/twilio/oneoff_sms_campaign_service_spec.rb b/spec/services/twilio/oneoff_sms_campaign_service_spec.rb new file mode 100644 index 000000000..aaa027930 --- /dev/null +++ b/spec/services/twilio/oneoff_sms_campaign_service_spec.rb @@ -0,0 +1,52 @@ +require 'rails_helper' + +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(:label1) { create(:label, account: account) } + let(:label2) { create(:label, account: account) } + let!(:campaign) do + create(:campaign, inbox: twilio_inbox, account: account, + audience: [{ type: 'Label', id: label1.id }, { type: 'Label', id: label2.id }]) + end + let(:twilio_client) { double } + let(:twilio_messages) { double } + + describe 'perform' do + before do + allow(::Twilio::REST::Client).to receive(:new).and_return(twilio_client) + allow(twilio_client).to receive(:messages).and_return(twilio_messages) + end + + it 'raises error if the campaign is completed' do + campaign.completed! + + expect { sms_campaign_service.perform }.to raise_error 'Completed Campaign' + end + + it 'raises error invalid campaign when its not a oneoff sms campaign' do + campaign = create(:campaign) + + expect { described_class.new(campaign: campaign).perform }.to raise_error "Invalid campaign #{campaign.id}" + end + + it 'send messages to contacts in the audience and marks the campaign completed' do + contact_with_label1, contact_with_label2, contact_with_both_labels = FactoryBot.create_list(:contact, 3, account: account) + contact_with_label1.update_labels([label1.title]) + contact_with_label2.update_labels([label2.title]) + contact_with_both_labels.update_labels([label1.title, label2.title]) + expect(twilio_messages).to receive(:create).with(body: campaign.message, + from: twilio_sms.phone_number, to: contact_with_label1.phone_number).once + expect(twilio_messages).to receive(:create).with(body: campaign.message, + from: twilio_sms.phone_number, to: contact_with_label2.phone_number).once + expect(twilio_messages).to receive(:create).with(body: campaign.message, + from: twilio_sms.phone_number, to: contact_with_both_labels.phone_number).once + + sms_campaign_service.perform + expect(campaign.reload.completed?).to eq true + end + end +end