diff --git a/app/assets/images/verify-illustration.png b/app/assets/images/confirm-illustration.png similarity index 100% rename from app/assets/images/verify-illustration.png rename to app/assets/images/confirm-illustration.png diff --git a/app/assets/images/verify-illustration.svg b/app/assets/images/confirm-illustration.svg similarity index 100% rename from app/assets/images/verify-illustration.svg rename to app/assets/images/confirm-illustration.svg diff --git a/app/assets/stylesheets/common.css.scss b/app/assets/stylesheets/common.css.scss index 7eec371b4..d4afea410 100644 --- a/app/assets/stylesheets/common.css.scss +++ b/app/assets/stylesheets/common.css.scss @@ -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 { diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index df0c8d841..dfacb9d5a 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -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' + 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 = 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 - 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 + session[:user] = user.id + cookies.permanent["_osm_username"] = user.display_name - 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') + "

" + 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 + redirect_to referer || welcome_path end - else - user = User.find_by_display_name(params[:display_name]) - - 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])) - else - flash[:error] = t('user.confirm.unknown token') - end - - redirect_to :action => 'login' + 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 index 4f5ecb621..000000000 --- a/app/views/site/verifysignup.html.erb +++ /dev/null @@ -1,13 +0,0 @@ -<% content_for :head do %> -<% end %> - -<%= content_for :heading do %> -

<%= t "verify_page.title" %>

-
-<% end %> - -

<%= t "verify_page.introduction_html" %>

- -

<%= t "verify_page.antispam_alert_html" %>

- -<%= link_to t('layouts.log_in'), login_path(:referer => request.fullpath), {:class => 'button', :title => t('layouts.log_in_tooltip')} %> diff --git a/app/views/user/confirm.html.erb b/app/views/user/confirm.html.erb index 3b68ddd5b..f8147ac78 100644 --- a/app/views/user/confirm.html.erb +++ b/app/views/user/confirm.html.erb @@ -1,19 +1,33 @@ - - <% content_for :heading do %> -

<%= t 'user.confirm.heading' %>

+

<%= t 'user.confirm.heading' %>

+
<% end %> -

<%= t 'user.confirm.press confirm button' %>

+<% if params[:confirm_string] %> + -<%= form_tag({}, { :id => "confirm" }) do %> - - - +

<%= t 'user.confirm.press confirm button' %>

+ + <%= form_tag({}, { :id => "confirm" }) do %> + + + + <% end %> + + +<% else %> +

+ <%= t "user.confirm.introduction_1" %> + + <%= t "user.confirm.introduction_2" %> + +

+ +

<%= t "user.confirm.antispam_alert_html" %>

+

<%= t "user.confirm.reconfirm_html", + :reconfirm => url_for(:action => 'confirm_resend')%>

<% end %> - - diff --git a/config/locales/en.yml b/config/locales/en.yml index 04573b6a7..b75038651 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1148,13 +1148,6 @@ en: to our takedown procedure or file directly at our on-line filing page. - verify_page: - title: Congratulations! - introduction_html: | - We sent a confirmation email to %{email}. 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." welcome_page: title: Welcome! introduction_html: | @@ -1736,8 +1729,6 @@ en: 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.

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 this wiki page." 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 send yourself a new confirmation email." + reconfirm_html: "If you need us to resend the confirmation email, click here." 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.

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." diff --git a/config/routes.rb b/config/routes.rb index 656bc5819..0168b6190 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb index 20ea74696..8b2238829 100644 --- a/test/functional/user_controller_test.rb +++ b/test/functional/user_controller_test.rb @@ -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 diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index fcab978c8..76397ae28 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -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'