From c00cac43b066d3244866bea1349e831770ae07c2 Mon Sep 17 00:00:00 2001 From: Ryan Jackson Date: Thu, 26 Sep 2019 15:40:35 -0400 Subject: [PATCH 1/6] Retry webhook emission if failure --- app/jobs/event_jobs.rb | 12 +++- app/lib/forms/events/create.rb | 1 + app/lib/forms/events/schedule_retry.rb | 55 +++++++++++++++++++ app/mailers/application_mailer.rb | 2 +- app/mailers/developer_mailer.rb | 11 ++++ app/models/event.rb | 1 + app/serializers/event_serializer.rb | 1 + .../webhook_failure.html.haml | 5 ++ app/views/layouts/mailer.html.erb | 13 ----- app/views/layouts/mailer.html.haml | 8 +++ app/views/layouts/mailer.text.erb | 1 - config/initializers/config.rb | 4 ++ config/settings.yml | 6 +- config/settings/development.yml | 2 + config/settings/test.yml | 6 +- .../20190926185450_add_retries_to_events.rb | 5 ++ db/schema.rb | 3 +- spec/factories/events.rb | 1 + spec/fixtures/developer/webhook_failure | 3 + spec/jobs/event_jobs/create_and_emit_spec.rb | 7 +++ spec/lib/forms/events/create_spec.rb | 1 + spec/lib/forms/events/schedule_retry_spec.rb | 29 ++++++++++ spec/mailers/developer_spec.rb | 20 +++++++ spec/mailers/previews/developer_preview.rb | 9 +++ spec/models/event_spec.rb | 1 + 25 files changed, 185 insertions(+), 22 deletions(-) create mode 100644 app/lib/forms/events/schedule_retry.rb create mode 100644 app/mailers/developer_mailer.rb create mode 100644 app/views/developer_mailer/webhook_failure.html.haml delete mode 100644 app/views/layouts/mailer.html.erb create mode 100644 app/views/layouts/mailer.html.haml delete mode 100644 app/views/layouts/mailer.text.erb create mode 100644 db/migrate/20190926185450_add_retries_to_events.rb create mode 100644 spec/fixtures/developer/webhook_failure create mode 100644 spec/lib/forms/events/schedule_retry_spec.rb create mode 100644 spec/mailers/developer_spec.rb create mode 100644 spec/mailers/previews/developer_preview.rb diff --git a/app/jobs/event_jobs.rb b/app/jobs/event_jobs.rb index 7ce276e..07c2361 100644 --- a/app/jobs/event_jobs.rb +++ b/app/jobs/event_jobs.rb @@ -18,9 +18,15 @@ def perform(data, event_object_type, event_object_id, organization_id, type) class Emit < ApplicationJob def perform(event_id) - Forms::Events::Emit.new( - event: Event.find(event_id) - ).save.raise_if_error + event = Event.find(event_id) + begin + Forms::Events::Emit.new( + event: event + ).save.raise_if_error + rescue StandardError => e + Forms::Events::ScheduleRetry.new(event: event).save.raise_if_error + raise e + end end end end diff --git a/app/lib/forms/events/create.rb b/app/lib/forms/events/create.rb index 2f8d051..99d7801 100644 --- a/app/lib/forms/events/create.rb +++ b/app/lib/forms/events/create.rb @@ -39,6 +39,7 @@ def save delegate_accessor :data, to: :event def create_event + event.retries = 0 event.save! success(event) end diff --git a/app/lib/forms/events/schedule_retry.rb b/app/lib/forms/events/schedule_retry.rb new file mode 100644 index 0000000..53a4bb4 --- /dev/null +++ b/app/lib/forms/events/schedule_retry.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Forms + module Events + class ScheduleRetry + include Formify::Form + attr_accessor :event + + validates_presence_of :event + + def save + with_advisory_lock_transaction(:events, event) do + validate_or_fail + .and_then { schedule_retry } + .and_then { success(event) } + end + end + + private + + def notify_provider + redis = Redis.new + key = 'webhooks/provider_last_notified_at' + + provider_last_notified_at = redis.get(key) + + return if provider_last_notified_at.present? && (Time.zone.now - Time.parse(provider_last_notified_at)) < 24.hours + + redis.set(key, Time.zone.now.to_s) + DeveloperMailer.webhook_failure(event.id).deliver_later + end + + def retry_back_off_in_seconds + @retry_back_off_in_seconds ||= Settings.application.webhooks.retry_back_off_in_seconds + end + + def retry_limit + @retry_limit ||= Settings.application.webhooks.retry_limit + end + + def schedule_retry + return success(event) if event.retries.present? && event.retries >= retry_limit + + notify_provider + + event.retries ||= 0 + event.retries += 1 + wait_in_seconds = retry_back_off_in_seconds**event.retries + EventJobs::Emit.perform_in(wait_in_seconds.seconds, event.id) + + success(event) + end + end + end +end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 286b223..56349a7 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,4 +1,4 @@ class ApplicationMailer < ActionMailer::Base - default from: 'from@example.com' + default from: Settings.mailer.from_email layout 'mailer' end diff --git a/app/mailers/developer_mailer.rb b/app/mailers/developer_mailer.rb new file mode 100644 index 0000000..227c951 --- /dev/null +++ b/app/mailers/developer_mailer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class DeveloperMailer < ApplicationMailer + def webhook_failure(event_id) + @event = Event.find(event_id) + mail( + subject: "Webhook failure at #{Time.zone.now}", + to: Settings.application.developer_email + ) + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 0a401e5..b615812 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -5,6 +5,7 @@ # id :string not null, primary key # data :text # event_object_type :string +# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null diff --git a/app/serializers/event_serializer.rb b/app/serializers/event_serializer.rb index 1234e74..4e69560 100644 --- a/app/serializers/event_serializer.rb +++ b/app/serializers/event_serializer.rb @@ -5,6 +5,7 @@ # id :string not null, primary key # data :text # event_object_type :string +# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null diff --git a/app/views/developer_mailer/webhook_failure.html.haml b/app/views/developer_mailer/webhook_failure.html.haml new file mode 100644 index 0000000..c9f453a --- /dev/null +++ b/app/views/developer_mailer/webhook_failure.html.haml @@ -0,0 +1,5 @@ +%h1 Webhook Failure + +%p + Event ID: #{@event.id} + Retries: #{@event.retries} diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb deleted file mode 100644 index cbd34d2..0000000 --- a/app/views/layouts/mailer.html.erb +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - - - <%= yield %> - - diff --git a/app/views/layouts/mailer.html.haml b/app/views/layouts/mailer.html.haml new file mode 100644 index 0000000..cbf6b8e --- /dev/null +++ b/app/views/layouts/mailer.html.haml @@ -0,0 +1,8 @@ +!!! +%html + %head + %meta{:content => "text/html; charset=utf-8", "http-equiv" => "Content-Type"}/ + :css + /* Email styles need to be inline */ + %body + = yield diff --git a/app/views/layouts/mailer.text.erb b/app/views/layouts/mailer.text.erb deleted file mode 100644 index 37f0bdd..0000000 --- a/app/views/layouts/mailer.text.erb +++ /dev/null @@ -1 +0,0 @@ -<%= yield %> diff --git a/config/initializers/config.rb b/config/initializers/config.rb index cf2f79a..3c30619 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -79,6 +79,7 @@ end required(:application).schema do + required(:developer_email).filled(:str?) required(:host_port).maybe(:int?) required(:host_url).filled(:str?) required(:login_url).maybe(:str?) @@ -86,6 +87,8 @@ optional(:theme).maybe(:str?) optional(:webhooks).schema do optional(:url).maybe(:str?) + optional(:retry_back_off_in_seconds).filled(:int?) + optional(:retry_limit).filled(:int?) optional(:key).maybe(:str?) end end @@ -102,6 +105,7 @@ required(:mailer).filled do schema do required(:delivery_method).filled(:str?) + required(:from_email).filled(:str?) optional(:smtp).filled do schema do required(:address).filled(:str?) diff --git a/config/settings.yml b/config/settings.yml index 51ece03..96e9a6f 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -20,13 +20,16 @@ api: root_secret_key: # required: The API key your application will use to send data to the Ledger Sync App application: - login_url: # required: the url where users will be redirect if authentication is required + developer_email: # required: email for developer notifications (e.g. webhook failure) host_port: # optional host_url: # required: where this app is hosted (e.g. `sync.example.com`) + login_url: # required: the url where users will be redirect if authentication is required name: 'Ledger Sync' # required: usually your company name theme: modern_treasury # optional: Name of theme in app/assets/stylesheets/themes webhooks: key: # required if url present: secret key used for signing webhooks + retry_back_off_in_seconds: 20 # required: retry_back_off_in_seconds ** retry_count + retry_limit: 10 # required url: # optional: URL to which webhooks will be sent authentication: @@ -39,6 +42,7 @@ customization: mailer: delivery_method: # required: letter_opener (only in development), smtp, test (only in test) disable_email_to_users: false # optional + from_email: # required: email address for replies # smtp: # required if delivery_method=smtp # address: # - required # authentication: # - required diff --git a/config/settings/development.yml b/config/settings/development.yml index 489e494..a44fe8a 100644 --- a/config/settings/development.yml +++ b/config/settings/development.yml @@ -1,4 +1,5 @@ application: + developer_email: me@example.com host_url: 'localhost' host_port: 3000 @@ -7,3 +8,4 @@ dev: mailer: delivery_method: 'letter_opener' + from_email: 'no-reply-dev@example.com' diff --git a/config/settings/test.yml b/config/settings/test.yml index bfc92e5..d2c040b 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -9,10 +9,12 @@ add_ons: api: root_secret_key: 'test_api_key' application: + developer_email: test@example.com host_url: 'lvh.me' host_port: 3000 webhooks: key: 'foobarbaz' - url: 'http://lvh.me' + url: 'http://127.0.0.1:3000' mailer: - delivery_method: 'test' \ No newline at end of file + delivery_method: 'test' + from_email: 'no-reply-test@example.com' \ No newline at end of file diff --git a/db/migrate/20190926185450_add_retries_to_events.rb b/db/migrate/20190926185450_add_retries_to_events.rb new file mode 100644 index 0000000..b5ef091 --- /dev/null +++ b/db/migrate/20190926185450_add_retries_to_events.rb @@ -0,0 +1,5 @@ +class AddRetriesToEvents < ActiveRecord::Migration[5.2] + def change + add_column :events, :retries, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 5905be3..72a8d80 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: 2019_09_19_162249) do +ActiveRecord::Schema.define(version: 2019_09_26_185450) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -31,6 +31,7 @@ t.string "organization_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "retries" t.index ["event_object_id", "event_object_type"], name: "index_events_on_event_object_id_and_event_object_type" t.index ["organization_id"], name: "index_events_on_organization_id" t.index ["type"], name: "index_events_on_type" diff --git a/spec/factories/events.rb b/spec/factories/events.rb index c5a6311..c667ece 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -5,6 +5,7 @@ # id :string not null, primary key # data :text # event_object_type :string +# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null diff --git a/spec/fixtures/developer/webhook_failure b/spec/fixtures/developer/webhook_failure new file mode 100644 index 0000000..93ff7c6 --- /dev/null +++ b/spec/fixtures/developer/webhook_failure @@ -0,0 +1,3 @@ +Developer#webhook_failure + +Hi, find me in app/views/developer/webhook_failure diff --git a/spec/jobs/event_jobs/create_and_emit_spec.rb b/spec/jobs/event_jobs/create_and_emit_spec.rb index df244e3..ec28ae4 100644 --- a/spec/jobs/event_jobs/create_and_emit_spec.rb +++ b/spec/jobs/event_jobs/create_and_emit_spec.rb @@ -14,6 +14,13 @@ let(:perform) { Sidekiq::Testing.inline! { described_class.perform_async(*args) } } let(:result) { described_class.new.perform(*args) } + before do + stub_request( + :post, + Settings.application.webhooks.url + ) + end + it { expect(result).to be_success } it { expect { result.value }.to change(EventJobs::Emit.jobs, :count).from(0).to(1) } end diff --git a/spec/lib/forms/events/create_spec.rb b/spec/lib/forms/events/create_spec.rb index 4fba9dc..c91531a 100644 --- a/spec/lib/forms/events/create_spec.rb +++ b/spec/lib/forms/events/create_spec.rb @@ -23,6 +23,7 @@ it { expect(result).to be_success } it { expect(value).to be_a(Event) } it { expect(JSON.parse(value.data)).to eq(event_object.serialize) } + it { expect(value.retries).to be_zero } describe '#event_object' do it { expect_error_with_missing_attribute(:event_object) } diff --git a/spec/lib/forms/events/schedule_retry_spec.rb b/spec/lib/forms/events/schedule_retry_spec.rb new file mode 100644 index 0000000..1369c25 --- /dev/null +++ b/spec/lib/forms/events/schedule_retry_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'formify/spec_helpers' + +describe Forms::Events::ScheduleRetry, type: :form do + include Formify::SpecHelpers + + + let(:event) { FactoryBot.create(:event) } + + + let(:attributes) do + { + event: event + } + end + + it { expect_valid } + it { expect(result).to be_success } + it { expect(value).to be_a(Event) } + + describe '#event' do + it { expect_error_with_missing_attribute(:event) } + xit { expect_error_with_attribute_value(:event, EVENT_BAD_VALUE, message: nil) } # :message is optional + xit { expect_valid_with_attribute_value(:event, EVENT_GOOD_VALUE) } + end + +end diff --git a/spec/mailers/developer_spec.rb b/spec/mailers/developer_spec.rb new file mode 100644 index 0000000..d43aaa1 --- /dev/null +++ b/spec/mailers/developer_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe DeveloperMailer, type: :mailer do + describe 'webhook_failure' do + let(:event) { FactoryBot.create(:event) } + let(:mail) { described_class.webhook_failure(event.id) } + + it 'renders the headers' do + expect(mail.subject).to include('Webhook failure at ') + expect(mail.to).to eq(['test@example.com']) + expect(mail.from).to eq(['no-reply-test@example.com']) + end + + it 'renders the body' do + expect(mail.body.encoded).to include("Event ID: #{event.id}") + end + end +end diff --git a/spec/mailers/previews/developer_preview.rb b/spec/mailers/previews/developer_preview.rb new file mode 100644 index 0000000..7825b08 --- /dev/null +++ b/spec/mailers/previews/developer_preview.rb @@ -0,0 +1,9 @@ +# Preview all emails at http://localhost:3000/rails/mailers/developer +class DeveloperPreview < ActionMailer::Preview + + # Preview this email at http://localhost:3000/rails/mailers/developer/webhook_failure + def webhook_failure + DeveloperMailer.webhook_failure + end + +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index bc0d7cb..abc49f8 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -5,6 +5,7 @@ # id :string not null, primary key # data :text # event_object_type :string +# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null From a09dfac1696329a2aa47ee2af0b6e5b4fec1521b Mon Sep 17 00:00:00 2001 From: Ryan Jackson Date: Thu, 26 Sep 2019 15:48:27 -0400 Subject: [PATCH 2/6] Add redis to travis-ci --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 98c9853..19d978b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ addons: postgresql: "9.6" services: - postgresql + - redis-server sudo: false language: ruby cache: bundler From 9e018eb65d9f4b934e93674e9c30c55690fab70e Mon Sep 17 00:00:00 2001 From: Ryan Jackson Date: Thu, 26 Sep 2019 19:54:23 -0400 Subject: [PATCH 3/6] Save event on retry --- app/lib/forms/events/schedule_retry.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/lib/forms/events/schedule_retry.rb b/app/lib/forms/events/schedule_retry.rb index 53a4bb4..722970e 100644 --- a/app/lib/forms/events/schedule_retry.rb +++ b/app/lib/forms/events/schedule_retry.rb @@ -45,6 +45,8 @@ def schedule_retry event.retries ||= 0 event.retries += 1 + event.save! + wait_in_seconds = retry_back_off_in_seconds**event.retries EventJobs::Emit.perform_in(wait_in_seconds.seconds, event.id) From 661e31a7a98b8774fd9a459f16d43ea83ff69928 Mon Sep 17 00:00:00 2001 From: Ryan Jackson Date: Sat, 28 Sep 2019 15:52:07 -0400 Subject: [PATCH 4/6] Use sidekiq retries --- app/jobs/event_jobs.rb | 12 ++-- app/lib/forms/events/create.rb | 1 - app/lib/forms/events/emit.rb | 17 +++++- app/lib/forms/events/schedule_retry.rb | 57 ------------------- app/models/event.rb | 1 - app/serializers/event_serializer.rb | 1 - config/initializers/config.rb | 2 - config/settings.yml | 2 - .../20190926185450_add_retries_to_events.rb | 5 -- db/schema.rb | 3 +- spec/factories/events.rb | 1 - spec/lib/forms/events/schedule_retry_spec.rb | 29 ---------- spec/models/event_spec.rb | 1 - 13 files changed, 20 insertions(+), 112 deletions(-) delete mode 100644 app/lib/forms/events/schedule_retry.rb delete mode 100644 db/migrate/20190926185450_add_retries_to_events.rb delete mode 100644 spec/lib/forms/events/schedule_retry_spec.rb diff --git a/app/jobs/event_jobs.rb b/app/jobs/event_jobs.rb index 07c2361..b9b293e 100644 --- a/app/jobs/event_jobs.rb +++ b/app/jobs/event_jobs.rb @@ -19,14 +19,10 @@ def perform(data, event_object_type, event_object_id, organization_id, type) class Emit < ApplicationJob def perform(event_id) event = Event.find(event_id) - begin - Forms::Events::Emit.new( - event: event - ).save.raise_if_error - rescue StandardError => e - Forms::Events::ScheduleRetry.new(event: event).save.raise_if_error - raise e - end + + Forms::Events::Emit.new( + event: event + ).save.raise_if_error end end end diff --git a/app/lib/forms/events/create.rb b/app/lib/forms/events/create.rb index 99d7801..2f8d051 100644 --- a/app/lib/forms/events/create.rb +++ b/app/lib/forms/events/create.rb @@ -39,7 +39,6 @@ def save delegate_accessor :data, to: :event def create_event - event.retries = 0 event.save! success(event) end diff --git a/app/lib/forms/events/emit.rb b/app/lib/forms/events/emit.rb index c4f0262..da76ebb 100644 --- a/app/lib/forms/events/emit.rb +++ b/app/lib/forms/events/emit.rb @@ -38,9 +38,22 @@ def emit response = HTTP.headers(headers).post(url, body: serialized_event_json_string) status = response.status - return failure(status.inspect) unless status.success? + return success(event) if status.success? - success(event) + notify_provider + failure(status.inspect) + end + + def notify_provider + redis = Redis.new + key = 'webhooks/provider_last_notified_at' + + provider_last_notified_at = redis.get(key) + + return if provider_last_notified_at.present? && (Time.zone.now - Time.parse(provider_last_notified_at)) < 24.hours + + redis.set(key, Time.zone.now.to_s) + DeveloperMailer.webhook_failure(event.id).deliver_later end def serialized_event_json_string diff --git a/app/lib/forms/events/schedule_retry.rb b/app/lib/forms/events/schedule_retry.rb deleted file mode 100644 index 722970e..0000000 --- a/app/lib/forms/events/schedule_retry.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -module Forms - module Events - class ScheduleRetry - include Formify::Form - attr_accessor :event - - validates_presence_of :event - - def save - with_advisory_lock_transaction(:events, event) do - validate_or_fail - .and_then { schedule_retry } - .and_then { success(event) } - end - end - - private - - def notify_provider - redis = Redis.new - key = 'webhooks/provider_last_notified_at' - - provider_last_notified_at = redis.get(key) - - return if provider_last_notified_at.present? && (Time.zone.now - Time.parse(provider_last_notified_at)) < 24.hours - - redis.set(key, Time.zone.now.to_s) - DeveloperMailer.webhook_failure(event.id).deliver_later - end - - def retry_back_off_in_seconds - @retry_back_off_in_seconds ||= Settings.application.webhooks.retry_back_off_in_seconds - end - - def retry_limit - @retry_limit ||= Settings.application.webhooks.retry_limit - end - - def schedule_retry - return success(event) if event.retries.present? && event.retries >= retry_limit - - notify_provider - - event.retries ||= 0 - event.retries += 1 - event.save! - - wait_in_seconds = retry_back_off_in_seconds**event.retries - EventJobs::Emit.perform_in(wait_in_seconds.seconds, event.id) - - success(event) - end - end - end -end diff --git a/app/models/event.rb b/app/models/event.rb index b615812..0a401e5 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -5,7 +5,6 @@ # id :string not null, primary key # data :text # event_object_type :string -# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null diff --git a/app/serializers/event_serializer.rb b/app/serializers/event_serializer.rb index 4e69560..1234e74 100644 --- a/app/serializers/event_serializer.rb +++ b/app/serializers/event_serializer.rb @@ -5,7 +5,6 @@ # id :string not null, primary key # data :text # event_object_type :string -# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null diff --git a/config/initializers/config.rb b/config/initializers/config.rb index 3c30619..2461115 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -87,8 +87,6 @@ optional(:theme).maybe(:str?) optional(:webhooks).schema do optional(:url).maybe(:str?) - optional(:retry_back_off_in_seconds).filled(:int?) - optional(:retry_limit).filled(:int?) optional(:key).maybe(:str?) end end diff --git a/config/settings.yml b/config/settings.yml index 96e9a6f..08d04a4 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -28,8 +28,6 @@ application: theme: modern_treasury # optional: Name of theme in app/assets/stylesheets/themes webhooks: key: # required if url present: secret key used for signing webhooks - retry_back_off_in_seconds: 20 # required: retry_back_off_in_seconds ** retry_count - retry_limit: 10 # required url: # optional: URL to which webhooks will be sent authentication: diff --git a/db/migrate/20190926185450_add_retries_to_events.rb b/db/migrate/20190926185450_add_retries_to_events.rb deleted file mode 100644 index b5ef091..0000000 --- a/db/migrate/20190926185450_add_retries_to_events.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddRetriesToEvents < ActiveRecord::Migration[5.2] - def change - add_column :events, :retries, :integer - end -end diff --git a/db/schema.rb b/db/schema.rb index 72a8d80..5905be3 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: 2019_09_26_185450) do +ActiveRecord::Schema.define(version: 2019_09_19_162249) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -31,7 +31,6 @@ t.string "organization_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.integer "retries" t.index ["event_object_id", "event_object_type"], name: "index_events_on_event_object_id_and_event_object_type" t.index ["organization_id"], name: "index_events_on_organization_id" t.index ["type"], name: "index_events_on_type" diff --git a/spec/factories/events.rb b/spec/factories/events.rb index c667ece..c5a6311 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -5,7 +5,6 @@ # id :string not null, primary key # data :text # event_object_type :string -# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null diff --git a/spec/lib/forms/events/schedule_retry_spec.rb b/spec/lib/forms/events/schedule_retry_spec.rb deleted file mode 100644 index 1369c25..0000000 --- a/spec/lib/forms/events/schedule_retry_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' -require 'formify/spec_helpers' - -describe Forms::Events::ScheduleRetry, type: :form do - include Formify::SpecHelpers - - - let(:event) { FactoryBot.create(:event) } - - - let(:attributes) do - { - event: event - } - end - - it { expect_valid } - it { expect(result).to be_success } - it { expect(value).to be_a(Event) } - - describe '#event' do - it { expect_error_with_missing_attribute(:event) } - xit { expect_error_with_attribute_value(:event, EVENT_BAD_VALUE, message: nil) } # :message is optional - xit { expect_valid_with_attribute_value(:event, EVENT_GOOD_VALUE) } - end - -end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index abc49f8..bc0d7cb 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -5,7 +5,6 @@ # id :string not null, primary key # data :text # event_object_type :string -# retries :integer # type :string # created_at :datetime not null # updated_at :datetime not null From 4f34cf29289ac0a6a5a25fc62bfdb9c985779286 Mon Sep 17 00:00:00 2001 From: Ryan Jackson Date: Sat, 28 Sep 2019 15:58:16 -0400 Subject: [PATCH 5/6] Fix tests --- app/views/developer_mailer/webhook_failure.html.haml | 1 - spec/lib/forms/events/create_spec.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/app/views/developer_mailer/webhook_failure.html.haml b/app/views/developer_mailer/webhook_failure.html.haml index c9f453a..d6b9588 100644 --- a/app/views/developer_mailer/webhook_failure.html.haml +++ b/app/views/developer_mailer/webhook_failure.html.haml @@ -2,4 +2,3 @@ %p Event ID: #{@event.id} - Retries: #{@event.retries} diff --git a/spec/lib/forms/events/create_spec.rb b/spec/lib/forms/events/create_spec.rb index c91531a..4fba9dc 100644 --- a/spec/lib/forms/events/create_spec.rb +++ b/spec/lib/forms/events/create_spec.rb @@ -23,7 +23,6 @@ it { expect(result).to be_success } it { expect(value).to be_a(Event) } it { expect(JSON.parse(value.data)).to eq(event_object.serialize) } - it { expect(value.retries).to be_zero } describe '#event_object' do it { expect_error_with_missing_attribute(:event_object) } From 66c296d20a83032a30f376fb66a32555a5a94b9f Mon Sep 17 00:00:00 2001 From: Ryan Jackson Date: Sat, 28 Sep 2019 17:06:18 -0400 Subject: [PATCH 6/6] Add tests --- app/controllers/application_controller.rb | 6 +++++ app/controllers/auth_controller.rb | 3 --- app/controllers/auth_tokens_controller.rb | 2 +- app/controllers/dev/base_controller.rb | 6 +++-- app/controllers/ledgers/test_controller.rb | 24 +++++++++++++++++ app/controllers/ledgers_controller.rb | 4 ++- app/decorators/ledger_decorator.rb | 5 +++- app/errors/routing_error.rb | 9 ------- app/lib/acl.rb | 2 -- app/lib/util/development_helper.rb | 9 +++++++ app/views/layouts/_nav.html.haml | 9 ++++--- app/views/ledgers/test/show.html.haml | 2 ++ config/initializers/warden.rb | 2 +- config/routes.rb | 1 + spec/decorators/ledger_decorator_spec.rb | 5 ++++ spec/features/dev/authentication_spec.rb | 30 ++++++++++++++++++++++ spec/features/dev/home_spec.rb | 15 +++++++++++ spec/features/dev/syncs_spec.rb | 22 ++++++++++++++++ spec/features/ledgers_spec.rb | 24 +++++++++++++++++ spec/lib/util/webhook_signer_spec.rb | 4 +-- spec/rails_helper.rb | 1 + spec/support/feature_helpers.rb | 12 ++++++--- 22 files changed, 168 insertions(+), 29 deletions(-) delete mode 100644 app/controllers/auth_controller.rb create mode 100644 app/controllers/ledgers/test_controller.rb delete mode 100644 app/errors/routing_error.rb delete mode 100644 app/lib/acl.rb create mode 100644 app/lib/util/development_helper.rb create mode 100644 app/views/ledgers/test/show.html.haml create mode 100644 spec/features/dev/authentication_spec.rb create mode 100644 spec/features/dev/home_spec.rb create mode 100644 spec/features/dev/syncs_spec.rb create mode 100644 spec/features/ledgers_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index cff0041..d505f4e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,12 @@ class ApplicationController < ActionController::Base private + def development_enabled? + Util::DevelopmentHelper.development_enabled? + end + + helper_method :development_enabled? + def raise_404(message: nil) raise ActionController::RoutingError, (message || 'Not Found') end diff --git a/app/controllers/auth_controller.rb b/app/controllers/auth_controller.rb deleted file mode 100644 index c804bd8..0000000 --- a/app/controllers/auth_controller.rb +++ /dev/null @@ -1,3 +0,0 @@ -class AuthController < ApplicationController - layout false -end diff --git a/app/controllers/auth_tokens_controller.rb b/app/controllers/auth_tokens_controller.rb index 36dbc1f..5c078f3 100644 --- a/app/controllers/auth_tokens_controller.rb +++ b/app/controllers/auth_tokens_controller.rb @@ -8,7 +8,7 @@ def destroy end def dev_login - raise_404 unless Rails.env.development? + raise_404 unless development_enabled? warden.logout authenticate!(:developer) diff --git a/app/controllers/dev/base_controller.rb b/app/controllers/dev/base_controller.rb index 44a0ac8..3772f91 100644 --- a/app/controllers/dev/base_controller.rb +++ b/app/controllers/dev/base_controller.rb @@ -7,7 +7,9 @@ class BaseController < UIController private def ensure_development - raise_404 unless Rails.env.development? + return if development_enabled? + + raise_404 end end -end \ No newline at end of file +end diff --git a/app/controllers/ledgers/test_controller.rb b/app/controllers/ledgers/test_controller.rb new file mode 100644 index 0000000..f1715fa --- /dev/null +++ b/app/controllers/ledgers/test_controller.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Ledgers + class TestController < DashboardBaseController + before_action :ensure_test_env + before_action :set_ledger, only: %i[show] + + private + + def ensure_test_env + return if Rails.env.test? + + raise_404 + end + + def set_ledger + @ledger = current_organization + .ledgers(kind: LedgerSync.adaptors.test.root_key.to_s) + .object + .find(params[:id]) + .decorate + end + end +end diff --git a/app/controllers/ledgers_controller.rb b/app/controllers/ledgers_controller.rb index 357e147..4802729 100644 --- a/app/controllers/ledgers_controller.rb +++ b/app/controllers/ledgers_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LedgersController < DashboardBaseController before_action :set_ledger, only: :show @@ -10,4 +12,4 @@ def show def set_ledger @ledger = Ledger.find(params[:id]).decorate end -end \ No newline at end of file +end diff --git a/app/decorators/ledger_decorator.rb b/app/decorators/ledger_decorator.rb index e1c5253..8004902 100644 --- a/app/decorators/ledger_decorator.rb +++ b/app/decorators/ledger_decorator.rb @@ -12,9 +12,12 @@ def name end def show_path - case kind + case kind.to_s when Util::QuickBooksOnline::KEY r.ledgers_quickbooks_online_path(self) + when LedgerSync.adaptors.test.root_key.to_s + raise NotImplementedError unless Rails.env.test? + r.ledgers_test_path(self) else raise NotImplementedError end diff --git a/app/errors/routing_error.rb b/app/errors/routing_error.rb deleted file mode 100644 index d5c47f8..0000000 --- a/app/errors/routing_error.rb +++ /dev/null @@ -1,9 +0,0 @@ -class RoutingError < APIError - def initialize - super( - "Invalid route. Please check that the path is typed correctly.", - type: 'routing_error', - status: 404 - ) - end -end diff --git a/app/lib/acl.rb b/app/lib/acl.rb deleted file mode 100644 index a0754c3..0000000 --- a/app/lib/acl.rb +++ /dev/null @@ -1,2 +0,0 @@ -# TODO: Remove before launch. Here for convenience. -ACL = LedgerSync \ No newline at end of file diff --git a/app/lib/util/development_helper.rb b/app/lib/util/development_helper.rb new file mode 100644 index 0000000..bc4be41 --- /dev/null +++ b/app/lib/util/development_helper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Util + module DevelopmentHelper + def self.development_enabled? + Rails.env.development? || (Rails.env.test? && ENV['DEVELOPMENT'] == 'true') + end + end +end diff --git a/app/views/layouts/_nav.html.haml b/app/views/layouts/_nav.html.haml index 9d01423..b155cb9 100644 --- a/app/views/layouts/_nav.html.haml +++ b/app/views/layouts/_nav.html.haml @@ -25,7 +25,8 @@ %li.nav-item.mr-3 = link_to '#', class: 'nav-link' do Log In - - if Rails.env.development? - %li.nav-item.mr-3 - = link_to dev_login_path, class: 'btn btn-danger' do - Dev Log In +- if development_enabled? + %ul + %li.nav-item.mr-3 + = link_to dev_login_path, class: 'btn btn-danger' do + Dev Log In diff --git a/app/views/ledgers/test/show.html.haml b/app/views/ledgers/test/show.html.haml new file mode 100644 index 0000000..a4679b1 --- /dev/null +++ b/app/views/ledgers/test/show.html.haml @@ -0,0 +1,2 @@ +.container + %h1 Test Adaptor - You made it! \ No newline at end of file diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index 5567223..2f76462 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -18,7 +18,7 @@ def result Warden::Strategies.add(:developer) do def valid? - raise 'Invalid strategy' unless Rails.env.development? + raise 'Invalid strategy' unless Util::DevelopmentHelper.development_enabled? true end diff --git a/config/routes.rb b/config/routes.rb index 981a11f..9b63dc9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -98,6 +98,7 @@ resources :ledgers, only: %i[index show] namespace :ledgers do + resources :tests, path: :test, only: %i[show], controller: :test resources :quickbooks_onlines, path: :quickbooks_online, only: %i[destroy new show update], controller: :quickbooks_online do collection do get :callback diff --git a/spec/decorators/ledger_decorator_spec.rb b/spec/decorators/ledger_decorator_spec.rb index d3139e3..030a726 100644 --- a/spec/decorators/ledger_decorator_spec.rb +++ b/spec/decorators/ledger_decorator_spec.rb @@ -13,6 +13,11 @@ it do ledger = create(:ledger, :test) + expect(ledger.decorate.show_path).to eq(r.ledgers_test_path(ledger)) + end + + it do + ledger = create(:ledger, kind: :asdf) expect { ledger.decorate.show_path }.to raise_error(NotImplementedError) end end diff --git a/spec/features/dev/authentication_spec.rb b/spec/features/dev/authentication_spec.rb new file mode 100644 index 0000000..4e7c27d --- /dev/null +++ b/spec/features/dev/authentication_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'formify/spec_helpers' + +describe 'dev/authentication', js: true, type: :feature do + def expect_authorized + expect_content('authorized') + end + + def expect_unauthorized + expect_content('unauthorized') + end + + it do + visit r.root_path + expect_no_content :invisible, 'Dev Log In' + visit r.auth_tokens_path + expect_unauthorized + end + + it do + ClimateControl.modify(DEVELOPMENT: 'true') do + visit r.root_path + click_on 'Dev Log In' + visit r.auth_tokens_path + expect_authorized + end + end +end diff --git a/spec/features/dev/home_spec.rb b/spec/features/dev/home_spec.rb new file mode 100644 index 0000000..a68df67 --- /dev/null +++ b/spec/features/dev/home_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'formify/spec_helpers' + +describe 'dev/home', js: true, type: :feature do + it do + ClimateControl.modify(DEVELOPMENT: 'true') do + visit r.root_path + click_on 'Dev Log In' + visit r.dev_path + expect_content 'Create Sync' + end + end +end diff --git a/spec/features/dev/syncs_spec.rb b/spec/features/dev/syncs_spec.rb new file mode 100644 index 0000000..a0f8276 --- /dev/null +++ b/spec/features/dev/syncs_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'sync_ledgers/create', js: true, type: :feature do + xit do + login + expect { visit r.new_dev_sync_path }.to raise_error(ActionController::RoutingError) + end + + it do + ClimateControl.modify(DEVELOPMENT: 'true') do + login(FactoryBot.create(:user, :admin)) + visit r.new_dev_sync_path + expect_content 'Create Sync' + expect do + click_on 'Send' + expect_content 'Operation' + end.to change(Sync, :count).from(0).to(1) + end + end +end diff --git a/spec/features/ledgers_spec.rb b/spec/features/ledgers_spec.rb new file mode 100644 index 0000000..41a4276 --- /dev/null +++ b/spec/features/ledgers_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'ledgers/test', js: true, type: :feature do + let(:ledger) { FactoryBot.create(:ledger) } + + it do + login + visit r.ledgers_path + expect_content 'Ledgers' + expect_content 'Connect QuickBooks' + end + + it do + login + ledger + visit r.ledgers_path + expect_content 'Test Ledger Adaptor' + click_on 'View' + expect_content 'You made it!' + expect_path ledger.decorate.show_path + end +end diff --git a/spec/lib/util/webhook_signer_spec.rb b/spec/lib/util/webhook_signer_spec.rb index 20b256f..1a6af6d 100644 --- a/spec/lib/util/webhook_signer_spec.rb +++ b/spec/lib/util/webhook_signer_spec.rb @@ -24,12 +24,12 @@ end end - it { expect { subject }.to raise_error } + it { expect { subject }.to raise_error(RuntimeError, 'Missing Settings.application.webhooks.key') } end context 'without data as string' do let(:data) { {} } - it { expect { subject }.to raise_error } + it { expect { subject }.to raise_error(RuntimeError, 'data must be a string') } end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 6dd2e54..0d9877f 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -13,6 +13,7 @@ ) SimpleCov.start do + add_filter '/channels' add_filter '/config/initializers' add_filter '/spec/support' end diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index 70f8632..ece2e56 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -31,10 +31,16 @@ def expect_404 # end # end - def expect_content(content) - raise 'content cannot be blank' if content.blank? + def expect_content(*args, **keywords) + raise 'content cannot be blank' if args.last.blank? - expect(page).to have_content(content, wait: 10) + expect(page).to have_content(*args, wait: 10, **keywords) + end + + def expect_no_content(*args, **keywords) + raise 'content cannot be blank' if args.last.blank? + + expect(page).not_to have_content(*args, wait: 10, **keywords) end def expect_count(selector, num = 1)