From 27baa935c1aef0ce8e226f5f0da66eb6989f11d5 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Sat, 13 Mar 2021 15:04:57 -0800 Subject: [PATCH 01/55] Begin writing tests for Mailbluster API calls --- app/helpers/external/mailbluster_helper.rb | 9 +++++++++ spec/helpers/external/mailbluster_helper_spec.rb | 16 ++++++++++++++++ spec/spec_helper.rb | 4 ++++ 3 files changed, 29 insertions(+) create mode 100644 app/helpers/external/mailbluster_helper.rb create mode 100644 spec/helpers/external/mailbluster_helper_spec.rb diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb new file mode 100644 index 00000000..2f57fa26 --- /dev/null +++ b/app/helpers/external/mailbluster_helper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module External + module Mailbluster + def create_lead(user) + # TODO + end + end +end \ No newline at end of file diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb new file mode 100644 index 00000000..b6d3016e --- /dev/null +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'webmock/rspec' + +describe MailblusterHelper, type: :helper do + let!(:user) { create(:user) } + + describe 'create_lead' do + it 'makes a post request to Mailbluster\'s API' do + create_lead(@user) + expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads"). + with { |req| req.body == "abc" } + end + end +end \ No newline at end of file diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2d680fb2..32fc961c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,6 +12,10 @@ SimpleCov.start 'rails' ENV['RAILS_ENV'] ||= 'test' require File.expand_path('../../config/environment', __FILE__) +# Prevent tests from making calls to the Internet +require 'webmock/rspec' +WebMock.disable_net_connect!(allow_localhost: true) + require 'rspec/rails' require 'shoulda/matchers' require 'webdrivers' From 5500fc8acf589f00b116b56ce7a73b4ef9ecd17d Mon Sep 17 00:00:00 2001 From: Ziyi Date: Sat, 13 Mar 2021 15:22:19 -0800 Subject: [PATCH 02/55] Init create_lead, fix MailblusterHelper name --- app/helpers/external/mailbluster_helper.rb | 21 +++++++++++++++++-- .../external/mailbluster_helper_spec.rb | 4 ++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 2f57fa26..c7e42a83 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -1,9 +1,26 @@ # frozen_string_literal: true module External - module Mailbluster + module MailblusterHelper def create_lead(user) # TODO + begin + uri = URI("api.mailbluster.com/api/leads") + http = Net::HTTP.new(uri.host, uri.port) + request = Net::HTTP::Post.new(uri.path, + {'Authorization' => ENV["MAILBLUSTER_API_KEY"]}) # TODO Authorization=APIKEY + request.body = {'firstName' => user.name, + 'email' => user.email, + 'subscribed' => true, + 'tags' => ["snapcon"], + 'overrideExisting' => true}.to_json + response = http.request(request) + puts response + return true + rescue => e + puts "ERROR #{e}" + return false + end end end -end \ No newline at end of file +end diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index b6d3016e..86881118 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'webmock/rspec' -describe MailblusterHelper, type: :helper do +describe External::MailblusterHelper, type: :helper do let!(:user) { create(:user) } describe 'create_lead' do @@ -13,4 +13,4 @@ describe MailblusterHelper, type: :helper do with { |req| req.body == "abc" } end end -end \ No newline at end of file +end From cc579731037eeed788445c03bdce622040703144 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Sat, 13 Mar 2021 15:29:25 -0800 Subject: [PATCH 03/55] Fixed spec @user --- app/helpers/external/mailbluster_helper.rb | 3 ++- spec/helpers/external/mailbluster_helper_spec.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index c7e42a83..474190bc 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -5,7 +5,7 @@ module External def create_lead(user) # TODO begin - uri = URI("api.mailbluster.com/api/leads") + uri = URI("http://api.mailbluster.com/api/leads") http = Net::HTTP.new(uri.host, uri.port) request = Net::HTTP::Post.new(uri.path, {'Authorization' => ENV["MAILBLUSTER_API_KEY"]}) # TODO Authorization=APIKEY @@ -14,6 +14,7 @@ module External 'subscribed' => true, 'tags' => ["snapcon"], 'overrideExisting' => true}.to_json + puts request response = http.request(request) puts response return true diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 86881118..aa30f8ca 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -8,7 +8,7 @@ describe External::MailblusterHelper, type: :helper do describe 'create_lead' do it 'makes a post request to Mailbluster\'s API' do - create_lead(@user) + create_lead(user) expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads"). with { |req| req.body == "abc" } end From d99487b19c73297965b40064b44acc76d7a0f055 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Sun, 14 Mar 2021 13:25:36 -0700 Subject: [PATCH 04/55] Start setting up WebMock to stub responses --- .../external/mailbluster_helper_spec.rb | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index aa30f8ca..75617259 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -8,9 +8,38 @@ describe External::MailblusterHelper, type: :helper do describe 'create_lead' do it 'makes a post request to Mailbluster\'s API' do + stub_request(:post, "http://api.mailbluster.com/api/leads") + .to_return(body: '{ + "message": "Lead created", + "lead": { + "id": 329395, + "firstName": "Richard", + "lastName": "Hendricks", + "fullName": "Richard Hendricks", + "email": "richard@example.com", + "timezone": "America/Los_Angeles", + "ipAddress": "162.213.1.246", + "subscribed": false, + "meta": { + "company": "Pied Piper", + "role": "CEO", + "continent": "North America", + "country": "United States", + "city": "San Jose", + "latitude": 37.3008, + "longitude": -121.9777, + "source": "Lead API" + }, + "tags": [ + "iPhone User", + "Startup" + ], + "createdAt": "2016-07-23T08:03:18.954Z", + "updatedAt": "2016-07-23T08:03:18.954Z" + } + }', status: 200) create_lead(user) - expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads"). - with { |req| req.body == "abc" } + expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads") end end end From 209d8480795938f537d8cfbb5adc5949c25f3fc2 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Sun, 14 Mar 2021 13:43:20 -0700 Subject: [PATCH 05/55] Test mailbluster request body --- app/helpers/external/mailbluster_helper.rb | 8 ++++---- spec/helpers/external/mailbluster_helper_spec.rb | 12 +++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 474190bc..ae2b4b7b 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -14,13 +14,13 @@ module External 'subscribed' => true, 'tags' => ["snapcon"], 'overrideExisting' => true}.to_json - puts request + puts request.body.to_json response = http.request(request) - puts response - return true + puts response.body.to_json + return response.to_json rescue => e puts "ERROR #{e}" - return false + return nil end end end diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 75617259..f3f45640 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -31,15 +31,17 @@ describe External::MailblusterHelper, type: :helper do "source": "Lead API" }, "tags": [ - "iPhone User", - "Startup" + "snapcon" ], - "createdAt": "2016-07-23T08:03:18.954Z", - "updatedAt": "2016-07-23T08:03:18.954Z" } }', status: 200) create_lead(user) - expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads") + expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads").with(body: { + 'firstName': user.name, + 'email': user.email, + 'subscribed': true, + 'tags': ["snapcon"], + 'overrideExisting': true}) end end end From bad73c5d8be11551cae1372b52d89ed38ee6987d Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Sun, 14 Mar 2021 13:54:44 -0700 Subject: [PATCH 06/55] Get WebMock matchers to work --- app/helpers/external/mailbluster_helper.rb | 6 ++++-- spec/helpers/external/mailbluster_helper_spec.rb | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index ae2b4b7b..11501c43 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -9,11 +9,13 @@ module External http = Net::HTTP.new(uri.host, uri.port) request = Net::HTTP::Post.new(uri.path, {'Authorization' => ENV["MAILBLUSTER_API_KEY"]}) # TODO Authorization=APIKEY - request.body = {'firstName' => user.name, + request.body = { 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, 'subscribed' => true, 'tags' => ["snapcon"], - 'overrideExisting' => true}.to_json + }.to_json puts request.body.to_json response = http.request(request) puts response.body.to_json diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index f3f45640..ea680b6c 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -37,11 +37,12 @@ describe External::MailblusterHelper, type: :helper do }', status: 200) create_lead(user) expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads").with(body: { - 'firstName': user.name, 'email': user.email, + 'firstName': user.name, + 'overrideExisting': true, 'subscribed': true, 'tags': ["snapcon"], - 'overrideExisting': true}) + }.to_json) end end end From f686f05710772be7ffd488295f8f2f38150f6099 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 16 Mar 2021 14:15:39 -0700 Subject: [PATCH 07/55] Make return stub value more accurate --- .../external/mailbluster_helper_spec.rb | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index ea680b6c..57a99977 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -9,32 +9,20 @@ describe External::MailblusterHelper, type: :helper do describe 'create_lead' do it 'makes a post request to Mailbluster\'s API' do stub_request(:post, "http://api.mailbluster.com/api/leads") - .to_return(body: '{ + .to_return(body: `{ "message": "Lead created", "lead": { "id": 329395, - "firstName": "Richard", - "lastName": "Hendricks", - "fullName": "Richard Hendricks", - "email": "richard@example.com", - "timezone": "America/Los_Angeles", - "ipAddress": "162.213.1.246", - "subscribed": false, - "meta": { - "company": "Pied Piper", - "role": "CEO", - "continent": "North America", - "country": "United States", - "city": "San Jose", - "latitude": 37.3008, - "longitude": -121.9777, - "source": "Lead API" - }, + "firstName": "#{user.name}", + "lastName": "", + "fullName": "#{user.name}", + "email": "#{user.email}", + "subscribed": true, "tags": [ "snapcon" ], } - }', status: 200) + }`, status: 200) create_lead(user) expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads").with(body: { 'email': user.email, From 340865ddbd646a89780755c5a1fe6179f90ef562 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Tue, 16 Mar 2021 14:28:39 -0700 Subject: [PATCH 08/55] Put user.variables in Mailbluster create test --- app/helpers/external/mailbluster_helper.rb | 12 +++++-- .../external/mailbluster_helper_spec.rb | 32 ++++++++++--------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 11501c43..57527267 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -2,6 +2,10 @@ module External module MailblusterHelper + def query_api() + + end + def create_lead(user) # TODO begin @@ -16,14 +20,18 @@ module External 'subscribed' => true, 'tags' => ["snapcon"], }.to_json - puts request.body.to_json response = http.request(request) - puts response.body.to_json + # puts request.body.to_json + # puts response.body.to_json return response.to_json rescue => e puts "ERROR #{e}" return nil end end + + def delete_lead(user) + + end end end diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 57a99977..c015033c 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -8,22 +8,23 @@ describe External::MailblusterHelper, type: :helper do describe 'create_lead' do it 'makes a post request to Mailbluster\'s API' do + response_body = "{ + \"message\": \"Lead created\", + \"lead\": { + \"id\": 329395, + \"firstName\": \"#{user.name}\", + \"lastName\": \"\", + \"fullName\": \"#{user.name}\", + \"email\": \"#{user.email}\", + \"subscribed\": true, + \"tags\": [ + \"snapcon\" + ], + } + }" stub_request(:post, "http://api.mailbluster.com/api/leads") - .to_return(body: `{ - "message": "Lead created", - "lead": { - "id": 329395, - "firstName": "#{user.name}", - "lastName": "", - "fullName": "#{user.name}", - "email": "#{user.email}", - "subscribed": true, - "tags": [ - "snapcon" - ], - } - }`, status: 200) - create_lead(user) + .to_return(body: response_body, status: 200) + response = create_lead(user) expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads").with(body: { 'email': user.email, 'firstName': user.name, @@ -31,6 +32,7 @@ describe External::MailblusterHelper, type: :helper do 'subscribed': true, 'tags': ["snapcon"], }.to_json) + expect(response).to eq(response_body) end end end From 4db96208e8179a53b326d3d9708b52e9e8b897b8 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 16 Mar 2021 15:16:56 -0700 Subject: [PATCH 09/55] Refactor Mailbluster API URL into a constant --- app/helpers/external/mailbluster_helper.rb | 47 +++++++++++----------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 57527267..d9b4062c 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -2,36 +2,35 @@ module External module MailblusterHelper - def query_api() - - end + MAILBLUSTER_URL = 'https://api.mailbluster.com/api/leads' + + def query_api; end def create_lead(user) # TODO - begin - uri = URI("http://api.mailbluster.com/api/leads") - http = Net::HTTP.new(uri.host, uri.port) - request = Net::HTTP::Post.new(uri.path, - {'Authorization' => ENV["MAILBLUSTER_API_KEY"]}) # TODO Authorization=APIKEY - request.body = { - 'email' => user.email, - 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, - 'tags' => ["snapcon"], - }.to_json - response = http.request(request) - # puts request.body.to_json - # puts response.body.to_json - return response.to_json - rescue => e - puts "ERROR #{e}" - return nil - end + + uri = URI(MAILBLUSTER_URL) + http = Net::HTTP.new(uri.host, uri.port) + request = Net::HTTP::Post.new(uri.path, + 'Authorization' => ENV['MAILBLUSTER_API_KEY']) # TODO: Authorization=APIKEY + request.body = { + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'tags' => ['snapcon'] + }.to_json + response = http.request(request) + # puts request.body.to_json + # puts response.body.to_json + response.to_json + rescue StandardError => e + puts "ERROR #{e}" + nil end def delete_lead(user) - + # TODO end end end From 59908e60e41beed9bca58a9ed754005001cefc99 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 16 Mar 2021 15:22:15 -0700 Subject: [PATCH 10/55] Fix rubocop complaints --- .../helpers/external/mailbluster_helper_spec.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index c015033c..aa3662e8 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' require 'webmock/rspec' -describe External::MailblusterHelper, type: :helper do +describe External::MailblusterHelper, type: :helper do let!(:user) { create(:user) } describe 'create_lead' do - it 'makes a post request to Mailbluster\'s API' do + it 'makes a post request to Mailbluster\'s API and gets the correct response' do response_body = "{ \"message\": \"Lead created\", \"lead\": { @@ -22,15 +22,16 @@ describe External::MailblusterHelper, type: :helper do ], } }" - stub_request(:post, "http://api.mailbluster.com/api/leads") + stub_request(:post, 'https://api.mailbluster.com/api/leads') .to_return(body: response_body, status: 200) response = create_lead(user) - expect(WebMock).to have_requested(:post, "api.mailbluster.com/api/leads").with(body: { - 'email': user.email, - 'firstName': user.name, + + expect(WebMock).to have_requested(:post, 'api.mailbluster.com/api/leads').with(body: { + 'email': user.email, + 'firstName': user.name, 'overrideExisting': true, - 'subscribed': true, - 'tags': ["snapcon"], + 'subscribed': true, + 'tags': ['snapcon'] }.to_json) expect(response).to eq(response_body) end From fddd6ecd9f1b77f7b2206b94b2ca4aaa1d3d53b3 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 16 Mar 2021 15:22:28 -0700 Subject: [PATCH 11/55] Add spec for deleting leads --- spec/helpers/external/mailbluster_helper_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index aa3662e8..143d0834 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -36,4 +36,17 @@ describe External::MailblusterHelper, type: :helper do expect(response).to eq(response_body) end end + + describe 'delete_lead' do + it 'correctly requests the right URL and gets a valid response' do + email_hash = Digest::MD5.hexdigest user.email + response_body = "{\"message\":\"Lead deleted\",\"leadHash\":\"#{email_hash}\"}" + stub_request(:delete, "https://api.mailbluster.com/api/leads/#{email_hash}") + .to_return(body: response_body) + response = delete_lead(user) + + expect(WebMock).to have_requested(:delete, "api.mailbluster.com/api/leads/#{email_hash}") + expect(response).to eq(response_body) + end + end end From 125c17838ca32826a13b6e782918dce70f88d2d5 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Wed, 17 Mar 2021 14:54:57 -0700 Subject: [PATCH 12/55] delete_lead WIP --- app/helpers/external/mailbluster_helper.rb | 23 ++++++++++++------- app/models/user.rb | 2 ++ .../external/mailbluster_helper_spec.rb | 13 +++++++---- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index d9b4062c..e7fb1788 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -2,17 +2,17 @@ module External module MailblusterHelper - MAILBLUSTER_URL = 'https://api.mailbluster.com/api/leads' + MAILBLUSTER_URL = 'https://api.mailbluster.com/api/leads/' - def query_api; end + # def query_api(user, method) + # TODO? General helper for all queries + # end def create_lead(user) - # TODO - uri = URI(MAILBLUSTER_URL) http = Net::HTTP.new(uri.host, uri.port) request = Net::HTTP::Post.new(uri.path, - 'Authorization' => ENV['MAILBLUSTER_API_KEY']) # TODO: Authorization=APIKEY + 'Authorization' => ENV['MAILBLUSTER_API_KEY']) request.body = { 'email' => user.email, 'firstName' => user.name, @@ -21,8 +21,6 @@ module External 'tags' => ['snapcon'] }.to_json response = http.request(request) - # puts request.body.to_json - # puts response.body.to_json response.to_json rescue StandardError => e puts "ERROR #{e}" @@ -30,7 +28,16 @@ module External end def delete_lead(user) - # TODO + email_hash = Digest::MD5.hexdigest user.email + uri = URI(MAILBLUSTER_URL + email_hash) + http = Net::HTTP.new(uri.host, uri.port) + request = Net::HTTP::Delete.new(uri.path, + 'Authorization' => ENV['MAILBLUSTER_API_KEY']) + response = http.request(request) + response.to_json + rescue StandardError => e + puts "ERROR #{e}" + nil end end end diff --git a/app/models/user.rb b/app/models/user.rb index 955c76d1..01ddf925 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -361,6 +361,8 @@ class User < ApplicationRecord User.count == 1 && User.first.email == 'deleted@localhost.osem' end + # TODO email_hash function for mailbluster + private def setup_role diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 143d0834..6c3f1731 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -5,6 +5,7 @@ require 'webmock/rspec' describe External::MailblusterHelper, type: :helper do let!(:user) { create(:user) } + url = 'https://api.mailbluster.com/api/leads/' describe 'create_lead' do it 'makes a post request to Mailbluster\'s API and gets the correct response' do @@ -22,7 +23,7 @@ describe External::MailblusterHelper, type: :helper do ], } }" - stub_request(:post, 'https://api.mailbluster.com/api/leads') + stub_request(:post, url) .to_return(body: response_body, status: 200) response = create_lead(user) @@ -40,12 +41,16 @@ describe External::MailblusterHelper, type: :helper do describe 'delete_lead' do it 'correctly requests the right URL and gets a valid response' do email_hash = Digest::MD5.hexdigest user.email - response_body = "{\"message\":\"Lead deleted\",\"leadHash\":\"#{email_hash}\"}" - stub_request(:delete, "https://api.mailbluster.com/api/leads/#{email_hash}") + response_body = "{ + \"message\":\"Lead deleted\", + \"leadHash\":\"#{email_hash}\" + }" + lead_url = url + email_hash.to_s + stub_request(:delete, lead_url) .to_return(body: response_body) response = delete_lead(user) - expect(WebMock).to have_requested(:delete, "api.mailbluster.com/api/leads/#{email_hash}") + expect(WebMock).to have_requested(:delete, lead_url) expect(response).to eq(response_body) end end From 0f6881414bd5212704321c32b71cdb66b06a59f8 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 17 Mar 2021 15:06:24 -0700 Subject: [PATCH 13/55] Fix up spec to properly pass under expectations --- app/helpers/external/mailbluster_helper.rb | 4 ++-- spec/helpers/external/mailbluster_helper_spec.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index e7fb1788..d00f7afa 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -21,7 +21,7 @@ module External 'tags' => ['snapcon'] }.to_json response = http.request(request) - response.to_json + response.body rescue StandardError => e puts "ERROR #{e}" nil @@ -34,7 +34,7 @@ module External request = Net::HTTP::Delete.new(uri.path, 'Authorization' => ENV['MAILBLUSTER_API_KEY']) response = http.request(request) - response.to_json + response.body rescue StandardError => e puts "ERROR #{e}" nil diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 6c3f1731..7371ba21 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -5,7 +5,7 @@ require 'webmock/rspec' describe External::MailblusterHelper, type: :helper do let!(:user) { create(:user) } - url = 'https://api.mailbluster.com/api/leads/' + url = 'http://api.mailbluster.com:443/api/leads/' describe 'create_lead' do it 'makes a post request to Mailbluster\'s API and gets the correct response' do @@ -27,7 +27,7 @@ describe External::MailblusterHelper, type: :helper do .to_return(body: response_body, status: 200) response = create_lead(user) - expect(WebMock).to have_requested(:post, 'api.mailbluster.com/api/leads').with(body: { + expect(WebMock).to have_requested(:post, url).with(body: { 'email': user.email, 'firstName': user.name, 'overrideExisting': true, @@ -44,7 +44,7 @@ describe External::MailblusterHelper, type: :helper do response_body = "{ \"message\":\"Lead deleted\", \"leadHash\":\"#{email_hash}\" - }" + }" lead_url = url + email_hash.to_s stub_request(:delete, lead_url) .to_return(body: response_body) From 803c98b158b8272dfcb9fde33e2a8dbc0b272124 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 17 Mar 2021 15:20:01 -0700 Subject: [PATCH 14/55] Ensure Mailbluster helper methods actually correctly cause changes in the end service --- app/helpers/external/mailbluster_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index d00f7afa..984c76f5 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -11,8 +11,10 @@ module External def create_lead(user) uri = URI(MAILBLUSTER_URL) http = Net::HTTP.new(uri.host, uri.port) + http.use_ssl = true request = Net::HTTP::Post.new(uri.path, 'Authorization' => ENV['MAILBLUSTER_API_KEY']) + request['Content-Type'] = 'application/json' request.body = { 'email' => user.email, 'firstName' => user.name, @@ -31,8 +33,10 @@ module External email_hash = Digest::MD5.hexdigest user.email uri = URI(MAILBLUSTER_URL + email_hash) http = Net::HTTP.new(uri.host, uri.port) + http.use_ssl = true request = Net::HTTP::Delete.new(uri.path, 'Authorization' => ENV['MAILBLUSTER_API_KEY']) + request['Content-Type'] = 'application/json' response = http.request(request) response.body rescue StandardError => e From 59cc57ebf837873d24a6d54aa79743b50cce3f77 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 19 Mar 2021 10:10:33 -0700 Subject: [PATCH 15/55] Use OSEM_NAME if available --- app/helpers/external/mailbluster_helper.rb | 2 +- spec/helpers/external/mailbluster_helper_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 984c76f5..894c9fc8 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -20,7 +20,7 @@ module External 'firstName' => user.name, 'overrideExisting' => true, 'subscribed' => true, - 'tags' => ['snapcon'] + 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] }.to_json response = http.request(request) response.body diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 7371ba21..7cb1560d 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -19,7 +19,7 @@ describe External::MailblusterHelper, type: :helper do \"email\": \"#{user.email}\", \"subscribed\": true, \"tags\": [ - \"snapcon\" + #{ENV['OSEM_NAME'] || 'snapcon'} ], } }" @@ -32,7 +32,7 @@ describe External::MailblusterHelper, type: :helper do 'firstName': user.name, 'overrideExisting': true, 'subscribed': true, - 'tags': ['snapcon'] + 'tags': [ENV['OSEM_NAME'] || 'snapcon'] }.to_json) expect(response).to eq(response_body) end From bbad082c21bc8d5dd12b2e5ca4e1344cb298f2b5 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 19 Mar 2021 10:11:07 -0700 Subject: [PATCH 16/55] Match actually used URL in helper --- spec/helpers/external/mailbluster_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 7cb1560d..2734be00 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -5,7 +5,7 @@ require 'webmock/rspec' describe External::MailblusterHelper, type: :helper do let!(:user) { create(:user) } - url = 'http://api.mailbluster.com:443/api/leads/' + url = 'https://api.mailbluster.com/api/leads/' describe 'create_lead' do it 'makes a post request to Mailbluster\'s API and gets the correct response' do From 09becbc3e0f81cf2a10db4c85cb64074ca22a404 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 19 Mar 2021 10:11:18 -0700 Subject: [PATCH 17/55] Prepare to add edit_lead spec --- app/helpers/external/mailbluster_helper.rb | 4 ++++ spec/helpers/external/mailbluster_helper_spec.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 894c9fc8..bc185d22 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -29,6 +29,10 @@ module External nil end + def edit_lead(user, add_tags, remove_tags) + puts 'PENDING' + end + def delete_lead(user) email_hash = Digest::MD5.hexdigest user.email uri = URI(MAILBLUSTER_URL + email_hash) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 2734be00..fa94ed37 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -38,6 +38,10 @@ describe External::MailblusterHelper, type: :helper do end end + describe 'edit_lead' do + pending + end + describe 'delete_lead' do it 'correctly requests the right URL and gets a valid response' do email_hash = Digest::MD5.hexdigest user.email From f98f35ec8d7b2897739753b0b538c93a59ae33fa Mon Sep 17 00:00:00 2001 From: Ziyi Date: Fri, 19 Mar 2021 11:13:39 -0700 Subject: [PATCH 18/55] Called create_lead in model, FactoryBot errors --- app/models/user.rb | 9 +++++++++ spec/factories/users.rb | 19 +++++++++++++++++++ spec/models/user_spec.rb | 39 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 01ddf925..9ff90c16 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -80,6 +80,8 @@ class User < ApplicationRecord after_save :touch_events + after_commit :mailbluster_create_lead, on: :create + # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} @@ -362,6 +364,9 @@ class User < ApplicationRecord end # TODO email_hash function for mailbluster + # def email_hash + # Digest::MD5.hexdigest user.email + # end private @@ -369,6 +374,10 @@ class User < ApplicationRecord self.is_admin = true if User.empty? end + def mailbluster_create_lead + ApplicationController.helpers.create_lead(self) + end + def touch_events event_users.each(&:touch) end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index fcec2048..8d04f5f7 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -80,8 +80,27 @@ FactoryBot.define do last_sign_in_at { Date.today } is_disabled { false } + url_mailbluster = 'https://api.mailbluster.com/api/leads/' + response_body = "{ + \"message\": \"Lead created\", + \"lead\": { + \"id\": 329395, + \"firstName\": \"#{name}\", + \"lastName\": \"\", + \"fullName\": \"#{name}\", + \"email\": \"#{email}\", + \"subscribed\": true, + \"tags\": [ + #{ENV['OSEM_NAME'] || 'snapcon'} + ], + } + }" + WebMock.stub_request(:post, url_mailbluster) + # Called by every user creation + after(:create) do |user| user.is_admin = false + # save with bang cause we want change in DB and not just in object instance user.save! end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2e890949..7d700853 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -49,7 +49,7 @@ require 'spec_helper' describe User do - + let(:user_admin) { create(:admin) } let(:conference) { create(:conference, short_title: 'oSC16', title: 'openSUSE Conference 2016') } let(:conference2) { create(:conference, short_title: 'oSC15', title: 'openSUSE Conference 2015') } @@ -530,4 +530,41 @@ describe User do expect(User.omniauth_providers).to eq [:suse, :google, :facebook, :github, :discourse] end end + + describe 'mailbluster' do + it 'creates a Mailbluster lead on creating a user' do + user_mailbluster = User.new(name: 'John Doe', email: 'snap@qwertyuiop.org') + + response_body = "{ + \"message\": \"Lead created\", + \"lead\": { + \"id\": 329395, + \"firstName\": \"#{user_mailbluster.name}\", + \"lastName\": \"\", + \"fullName\": \"#{user_mailbluster.name}\", + \"email\": \"#{user_mailbluster.email}\", + \"subscribed\": true, + \"tags\": [ + #{ENV['OSEM_NAME'] || 'snapcon'} + ], + } + }" + stub_request(:post, url_mailbluster) + .to_return(body: response_body, status: 200) + + # Do not request before save + expect(WebMock).not_to have_requested(:post, url) + + # Request after save + user_mailbluster.save! + + expect(WebMock).to have_requested(:post, url).with(body: { + 'email': user_mailbluster.email, + 'firstName': user_mailbluster.name, + 'overrideExisting': true, + 'subscribed': true, + 'tags': [ENV['OSEM_NAME'] || 'snapcon'] + }.to_json) + end + end end From 5ff93061d96b29c90aae2cedbd3fe0d2a536c3fd Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 19 Mar 2021 16:50:27 -0700 Subject: [PATCH 19/55] Properly stub MailBluster calls in FactoryBot user creation --- spec/factories/users.rb | 34 ++++++++++++++++++---------------- spec/models/user_spec.rb | 5 +++-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 8d04f5f7..fdd60d67 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -80,22 +80,24 @@ FactoryBot.define do last_sign_in_at { Date.today } is_disabled { false } - url_mailbluster = 'https://api.mailbluster.com/api/leads/' - response_body = "{ - \"message\": \"Lead created\", - \"lead\": { - \"id\": 329395, - \"firstName\": \"#{name}\", - \"lastName\": \"\", - \"fullName\": \"#{name}\", - \"email\": \"#{email}\", - \"subscribed\": true, - \"tags\": [ - #{ENV['OSEM_NAME'] || 'snapcon'} - ], - } - }" - WebMock.stub_request(:post, url_mailbluster) + after(:build) do |user| + url_mailbluster = 'https://api.mailbluster.com/api/leads/' + response_body = "{ + \"message\": \"Lead created\", + \"lead\": { + \"id\": 329395, + \"firstName\": \"#{user.name}\", + \"lastName\": \"\", + \"fullName\": \"#{user.name}\", + \"email\": \"#{user.email}\", + \"subscribed\": true, + \"tags\": [ + #{ENV['OSEM_NAME'] || 'snapcon'} + ], + } + }" + WebMock.stub_request(:post, url_mailbluster) + end # Called by every user creation after(:create) do |user| diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7d700853..5df95d61 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -533,7 +533,8 @@ describe User do describe 'mailbluster' do it 'creates a Mailbluster lead on creating a user' do - user_mailbluster = User.new(name: 'John Doe', email: 'snap@qwertyuiop.org') + user_mailbluster = build(:user) + url = 'https://api.mailbluster.com/api/leads/' response_body = "{ \"message\": \"Lead created\", @@ -549,7 +550,7 @@ describe User do ], } }" - stub_request(:post, url_mailbluster) + stub_request(:post, url) .to_return(body: response_body, status: 200) # Do not request before save From 92926effa54b4ee45d6188e27ab2f670412cb31a Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 23 Mar 2021 12:55:04 -0700 Subject: [PATCH 20/55] Use HTTParty to make connection logic much simpler --- Gemfile | 3 + Gemfile.lock | 1 + app/helpers/external/mailbluster_helper.rb | 64 +++++++++++++--------- 3 files changed, 43 insertions(+), 25 deletions(-) diff --git a/Gemfile b/Gemfile index 05c7a4bd..4391debb 100644 --- a/Gemfile +++ b/Gemfile @@ -221,6 +221,9 @@ gem 'dalli' gem 'icalendar' +# for making external requests easier +gem 'httparty' + # Use guard and spring for testing in development group :development do # to launch specs when files are modified diff --git a/Gemfile.lock b/Gemfile.lock index 5b7365da..8d32a279 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -714,6 +714,7 @@ DEPENDENCIES guard-rspec haml-lint haml-rails + httparty icalendar iso-639 jquery-datatables diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index bc185d22..8a8d770c 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +require 'httparty' module External module MailblusterHelper @@ -9,40 +10,53 @@ module External # end def create_lead(user) - uri = URI(MAILBLUSTER_URL) - http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = true - request = Net::HTTP::Post.new(uri.path, - 'Authorization' => ENV['MAILBLUSTER_API_KEY']) - request['Content-Type'] = 'application/json' - request.body = { - 'email' => user.email, - 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, - 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] - }.to_json - response = http.request(request) - response.body + options = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + }, + body: { + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] + }.to_json + } + HTTParty.post(MAILBLUSTER_URL, options).parsed_response rescue StandardError => e puts "ERROR #{e}" nil end - def edit_lead(user, add_tags, remove_tags) - puts 'PENDING' + def edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) + options = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + }, + body: { + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'addTags' => add_tags, + 'removeTags' => remove_tags + }.to_json + } + email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) + HTTParty.put(MAILBLUSTER_URL + email_hash, options).parsed_response end def delete_lead(user) email_hash = Digest::MD5.hexdigest user.email - uri = URI(MAILBLUSTER_URL + email_hash) - http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = true - request = Net::HTTP::Delete.new(uri.path, - 'Authorization' => ENV['MAILBLUSTER_API_KEY']) - request['Content-Type'] = 'application/json' - response = http.request(request) - response.body + options = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + } + } + HTTParty.delete(MAILBLUSTER_URL + email_hash, options).parsed_response rescue StandardError => e puts "ERROR #{e}" nil From d75ba79b3d186e4c78d2a3894aa346e8697a3098 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 23 Mar 2021 13:01:22 -0700 Subject: [PATCH 21/55] Fix up RSpec tests --- spec/factories/users.rb | 1 + spec/helpers/external/mailbluster_helper_spec.rb | 5 +++++ spec/spec_helper.rb | 4 ++-- spec/support/external_request.rb | 5 +++++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index fdd60d67..75af7370 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -97,6 +97,7 @@ FactoryBot.define do } }" WebMock.stub_request(:post, url_mailbluster) + .to_return(body: response_body, status: 200) end # Called by every user creation diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index fa94ed37..d06fff0c 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -5,6 +5,11 @@ require 'webmock/rspec' describe External::MailblusterHelper, type: :helper do let!(:user) { create(:user) } + + before(:each) do + WebMock.reset_executed_requests! + end + url = 'https://api.mailbluster.com/api/leads/' describe 'create_lead' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 32fc961c..6021d4ab 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,8 +13,8 @@ ENV['RAILS_ENV'] ||= 'test' require File.expand_path('../../config/environment', __FILE__) # Prevent tests from making calls to the Internet -require 'webmock/rspec' -WebMock.disable_net_connect!(allow_localhost: true) +# require 'webmock/rspec' +# WebMock.disable_net_connect!(allow_localhost: true) require 'rspec/rails' require 'shoulda/matchers' diff --git a/spec/support/external_request.rb b/spec/support/external_request.rb index c968acd3..fbedd9e3 100644 --- a/spec/support/external_request.rb +++ b/spec/support/external_request.rb @@ -11,6 +11,7 @@ RSpec.configure do |config| config.before(:each) do mock_commercial_request mock_image_request + mock_default_mailbluster end end @@ -39,3 +40,7 @@ def mock_image_request WebMock.stub_request(:post, 'https://api.cloudinary.com/v1_1/snapcon/image/destroy') .to_return(status: 200, body: {}.to_json, headers: {}) end + +def mock_default_mailbluster + WebMock.stub_request(:any, /api.mailbluster.com/) +end From 58f130c132017f61d16a1c7416e042f238364c91 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 23 Mar 2021 13:05:45 -0700 Subject: [PATCH 22/55] Fix Rubocop linting issues --- app/helpers/external/mailbluster_helper.rb | 11 ++++++----- app/models/user.rb | 2 +- spec/models/user_spec.rb | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 8a8d770c..5c39bf2f 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require 'httparty' module External @@ -12,10 +13,10 @@ module External def create_lead(user) options = { headers: { - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/json', 'Authorization' => ENV['MAILBLUSTER_API_KEY'] }, - body: { + body: { 'email' => user.email, 'firstName' => user.name, 'overrideExisting' => true, @@ -32,10 +33,10 @@ module External def edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) options = { headers: { - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/json', 'Authorization' => ENV['MAILBLUSTER_API_KEY'] }, - body: { + body: { 'email' => user.email, 'firstName' => user.name, 'overrideExisting' => true, @@ -52,7 +53,7 @@ module External email_hash = Digest::MD5.hexdigest user.email options = { headers: { - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/json', 'Authorization' => ENV['MAILBLUSTER_API_KEY'] } } diff --git a/app/models/user.rb b/app/models/user.rb index 9ff90c16..f36a25d1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -363,7 +363,7 @@ class User < ApplicationRecord User.count == 1 && User.first.email == 'deleted@localhost.osem' end - # TODO email_hash function for mailbluster + # TODO: email_hash function for mailbluster # def email_hash # Digest::MD5.hexdigest user.email # end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5df95d61..cd3bac07 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -49,7 +49,7 @@ require 'spec_helper' describe User do - + let(:user_admin) { create(:admin) } let(:conference) { create(:conference, short_title: 'oSC16', title: 'openSUSE Conference 2016') } let(:conference2) { create(:conference, short_title: 'oSC15', title: 'openSUSE Conference 2015') } From c2b9275e022c11410d21acd2fb29b8bcd1c552f2 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Tue, 23 Mar 2021 13:45:05 -0700 Subject: [PATCH 23/55] Spec for delete_lead WIP --- app/helpers/external/mailbluster_helper.rb | 2 - app/models/user.rb | 5 ++ .../external/mailbluster_helper_spec.rb | 61 ++++++++++++++++++- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index 5c39bf2f..b1dc93b4 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -39,8 +39,6 @@ module External body: { 'email' => user.email, 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, 'addTags' => add_tags, 'removeTags' => remove_tags }.to_json diff --git a/app/models/user.rb b/app/models/user.rb index f36a25d1..fbf19a67 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,6 +81,7 @@ class User < ApplicationRecord after_save :touch_events after_commit :mailbluster_create_lead, on: :create + after_commit :mailbluster_delete_lead, on: :destroy # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} @@ -378,6 +379,10 @@ class User < ApplicationRecord ApplicationController.helpers.create_lead(self) end + def mailbluster_delete_lead + ApplicationController.helpers.delete_lead(self) + end + def touch_events event_users.each(&:touch) end diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index d06fff0c..8e4933a4 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -44,7 +44,66 @@ describe External::MailblusterHelper, type: :helper do end describe 'edit_lead' do - pending + it 'makes a put request to Mailbluster\'s API to change the email and gets the correct response' do + response_body = "{ + \"message\": \"Lead updated\", + \"lead\": { + \"id\": 329395, + \"firstName\": \"#{user.name}\", + \"lastName\": \"\", + \"fullName\": \"#{user.name}\", + \"email\": \"#{user.email}\", + \"subscribed\": true, + \"tags\": [ + #{ENV['OSEM_NAME'] || 'snapcon'} + ], + } + }" + stub_request(:put, url) + .to_return(body: response_body, status: 200) + add_tags = ['2021'] + old_email = user.email + user.email = "new@new.org" + user.save + response = edit_lead(user, old_email: old_email) + + expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(old_email)).with(body: { + 'email': user.email, + 'firstName': user.name, + 'addTags': add_tags, + 'removeTags': [] + }.to_json) + expect(response).to eq(response_body) + end + + it 'makes a put request to Mailbluster\'s API to add a tag and gets the correct response' do + response_body = "{ + \"message\": \"Lead updated\", + \"lead\": { + \"id\": 329395, + \"firstName\": \"#{user.name}\", + \"lastName\": \"\", + \"fullName\": \"#{user.name}\", + \"email\": \"#{user.email}\", + \"subscribed\": true, + \"tags\": [ + #{ENV['OSEM_NAME'] || 'snapcon'}, '2021' + ], + } + }" + stub_request(:put, url + Digest::MD5.hexdigest(user.email)) + .to_return(body: response_body, status: 200) + add_tags = ['2021'] + response = edit_lead(user, add_tags: add_tags) + + expect(WebMock).to have_requested(:put, url).with(body: { + 'email': user.email, + 'firstName': user.name, + 'addTags': add_tags, + 'removeTags': [] + }.to_json) + expect(response).to eq(response_body) + end end describe 'delete_lead' do From 2f9b3feabc75e104eaaad74a2d0b1bc163a529f2 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 23 Mar 2021 15:00:02 -0700 Subject: [PATCH 24/55] Fix Rubocop linting --- app/helpers/external/mailbluster_helper.rb | 8 ++++---- spec/helpers/external/mailbluster_helper_spec.rb | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb index b1dc93b4..0992470a 100644 --- a/app/helpers/external/mailbluster_helper.rb +++ b/app/helpers/external/mailbluster_helper.rb @@ -37,10 +37,10 @@ module External 'Authorization' => ENV['MAILBLUSTER_API_KEY'] }, body: { - 'email' => user.email, - 'firstName' => user.name, - 'addTags' => add_tags, - 'removeTags' => remove_tags + 'email' => user.email, + 'firstName' => user.name, + 'addTags' => add_tags, + 'removeTags' => remove_tags }.to_json } email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index 8e4933a4..bfa0e96b 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -63,14 +63,14 @@ describe External::MailblusterHelper, type: :helper do .to_return(body: response_body, status: 200) add_tags = ['2021'] old_email = user.email - user.email = "new@new.org" + user.email = 'new@new.org' user.save response = edit_lead(user, old_email: old_email) expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(old_email)).with(body: { - 'email': user.email, - 'firstName': user.name, - 'addTags': add_tags, + 'email': user.email, + 'firstName': user.name, + 'addTags': add_tags, 'removeTags': [] }.to_json) expect(response).to eq(response_body) @@ -97,9 +97,9 @@ describe External::MailblusterHelper, type: :helper do response = edit_lead(user, add_tags: add_tags) expect(WebMock).to have_requested(:put, url).with(body: { - 'email': user.email, - 'firstName': user.name, - 'addTags': add_tags, + 'email': user.email, + 'firstName': user.name, + 'addTags': add_tags, 'removeTags': [] }.to_json) expect(response).to eq(response_body) From 31e4924c194b86720b890d0781cde8a55a589161 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 23 Mar 2021 22:28:52 -0700 Subject: [PATCH 25/55] Add Active Job definitions for executing Mailbluster requests --- app/jobs/mailbluster_create_lead_job.rb | 9 +++++++++ app/jobs/mailbluster_delete_lead_job_job.rb | 7 +++++++ app/jobs/mailbluster_edit_lead_job.rb | 10 ++++++++++ 3 files changed, 26 insertions(+) create mode 100644 app/jobs/mailbluster_create_lead_job.rb create mode 100644 app/jobs/mailbluster_delete_lead_job_job.rb create mode 100644 app/jobs/mailbluster_edit_lead_job.rb diff --git a/app/jobs/mailbluster_create_lead_job.rb b/app/jobs/mailbluster_create_lead_job.rb new file mode 100644 index 00000000..63ef6b33 --- /dev/null +++ b/app/jobs/mailbluster_create_lead_job.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class MailblusterCreateLeadJob < ApplicationJob + queue_as :default + + def perform(user) + ApplicationController.helpers.create_lead(user) + end +end diff --git a/app/jobs/mailbluster_delete_lead_job_job.rb b/app/jobs/mailbluster_delete_lead_job_job.rb new file mode 100644 index 00000000..3430d01b --- /dev/null +++ b/app/jobs/mailbluster_delete_lead_job_job.rb @@ -0,0 +1,7 @@ +class MailblusterDeleteLeadJobJob < ApplicationJob + queue_as :default + + def perform(user) + ApplicationController.helpers.delete_lead(user) + end +end diff --git a/app/jobs/mailbluster_edit_lead_job.rb b/app/jobs/mailbluster_edit_lead_job.rb new file mode 100644 index 00000000..3dd16ef4 --- /dev/null +++ b/app/jobs/mailbluster_edit_lead_job.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class MailblusterEditLeadJob < ApplicationJob + queue_as :default + + def perform(user, add_tags: [], remove_tags: [], old_email: nil) + ApplicationController.helpers.edit_lead(user, + add_tags: add_tags, remove_tags: remove_tags, old_email: old_email) + end +end \ No newline at end of file From 6b251cb02a9a58f93769653c16b2c8c345e20db0 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 25 Mar 2021 16:40:04 -0700 Subject: [PATCH 26/55] Use Active Jobs to spin Mailbluster requests off into a separate thread --- app/models/user.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index fbf19a67..87f5eb73 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -376,11 +376,15 @@ class User < ApplicationRecord end def mailbluster_create_lead - ApplicationController.helpers.create_lead(self) + MailblusterCreateLeadJob.perform_later self end def mailbluster_delete_lead - ApplicationController.helpers.delete_lead(self) + MailblusterDeleteLeadJob.perform_later self + end + + def mailbluster_update_email + MailblusterEditLeadJob.perform_later(self, old_email: email_before_last_save) end def touch_events From e620dadc1cfcae62bb7d25c9bb1c3a2396459a98 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 25 Mar 2021 16:43:18 -0700 Subject: [PATCH 27/55] Fix some RSpec issues --- spec/helpers/external/mailbluster_helper_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/helpers/external/mailbluster_helper_spec.rb index bfa0e96b..125c862d 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/helpers/external/mailbluster_helper_spec.rb @@ -59,18 +59,17 @@ describe External::MailblusterHelper, type: :helper do ], } }" - stub_request(:put, url) - .to_return(body: response_body, status: 200) - add_tags = ['2021'] old_email = user.email user.email = 'new@new.org' user.save + stub_request(:put, url + Digest::MD5.hexdigest(old_email)) + .to_return(body: response_body, status: 200) response = edit_lead(user, old_email: old_email) expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(old_email)).with(body: { 'email': user.email, 'firstName': user.name, - 'addTags': add_tags, + 'addTags': [], 'removeTags': [] }.to_json) expect(response).to eq(response_body) @@ -96,7 +95,7 @@ describe External::MailblusterHelper, type: :helper do add_tags = ['2021'] response = edit_lead(user, add_tags: add_tags) - expect(WebMock).to have_requested(:put, url).with(body: { + expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(user.email)).with(body: { 'email': user.email, 'firstName': user.name, 'addTags': add_tags, From b357d13c39eddc8dd60c17b0e68dd8a2256d7cce Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 25 Mar 2021 16:44:51 -0700 Subject: [PATCH 28/55] Use better and clearer shorthand for callbacks --- app/models/user.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 87f5eb73..04f51c4c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -80,8 +80,9 @@ class User < ApplicationRecord after_save :touch_events - after_commit :mailbluster_create_lead, on: :create - after_commit :mailbluster_delete_lead, on: :destroy + after_create_commit :mailbluster_create_lead + after_destory_commit :mailbluster_delete_lead + after_update_commit :mailbluster_update_email, if: :saved_change_to_email? # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} From 560a3dbe70c7bed1ef43134214db14fae6001da5 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Thu, 25 Mar 2021 17:00:58 -0700 Subject: [PATCH 29/55] CAUTION: deleted user model spec for mailbluster --- spec/models/user_spec.rb | 38 -------------------------------------- 1 file changed, 38 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cd3bac07..2e890949 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -530,42 +530,4 @@ describe User do expect(User.omniauth_providers).to eq [:suse, :google, :facebook, :github, :discourse] end end - - describe 'mailbluster' do - it 'creates a Mailbluster lead on creating a user' do - user_mailbluster = build(:user) - url = 'https://api.mailbluster.com/api/leads/' - - response_body = "{ - \"message\": \"Lead created\", - \"lead\": { - \"id\": 329395, - \"firstName\": \"#{user_mailbluster.name}\", - \"lastName\": \"\", - \"fullName\": \"#{user_mailbluster.name}\", - \"email\": \"#{user_mailbluster.email}\", - \"subscribed\": true, - \"tags\": [ - #{ENV['OSEM_NAME'] || 'snapcon'} - ], - } - }" - stub_request(:post, url) - .to_return(body: response_body, status: 200) - - # Do not request before save - expect(WebMock).not_to have_requested(:post, url) - - # Request after save - user_mailbluster.save! - - expect(WebMock).to have_requested(:post, url).with(body: { - 'email': user_mailbluster.email, - 'firstName': user_mailbluster.name, - 'overrideExisting': true, - 'subscribed': true, - 'tags': [ENV['OSEM_NAME'] || 'snapcon'] - }.to_json) - end - end end From efdf1f03dfa1ef96a39e7764c6e609256794fee2 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Thu, 25 Mar 2021 17:21:27 -0700 Subject: [PATCH 30/55] Call mailbluster helpers when registering / cancelling reg to conference --- app/controllers/conference_registrations_controller.rb | 3 +++ app/models/user.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 8d4ae8a1..6a64ff8a 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -57,6 +57,8 @@ class ConferenceRegistrationsController < ApplicationController sign_in(@registration.user) end + MailblusterEditLeadJob.perform_later(@user, add_tags: ["snapcon-#{@conference.short_title}"]) + if @conference.tickets.visible.any? && !current_user.supports?(@conference) redirect_to conference_tickets_path(@conference.short_title), notice: 'You are now registered and will be receiving E-Mail notifications.' @@ -87,6 +89,7 @@ class ConferenceRegistrationsController < ApplicationController def destroy if @registration.destroy + MailblusterEditLeadJob.perform_later(@user, remove_tags: ["snapcon-#{@conference.short_title}"]) redirect_to root_path, notice: "You are not registered for #{@conference.title} anymore!" else diff --git a/app/models/user.rb b/app/models/user.rb index 04f51c4c..e4de8e8c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,7 +81,7 @@ class User < ApplicationRecord after_save :touch_events after_create_commit :mailbluster_create_lead - after_destory_commit :mailbluster_delete_lead + after_destroy_commit :mailbluster_delete_lead after_update_commit :mailbluster_update_email, if: :saved_change_to_email? # add scope From 43911c4a38f1ddac60d0e3daa48223aea0c233e5 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Thu, 25 Mar 2021 17:30:43 -0700 Subject: [PATCH 31/55] Finished spec, added FIXME for caveat in email change --- app/models/user.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/user.rb b/app/models/user.rb index e4de8e8c..8b95fee2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -385,6 +385,7 @@ class User < ApplicationRecord end def mailbluster_update_email + # FIXME: May fail if multiple saves occur in one commit MailblusterEditLeadJob.perform_later(self, old_email: email_before_last_save) end From b980c88085ff45916e80d3a1e6fa74bfe5257900 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 25 Mar 2021 18:57:58 -0700 Subject: [PATCH 32/55] Fix typo in Mailbluster delete lead job --- app/jobs/mailbluster_delete_lead_job_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/mailbluster_delete_lead_job_job.rb b/app/jobs/mailbluster_delete_lead_job_job.rb index 3430d01b..972a2cb6 100644 --- a/app/jobs/mailbluster_delete_lead_job_job.rb +++ b/app/jobs/mailbluster_delete_lead_job_job.rb @@ -1,4 +1,4 @@ -class MailblusterDeleteLeadJobJob < ApplicationJob +class MailblusterDeleteLeadJob < ApplicationJob queue_as :default def perform(user) From 02db4211ae71be3d40893cd72061071563d7695d Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 25 Mar 2021 20:16:09 -0700 Subject: [PATCH 33/55] Properly rename job file --- ...ster_delete_lead_job_job.rb => mailbluster_delete_lead_job.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/jobs/{mailbluster_delete_lead_job_job.rb => mailbluster_delete_lead_job.rb} (100%) diff --git a/app/jobs/mailbluster_delete_lead_job_job.rb b/app/jobs/mailbluster_delete_lead_job.rb similarity index 100% rename from app/jobs/mailbluster_delete_lead_job_job.rb rename to app/jobs/mailbluster_delete_lead_job.rb From 1c1d09fefc4df8c61f50d0affa3fdee6031c15b7 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Tue, 30 Mar 2021 22:21:03 -0700 Subject: [PATCH 34/55] Fix Rubocop linting issues --- app/jobs/mailbluster_edit_lead_job.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/jobs/mailbluster_edit_lead_job.rb b/app/jobs/mailbluster_edit_lead_job.rb index 3dd16ef4..cb0a5832 100644 --- a/app/jobs/mailbluster_edit_lead_job.rb +++ b/app/jobs/mailbluster_edit_lead_job.rb @@ -5,6 +5,6 @@ class MailblusterEditLeadJob < ApplicationJob def perform(user, add_tags: [], remove_tags: [], old_email: nil) ApplicationController.helpers.edit_lead(user, - add_tags: add_tags, remove_tags: remove_tags, old_email: old_email) + add_tags: add_tags, remove_tags: remove_tags, old_email: old_email) end -end \ No newline at end of file +end From f0582c25bf5fe75c768c85d92a0b7814617691e3 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 31 Mar 2021 21:05:59 -0700 Subject: [PATCH 35/55] Move Mailbluster business logic to services/ instead of helpers/ --- app/jobs/mailbluster_create_lead_job.rb | 2 +- app/jobs/mailbluster_delete_lead_job.rb | 2 +- app/jobs/mailbluster_edit_lead_job.rb | 3 +- app/services/mailbluster_manager.rb | 49 +++++++++++++++++++ .../mailbluster_manager_spec.rb} | 10 ++-- 5 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 app/services/mailbluster_manager.rb rename spec/{helpers/external/mailbluster_helper_spec.rb => services/mailbluster_manager_spec.rb} (92%) diff --git a/app/jobs/mailbluster_create_lead_job.rb b/app/jobs/mailbluster_create_lead_job.rb index 63ef6b33..701719d7 100644 --- a/app/jobs/mailbluster_create_lead_job.rb +++ b/app/jobs/mailbluster_create_lead_job.rb @@ -4,6 +4,6 @@ class MailblusterCreateLeadJob < ApplicationJob queue_as :default def perform(user) - ApplicationController.helpers.create_lead(user) + MailblusterManager.create_lead(user) end end diff --git a/app/jobs/mailbluster_delete_lead_job.rb b/app/jobs/mailbluster_delete_lead_job.rb index 972a2cb6..387d1cc0 100644 --- a/app/jobs/mailbluster_delete_lead_job.rb +++ b/app/jobs/mailbluster_delete_lead_job.rb @@ -2,6 +2,6 @@ class MailblusterDeleteLeadJob < ApplicationJob queue_as :default def perform(user) - ApplicationController.helpers.delete_lead(user) + MailblusterManager.delete_lead(user) end end diff --git a/app/jobs/mailbluster_edit_lead_job.rb b/app/jobs/mailbluster_edit_lead_job.rb index cb0a5832..ff5d07ce 100644 --- a/app/jobs/mailbluster_edit_lead_job.rb +++ b/app/jobs/mailbluster_edit_lead_job.rb @@ -4,7 +4,6 @@ class MailblusterEditLeadJob < ApplicationJob queue_as :default def perform(user, add_tags: [], remove_tags: [], old_email: nil) - ApplicationController.helpers.edit_lead(user, - add_tags: add_tags, remove_tags: remove_tags, old_email: old_email) + MailblusterManager.edit_lead(user, add_tags: add_tags, remove_tags: remove_tags, old_email: old_email) end end diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb new file mode 100644 index 00000000..6360413d --- /dev/null +++ b/app/services/mailbluster_manager.rb @@ -0,0 +1,49 @@ +class MailblusterManager + include HTTParty + base_uri 'https://api.mailbluster.com/api/leads/' + + def self.create_lead(user) + options = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + }, + body: { + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] + }.to_json + } + post('/', options).parsed_response + end + + def self.edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) + options = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + }, + body: { + 'email' => user.email, + 'firstName' => user.name, + 'addTags' => add_tags, + 'removeTags' => remove_tags + }.to_json + } + email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) + put("/#{email_hash}", options).parsed_response + end + + def self.delete_lead(user) + email_hash = Digest::MD5.hexdigest user.email + options = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + } + } + delete("/#{email_hash}", options).parsed_response + end +end \ No newline at end of file diff --git a/spec/helpers/external/mailbluster_helper_spec.rb b/spec/services/mailbluster_manager_spec.rb similarity index 92% rename from spec/helpers/external/mailbluster_helper_spec.rb rename to spec/services/mailbluster_manager_spec.rb index 125c862d..8c031df0 100644 --- a/spec/helpers/external/mailbluster_helper_spec.rb +++ b/spec/services/mailbluster_manager_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'webmock/rspec' -describe External::MailblusterHelper, type: :helper do +describe MailblusterManager, type: :model do let!(:user) { create(:user) } before(:each) do @@ -30,7 +30,7 @@ describe External::MailblusterHelper, type: :helper do }" stub_request(:post, url) .to_return(body: response_body, status: 200) - response = create_lead(user) + response = MailblusterManager.create_lead(user) expect(WebMock).to have_requested(:post, url).with(body: { 'email': user.email, @@ -64,7 +64,7 @@ describe External::MailblusterHelper, type: :helper do user.save stub_request(:put, url + Digest::MD5.hexdigest(old_email)) .to_return(body: response_body, status: 200) - response = edit_lead(user, old_email: old_email) + response = MailblusterManager.edit_lead(user, old_email: old_email) expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(old_email)).with(body: { 'email': user.email, @@ -93,7 +93,7 @@ describe External::MailblusterHelper, type: :helper do stub_request(:put, url + Digest::MD5.hexdigest(user.email)) .to_return(body: response_body, status: 200) add_tags = ['2021'] - response = edit_lead(user, add_tags: add_tags) + response = MailblusterManager.edit_lead(user, add_tags: add_tags) expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(user.email)).with(body: { 'email': user.email, @@ -115,7 +115,7 @@ describe External::MailblusterHelper, type: :helper do lead_url = url + email_hash.to_s stub_request(:delete, lead_url) .to_return(body: response_body) - response = delete_lead(user) + response = MailblusterManager.delete_lead(user) expect(WebMock).to have_requested(:delete, lead_url) expect(response).to eq(response_body) From a49f5c7f7285c5e497af507afcb374464ba7f0fa Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 31 Mar 2021 21:08:28 -0700 Subject: [PATCH 36/55] Properly fire update_email hook after commit which changes user email --- app/models/concerns/track_saved_changes.rb | 50 ++++++++++++++++++++++ app/models/user.rb | 9 +++- 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 app/models/concerns/track_saved_changes.rb diff --git a/app/models/concerns/track_saved_changes.rb b/app/models/concerns/track_saved_changes.rb new file mode 100644 index 00000000..03596050 --- /dev/null +++ b/app/models/concerns/track_saved_changes.rb @@ -0,0 +1,50 @@ +# https://github.com/ccmcbeck/after-commit +module TrackSavedChanges + extend ActiveSupport::Concern + + included do + # expose the details if consumer wants to do more + attr_reader :saved_changes_history, :saved_changes_unfiltered + after_initialize :reset_saved_changes + after_save :track_saved_changes + end + + # on initalize, but useful for fine grain control + def reset_saved_changes + @saved_changes_unfiltered = {} + @saved_changes_history = [] + end + + # filter out any changes that result in the original value + def saved_changes + @saved_changes_unfiltered.reject { |k,v| v[0] == v[1] } + end + + private + + # on save + def track_saved_changes + # maintain an array of ActiveModel::Dirty.changes + @saved_changes_history << changes.dup + # accumulate the most recent changes + @saved_changes_history.last.each_pair { |k, v| track_saved_change k, v } + end + + # v is an an array of [prev, current] + def track_saved_change(k, v) + if @saved_changes_unfiltered.key? k + @saved_changes_unfiltered[k][1] = track_saved_value v[1] + else + @saved_changes_unfiltered[k] = v.dup + end + end + + # type safe dup inspred by http://stackoverflow.com/a/20955038 + def track_saved_value(v) + begin + v.dup + rescue TypeError + v + end + end +end \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 8b95fee2..c3efd46d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,7 +82,12 @@ class User < ApplicationRecord after_create_commit :mailbluster_create_lead after_destroy_commit :mailbluster_delete_lead - after_update_commit :mailbluster_update_email, if: :saved_change_to_email? + # Note that because a commit may cause multiple changes + # which are not fully tracked by ActiveRecord::Dirty, + # we must resort to using a ActiveRecord::Concern which accumulates + # changes from a commit (app/models/concerns/track_saved_changes.rb) + # See https://github.com/ccmcbeck/after-commit + after_update_commit :mailbluster_update_email, if: ->(obj){ obj.saved_changes.key? 'email' } # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} @@ -386,7 +391,7 @@ class User < ApplicationRecord def mailbluster_update_email # FIXME: May fail if multiple saves occur in one commit - MailblusterEditLeadJob.perform_later(self, old_email: email_before_last_save) + MailblusterEditLeadJob.perform_later(self, old_email: self.saved_changes['email'][0]) end def touch_events From 66c27289983bf48e8f8a8290706565312b46fff6 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 31 Mar 2021 21:22:28 -0700 Subject: [PATCH 37/55] Fix Rubocop linting issues --- app/models/concerns/track_saved_changes.rb | 22 ++++++++++------------ app/models/user.rb | 2 +- app/services/mailbluster_manager.rb | 2 +- spec/services/mailbluster_manager_spec.rb | 8 ++++---- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/app/models/concerns/track_saved_changes.rb b/app/models/concerns/track_saved_changes.rb index 03596050..660e347d 100644 --- a/app/models/concerns/track_saved_changes.rb +++ b/app/models/concerns/track_saved_changes.rb @@ -17,7 +17,7 @@ module TrackSavedChanges # filter out any changes that result in the original value def saved_changes - @saved_changes_unfiltered.reject { |k,v| v[0] == v[1] } + @saved_changes_unfiltered.reject { |_k, v| v[0] == v[1] } end private @@ -31,20 +31,18 @@ module TrackSavedChanges end # v is an an array of [prev, current] - def track_saved_change(k, v) - if @saved_changes_unfiltered.key? k - @saved_changes_unfiltered[k][1] = track_saved_value v[1] + def track_saved_change(key, value) + if @saved_changes_unfiltered.key? key + @saved_changes_unfiltered[key][1] = track_saved_value value[1] else - @saved_changes_unfiltered[k] = v.dup + @saved_changes_unfiltered[key] = value.dup end end # type safe dup inspred by http://stackoverflow.com/a/20955038 - def track_saved_value(v) - begin - v.dup - rescue TypeError - v - end + def track_saved_value(value) + value.dup + rescue TypeError + value end -end \ No newline at end of file +end diff --git a/app/models/user.rb b/app/models/user.rb index c3efd46d..331ce805 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -391,7 +391,7 @@ class User < ApplicationRecord def mailbluster_update_email # FIXME: May fail if multiple saves occur in one commit - MailblusterEditLeadJob.perform_later(self, old_email: self.saved_changes['email'][0]) + MailblusterEditLeadJob.perform_later(self, old_email: saved_changes['email'][0]) end def touch_events diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index 6360413d..cb623d4a 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -46,4 +46,4 @@ class MailblusterManager } delete("/#{email_hash}", options).parsed_response end -end \ No newline at end of file +end diff --git a/spec/services/mailbluster_manager_spec.rb b/spec/services/mailbluster_manager_spec.rb index 8c031df0..9f3d1be8 100644 --- a/spec/services/mailbluster_manager_spec.rb +++ b/spec/services/mailbluster_manager_spec.rb @@ -30,7 +30,7 @@ describe MailblusterManager, type: :model do }" stub_request(:post, url) .to_return(body: response_body, status: 200) - response = MailblusterManager.create_lead(user) + response = described_class.create_lead(user) expect(WebMock).to have_requested(:post, url).with(body: { 'email': user.email, @@ -64,7 +64,7 @@ describe MailblusterManager, type: :model do user.save stub_request(:put, url + Digest::MD5.hexdigest(old_email)) .to_return(body: response_body, status: 200) - response = MailblusterManager.edit_lead(user, old_email: old_email) + response = described_class.edit_lead(user, old_email: old_email) expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(old_email)).with(body: { 'email': user.email, @@ -93,7 +93,7 @@ describe MailblusterManager, type: :model do stub_request(:put, url + Digest::MD5.hexdigest(user.email)) .to_return(body: response_body, status: 200) add_tags = ['2021'] - response = MailblusterManager.edit_lead(user, add_tags: add_tags) + response = described_class.edit_lead(user, add_tags: add_tags) expect(WebMock).to have_requested(:put, url + Digest::MD5.hexdigest(user.email)).with(body: { 'email': user.email, @@ -115,7 +115,7 @@ describe MailblusterManager, type: :model do lead_url = url + email_hash.to_s stub_request(:delete, lead_url) .to_return(body: response_body) - response = MailblusterManager.delete_lead(user) + response = described_class.delete_lead(user) expect(WebMock).to have_requested(:delete, lead_url) expect(response).to eq(response_body) From d6341ff9320c8d342654650f46e3869d2e53c72e Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 31 Mar 2021 21:28:12 -0700 Subject: [PATCH 38/55] Prepare to add Mailbluster update name --- app/models/user.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 331ce805..fcfe864c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -88,6 +88,7 @@ class User < ApplicationRecord # changes from a commit (app/models/concerns/track_saved_changes.rb) # See https://github.com/ccmcbeck/after-commit after_update_commit :mailbluster_update_email, if: ->(obj){ obj.saved_changes.key? 'email' } + after_update_commit :mailbluster_update_name, if: ->(obj){ obj.saved_changes.key? 'name' } # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} @@ -394,6 +395,10 @@ class User < ApplicationRecord MailblusterEditLeadJob.perform_later(self, old_email: saved_changes['email'][0]) end + def mailbluster_update_name + # TODO + end + def touch_events event_users.each(&:touch) end From 8c184d4f30725a222c0319e2afb9c849eb6f15e5 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Wed, 31 Mar 2021 21:34:55 -0700 Subject: [PATCH 39/55] Edit lead when user changes name --- app/models/user.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index fcfe864c..7199cfe3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -391,12 +391,11 @@ class User < ApplicationRecord end def mailbluster_update_email - # FIXME: May fail if multiple saves occur in one commit MailblusterEditLeadJob.perform_later(self, old_email: saved_changes['email'][0]) end def mailbluster_update_name - # TODO + MailblusterEditLeadJob.perform_later self end def touch_events From b0104cac0999dabd851695a6b961a1da43984f01 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 21:35:32 -0700 Subject: [PATCH 40/55] Actually remove redundant code from helpers --- app/helpers/external/mailbluster_helper.rb | 64 ---------------------- 1 file changed, 64 deletions(-) delete mode 100644 app/helpers/external/mailbluster_helper.rb diff --git a/app/helpers/external/mailbluster_helper.rb b/app/helpers/external/mailbluster_helper.rb deleted file mode 100644 index 0992470a..00000000 --- a/app/helpers/external/mailbluster_helper.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -require 'httparty' - -module External - module MailblusterHelper - MAILBLUSTER_URL = 'https://api.mailbluster.com/api/leads/' - - # def query_api(user, method) - # TODO? General helper for all queries - # end - - def create_lead(user) - options = { - headers: { - 'Content-Type' => 'application/json', - 'Authorization' => ENV['MAILBLUSTER_API_KEY'] - }, - body: { - 'email' => user.email, - 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, - 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] - }.to_json - } - HTTParty.post(MAILBLUSTER_URL, options).parsed_response - rescue StandardError => e - puts "ERROR #{e}" - nil - end - - def edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) - options = { - headers: { - 'Content-Type' => 'application/json', - 'Authorization' => ENV['MAILBLUSTER_API_KEY'] - }, - body: { - 'email' => user.email, - 'firstName' => user.name, - 'addTags' => add_tags, - 'removeTags' => remove_tags - }.to_json - } - email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) - HTTParty.put(MAILBLUSTER_URL + email_hash, options).parsed_response - end - - def delete_lead(user) - email_hash = Digest::MD5.hexdigest user.email - options = { - headers: { - 'Content-Type' => 'application/json', - 'Authorization' => ENV['MAILBLUSTER_API_KEY'] - } - } - HTTParty.delete(MAILBLUSTER_URL + email_hash, options).parsed_response - rescue StandardError => e - puts "ERROR #{e}" - nil - end - end -end From dcec2a6e01c5b9d026102f4bd36418e345d69879 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 21:47:28 -0700 Subject: [PATCH 41/55] Avoid repeating headers --- app/services/mailbluster_manager.rb | 33 +++++++++++++---------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index cb623d4a..796a7f3d 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -1,13 +1,19 @@ class MailblusterManager include HTTParty base_uri 'https://api.mailbluster.com/api/leads/' + @@auth_headers = { + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => ENV['MAILBLUSTER_API_KEY'] + } + } + + def query_api(method, user, body) + #TODO + end def self.create_lead(user) - options = { - headers: { - 'Content-Type' => 'application/json', - 'Authorization' => ENV['MAILBLUSTER_API_KEY'] - }, + options = @@auth_headers.merge({ body: { 'email' => user.email, 'firstName' => user.name, @@ -15,35 +21,26 @@ class MailblusterManager 'subscribed' => true, 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] }.to_json - } + }) post('/', options).parsed_response end def self.edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) - options = { - headers: { - 'Content-Type' => 'application/json', - 'Authorization' => ENV['MAILBLUSTER_API_KEY'] - }, + options = @@auth_headers.merge({ body: { 'email' => user.email, 'firstName' => user.name, 'addTags' => add_tags, 'removeTags' => remove_tags }.to_json - } + }) email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) put("/#{email_hash}", options).parsed_response end def self.delete_lead(user) email_hash = Digest::MD5.hexdigest user.email - options = { - headers: { - 'Content-Type' => 'application/json', - 'Authorization' => ENV['MAILBLUSTER_API_KEY'] - } - } + options = @@auth_headers delete("/#{email_hash}", options).parsed_response end end From ea681c030ccce88c6fa79c1dc816bbb7b4a950e3 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Thu, 1 Apr 2021 21:55:16 -0700 Subject: [PATCH 42/55] DRY query_api in mailbluster_manager --- app/services/mailbluster_manager.rb | 31 +++++++++++++---------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index 796a7f3d..b9f1e070 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -8,39 +8,36 @@ class MailblusterManager } } - def query_api(method, user, body) + def query_api(method, path, body) #TODO + options = @@auth_headers.merge({ + body: body.to_json + }) + method(path, options).parsed_response end def self.create_lead(user) - options = @@auth_headers.merge({ - body: { - 'email' => user.email, - 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, - 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] - }.to_json + query_api(post, '/', { + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] }) - post('/', options).parsed_response end def self.edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) - options = @@auth_headers.merge({ - body: { + email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) + query_api(put, "/#{email_hash}", { 'email' => user.email, 'firstName' => user.name, 'addTags' => add_tags, 'removeTags' => remove_tags - }.to_json }) - email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) - put("/#{email_hash}", options).parsed_response end def self.delete_lead(user) email_hash = Digest::MD5.hexdigest user.email - options = @@auth_headers - delete("/#{email_hash}", options).parsed_response + query_api(delete, "/#{email_hash}", {}) end end From 4afb92596de3fcc0f1134274adcdba28cae87a3d Mon Sep 17 00:00:00 2001 From: Ziyi Date: Thu, 1 Apr 2021 21:57:19 -0700 Subject: [PATCH 43/55] self.query_api --- app/services/mailbluster_manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index b9f1e070..2208d0f6 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -8,7 +8,7 @@ class MailblusterManager } } - def query_api(method, path, body) + def self.query_api(method, path, body) #TODO options = @@auth_headers.merge({ body: body.to_json From 2e3cdad0bcfcbc07705300fa782dd670a6f216d2 Mon Sep 17 00:00:00 2001 From: Ziyi Date: Thu, 1 Apr 2021 22:01:09 -0700 Subject: [PATCH 44/55] Try fixing query_api 0 argument error --- app/services/mailbluster_manager.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index 2208d0f6..db6cb6f9 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -13,11 +13,11 @@ class MailblusterManager options = @@auth_headers.merge({ body: body.to_json }) - method(path, options).parsed_response + send(method, path, options).parsed_response end def self.create_lead(user) - query_api(post, '/', { + query_api(:post, '/', { 'email' => user.email, 'firstName' => user.name, 'overrideExisting' => true, @@ -28,7 +28,7 @@ class MailblusterManager def self.edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) - query_api(put, "/#{email_hash}", { + query_api(:put, "/#{email_hash}", { 'email' => user.email, 'firstName' => user.name, 'addTags' => add_tags, @@ -38,6 +38,6 @@ class MailblusterManager def self.delete_lead(user) email_hash = Digest::MD5.hexdigest user.email - query_api(delete, "/#{email_hash}", {}) + query_api(:delete, "/#{email_hash}", {}) end end From dd2b229761383ef11ee75edf30841124a9869087 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 22:08:25 -0700 Subject: [PATCH 45/55] Rubocop linting issues fixed --- app/services/mailbluster_manager.rb | 33 +++++++++++++---------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index db6cb6f9..ace60586 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -1,7 +1,7 @@ class MailblusterManager include HTTParty base_uri 'https://api.mailbluster.com/api/leads/' - @@auth_headers = { + @auth_headers = { headers: { 'Content-Type' => 'application/json', 'Authorization' => ENV['MAILBLUSTER_API_KEY'] @@ -9,31 +9,28 @@ class MailblusterManager } def self.query_api(method, path, body) - #TODO - options = @@auth_headers.merge({ - body: body.to_json - }) + options = @auth_headers.merge(body: body.to_json) send(method, path, options).parsed_response end def self.create_lead(user) - query_api(:post, '/', { - 'email' => user.email, - 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, - 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] - }) + query_api(:post, '/', + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] + ) end def self.edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) - query_api(:put, "/#{email_hash}", { - 'email' => user.email, - 'firstName' => user.name, - 'addTags' => add_tags, - 'removeTags' => remove_tags - }) + query_api(:put, "/#{email_hash}", + 'email' => user.email, + 'firstName' => user.name, + 'addTags' => add_tags, + 'removeTags' => remove_tags + ) end def self.delete_lead(user) From 11785c665ee1c6c9ab4e8bd241c883e7d3fa9673 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 22:59:53 -0700 Subject: [PATCH 46/55] Actually properly track saved changes within the User model --- app/models/user.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/user.rb b/app/models/user.rb index 7199cfe3..4eb10a1d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -53,6 +53,7 @@ class UserDisabled < StandardError end class User < ApplicationRecord + include TrackSavedChanges rolify # prevent N+1 queries with has_cached_role? by preloading roles *always* default_scope { preload(:roles) } From 9361f0bfd5c10d90b8ee5ba9a6a73ddfc50d2e58 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 23:00:17 -0700 Subject: [PATCH 47/55] Remove redundant comments --- spec/spec_helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6021d4ab..2d680fb2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,10 +12,6 @@ SimpleCov.start 'rails' ENV['RAILS_ENV'] ||= 'test' require File.expand_path('../../config/environment', __FILE__) -# Prevent tests from making calls to the Internet -# require 'webmock/rspec' -# WebMock.disable_net_connect!(allow_localhost: true) - require 'rspec/rails' require 'shoulda/matchers' require 'webdrivers' From d780c2a6b3e2765d655e925ae1cbafabdb3b7777 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 23:10:43 -0700 Subject: [PATCH 48/55] Make body argument for query_api more clear --- app/services/mailbluster_manager.rb | 30 ++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index ace60586..3925d1dd 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -8,33 +8,33 @@ class MailblusterManager } } - def self.query_api(method, path, body) + def self.query_api(method, path, body: {}) options = @auth_headers.merge(body: body.to_json) send(method, path, options).parsed_response end def self.create_lead(user) - query_api(:post, '/', - 'email' => user.email, - 'firstName' => user.name, - 'overrideExisting' => true, - 'subscribed' => true, - 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] - ) + query_api(:post, '/', body: { + 'email' => user.email, + 'firstName' => user.name, + 'overrideExisting' => true, + 'subscribed' => true, + 'tags' => [ENV['OSEM_NAME'] || 'snapcon'] + }) end def self.edit_lead(user, add_tags: [], remove_tags: [], old_email: nil) email_hash = Digest::MD5.hexdigest(old_email.presence || user.email) - query_api(:put, "/#{email_hash}", - 'email' => user.email, - 'firstName' => user.name, - 'addTags' => add_tags, - 'removeTags' => remove_tags - ) + query_api(:put, "/#{email_hash}", body: { + 'email' => user.email, + 'firstName' => user.name, + 'addTags' => add_tags, + 'removeTags' => remove_tags + }) end def self.delete_lead(user) email_hash = Digest::MD5.hexdigest user.email - query_api(:delete, "/#{email_hash}", {}) + query_api(:delete, "/#{email_hash}") end end From ce90dc5fc2ebb6e9a73566836c85ad915f342d4d Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Thu, 1 Apr 2021 23:20:21 -0700 Subject: [PATCH 49/55] Add spec for query_api method --- spec/services/mailbluster_manager_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/services/mailbluster_manager_spec.rb b/spec/services/mailbluster_manager_spec.rb index 9f3d1be8..c1bffa27 100644 --- a/spec/services/mailbluster_manager_spec.rb +++ b/spec/services/mailbluster_manager_spec.rb @@ -12,6 +12,22 @@ describe MailblusterManager, type: :model do url = 'https://api.mailbluster.com/api/leads/' + describe 'query_api' do + it 'translates :get to a get request' do + stub_request(:get, url) + described_class.query_api(:get, '/') + + expect(WebMock).to have_requested(:get, url) + end + + it 'translates :post to a post request' do + stub_request(:post, url + 'path') + described_class.query_api(:post, '/path', body: { key: 'value' }) + + expect(WebMock).to have_requested(:post, url + 'path').with(body: { key: 'value' }) + end + end + describe 'create_lead' do it 'makes a post request to Mailbluster\'s API and gets the correct response' do response_body = "{ From 29804c8681ed8ffd9a529026a878b6f5713e6f66 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Wed, 7 Apr 2021 22:48:57 -0700 Subject: [PATCH 50/55] Remove name conflict change which caused PaperTrail to fail --- app/models/concerns/track_saved_changes.rb | 32 +++++++++++----------- app/models/user.rb | 4 +-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/models/concerns/track_saved_changes.rb b/app/models/concerns/track_saved_changes.rb index 660e347d..a7e928d4 100644 --- a/app/models/concerns/track_saved_changes.rb +++ b/app/models/concerns/track_saved_changes.rb @@ -4,43 +4,43 @@ module TrackSavedChanges included do # expose the details if consumer wants to do more - attr_reader :saved_changes_history, :saved_changes_unfiltered - after_initialize :reset_saved_changes - after_save :track_saved_changes + # attr_reader :ts_saved_changes_history, :ts_saved_changes_unfiltered + after_initialize :ts_reset_saved_changes + after_save :ts_track_saved_changes end # on initalize, but useful for fine grain control - def reset_saved_changes - @saved_changes_unfiltered = {} - @saved_changes_history = [] + def ts_reset_saved_changes + @ts_saved_changes_unfiltered = {} + @ts_saved_changes_history = [] end # filter out any changes that result in the original value - def saved_changes - @saved_changes_unfiltered.reject { |_k, v| v[0] == v[1] } + def ts_saved_changes + @ts_saved_changes_unfiltered.reject { |_k, v| v[0] == v[1] } end private # on save - def track_saved_changes + def ts_track_saved_changes # maintain an array of ActiveModel::Dirty.changes - @saved_changes_history << changes.dup + @ts_saved_changes_history << changes.dup # accumulate the most recent changes - @saved_changes_history.last.each_pair { |k, v| track_saved_change k, v } + @ts_saved_changes_history.last.each_pair { |k, v| ts_track_saved_change k, v } end # v is an an array of [prev, current] - def track_saved_change(key, value) - if @saved_changes_unfiltered.key? key - @saved_changes_unfiltered[key][1] = track_saved_value value[1] + def ts_track_saved_change(key, value) + if @ts_saved_changes_unfiltered.key? key + @ts_saved_changes_unfiltered[key][1] = ts_track_saved_value value[1] else - @saved_changes_unfiltered[key] = value.dup + @ts_saved_changes_unfiltered[key] = value.dup end end # type safe dup inspred by http://stackoverflow.com/a/20955038 - def track_saved_value(value) + def ts_track_saved_value(value) value.dup rescue TypeError value diff --git a/app/models/user.rb b/app/models/user.rb index 4eb10a1d..27b8ccc2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -88,8 +88,8 @@ class User < ApplicationRecord # we must resort to using a ActiveRecord::Concern which accumulates # changes from a commit (app/models/concerns/track_saved_changes.rb) # See https://github.com/ccmcbeck/after-commit - after_update_commit :mailbluster_update_email, if: ->(obj){ obj.saved_changes.key? 'email' } - after_update_commit :mailbluster_update_name, if: ->(obj){ obj.saved_changes.key? 'name' } + after_update_commit :mailbluster_update_email, if: ->(obj){ obj.ts_saved_changes.key? 'email' } + after_update_commit :mailbluster_update_name, if: ->(obj){ obj.ts_saved_changes.key? 'name' } # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} From 3d9b6400b97be6b58725f9a482a0503ba78f2875 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 9 Apr 2021 00:42:14 -0700 Subject: [PATCH 51/55] Fully address AR callback issues --- app/models/concerns/track_saved_changes.rb | 2 +- app/models/user.rb | 23 ++++++++-------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/app/models/concerns/track_saved_changes.rb b/app/models/concerns/track_saved_changes.rb index a7e928d4..db75e625 100644 --- a/app/models/concerns/track_saved_changes.rb +++ b/app/models/concerns/track_saved_changes.rb @@ -25,7 +25,7 @@ module TrackSavedChanges # on save def ts_track_saved_changes # maintain an array of ActiveModel::Dirty.changes - @ts_saved_changes_history << changes.dup + @ts_saved_changes_history << previous_changes.dup # accumulate the most recent changes @ts_saved_changes_history.last.each_pair { |k, v| ts_track_saved_change k, v } end diff --git a/app/models/user.rb b/app/models/user.rb index 27b8ccc2..09707336 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,15 +81,11 @@ class User < ApplicationRecord after_save :touch_events - after_create_commit :mailbluster_create_lead - after_destroy_commit :mailbluster_delete_lead - # Note that because a commit may cause multiple changes - # which are not fully tracked by ActiveRecord::Dirty, - # we must resort to using a ActiveRecord::Concern which accumulates - # changes from a commit (app/models/concerns/track_saved_changes.rb) - # See https://github.com/ccmcbeck/after-commit - after_update_commit :mailbluster_update_email, if: ->(obj){ obj.ts_saved_changes.key? 'email' } - after_update_commit :mailbluster_update_name, if: ->(obj){ obj.ts_saved_changes.key? 'name' } + # after_create_commit :mailbluster_create_lead + after_commit :mailbluster_create_lead, on: :create + # after_destroy_commit :mailbluster_delete_lead + after_commit :mailbluster_delete_lead, on: :destroy + after_commit :mailbluster_update_lead, on: :update, if: ->(user){ ['name','email'].any? { |key| user.ts_saved_changes.key? key } } # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} @@ -391,12 +387,9 @@ class User < ApplicationRecord MailblusterDeleteLeadJob.perform_later self end - def mailbluster_update_email - MailblusterEditLeadJob.perform_later(self, old_email: saved_changes['email'][0]) - end - - def mailbluster_update_name - MailblusterEditLeadJob.perform_later self + def mailbluster_update_lead + MailblusterEditLeadJob.perform_later(self, old_email: ts_saved_changes.fetch('email', [nil])[0]) + ts_reset_saved_changes end def touch_events From 757d16919daf2234311260bfce99094fc7273739 Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 9 Apr 2021 00:45:03 -0700 Subject: [PATCH 52/55] Fix rubocop error with missing space after comma --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 09707336..b3ebe306 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -85,7 +85,7 @@ class User < ApplicationRecord after_commit :mailbluster_create_lead, on: :create # after_destroy_commit :mailbluster_delete_lead after_commit :mailbluster_delete_lead, on: :destroy - after_commit :mailbluster_update_lead, on: :update, if: ->(user){ ['name','email'].any? { |key| user.ts_saved_changes.key? key } } + after_commit :mailbluster_update_lead, on: :update, if: ->(user){ ['name', 'email'].any? { |key| user.ts_saved_changes.key? key } } # add scope scope :comment_notifiable, ->(conference) {joins(:roles).where('roles.name IN (?)', [:organizer, :cfp]).where('roles.resource_type = ? AND roles.resource_id = ?', 'Conference', conference.id)} From cacb26582955d098874f9470088a953ffe897caf Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 9 Apr 2021 10:55:46 -0700 Subject: [PATCH 53/55] Get destroy job working properly and prevent misfires on updated users --- app/models/user.rb | 8 +++++--- app/services/mailbluster_manager.rb | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index b3ebe306..2f844c4e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,9 +81,9 @@ class User < ApplicationRecord after_save :touch_events - # after_create_commit :mailbluster_create_lead + # Note that using after_create_commit and after_update_commit does not work. + # See https://github.com/CactusPuppy/snapcon/pull/43#discussion_r609458034 after_commit :mailbluster_create_lead, on: :create - # after_destroy_commit :mailbluster_delete_lead after_commit :mailbluster_delete_lead, on: :destroy after_commit :mailbluster_update_lead, on: :update, if: ->(user){ ['name', 'email'].any? { |key| user.ts_saved_changes.key? key } } @@ -381,10 +381,12 @@ class User < ApplicationRecord def mailbluster_create_lead MailblusterCreateLeadJob.perform_later self + ts_reset_saved_changes end def mailbluster_delete_lead - MailblusterDeleteLeadJob.perform_later self + MailblusterDeleteLeadJob.perform_later self.email + ts_reset_saved_changes end def mailbluster_update_lead diff --git a/app/services/mailbluster_manager.rb b/app/services/mailbluster_manager.rb index 3925d1dd..d74ea76a 100644 --- a/app/services/mailbluster_manager.rb +++ b/app/services/mailbluster_manager.rb @@ -33,8 +33,8 @@ class MailblusterManager }) end - def self.delete_lead(user) - email_hash = Digest::MD5.hexdigest user.email + def self.delete_lead(email) + email_hash = Digest::MD5.hexdigest email query_api(:delete, "/#{email_hash}") end end From 34ac9791092a1a2fa95c0896f98aa15e1fef862a Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 9 Apr 2021 11:53:54 -0700 Subject: [PATCH 54/55] Fix spec that was not updated with update to delete_lead --- spec/services/mailbluster_manager_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/mailbluster_manager_spec.rb b/spec/services/mailbluster_manager_spec.rb index c1bffa27..52851089 100644 --- a/spec/services/mailbluster_manager_spec.rb +++ b/spec/services/mailbluster_manager_spec.rb @@ -131,7 +131,7 @@ describe MailblusterManager, type: :model do lead_url = url + email_hash.to_s stub_request(:delete, lead_url) .to_return(body: response_body) - response = described_class.delete_lead(user) + response = described_class.delete_lead(user.email) expect(WebMock).to have_requested(:delete, lead_url) expect(response).to eq(response_body) From d6da99cc1f00ccde8923f4c0c99f75f4d85f5e7a Mon Sep 17 00:00:00 2001 From: CactusPuppy Date: Fri, 9 Apr 2021 11:54:11 -0700 Subject: [PATCH 55/55] Fix Rubocop complaints --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 2f844c4e..ddcf63bb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -385,7 +385,7 @@ class User < ApplicationRecord end def mailbluster_delete_lead - MailblusterDeleteLeadJob.perform_later self.email + MailblusterDeleteLeadJob.perform_later email ts_reset_saved_changes end