From 290535b55768109f6d941fbfa233620a0d1a337c Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 27 Aug 2013 21:22:35 +1000 Subject: [PATCH 1/7] Add newsletter subscription checkbox --- app/models/member.rb | 2 +- app/views/devise/registrations/edit.html.haml | 7 +++++++ app/views/devise/registrations/new.html.haml | 8 +++++++- db/migrate/20130827105823_add_newsletter_to_member.rb | 5 +++++ db/schema.rb | 5 ++++- spec/views/devise/registrations/edit_spec.rb | 4 ++++ spec/views/devise/registrations/new_spec.rb | 4 ++++ 7 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20130827105823_add_newsletter_to_member.rb diff --git a/app/models/member.rb b/app/models/member.rb index 4d93bce4a..40265cbb8 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -38,7 +38,7 @@ class Member < ActiveRecord::Base # Setup accessible (or protected) attributes for your model attr_accessible :login_name, :email, :password, :password_confirmation, - :remember_me, :login, :tos_agreement, :show_email, + :remember_me, :login, :tos_agreement, :show_email, :newsletter, :location, :latitude, :longitude, :send_notification_email, :bio # set up geocoding diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 5fa1bcab4..4dfcacee4 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -21,6 +21,13 @@ = f.check_box :send_notification_email Receive emailed copies of Inbox notifications. + .control-group + .controls + = f.check_box :newsletter + Subscribe to the + = succeed "." do + = link_to 'Growstuff newsletter', 'http://blog.growstuff.org/category/newsletter' + %h2 Profile details .control-group %label.control-label diff --git a/app/views/devise/registrations/new.html.haml b/app/views/devise/registrations/new.html.haml index e84902314..e83776206 100644 --- a/app/views/devise/registrations/new.html.haml +++ b/app/views/devise/registrations/new.html.haml @@ -29,7 +29,13 @@ .controls = f.check_box :tos_agreement I agree to the - = link_to('Terms of Service', url_for(:action => 'tos', :controller => '/policy')) + = succeed "." do + = link_to 'Terms of Service', url_for(:action => 'tos', :controller => '/policy') + .controls + = f.check_box :newsletter + Subscribe to the + = succeed "." do + = link_to 'Growstuff newsletter', 'http://blog.growstuff.org/category/newsletter' .form-actions = f.submit "Sign up", :class => 'btn btn-primary' diff --git a/db/migrate/20130827105823_add_newsletter_to_member.rb b/db/migrate/20130827105823_add_newsletter_to_member.rb new file mode 100644 index 000000000..919e5222f --- /dev/null +++ b/db/migrate/20130827105823_add_newsletter_to_member.rb @@ -0,0 +1,5 @@ +class AddNewsletterToMember < ActiveRecord::Migration + def change + add_column :members, :newsletter, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 2237703ee..ffa72fea8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130821073736) do +ActiveRecord::Schema.define(:version => 20130827105823) do create_table "account_types", :force => true do |t| t.string "name", :null => false @@ -116,6 +116,8 @@ ActiveRecord::Schema.define(:version => 20130821073736) do t.float "longitude" t.boolean "send_notification_email", :default => true t.text "bio" + t.integer "plantings_count" + t.boolean "newsletter" end add_index "members", ["confirmation_token"], :name => "index_users_on_confirmation_token", :unique => true @@ -192,6 +194,7 @@ ActiveRecord::Schema.define(:version => 20130821073736) do t.string "slug" t.string "sunniness" t.string "planted_from" + t.integer "owner_id" end add_index "plantings", ["slug"], :name => "index_plantings_on_slug", :unique => true diff --git a/spec/views/devise/registrations/edit_spec.rb b/spec/views/devise/registrations/edit_spec.rb index 2f3981fc6..cfd18c6de 100644 --- a/spec/views/devise/registrations/edit_spec.rb +++ b/spec/views/devise/registrations/edit_spec.rb @@ -30,6 +30,10 @@ describe 'devise/registrations/edit.html.haml', :type => "view" do it 'has a checkbox for email notifications' do assert_select "input[id=member_send_notification_email][type=checkbox]" end + + it 'has a checkbox for newsletter subscription' do + assert_select "input[id=member_newsletter][type=checkbox]" + end end context 'profile section' do diff --git a/spec/views/devise/registrations/new_spec.rb b/spec/views/devise/registrations/new_spec.rb index 48bf90692..b17b000b7 100644 --- a/spec/views/devise/registrations/new_spec.rb +++ b/spec/views/devise/registrations/new_spec.rb @@ -14,5 +14,9 @@ describe 'devise/registrations/new.html.haml', :type => "view" do it 'should have some fields' do rendered.should contain 'Email' end + + it 'has a checkbox for newsletter subscription' do + assert_select "input[id=member_newsletter][type=checkbox]" + end end end From 30bb87753784be76c2ce8330ae8009fb5668aee4 Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 27 Aug 2013 21:34:30 +1000 Subject: [PATCH 2/7] Added wants_newsletter scope to Member also cleaned up member tests which were using FG.build instead of .create --- app/models/member.rb | 1 + spec/factories/member.rb | 4 ++++ spec/models/member_spec.rb | 28 ++++++++++------------------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/app/models/member.rb b/app/models/member.rb index 40265cbb8..8a6b5e84f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -28,6 +28,7 @@ class Member < ActiveRecord::Base scope :confirmed, where('confirmed_at IS NOT NULL') scope :located, where("location <> ''") scope :recently_signed_in, reorder('updated_at DESC') + scope :wants_newsletter, where(:newsletter => true) # Include default devise modules. Others available are: # :token_authenticatable, :confirmable, diff --git a/spec/factories/member.rb b/spec/factories/member.rb index 0f1941436..19836a628 100644 --- a/spec/factories/member.rb +++ b/spec/factories/member.rb @@ -15,6 +15,10 @@ FactoryGirl.define do tos_agreement false end + factory :newsletter_recipient_member do + newsletter true + end + factory :no_bio_member do bio nil end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 2dc6be3c6..b19b087d0 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -4,15 +4,10 @@ describe 'member' do context 'valid member' do before(:each) do - @member = FactoryGirl.build(:member) - end - - it 'should save a basic member' do - @member.save.should be_true + @member = FactoryGirl.create(:member) end it 'should be fetchable from the database' do - @member.save @member2 = Member.find(@member.id) @member2.should be_an_instance_of Member @member2.login_name.should match(/member\d+/) @@ -20,40 +15,32 @@ describe 'member' do end it 'should have a friendly slug' do - @member.save @member.slug.should match(/member\d+/) end it 'has a bio' do @member.bio = 'I love seeds' - @member.save @member.bio.should eq 'I love seeds' end it 'should have a default garden' do - @member.save @member.gardens.count.should == 1 end it 'should have a accounts entry' do - @member.save @member.account.should be_an_instance_of Account end it "should have a default-type account by default" do - @member.save @member.account.account_type.name.should eq Growstuff::Application.config.default_account_type - @member.is_paid?.should be_false end it "doesn't show email by default" do - @member.save @member.show_email.should be_false end it 'should stringify as the login_name' do - @member.save @member.to_s.should match(/member\d+/) "#{@member}".should match(/member\d+/) end @@ -64,12 +51,10 @@ describe 'member' do end it 'should be able to fetch gardens' do - @member.save @member.gardens.first.name.should eq "Garden" end it 'has many plantings through gardens' do - @member.save @planting = FactoryGirl.create(:planting, :garden => @member.gardens.first ) @@ -77,14 +62,12 @@ describe 'member' do end it "has many comments" do - @member.save @comment1 = FactoryGirl.create(:comment, :author => @member) @comment2 = FactoryGirl.create(:comment, :author => @member) @member.comments.length.should == 2 end it "has many forums" do - @member.save @forum1 = FactoryGirl.create(:forum, :owner => @member) @forum2 = FactoryGirl.create(:forum, :owner => @member) @member.forums.length.should == 2 @@ -117,6 +100,15 @@ describe 'member' do end end + context 'newsletter scope' do + it 'finds newsletter recipients' do + @member1 = FactoryGirl.create(:member) + @member2 = FactoryGirl.create(:newsletter_recipient_member) + Member.wants_newsletter.should include @member2 + Member.wants_newsletter.should_not include @member1 + end + end + context 'same :login_name' do it "should not allow two members with the same login_name" do FactoryGirl.create(:member, :login_name => "bob") From 16f4e153f23ee9102857db40ed52d08b1fcbe95b Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 27 Aug 2013 21:49:54 +1000 Subject: [PATCH 3/7] Made admin/newsletter page with list of subscribers --- app/controllers/admin_controller.rb | 8 ++++++++ app/views/admin/index.html.haml | 1 + app/views/admin/newsletter.html.haml | 6 ++++++ config/routes.rb | 1 + spec/controllers/admin_controller_spec.rb | 15 +++++++++++++++ spec/views/admin/index_spec.rb | 4 ++++ spec/views/admin/newsletter_spec.rb | 16 ++++++++++++++++ 7 files changed, 51 insertions(+) create mode 100644 app/views/admin/newsletter.html.haml create mode 100644 spec/views/admin/newsletter_spec.rb diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 272e14eea..cfbb5c014 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -5,4 +5,12 @@ class AdminController < ApplicationController format.html # index.html.haml end end + + def newsletter + authorize! :manage, :all + @members = Member.confirmed.wants_newsletter.all + respond_to do |format| + format.html # index.html.haml + end + end end diff --git a/app/views/admin/index.html.haml b/app/views/admin/index.html.haml index 0ce432ccd..b9f814df8 100644 --- a/app/views/admin/index.html.haml +++ b/app/views/admin/index.html.haml @@ -7,6 +7,7 @@ %li= link_to "Products", products_path %li= link_to "Roles", roles_path %li= link_to "Forums", forums_path + %li= link_to "Newsletter subscribers", admin_newsletter_path %h2 Orders diff --git a/app/views/admin/newsletter.html.haml b/app/views/admin/newsletter.html.haml new file mode 100644 index 000000000..adf75ce6b --- /dev/null +++ b/app/views/admin/newsletter.html.haml @@ -0,0 +1,6 @@ +-content_for :title, 'Newsletter subscribers' + +%p +- @members.each do |m| + = m.email + %br/ diff --git a/config/routes.rb b/config/routes.rb index 341aacec8..62146251a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -117,6 +117,7 @@ Growstuff::Application.routes.draw do match '/admin/orders' => 'admin/orders#index' match '/admin/orders/:action' => 'admin/orders#:action' match '/admin' => 'admin#index' + match '/admin/newsletter' => 'admin#newsletter', :as => :admin_newsletter match '/admin/:action' => 'admin#:action' end diff --git a/spec/controllers/admin_controller_spec.rb b/spec/controllers/admin_controller_spec.rb index 587324b52..76f56b78a 100644 --- a/spec/controllers/admin_controller_spec.rb +++ b/spec/controllers/admin_controller_spec.rb @@ -2,4 +2,19 @@ require 'spec_helper' describe AdminController do + login_member(:admin_member) + + describe "GET admin/newsletter" do + it 'fetches the admin newsletter page' do + get :newsletter + response.should be_success + response.should render_template("admin/newsletter") + end + + it 'assigns @members' do + m = FactoryGirl.create(:newsletter_recipient_member) + get :newsletter + assigns(:members).should eq [m] + end + end end diff --git a/spec/views/admin/index_spec.rb b/spec/views/admin/index_spec.rb index d63f897ee..fe92a4193 100644 --- a/spec/views/admin/index_spec.rb +++ b/spec/views/admin/index_spec.rb @@ -14,4 +14,8 @@ describe 'admin/index.html.haml', :type => "view" do assert_select "a", :href => roles_path assert_select "a", :href => forums_path end + + it "has a link to newsletter subscribers" do + rendered.should contain "Newsletter subscribers" + end end diff --git a/spec/views/admin/newsletter_spec.rb b/spec/views/admin/newsletter_spec.rb new file mode 100644 index 000000000..a0d871cd4 --- /dev/null +++ b/spec/views/admin/newsletter_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe 'admin/newsletter.html.haml', :type => "view" do + before(:each) do + @member = FactoryGirl.create(:admin_member) + sign_in @member + controller.stub(:current_user) { @member } + @subscriber = FactoryGirl.create(:newsletter_recipient_member) + assign(:members, [@subscriber]) + render + end + + it "lists newsletter subscribers by email" do + rendered.should contain @subscriber.email + end +end From f747db7518e7275a3723584c65c7dc50cf44a5f7 Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 27 Aug 2013 22:45:18 +1000 Subject: [PATCH 4/7] newsletter subscription/unsubscription via Mailchimp API --- Gemfile | 2 ++ Gemfile.lock | 8 ++++++++ app/models/member.rb | 36 +++++++++++++++++++++++++++++++++++- config/application.rb | 5 +++++ credentials.example | 6 ++++++ 5 files changed, 56 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 04f2ea9b9..6abaeeeeb 100644 --- a/Gemfile +++ b/Gemfile @@ -10,6 +10,8 @@ gem 'unicorn' # http server gem 'cancan' # for checking member privileges +gem 'gibbon' # for Mailchimp newsletter subscriptions + # vendored activemerchant for testing- needed for bogus paypal # gateway monkeypatch gem 'activemerchant', '1.33.0', diff --git a/Gemfile.lock b/Gemfile.lock index e82224106..7dc919be9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -106,6 +106,9 @@ GEM friendly_id (4.0.9) fssm (0.2.10) geocoder (1.1.8) + gibbon (1.0.0) + httparty + multi_json (>= 1.3.4) gravatar-ultimate (1.0.3) haml (4.0.3) tilt @@ -116,6 +119,9 @@ GEM railties (>= 3.1, < 4.1) hashie (2.0.5) hike (1.2.3) + httparty (0.11.0) + multi_json (~> 1.0) + multi_xml (>= 0.5.2) i18n (0.6.1) journey (1.0.4) jquery-rails (3.0.4) @@ -136,6 +142,7 @@ GEM mime-types (1.23) mini_portile (0.5.1) multi_json (1.7.7) + multi_xml (0.5.5) newrelic_rpm (3.6.5.130) nokogiri (1.6.0) mini_portile (~> 0.5.0) @@ -251,6 +258,7 @@ DEPENDENCIES flickraw friendly_id geocoder + gibbon gravatar-ultimate haml haml-rails diff --git a/app/models/member.rb b/app/models/member.rb index 8a6b5e84f..90e3d8932 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -81,6 +81,8 @@ class Member < ActiveRecord::Base # which can happen sometimes especially with FactoryGirl associations after_create {|member| Account.find_or_create_by_member_id(:member_id => member.id) } + after_save :update_newsletter_subscription + # allow login via either login_name or email address def self.find_first_by_auth_conditions(warden_conditions) conditions = warden_conditions.dup @@ -202,7 +204,8 @@ class Member < ActiveRecord::Base return interesting_members end - protected + private + def empty_unwanted_geocodes if self.location.to_s == '' self.latitude = nil @@ -210,4 +213,35 @@ class Member < ActiveRecord::Base end end + def update_newsletter_subscription + if confirmed_at_changed? and newsletter # just signed up + newsletter_subscribe + elsif confirmed_at # i.e. after member's confirmed their account + if newsletter_changed? # edited member settings + if newsletter + newsletter_subscribe + else + newsletter_unsubscribe + end + end + end + end + + def newsletter_subscribe + gb = Gibbon::API.new + res = gb.lists.subscribe({ + :id => ENV['MAILCHIMP_NEWSLETTER_ID'], + :email => { :email => email }, + :double_optin => false # they alredy confirmed their email with us + }) + end + + def newsletter_unsubscribe + gb = Gibbon::API.new + res = gb.lists.unsubscribe({ + :id => ENV['MAILCHIMP_NEWSLETTER_ID'], + :email => { :email => email } + }) + end + end diff --git a/config/application.rb b/config/application.rb index 0648a5322..c0ff21d1a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -78,5 +78,10 @@ module Growstuff :openssl_verify_mode => 'none' } + Gibbon::API.api_key = ENV['MAILCHIMP_APIKEY'] + Gibbon::API.timeout = 10 + Gibbon::API.throws_exceptions = false + config.newsletter_list_id = ENV['MAILCHIMP_NEWSLETTER_ID'] + end end diff --git a/credentials.example b/credentials.example index 74b7fa9df..60c68ddec 100755 --- a/credentials.example +++ b/credentials.example @@ -31,3 +31,9 @@ export TWITTER_SECRET="" # Get Flickr key from http://www.flickr.com/services/apps/create/apply/ export FLICKR_KEY="" export FLICKR_SECRET="" + +# Used for subscribing/unsubscribing people from the newsletter +export MAILCHIMP_APIKEY="" +# Used to identify the newsletter via the API +# Fetch list IDs using Gibbon: puts gb.lists.list.to_yaml +export MAILCHIMP_NEWSLETTER_ID="" From 1433a51b9142c5220988de129c42f467cc93f63b Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 27 Aug 2013 22:54:30 +1000 Subject: [PATCH 5/7] Add text to show we're lovely people and not spammers --- app/views/devise/registrations/_newsletter_blurb.html.haml | 5 +++++ app/views/devise/registrations/edit.html.haml | 6 +++--- app/views/devise/registrations/new.html.haml | 7 ++++--- 3 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 app/views/devise/registrations/_newsletter_blurb.html.haml diff --git a/app/views/devise/registrations/_newsletter_blurb.html.haml b/app/views/devise/registrations/_newsletter_blurb.html.haml new file mode 100644 index 000000000..a731d6724 --- /dev/null +++ b/app/views/devise/registrations/_newsletter_blurb.html.haml @@ -0,0 +1,5 @@ +The #{Growstuff::Application.config.site_name} newsletter is sent out +every few weeks. Check out our += link_to 'past newsletters', 'http://blog.growstuff.org/category/newsletter/' +if you want to see the sort of thing we'll send you. Of course, we'll never spam you +or sell your email address, and you can unsubscribe any time. diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 4dfcacee4..28dfdb223 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -24,9 +24,9 @@ .control-group .controls = f.check_box :newsletter - Subscribe to the - = succeed "." do - = link_to 'Growstuff newsletter', 'http://blog.growstuff.org/category/newsletter' + Subscribe to the #{Growstuff::Application.config.site_name} newsletter + .help-inline + = render :partial => 'newsletter_blurb' %h2 Profile details .control-group diff --git a/app/views/devise/registrations/new.html.haml b/app/views/devise/registrations/new.html.haml index e83776206..4364ac3be 100644 --- a/app/views/devise/registrations/new.html.haml +++ b/app/views/devise/registrations/new.html.haml @@ -31,11 +31,12 @@ I agree to the = succeed "." do = link_to 'Terms of Service', url_for(:action => 'tos', :controller => '/policy') + .control-group .controls = f.check_box :newsletter - Subscribe to the - = succeed "." do - = link_to 'Growstuff newsletter', 'http://blog.growstuff.org/category/newsletter' + Subscribe to the #{Growstuff::Application.config.site_name} newsletter + .help-inline + = render :partial => 'newsletter_blurb' .form-actions = f.submit "Sign up", :class => 'btn btn-primary' From 411c5bcd0422b0172c11013602490d81ff22ed62 Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 27 Aug 2013 23:09:15 +1000 Subject: [PATCH 6/7] split the difference: pre-checked newsletter signup + friendly text --- app/views/devise/registrations/new.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/devise/registrations/new.html.haml b/app/views/devise/registrations/new.html.haml index 4364ac3be..58d93f902 100644 --- a/app/views/devise/registrations/new.html.haml +++ b/app/views/devise/registrations/new.html.haml @@ -33,7 +33,7 @@ = link_to 'Terms of Service', url_for(:action => 'tos', :controller => '/policy') .control-group .controls - = f.check_box :newsletter + = f.check_box :newsletter, :checked => true Subscribe to the #{Growstuff::Application.config.site_name} newsletter .help-inline = render :partial => 'newsletter_blurb' From 0c5fae1274805a8b1a3f2d6348992b3541bbba6a Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 28 Aug 2013 10:42:40 +1000 Subject: [PATCH 7/7] Send Growstuff login_name to Mailchimp This lets us do mail merges with the member's name, as well as letting us segment mailouts by people who've signed up for Growstuff and those who came to the newsletter directly without having a Growstuff account. --- app/models/member.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/member.rb b/app/models/member.rb index 90e3d8932..d25834af8 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -232,6 +232,7 @@ class Member < ActiveRecord::Base res = gb.lists.subscribe({ :id => ENV['MAILCHIMP_NEWSLETTER_ID'], :email => { :email => email }, + :merge_vars => { :login_name => login_name }, :double_optin => false # they alredy confirmed their email with us }) end