Improve the unsubscribe to digest experience. Give a link in case it

fails, provide a different message if you are logged in as a different
user, increase expiry to 2 months from 1 week.

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 current_user.present? && (@user != current_user)
-
+      @different_user = true
-    if @user.present?
+      return
-      @user.update_column(:email_digests, false)
-    else
-      @not_found = true
     end
+
+    if @user.blank?
+      @not_found = true
+      return
+    end
+
+    @user.update_column(:email_digests, false)
+    @success = true
   end
 
   def resubscribe

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
 

app/views/email/resubscribe.html.erb

@@ -1,7 +1,6 @@
 <div class='container'>
-
   <h2><%= t :'resubscribe.title' %></h2>
+  <br/>
 
   <p><%= t :'resubscribe.description' %></p>
-
 </div>

app/views/email/unsubscribe.html.erb

@@ -1,7 +1,8 @@
 <div class='container'>
 
-  <%- unless @not_found %>
+  <%- if @success %>
     <h2><%= t :'unsubscribed.title' %></h2>
+    <br/>
 
     <p><%= t :'unsubscribed.description' %></p>
 
@@ -11,10 +12,18 @@
       <%= submit_tag t(:'resubscribe.action'), class: 'btn btn-danger' %>
     <% end %>
   <%- else %>
-    <h2><%= t :'unsubscribed.not_found' %></h2>
+    <h2><%= t :'unsubscribed.error' %></h2>
-    <p><%= t :'unsubscribed.not_found_description' %></p>
+    <br/>
+    <%- if @different_user %>
+      <p><%= t :'unsubscribed.different_user_description' %></p>
+    <%- end %>
+
+    <%- if @not_found %>
+      <p><%= t :'unsubscribed.not_found_description' %></p>
+    <%- end %>
+
+    <p><%=raw(t :'unsubscribed.preferences_link') %></p>
+
   <%- end %>
 
-
-
 </div>

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 <a href='/my/preferences'>preferences page</a>"
+    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:

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
+      it "sets the appropriate instance variables" do
-        assigns(:not_found).should be_blank
+        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
+      it 'sets the appropriate instance variables' do
-        assigns(:not_found).should be_true
+        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
+      it 'sets the appropriate instance variables' do
-        assigns(:not_found).should be_blank
+        assigns(:success).should be_present
       end
     end