Hook up user confirmation page
authorJohn Firebaugh <john.firebaugh@gmail.com>
Thu, 1 Aug 2013 19:38:15 +0000 (12:38 -0700)
committerJohn Firebaugh <john.firebaugh@gmail.com>
Mon, 12 Aug 2013 20:34:44 +0000 (13:34 -0700)
app/assets/images/confirm-illustration.png [moved from app/assets/images/verify-illustration.png with 100% similarity]
app/assets/images/confirm-illustration.svg [moved from app/assets/images/verify-illustration.svg with 100% similarity]
app/assets/stylesheets/common.css.scss
app/controllers/user_controller.rb
app/views/site/verifysignup.html.erb [deleted file]
app/views/user/confirm.html.erb
config/locales/en.yml
config/routes.rb
test/functional/user_controller_test.rb
test/integration/user_creation_test.rb

index 7eec371..d4afea4 100644 (file)
@@ -1046,7 +1046,7 @@ ul.results-list li { border-bottom: 1px solid #ccc; }
 }
 
 /* Overrides for pages that use new layout conventions */
-.site-verifysignup,
+.user-confirm,
 .site-copyright,
 .site-welcome {
   #content {
@@ -1055,40 +1055,48 @@ ul.results-list li { border-bottom: 1px solid #ccc; }
 }
 
 .user-new,
-.user-create {
+.user-create,
+.user-confirm {
   .content-heading {
     height: 200px;
   }
+}
 
-  #content.pad2 {
+.user-new,
+.user-create {
+  #content {
     padding: 0;
   }
 }
 
-.header-illustration { 
+.header-illustration {
   background-position: 0 0;
   background-repeat: no-repeat;
   position: absolute;
-}
-
-.header-illustration.new-user-main {
   height: 200px;
   width: 100%;
   left: 0;
   bottom: 0;
-  background-image: image-url("sign-up-illustration.png");
-}
 
-.header-illustration.new-user-arm {
-  height: 110px;
-  width: 130px;
-  left: 260px;
-  top: 160px;
-  background-image: image-url("sign-up-illustration-arm.png");
+  &.new-user-main {
+    background-image: image-url("sign-up-illustration.png");
+  }
+
+  &.confirm-main {
+    background-image: image-url("confirm-illustration.png");
+  }
+
+  &.new-user-arm {
+    height: 110px;
+    width: 130px;
+    left: 260px;
+    top: 160px;
+    background-image: image-url("sign-up-illustration-arm.png");
+  }
 }
 
 @media only screen and (max-width:770px) {
-  .new-user-illustration.new-user-arm { display: none;}
+  .header-illustration.new-user-arm { display: none;}
 }
 
 .wrapper {
@@ -2282,25 +2290,6 @@ a.button {
   background: #fff;
 }
 
-/* Rules for the "Verify signup" page */
-.site-verifysignup .content-heading { 
-  height: 200px;  
-}
-
-.header-illustration.verify-main { 
-  background-position: 0 0;
-  background-repeat: no-repeat;
-  position: absolute;
-}
-
-.header-illustration.verify-main {
-  height: 200px;
-  width: 100%;
-  left: 0;
-  bottom: 0;
-  background-image: image-url("verify-illustration.png");
-}
-
 /* Rules for the "Welcome" page */
 .site-welcome {
   .center {
index df0c8d8..dfacb9d 100644 (file)
@@ -89,17 +89,12 @@ class UserController < ApplicationController
           flash[:piwik_goal] = PIWIK_SIGNUP_GOAL if defined?(PIWIK_SIGNUP_GOAL)
 
           if @user.status == "active"
-            flash[:notice] = t 'user.new.flash welcome', :email => @user.email
             session[:referer] = welcome_path
-
             successful_login(@user)
           else
-            flash[:notice] = t 'user.new.flash create success message', :email => @user.email
             session[:token] = @user.tokens.create.token
-
             Notifier.signup_confirm(@user, @user.tokens.create(:referer => welcome_path)).deliver
-
-            redirect_to :action => 'login', :referer => params[:referer]
+            redirect_to :action => 'confirm', :display_name => @user.display_name
           end
         else
           render :action => 'new', :referer => params[:referer]
@@ -302,56 +297,41 @@ class UserController < ApplicationController
   end
 
   def confirm
-    if request.post?
-      if token = UserToken.find_by_token(params[:confirm_string])
-        if token.user.active?
-          flash[:error] = t('user.confirm.already active')
-          redirect_to :action => 'login'
-        else
-          user = token.user
-          user.status = "active"
-          user.email_valid = true
-          user.save!
-          referer = token.referer
-          token.destroy
-
-          if session[:token]
-            token = UserToken.find_by_token(session[:token])
-            session.delete(:token)
-          else
-            token = nil
-          end
-
-          if token.nil? or token.user != user
-            flash[:notice] = t('user.confirm.success')
-            redirect_to :action => :login, :referer => referer
-          else
-            token.destroy
-
-            session[:user] = user.id
-            cookies.permanent["_osm_username"] = user.display_name
-
-            if referer.nil?
-              flash[:notice] = t('user.confirm.success') + "<br /><br />" + t('user.confirm.before you start')
-              redirect_to :action => :account, :display_name => user.display_name
-            else
-              flash[:notice] = t('user.confirm.success')
-              redirect_to referer
-            end
-          end
-        end
+    if request.post? && (token = UserToken.find_by_token(params[:confirm_string]))
+      if token.user.active?
+        flash[:error] = t('user.confirm.already active')
+        redirect_to :action => 'login'
       else
-        user = User.find_by_display_name(params[:display_name])
+        user = token.user
+        user.status = "active"
+        user.email_valid = true
+        user.save!
+        referer = token.referer
+        token.destroy
 
-        if user and user.active?
-          flash[:error] = t('user.confirm.already active')
-        elsif user
-          flash[:error] = t('user.confirm.unknown token') + t('user.confirm.reconfirm', :reconfirm => url_for(:action => 'confirm_resend', :display_name => params[:display_name]))
+        if session[:token]
+          token = UserToken.find_by_token(session[:token])
+          session.delete(:token)
         else
-          flash[:error] = t('user.confirm.unknown token')
+          token = nil
         end
 
-        redirect_to :action => 'login'
+        if token.nil? or token.user != user
+          flash[:notice] = t('user.confirm.success')
+          redirect_to :action => :login, :referer => referer
+        else
+          token.destroy
+
+          session[:user] = user.id
+          cookies.permanent["_osm_username"] = user.display_name
+
+          redirect_to referer || welcome_path
+        end
+      end
+    else
+      user = User.find_by_display_name(params[:display_name])
+      if !user || user.active?
+        redirect_to root_path
       end
     end
   end
@@ -517,7 +497,7 @@ private
     if user = User.authenticate(:username => username, :password => password)
       successful_login(user)
     elsif user = User.authenticate(:username => username, :password => password, :pending => true)
-      failed_login t('user.login.account not active', :reconfirm => url_for(:action => 'confirm_resend', :display_name => user.display_name))
+      unconfirmed_login(user)
     elsif User.authenticate(:username => username, :password => password, :suspended => true)
       failed_login t('user.login.account is suspended', :webmaster => "mailto:webmaster@openstreetmap.org")
     else
@@ -548,7 +528,7 @@ private
         if user = User.find_by_openid_url(identity_url)
           case user.status
             when "pending" then
-              failed_login t('user.login.account not active', :reconfirm => url_for(:action => 'confirm_resend', :display_name => user.display_name))
+              unconfirmed_login(user)
             when "active", "confirmed" then
               successful_login(user)
             when "suspended" then
@@ -679,6 +659,15 @@ private
     session.delete(:referer)
   end
 
+  ##
+  #
+  def unconfirmed_login(user)
+    redirect_to :action => 'confirm', :display_name => user.display_name
+
+    session.delete(:remember_me)
+    session.delete(:referer)
+  end
+
   ##
   # update a user's details
   def update_user(user, params)
diff --git a/app/views/site/verifysignup.html.erb b/app/views/site/verifysignup.html.erb
deleted file mode 100644 (file)
index 4f5ecb6..0000000
+++ /dev/null
@@ -1,13 +0,0 @@
-<% content_for :head do %>
-<% end %>
-
-<%= content_for :heading do %>
-  <h2><%= t "verify_page.title" %></h2>
-  <div class='header-illustration verify-main'></div>
-<% end %>
-
-<h1><%= t "verify_page.introduction_html" %></h1>
-
-<p class='deemphasize'><%= t "verify_page.antispam_alert_html" %></p>
-
-<%= link_to t('layouts.log_in'), login_path(:referer => request.fullpath), {:class => 'button', :title => t('layouts.log_in_tooltip')} %>
index 3b68ddd..f8147ac 100644 (file)
@@ -1,19 +1,33 @@
-<script>
-$("#content").hide();
-</script>
-
 <% content_for :heading do %>
-  <h1><%= t 'user.confirm.heading' %></h1>
+  <h2><%= t 'user.confirm.heading' %></h2>
+  <div class='header-illustration confirm-main'></div>
 <% end %>
 
-<p><%= t 'user.confirm.press confirm button' %></p>
+<% if params[:confirm_string] %>
+  <script>
+  $("#content").hide();
+  </script>
 
-<%= form_tag({}, { :id => "confirm" }) do %>
-  <input type="display_name" name="confirm_string" value="<%= params[:display_name] %>">
-  <input type="hidden" name="confirm_string" value="<%= params[:confirm_string] %>">
-  <input type="submit" name="confirm_action" value="<%= t 'user.confirm.button' %>">
-<% end %>
+  <p><%= t 'user.confirm.press confirm button' %></p>
+
+  <%= form_tag({}, { :id => "confirm" }) do %>
+    <input type="display_name" name="confirm_string" value="<%= params[:display_name] %>">
+    <input type="hidden" name="confirm_string" value="<%= params[:confirm_string] %>">
+    <input type="submit" name="confirm_action" value="<%= t 'user.confirm.button' %>">
+  <% end %>
 
-<script>
-$("#confirm").submit();
-</script>
+  <script>
+  $("#confirm").submit();
+  </script>
+<% else %>
+  <h1>
+    <%= t "user.confirm.introduction_1" %>
+    <span class="deemphasize">
+      <%= t "user.confirm.introduction_2" %>
+    </span>
+  </h1>
+
+  <p class='deemphasize'><%= t "user.confirm.antispam_alert_html" %></p>
+  <p class='deemphasize'><%= t "user.confirm.reconfirm_html",
+                               :reconfirm => url_for(:action => 'confirm_resend')%></p>
+<% end %>
index 04573b6..b750386 100644 (file)
@@ -1148,13 +1148,6 @@ en:
         to our <a href="http://www.osmfoundation.org/wiki/License/Takedown_procedure">takedown
         procedure</a> or file directly at our
         <a href="http://dmca.openstreetmap.org/">on-line filing page</a>.
-  verify_page:
-    title: Congratulations!
-    introduction_html: |
-      We sent a confirmation email to %{email}. <span class='deemphasize'>Confirm your account 
-      by clicking on the link in the email and you'll be able to start mapping.</span>
-    antispam_alert_html: | 
-      If you use an antispam system which sends confirmation requests then please make sure you whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests."
   welcome_page:
     title: Welcome!
     introduction_html: |
@@ -1736,8 +1729,6 @@ en:
           </li>
         </ul>
       continue: Sign Up
-      flash welcome: "Thanks for signing up. We've sent a welcome message to %{email} with some hints on getting started."
-      flash create success message: "Thanks for signing up. We've sent a confirmation note to %{email} and as soon as you confirm your account you'll be able to get mapping.<br /><br />If you use an antispam system which sends confirmation requests then please make sure you whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests."
       terms accepted: "Thanks for accepting the new contributor terms!"
       terms declined: "We are sorry that you have decided to not accept the new Contributor Terms. For more information, please see <a href=\"%{url}\">this wiki page</a>."
       terms declined url: http://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined
@@ -1881,14 +1872,19 @@ en:
       flash update success confirm needed: "User information updated successfully. Check your email for a note to confirm your new email address."
       flash update success: "User information updated successfully."
     confirm:
-      heading: Confirm a user account
+      heading: Check your email!
+      introduction_1: |
+        We sent you a confirmation email.
+      introduction_2: |
+        Confirm your account by clicking on the link in the email and you'll be able to start mapping.
+      antispam_alert_html: |
+        If you use an antispam system which sends confirmation requests then please make sure you
+        whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests.
       press confirm button: "Press the confirm button below to activate your account."
       button: Confirm
-      success: "Confirmed your account, thanks for signing up!"
-      before you start: "We know you're probably in a hurry to start mapping, but before you do you might like to fill in some more information about yourself in the form below."
       already active: "This account has already been confirmed."
       unknown token: "That token doesn't seem to exist."
-      reconfirm: "If it's been a while since you signed up you might need to <a href=\"%{reconfirm}\">send yourself a new confirmation email</a>."
+      reconfirm_html: "If you need us to resend the confirmation email, <a href=\"%{reconfirm}\">click here</a>."
     confirm_resend:
       success: "We've sent a new confirmation note to %{email} and as soon as you confirm your account you'll be able to get mapping.<br /><br />If you use an antispam system which sends confirmation requests then please make sure you whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests."
       failure: "User %{name} not found."
index 656bc58..0168b61 100644 (file)
@@ -126,7 +126,6 @@ OpenStreetMap::Application.routes.draw do
   match '/copyright/:copyright_locale' => 'site#copyright', :via => :get
   match '/copyright' => 'site#copyright', :via => :get
   match '/welcome' => 'site#welcome', :via => :get, :as => :welcome
-  match '/verifysignup' => 'site#verifysignup', :via => :get, :as => :verifysignup
   match '/history' => 'changeset#list', :via => :get
   match '/history/feed' => 'changeset#feed', :via => :get, :format => :atom
   match '/export' => 'site#index', :export => true, :via => :get
index 20ea746..8b22388 100644 (file)
@@ -238,8 +238,8 @@ class UserControllerTest < ActionController::TestCase
     assert_match /#{@url}/, register_email.body.to_s
 
     # Check the page
-    assert_redirected_to :action => 'login', :referer => nil
-      
+    assert_redirected_to :action => 'confirm', :display_name => display_name
+
     ActionMailer::Base.deliveries.clear
   end
   
index fcab978..76397ae 100644 (file)
@@ -82,7 +82,7 @@ class UserCreationTest < ActionController::IntegrationTest
 
       # Check the page
       assert_response :success
-      assert_template 'login'
+      assert_template 'user/confirm'
 
       ActionMailer::Base.deliveries.clear
     end
@@ -126,17 +126,17 @@ class UserCreationTest < ActionController::IntegrationTest
 
     # Check the page
     assert_response :success
-    assert_template 'login'
+    assert_template 'user/confirm'
 
     ActionMailer::Base.deliveries.clear
 
     # Go to the confirmation page
-    get 'user/confirm', { :confirm_string => confirm_string }
+    get "/user/#{display_name}/confirm", { :confirm_string => confirm_string }
     assert_response :success
     assert_template 'user/confirm'
 
-    post 'user/confirm', { :confirm_string => confirm_string, :confirm_action => 'submit' }
-    assert_response :redirect # to trace/mine in original referrer
+    post "user/#{display_name}/confirm", { :confirm_string => confirm_string }
+    assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template 'site/welcome'
@@ -163,7 +163,7 @@ class UserCreationTest < ActionController::IntegrationTest
 
     # Check the page
     assert_response :success
-    assert_template 'login'
+    assert_template 'user/confirm'
 
     ActionMailer::Base.deliveries.clear
   end
@@ -219,17 +219,17 @@ class UserCreationTest < ActionController::IntegrationTest
 
     # Check the page
     assert_response :success
-    assert_template 'login'
+    assert_template 'user/confirm'
 
     ActionMailer::Base.deliveries.clear
 
     # Go to the confirmation page
-    get 'user/confirm', { :confirm_string => confirm_string }
+    get "/user/#{display_name}/confirm", { :confirm_string => confirm_string }
     assert_response :success
     assert_template 'user/confirm'
 
-    post 'user/confirm', { :confirm_string => confirm_string, :confirm_action => 'submit' }
-    assert_response :redirect # to trace/mine in original referrer
+    post "/user/#{display_name}/confirm", { :confirm_string => confirm_string }
+    assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template 'site/welcome'