diff --git a/app/controllers/email_controller.rb b/app/controllers/email_controller.rb
index 0845570229..7fe28798df 100644
--- a/app/controllers/email_controller.rb
+++ b/app/controllers/email_controller.rb
@@ -13,13 +13,18 @@ class EmailController < ApplicationController
@user = User.find_by_temporary_key(params[:key])
# Don't allow the use of a key while logged in as a different user
- @user = nil if current_user.present? && (@user != current_user)
-
- if @user.present?
- @user.update_column(:email_digests, false)
- else
- @not_found = true
+ if current_user.present? && (@user != current_user)
+ @different_user = true
+ return
end
+
+ if @user.blank?
+ @not_found = true
+ return
+ end
+
+ @user.update_column(:email_digests, false)
+ @success = true
end
def resubscribe
diff --git a/app/models/user.rb b/app/models/user.rb
index 4b8e07673b..574152f564 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -185,7 +185,7 @@ class User < ActiveRecord::Base
# Use a temporary key to find this user, store it in redis with an expiry
def temporary_key
key = SecureRandom.hex(32)
- $redis.setex "temporary_key:#{key}", 1.week, id.to_s
+ $redis.setex "temporary_key:#{key}", 2.months, id.to_s
key
end
diff --git a/app/views/email/resubscribe.html.erb b/app/views/email/resubscribe.html.erb
index f83d460de7..1cef86a7f0 100644
--- a/app/views/email/resubscribe.html.erb
+++ b/app/views/email/resubscribe.html.erb
@@ -1,7 +1,6 @@
-
<%= t :'resubscribe.title' %>
+
<%= t :'resubscribe.description' %>
-
diff --git a/app/views/email/unsubscribe.html.erb b/app/views/email/unsubscribe.html.erb
index e15f6032a1..488442965f 100644
--- a/app/views/email/unsubscribe.html.erb
+++ b/app/views/email/unsubscribe.html.erb
@@ -1,7 +1,8 @@
- <%- unless @not_found %>
+ <%- if @success %>
<%= t :'unsubscribed.title' %>
+
<%= t :'unsubscribed.description' %>
@@ -11,10 +12,18 @@
<%= submit_tag t(:'resubscribe.action'), class: 'btn btn-danger' %>
<% end %>
<%- else %>
-
<%= t :'unsubscribed.not_found' %>
-
<%= t :'unsubscribed.not_found_description' %>
+
<%= t :'unsubscribed.error' %>
+
+ <%- if @different_user %>
+
<%= t :'unsubscribed.different_user_description' %>
+ <%- end %>
+
+ <%- if @not_found %>
+
<%= t :'unsubscribed.not_found_description' %>
+ <%- end %>
+
+
<%=raw(t :'unsubscribed.preferences_link') %>
+
<%- end %>
-
-
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 5330311af2..f9472290fa 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -460,7 +460,9 @@ en:
title: 'Unsubscribed'
description: "You have been unsubscribed. We won't contact you again!"
oops: "In case you didn't mean to do this, click below."
- not_found: "Error Unsubscribing"
+ error: "Error Unsubscribing"
+ preferences_link: "You can also unsubscribe from digest emails on your preferences page"
+ different_user_description: "You are currently logged in as a different user than the one who the digest was mailed to. Please log out and try again."
not_found_description: "Sorry, we couldn't unsubscribe you. It's possible the link in your email has expired."
resubscribe:
diff --git a/spec/controllers/email_controller_spec.rb b/spec/controllers/email_controller_spec.rb
index 0d1d8d6b34..e6fb69bdaf 100644
--- a/spec/controllers/email_controller_spec.rb
+++ b/spec/controllers/email_controller_spec.rb
@@ -50,8 +50,18 @@ describe EmailController do
user.email_digests.should be_false
end
- it "does not set the not_found instance variable" do
- assigns(:not_found).should be_blank
+ it "sets the appropriate instance variables" do
+ assigns(:success).should be_present
+ end
+ end
+
+ context "with an expired key or invalid key" do
+ before do
+ get :unsubscribe, key: 'watwatwat'
+ end
+
+ it "sets the appropriate instance variables" do
+ assigns(:success).should be_blank
end
end
@@ -67,8 +77,9 @@ describe EmailController do
user.email_digests.should be_true
end
- it 'sets not found' do
- assigns(:not_found).should be_true
+ it 'sets the appropriate instance variables' do
+ assigns(:success).should be_blank
+ assigns(:different_user).should be_present
end
end
@@ -84,8 +95,8 @@ describe EmailController do
user.email_digests.should be_false
end
- it "doesn't set not found" do
- assigns(:not_found).should be_blank
+ it 'sets the appropriate instance variables' do
+ assigns(:success).should be_present
end
end