Merge branch 'fix/admin_emails_unsubscribe' into 'master'

Fix admin emails unsubscribe

Fixes #151

See merge request !152

app/controllers/unsubscribes_controller.rb

@@ -15,8 +15,9 @@ class UnsubscribesController < ApplicationController
   end
 
   protected
+
   def get_user
-    @email = CGI.unescape(params[:email])
+    @email = Base64.urlsafe_decode64(params[:email])
     User.where(email: @email).first
   end
 end

app/mailers/emails/admin_notification.rb

@@ -2,9 +2,9 @@ module Emails
   module AdminNotification
     def send_admin_notification(user_id, subject, body)
       email = recipient(user_id)
-      @unsubscribe_url = unsubscribe_url(email: CGI.escape(email))
+      @unsubscribe_url = unsubscribe_url(email: Base64.urlsafe_encode64(email))
       @body = body
       mail to: email, subject: subject
     end
   end
-end
\ No newline at end of file
+end

app/views/unsubscribes/show.html.haml

 %h3.page-title Unsubscribe from Admin notifications
 
 %hr
-= form_tag unsubscribe_path(@email) do
+= form_tag unsubscribe_path(Base64.urlsafe_encode64(@email)) do
   %p
     Yes, I want to unsubscribe
     %strong= @email

features/steps/user.rb

@@ -7,4 +7,21 @@ class Spinach::Features::User < Spinach::FeatureSteps
   step 'I should see user "John Doe" page' do
     expect(page.title).to match(/^\s*John Doe/)
   end
+
+  step 'I visit unsubscribe link' do
+    email = Base64.urlsafe_encode64("joh@doe.org")
+    visit "/unsubscribes/#{email}"
+  end
+
+  step 'I should see unsubscribe text and button' do
+    page.should have_content "Unsubscribe from Admin notifications Yes, I want to unsubscribe joh@doe.org from any further admin emails."
+  end
+
+  step 'I press the unsubscribe button' do
+    click_button("Unsubscribe")
+  end
+
+  step 'I should be unsubscribed' do
+    current_path.should == root_path
+  end
 end

features/user.feature

@@ -67,3 +67,10 @@ Feature: User
     And I should see project "Enterprise"
     And I should not see project "Internal"
     And I should not see project "Community"
+
+  Scenario: I unsubscribe from admin notifications
+    Given I sign in as "John Doe"
+    When I visit unsubscribe link
+    Then I should see unsubscribe text and button
+    And I press the unsubscribe button
+    Then I should be unsubscribed

spec/controllers/unsubscribes_controller_spec.rb

@@ -5,13 +5,13 @@ describe UnsubscribesController do
 
   describe "show" do
     it "responds with success" do
-      get :show, email: CGI.escape('me@example.com')
+      get :show, email:  Base64.urlsafe_encode64('me@example.com')
 
       assert_response :success
     end
 
     it "behaves the same if email address isn't known in the system" do
-      get :show, email: CGI.escape('i@dont_exists.com')
+      get :show, email: Base64.urlsafe_encode64('i@dont_exists.com')
 
       assert_response :success
     end
@@ -19,14 +19,14 @@ describe UnsubscribesController do
 
   describe "create" do
     it "unsubscribes the connected user" do
-      post :create, email: CGI.escape('me@example.com')
+      post :create, email: Base64.urlsafe_encode64('me@example.com')
 
       assert user.reload.admin_email_unsubscribed_at
     end
 
     # Don't tell if the email does not exists
     it "behaves the same if email address isn't known in the system" do
-      post :create, email: CGI.escape('i@dont_exists.com')
+      post :create, email: Base64.urlsafe_encode64('i@dont_exists.com')
 
       assert_response :redirect
     end

spec/mailers/notify_spec.rb

@@ -622,4 +622,30 @@ describe Notify do
       should have_body_text /#{diff_path}/
     end
   end
+
+  describe 'admin notification' do
+    let(:example_site_path) { root_path }
+    let(:user) { create(:user) }
+
+    subject { @email = Notify.send_admin_notification(user.id, 'Admin announcement','Text') }
+
+    it 'is sent as the author' do
+      sender = subject.header[:from].addrs[0]
+      sender.display_name.should eq("GitLab")
+      sender.address.should eq(gitlab_sender)
+    end
+
+    it 'is sent to recipient' do
+      should deliver_to user.email
+    end
+
+    it 'has the correct subject' do
+      should have_subject 'Admin announcement'
+    end
+
+    it 'includes unsubscribe link' do
+      unsubscribe_link = "http://localhost/unsubscribes/#{Base64.urlsafe_encode64(user.email)}"
+      should have_body_text(unsubscribe_link)
+    end
+  end
 end