diff --git a/Gemfile.lock b/Gemfile.lock index 8d7c29d..59ee4e6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -192,5 +192,8 @@ DEPENDENCIES unicorn webmock +RUBY VERSION + ruby 2.2.3p173 + BUNDLED WITH - 1.10.6 + 1.15.1 diff --git a/app/models.rb b/app/models.rb index c087289..0449738 100644 --- a/app/models.rb +++ b/app/models.rb @@ -83,6 +83,7 @@ module Models require 'app/models/city' require 'app/models/event' require 'app/models/publisher' +require 'app/models/sms_credentials' require 'app/models/subscription' # access model class constants without qualifying namespace diff --git a/app/models/publisher.rb b/app/models/publisher.rb index f48a53a..67eb540 100644 --- a/app/models/publisher.rb +++ b/app/models/publisher.rb @@ -2,6 +2,7 @@ module Citygram::Models class Publisher < Sequel::Model one_to_many :subscriptions one_to_many :events + many_to_one :sms_credentials plugin :url_validation plugin :serialization, :pg_array, :tags @@ -19,6 +20,9 @@ def active where(active: true) end end + + delegate :credential_name, :from_number, :account_sid, :auth_token, to: :sms_credentials + def validate super diff --git a/app/models/sms_credentials.rb b/app/models/sms_credentials.rb new file mode 100644 index 0000000..00476a6 --- /dev/null +++ b/app/models/sms_credentials.rb @@ -0,0 +1,12 @@ +module Citygram::Models + class SmsCredentials < Sequel::Model + dataset_module do + end + + def validate + super + validates_presence [:from_number, :account_sid, :auth_token] + validates_unique :account_sid + end + end +end \ No newline at end of file diff --git a/app/models/subscription.rb b/app/models/subscription.rb index a0a6e51..922ab96 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -112,6 +112,8 @@ def last_notification def last_notification_date self.last_notification.strftime("%b %d, %Y") end + + delegate :credential_name, :from_number, :account_sid, :auth_token, to: :publisher def validate super diff --git a/app/modules/sms_sender.rb b/app/modules/sms_sender.rb new file mode 100644 index 0000000..541ea4a --- /dev/null +++ b/app/modules/sms_sender.rb @@ -0,0 +1,15 @@ +module Citygram + module SmsSender + + def send_sms(subscription, body) + Citygram::App.logger.info("Sending SMS via #{subscription.credential_name}") + Citygram::Services::Channels::SMS.sms( + subscription.account_sid, + subscription.auth_token, { + from: subscription.from_number, + to: subscription.phone_number, + body: body + }) + end + end +end \ No newline at end of file diff --git a/app/services/channels.rb b/app/services/channels.rb index 33f0f8b..0158286 100644 --- a/app/services/channels.rb +++ b/app/services/channels.rb @@ -21,6 +21,7 @@ class NotificationFailure < StandardError; end end end +require 'app/modules/sms_sender' require 'app/services/channels/base' require 'app/services/channels/email' require 'app/services/channels/sms' diff --git a/app/services/channels/sms.rb b/app/services/channels/sms.rb index 0e9e1c5..a27b6e4 100644 --- a/app/services/channels/sms.rb +++ b/app/services/channels/sms.rb @@ -1,9 +1,8 @@ module Citygram::Services::Channels class SMS < Base - FROM_NUMBER = ENV.fetch('TWILIO_FROM_NUMBER').freeze + + include ::Citygram::SmsSender - TWILIO_ACCOUNT_SID = ENV.fetch('TWILIO_ACCOUNT_SID').freeze - TWILIO_AUTH_TOKEN = ENV.fetch('TWILIO_AUTH_TOKEN').freeze UNSUBSCRIBE_ERROR_CODES = [ 21211, # number cannot exist - https://www.twilio.com/docs/errors/21211 @@ -11,17 +10,13 @@ class SMS < Base 21614, # not a valid mobile number - https://www.twilio.com/docs/errors/21614 ].freeze - def self.sms(*args) - client = Twilio::REST::Client.new(TWILIO_ACCOUNT_SID, TWILIO_AUTH_TOKEN) + def self.sms(sid, token, *args) + client = Twilio::REST::Client.new(sid, token) client.account.messages.create(*args) end def call - self.class.sms( - from: FROM_NUMBER, - to: subscription.phone_number, - body: event.title - ) + send_sms(subscription, event.title) rescue Twilio::REST::RequestError => e Citygram::App.logger.error(e) diff --git a/app/workers.rb b/app/workers.rb index 5de3a20..02d7f55 100644 --- a/app/workers.rb +++ b/app/workers.rb @@ -5,12 +5,14 @@ module Workers end end +require 'app/modules/sms_sender' require 'app/workers/publisher_poll' require 'app/workers/notifier' require 'app/workers/subscription_confirmation' require 'app/workers/reminder_notification' require 'app/workers/middleware/database_connections' + Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Citygram::Workers::Middleware::DatabaseConnections, Sequel::Model.db diff --git a/app/workers/reminder_notification.rb b/app/workers/reminder_notification.rb index 9a05b22..5617c82 100644 --- a/app/workers/reminder_notification.rb +++ b/app/workers/reminder_notification.rb @@ -1,6 +1,7 @@ module Citygram::Workers class ReminderNotification include Sidekiq::Worker + include ::Citygram::SmsSender sidekiq_options retry: 5 def reminder_url(subscription) @@ -23,11 +24,7 @@ def perform(subscription_id) # ...but if there is an error sending this message, tomorrow's might work subscription.remind! [body_1, body_2].each do |body| - Citygram::Services::Channels::SMS.sms( - from: Citygram::Services::Channels::SMS::FROM_NUMBER, - to: subscription.phone_number, - body: body - ) + send_sms(subscription, body) end rescue Twilio::REST::RequestError => e Citygram::App.logger.error(e) diff --git a/app/workers/subscription_confirmation.rb b/app/workers/subscription_confirmation.rb index 9b4e94e..4f946e7 100644 --- a/app/workers/subscription_confirmation.rb +++ b/app/workers/subscription_confirmation.rb @@ -1,6 +1,8 @@ module Citygram::Workers class SubscriptionConfirmation include Sidekiq::Worker + include ::Citygram::SmsSender + sidekiq_options retry: 5 def digest_url(subscription) @@ -15,12 +17,7 @@ def perform(subscription_id) case subscription.channel when 'sms' body = "Welcome! You are now subscribed to #{publisher.title} in #{publisher.city}. To see current Citygrams please visit #{digest_url(subscription)}. To unsubscribe from all messages, reply REMOVE." - - Citygram::Services::Channels::SMS.sms( - from: Citygram::Services::Channels::SMS::FROM_NUMBER, - to: subscription.phone_number, - body: body - ) + send_sms(subscription, body) when 'email' body = <<-BODY.dedent

Thank you for subscribing! View Citygrams in a browser.

diff --git a/db/migrations/031_create_sms_credentials.rb b/db/migrations/031_create_sms_credentials.rb new file mode 100644 index 0000000..56c1316 --- /dev/null +++ b/db/migrations/031_create_sms_credentials.rb @@ -0,0 +1,17 @@ +Sequel.migration do + up do + create_table :sms_credentials do + primary_key :id + String :credential_name + String :from_number + String :account_sid + String :auth_token + DateTime :updated_at + DateTime :created_at + end + end + + down do + drop_table :sms_credentials + end +end diff --git a/db/migrations/032_add_sms_credentials_to_publisher.rb b/db/migrations/032_add_sms_credentials_to_publisher.rb new file mode 100644 index 0000000..50bcace --- /dev/null +++ b/db/migrations/032_add_sms_credentials_to_publisher.rb @@ -0,0 +1,13 @@ +Sequel.migration do + up do + alter_table(:publishers) do + add_foreign_key :sms_credentials_id, :sms_credentials + end + end + + down do + alter_table(:publishers) do + drop_foreign_key :sms_credentials_id + end + end +end diff --git a/db/schema.sql b/db/schema.sql index f3b294a..5f6b941 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -2,12 +2,17 @@ -- PostgreSQL database dump -- +-- Dumped from database version 9.5.2 +-- Dumped by pg_dump version 10.1 + SET statement_timeout = 0; SET lock_timeout = 0; +SET idle_in_transaction_session_timeout = 0; SET client_encoding = 'UTF8'; SET standard_conforming_strings = on; SET check_function_bodies = false; SET client_min_messages = warning; +SET row_security = off; -- -- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: - @@ -23,269 +28,6 @@ CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog; COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language'; --- --- Name: pg_stat_statements; Type: EXTENSION; Schema: -; Owner: - --- - -CREATE EXTENSION IF NOT EXISTS pg_stat_statements WITH SCHEMA public; - - --- --- Name: EXTENSION pg_stat_statements; Type: COMMENT; Schema: -; Owner: - --- - -COMMENT ON EXTENSION pg_stat_statements IS 'track execution statistics of all SQL statements executed'; - - --- --- Name: postgis; Type: EXTENSION; Schema: -; Owner: - --- - -CREATE EXTENSION IF NOT EXISTS postgis WITH SCHEMA public; - - --- --- Name: EXTENSION postgis; Type: COMMENT; Schema: -; Owner: - --- - -COMMENT ON EXTENSION postgis IS 'PostGIS geometry, geography, and raster spatial types and functions'; - - --- --- Name: uuid-ossp; Type: EXTENSION; Schema: -; Owner: - --- - -CREATE EXTENSION IF NOT EXISTS "uuid-ossp" WITH SCHEMA public; - - --- --- Name: EXTENSION "uuid-ossp"; Type: COMMENT; Schema: -; Owner: - --- - -COMMENT ON EXTENSION "uuid-ossp" IS 'generate universally unique identifiers (UUIDs)'; - - -SET search_path = public, pg_catalog; - -SET default_tablespace = ''; - -SET default_with_oids = false; - --- --- Name: events; Type: TABLE; Schema: public; Owner: -; Tablespace: --- - -CREATE TABLE events ( - id integer NOT NULL, - title text, - description text, - geom geometry, - updated_at timestamp without time zone, - created_at timestamp without time zone, - publisher_id integer, - feature_id text, - properties json DEFAULT '{}'::json -); - - --- --- Name: events_id_seq; Type: SEQUENCE; Schema: public; Owner: - --- - -CREATE SEQUENCE events_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - - --- --- Name: events_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - --- - -ALTER SEQUENCE events_id_seq OWNED BY events.id; - - --- --- Name: http_requests; Type: TABLE; Schema: public; Owner: -; Tablespace: --- - -CREATE TABLE http_requests ( - id uuid DEFAULT uuid_generate_v4() NOT NULL, - scheme character varying(255), - userinfo text, - host text, - port integer, - path text, - query text, - fragment text, - method character varying(255), - response_status integer, - duration integer, - started_at timestamp without time zone -); - - --- --- Name: publishers; Type: TABLE; Schema: public; Owner: -; Tablespace: --- - -CREATE TABLE publishers ( - id integer NOT NULL, - title text, - endpoint text, - updated_at timestamp without time zone, - created_at timestamp without time zone, - active boolean, - city text, - icon text, - visible boolean DEFAULT true, - state text, - description text, - tags text[] DEFAULT '{}'::text[] NOT NULL, - event_display_endpoint text, - events_are_polygons boolean -); - - --- --- Name: publishers_id_seq; Type: SEQUENCE; Schema: public; Owner: - --- - -CREATE SEQUENCE publishers_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - - --- --- Name: publishers_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - --- - -ALTER SEQUENCE publishers_id_seq OWNED BY publishers.id; - - --- --- Name: schema_info; Type: TABLE; Schema: public; Owner: -; Tablespace: --- - -CREATE TABLE schema_info ( - version integer DEFAULT 0 NOT NULL -); - - --- --- Name: subscriptions; Type: TABLE; Schema: public; Owner: -; Tablespace: --- - -CREATE TABLE subscriptions ( - geom geometry, - updated_at timestamp without time zone, - created_at timestamp without time zone, - publisher_id integer, - channel text NOT NULL, - phone_number text, - email_address text, - webhook_url text, - unsubscribed_at timestamp without time zone, - id uuid DEFAULT uuid_generate_v4() NOT NULL, - last_notified timestamp without time zone -); - - --- --- Name: id; Type: DEFAULT; Schema: public; Owner: - --- - -ALTER TABLE ONLY events ALTER COLUMN id SET DEFAULT nextval('events_id_seq'::regclass); - - --- --- Name: id; Type: DEFAULT; Schema: public; Owner: - --- - -ALTER TABLE ONLY publishers ALTER COLUMN id SET DEFAULT nextval('publishers_id_seq'::regclass); - - --- --- Name: events_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: --- - -ALTER TABLE ONLY events - ADD CONSTRAINT events_pkey PRIMARY KEY (id); - - --- --- Name: http_requests_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: --- - -ALTER TABLE ONLY http_requests - ADD CONSTRAINT http_requests_pkey PRIMARY KEY (id); - - --- --- Name: publishers_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: --- - -ALTER TABLE ONLY publishers - ADD CONSTRAINT publishers_pkey PRIMARY KEY (id); - - --- --- Name: subscriptions_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: --- - -ALTER TABLE ONLY subscriptions - ADD CONSTRAINT subscriptions_pkey PRIMARY KEY (id); - - --- --- Name: events_geom_gist; Type: INDEX; Schema: public; Owner: -; Tablespace: --- - -CREATE INDEX events_geom_gist ON events USING gist (geom); - - --- --- Name: events_publisher_id_feature_id_index; Type: INDEX; Schema: public; Owner: -; Tablespace: --- - -CREATE UNIQUE INDEX events_publisher_id_feature_id_index ON events USING btree (publisher_id, feature_id); - - --- --- Name: publishers_endpoint_index; Type: INDEX; Schema: public; Owner: -; Tablespace: --- - -CREATE UNIQUE INDEX publishers_endpoint_index ON publishers USING btree (endpoint); - - --- --- Name: subscriptions_geom_gist; Type: INDEX; Schema: public; Owner: -; Tablespace: --- - -CREATE INDEX subscriptions_geom_gist ON subscriptions USING gist (geom); - - --- --- Name: events_publisher_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - --- - -ALTER TABLE ONLY events - ADD CONSTRAINT events_publisher_id_fkey FOREIGN KEY (publisher_id) REFERENCES publishers(id); - - --- --- Name: subscriptions_publisher_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - --- - -ALTER TABLE ONLY subscriptions - ADD CONSTRAINT subscriptions_publisher_id_fkey FOREIGN KEY (publisher_id) REFERENCES publishers(id); - - -- -- PostgreSQL database dump complete -- diff --git a/docker-compose.yml b/docker-compose.yml index 404d068..5f9e869 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,6 +5,8 @@ services: image: citygram/data networks: - citygram + ports: + - "25432:5432" container_name: citygram_db redis: image: redis:3.0.3 diff --git a/lib/database_helper.rb b/lib/database_helper.rb index 24fa147..8e146fd 100644 --- a/lib/database_helper.rb +++ b/lib/database_helper.rb @@ -82,7 +82,7 @@ def self.drop_db def self.schema_dump `rm #{schema_path}` - pg_command("pg_dump -i -s -x -O -f #{schema_path} #{db_name}") + pg_command("pg_dump -s -x -O -f #{schema_path} #{db_name}") end def self.rollback_db(version = nil) diff --git a/spec/factories.rb b/spec/factories.rb index 8a88c06..51a37e7 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -8,6 +8,7 @@ sequence(:city) { |n| "City-#{n}" } icon 'balloons.png' active true + sms_credentials end factory :event, class: Citygram::Models::Event do @@ -17,6 +18,13 @@ feature_id { SecureRandom.hex(10) } geom FixtureHelpers.fixture('disjoint-geom.geojson') end + + factory :sms_credentials, class: Citygram::Models::SmsCredentials do + credential_name "test-credential" + from_number "15555555555" + sequence(:account_sid) { |n| "dev-account-sid-#{n}" } + auth_token "dev-auth-token" + end factory :subscription, class: Citygram::Models::Subscription do publisher diff --git a/spec/models/sms_credentials_spec.rb b/spec/models/sms_credentials_spec.rb new file mode 100644 index 0000000..0d2255d --- /dev/null +++ b/spec/models/sms_credentials_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Citygram::Models::SmsCredentials do + it 'should be retrievable from the factory' do + creds = build(:sms_credentials) + expect(creds).to be_valid + end + + it 'should require a from number' do + creds = build(:sms_credentials, from_number: nil) + expect(creds).not_to be_valid + end + + it 'should require a sid' do + creds = build(:sms_credentials, account_sid: nil) + expect(creds).not_to be_valid + end + + it 'should require an auth_token' do + creds = build(:sms_credentials, auth_token: nil) + expect(creds).not_to be_valid + end +end \ No newline at end of file diff --git a/spec/services/channels/sms_spec.rb b/spec/services/channels/sms_spec.rb index 935a8ac..57a2ca3 100644 --- a/spec/services/channels/sms_spec.rb +++ b/spec/services/channels/sms_spec.rb @@ -4,11 +4,13 @@ subject { Citygram::Services::Channels::SMS } let(:subscription) { create(:subscription, channel: 'sms', phone_number: '+15559874567') } + let(:publisher) { subscription.publisher } + let(:sms_credentials) { publisher.sms_credentials } let(:event) { create(:event) } - let(:from_number) { ENV.fetch('TWILIO_FROM_NUMBER') } - let(:account_sid) { ENV.fetch('TWILIO_ACCOUNT_SID') } - let(:auth_token) { ENV.fetch('TWILIO_AUTH_TOKEN') } + let(:from_number) { sms_credentials.from_number } + let(:account_sid) { sms_credentials.account_sid } + let(:auth_token) { sms_credentials.auth_token } let(:sms_endpoint) do "https://%s:%s@api.twilio.com/2010-04-01/Accounts/%s/Messages.json" % [account_sid, auth_token, account_sid] end @@ -93,7 +95,7 @@ args = { from: from_number, to: subscription.phone_number, body: event.title } expect(Citygram::Services::Channels::SMS).to receive(:sms). - with(args). + with(account_sid, auth_token, args). and_raise(unsubscribed_error) expect { diff --git a/spec/workers/reminder_notification_spec.rb b/spec/workers/reminder_notification_spec.rb index e06214c..2fafe41 100644 --- a/spec/workers/reminder_notification_spec.rb +++ b/spec/workers/reminder_notification_spec.rb @@ -3,6 +3,7 @@ describe Citygram::Workers::ReminderNotification do let(:reminder){ Citygram::Workers::ReminderNotification.new } let(:publisher) { subscription.publisher } + let(:sms_credentials) { publisher.sms_credentials } let!(:subscription) { create(:subscription, channel: 'sms', phone_number: '212-555-1234', last_notified: 3.weeks.ago) } subject { reminder } @@ -44,10 +45,10 @@ body_1 = subject.reminder_message(subscription) body_2 = subject.unsub_message(subscription) [body_1, body_2].each do |body| - stub_request(:post, "https://dev-account-sid:dev-auth-token@api.twilio.com/2010-04-01/Accounts/dev-account-sid/Messages.json"). + stub_request(:post, "https://#{sms_credentials.account_sid}:#{sms_credentials.auth_token}@api.twilio.com/2010-04-01/Accounts/#{sms_credentials.account_sid}/Messages.json"). with(body: { "Body" => body, - "From"=>"15555555555", + "From"=> '15555555555', "To"=>"+12125551234"}). to_return(status: 200, body: { 'sid' => 'SM10ea1dce707f4bedb44204c9fbc02e39', diff --git a/spec/workers/subscription_confirmation_spec.rb b/spec/workers/subscription_confirmation_spec.rb index ffe8608..db730d7 100644 --- a/spec/workers/subscription_confirmation_spec.rb +++ b/spec/workers/subscription_confirmation_spec.rb @@ -38,12 +38,13 @@ context 'sms' do let(:publisher) { subscription.publisher } + let(:sms_credentials) { publisher.sms_credentials } let!(:subscription) { create(:subscription, channel: 'sms', phone_number: '212-555-1234') } before do body = "Welcome! You are now subscribed to #{publisher.title} in #{publisher.city}. To see current Citygrams please visit #{subject.digest_url(subscription)}. To unsubscribe from all messages, reply REMOVE." - stub_request(:post, "https://dev-account-sid:dev-auth-token@api.twilio.com/2010-04-01/Accounts/dev-account-sid/Messages.json"). + stub_request(:post, "https://#{sms_credentials.account_sid}:#{sms_credentials.auth_token}@api.twilio.com/2010-04-01/Accounts/#{sms_credentials.account_sid}/Messages.json"). with(body: { "Body" => body, "From"=>"15555555555",