From: Milan Cvetkovic <mcvetkovic@microsoft.com>
Date: Wed, 6 Dec 2023 10:31:52 +0000 (+0000)
Subject: Merge login and terms screens, assume TOU and contributor terms are accepted on ... 
X-Git-Tag: live~796^2~10
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/1276fb944a8024884f3121d21577ef892353dfd1

Merge login and terms screens, assume TOU and contributor terms are accepted on /user/new form

This eliminates the need for "terms" screen after /user/new form..
Terms screen is still required for legacy users who never accepted the terms.
---

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index c55a177b4..e1ecfef80 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -103,12 +103,12 @@ class UsersController < ApplicationController
         render :action => "new"
       elsif current_user.auth_provider.present?
         # Verify external authenticator before moving on
-        session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation")
+        session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation", "consider_pd")
         redirect_to auth_url(current_user.auth_provider, current_user.auth_uid), :status => :temporary_redirect
       else
         # Save the user record
-        session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation")
-        redirect_to :action => :terms
+        session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation", "consider_pd")
+        save_new_user
       end
     end
   end
@@ -168,48 +168,6 @@ class UsersController < ApplicationController
       referer = safe_referer(params[:referer]) if params[:referer]
 
       redirect_to referer || edit_account_path
-    else
-      new_user = session.delete(:new_user)
-      verified_email = new_user.delete("verified_email")
-
-      self.current_user = User.new(new_user)
-
-      if check_signup_allowed(current_user.email)
-        current_user.data_public = true
-        current_user.description = "" if current_user.description.nil?
-        current_user.creation_ip = request.remote_ip
-        current_user.languages = http_accept_language.user_preferred_languages
-        current_user.terms_agreed = Time.now.utc
-        current_user.tou_agreed = Time.now.utc
-        current_user.terms_seen = true
-
-        if current_user.auth_uid.blank?
-          current_user.auth_provider = nil
-          current_user.auth_uid = nil
-        elsif current_user.email == verified_email
-          current_user.activate
-        end
-
-        if current_user.save
-          SIGNUP_IP_LIMITER&.update(request.remote_ip)
-          SIGNUP_EMAIL_LIMITER&.update(canonical_email(current_user.email))
-
-          flash[:matomo_goal] = Settings.matomo["goals"]["signup"] if defined?(Settings.matomo)
-
-          referer = welcome_path(welcome_options)
-
-          if current_user.status == "active"
-            session[:referer] = referer
-            successful_login(current_user)
-          else
-            session[:pending_user] = current_user.id
-            UserMailer.signup_confirm(current_user, current_user.generate_token_for(:new_user), referer).deliver_later
-            redirect_to :controller => :confirmations, :action => :confirm, :display_name => current_user.display_name
-          end
-        else
-          render :action => "new", :referer => params[:referer]
-        end
-      end
     end
   end
 
@@ -268,7 +226,7 @@ class UsersController < ApplicationController
       session[:new_user]["auth_uid"] = uid
       session[:new_user]["verified_email"] = email if email_verified
 
-      redirect_to :action => "terms"
+      save_new_user
     else
       user = User.find_by(:auth_provider => provider, :auth_uid => uid)
 
@@ -308,6 +266,50 @@ class UsersController < ApplicationController
 
   private
 
+  def save_new_user
+    new_user = session.delete(:new_user)
+    verified_email = new_user.delete("verified_email")
+
+    self.current_user = User.new(new_user)
+
+    if check_signup_allowed(current_user.email)
+      current_user.data_public = true
+      current_user.description = "" if current_user.description.nil?
+      current_user.creation_ip = request.remote_ip
+      current_user.languages = http_accept_language.user_preferred_languages
+      current_user.terms_agreed = Time.now.utc
+      current_user.tou_agreed = Time.now.utc
+      current_user.terms_seen = true
+
+      if current_user.auth_uid.blank?
+        current_user.auth_provider = nil
+        current_user.auth_uid = nil
+      elsif current_user.email == verified_email
+        current_user.activate
+      end
+
+      if current_user.save
+        SIGNUP_IP_LIMITER&.update(request.remote_ip)
+        SIGNUP_EMAIL_LIMITER&.update(canonical_email(current_user.email))
+
+        flash[:matomo_goal] = Settings.matomo["goals"]["signup"] if defined?(Settings.matomo)
+
+        referer = welcome_path(welcome_options)
+
+        if current_user.status == "active"
+          session[:referer] = referer
+          successful_login(current_user)
+        else
+          session[:pending_user] = current_user.id
+          UserMailer.signup_confirm(current_user, current_user.generate_token_for(:new_user), referer).deliver_later
+          redirect_to :controller => :confirmations, :action => :confirm, :display_name => current_user.display_name
+        end
+      else
+        render :action => "new", :referer => params[:referer]
+      end
+    end
+  end
+
   def welcome_options
     uri = URI(session[:referer]) if session[:referer].present?
 
@@ -336,7 +338,8 @@ class UsersController < ApplicationController
   def user_params
     params.require(:user).permit(:email, :email_confirmation, :display_name,
                                  :auth_provider, :auth_uid,
-                                 :pass_crypt, :pass_crypt_confirmation)
+                                 :pass_crypt, :pass_crypt_confirmation,
+                                 :consider_pd)
   end
 
   ##
diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb
index 1b65ec506..733530104 100644
--- a/app/views/users/new.html.erb
+++ b/app/views/users/new.html.erb
@@ -40,10 +40,31 @@
     <%= f.password_field :pass_crypt, :tabindex => 6 %>
     <%= f.password_field :pass_crypt_confirmation, :tabindex => 7 %>
 
+    <p class="mb-3 text-muted"><%= t(".by_signing_up_html",
+                                     :tou_link => link_to(t("layouts.tou"),
+                                                          "https://wiki.osmfoundation.org/wiki/Terms_of_Use",
+                                                          :target => :new),
+                                     :contributor_terms_link => link_to(t(".contributor_terms"),
+                                                                        t(".contributor_terms_url"),
+                                                                        :target => :new)) %></p>
+
+    <div class="mb-3">
+      <%= submit_tag(t(".continue"), :name => "continue", :id => "continue", :class => "btn btn-primary", :tabindex => 8) %>
+    </div>
+
+    <div class="mb-3">
+      <div class="form-check">
+        <%= check_box("user", "consider_pd", :class => "form-check-input") %>
+          <label for="user_consider_pd" class="form-check-label">
+            <%= t ".consider_pd" %>
+          </label>
+        <span class="minorNote">(<%= link_to(t(".consider_pd_why"), t(".consider_pd_why_url"), :target => :new) %>)</span>
+      </div>
+    </div>
+
     <div id="auth_prompt">
       <p><%= link_to t(".use external auth"), "#", :id => "auth_enable" %></p>
     </div>
 
-    <%= f.primary t(".continue"), :tabindex => 8 %>
   <% end %>
 </div>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7f6563a1a..fdb5e3f49 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2735,6 +2735,9 @@ en:
         paragraph_1: Unlike other maps, OpenStreetMap is completely created by people like you, and it's free for anyone to fix, update, download and use.
         paragraph_2: Sign up to get started contributing. We'll send an email to confirm your account.
       display name description: "Your publicly displayed username. You can change this later in the preferences."
+      by_signing_up_html: "By signing up, you agree to our %{contributor_terms_link} and %{tou_link}."
+      contributor_terms_url: "https://wiki.osmfoundation.org/wiki/Licence/Contributor_Terms"
+      contributor_terms: "Contributor terms"
       external auth: "Third Party Authentication:"
       use external auth: "Alternatively, use a third party to log in"
       auth no password: "With third party authentication a password is not required, but some extra tools or server may still need one."
@@ -2744,6 +2747,9 @@ en:
       privacy_policy: privacy policy
       privacy_policy_url: https://wiki.osmfoundation.org/wiki/Privacy_Policy
       privacy_policy_title: OSMF privacy policy including section on email addresses
+      consider_pd: "In addition to the above, I consider my contributions to be in the Public Domain"
+      consider_pd_why: "what's this?"
+      consider_pd_why_url: https://wiki.osmfoundation.org/wiki/Licence_and_Legal_FAQ/Why_would_I_want_my_contributions_to_be_public_domain
     terms:
       title: "Terms"
       heading: "Terms"
diff --git a/test/controllers/confirmations_controller_test.rb b/test/controllers/confirmations_controller_test.rb
index 0f4315e4f..79213441f 100644
--- a/test/controllers/confirmations_controller_test.rb
+++ b/test/controllers/confirmations_controller_test.rb
@@ -38,7 +38,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
   def test_confirm_get
     user = build(:user, :pending)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     get user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
@@ -50,7 +49,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     # Get the confirmation page
@@ -71,7 +69,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
@@ -85,7 +82,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
@@ -96,7 +92,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
@@ -111,7 +106,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
@@ -125,7 +119,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
@@ -136,7 +129,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
@@ -151,7 +143,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     travel 2.weeks do
@@ -165,7 +156,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
@@ -183,7 +173,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = build(:user, :pending)
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
     confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     User.find_by(:display_name => user.display_name).hide!
@@ -201,7 +190,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
   def test_confirm_resend_success
     user = build(:user, :pending)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
 
     assert_difference "ActionMailer::Base.deliveries.size", 1 do
       perform_enqueued_jobs do
@@ -220,25 +208,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     ActionMailer::Base.deliveries.clear
   end
 
-  def test_confirm_resend_no_token
-    user = build(:user, :pending)
-    # only complete first half of registration
-    post user_new_path, :params => { :user => user.attributes }
-
-    assert_no_difference "ActionMailer::Base.deliveries.size" do
-      perform_enqueued_jobs do
-        get user_confirm_resend_path(user)
-      end
-    end
-
-    assert_redirected_to login_path
-    assert_match "User #{user.display_name} not found.", flash[:error]
-  end
-
   def test_confirm_resend_deleted
     user = build(:user, :pending)
     post user_new_path, :params => { :user => user.attributes }
-    post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
 
     User.find_by(:display_name => user.display_name).hide!
 
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 62bb34279..0c2d7d342 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -106,18 +106,10 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
   def test_new_success
     user = build(:user, :pending)
 
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
     assert_difference "User.count", 1 do
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
         perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+          post user_new_path, :params => { :user => user.attributes }
         end
       end
     end
@@ -151,55 +143,14 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_select "form > div > input.is-invalid#user_email"
   end
 
-  def test_save_duplicate_email
-    user = build(:user, :pending)
-
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
-    # Now create another user with that email
-    create(:user, :email => user.email)
-
-    # Check that the second half of registration fails
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-        end
-      end
-    end
-
-    assert_response :success
-    assert_template "new"
-    assert_select "form > div > input.is-invalid#user_email"
-  end
-
-  def test_save_duplicate_email_uppercase
+  def test_new_duplicate_email_uppercase
     user = build(:user, :pending)
-
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
-    # Now create another user with that email, but uppercased
     create(:user, :email => user.email.upcase)
 
-    # Check that the second half of registration fails
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+          post user_new_path, :params => { :user => user.attributes }
         end
       end
     end
@@ -209,26 +160,14 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_select "form > div > input.is-invalid#user_email"
   end
 
-  def test_save_duplicate_name
+  def test_new_duplicate_name
     user = build(:user, :pending)
-
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
-    # Now create another user with that display name
     create(:user, :display_name => user.display_name)
 
-    # Check that the second half of registration fails
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+          post user_new_path, :params => { :user => user.attributes }
         end
       end
     end
@@ -238,26 +177,14 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_select "form > div > input.is-invalid#user_display_name"
   end
 
-  def test_save_duplicate_name_uppercase
+  def test_new_duplicate_name_uppercase
     user = build(:user, :pending)
-
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
-    # Now create another user with that display_name, but uppercased
     create(:user, :display_name => user.display_name.upcase)
 
-    # Check that the second half of registration fails
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+          post user_new_path, :params => { :user => user.attributes }
         end
       end
     end
@@ -267,18 +194,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_select "form > div > input.is-invalid#user_display_name"
   end
 
-  def test_save_blocked_domain
+  def test_new_blocked_domain
     user = build(:user, :pending, :email => "user@example.net")
 
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
     # Now block that domain
     create(:acl, :domain => "example.net", :k => "no_account_creation")
 
@@ -286,7 +204,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+          post user_new_path, :params => { :user => user.attributes }
         end
       end
     end
@@ -298,18 +216,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
   def test_save_referer_params
     user = build(:user, :pending)
 
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes, :referer => "/edit?editor=id#map=1/2/3" }
-        end
-      end
-    end
-
     assert_difference "User.count", 1 do
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
-        post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+        post user_new_path, :params => { :user => user.attributes, :referer => "/edit?editor=id#map=1/2/3" }
         assert_enqueued_with :job => ActionMailer::MailDeliveryJob,
                              :args => proc { |args| args[3][:args][2] == welcome_path(:editor => "id", :zoom => 1, :lat => 2, :lon => 3) }
         perform_enqueued_jobs
@@ -319,24 +228,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     ActionMailer::Base.deliveries.clear
   end
 
-  def test_terms_new_user
-    user = build(:user, :pending)
-
-    # Set up our user as being half-way through registration
-    assert_no_difference "User.count" do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        perform_enqueued_jobs do
-          post user_new_path, :params => { :user => user.attributes }
-        end
-      end
-    end
-
-    get user_terms_path
-
-    assert_response :success
-    assert_template :terms
-  end
-
   def test_terms_agreed
     user = create(:user, :terms_seen => true, :terms_agreed => Date.yesterday)
 
diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb
index 1f749f957..8b6cdb19c 100644
--- a/test/integration/user_creation_test.rb
+++ b/test/integration/user_creation_test.rb
@@ -35,7 +35,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => dup_email,
                                        :display_name => display_name,
                                        :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" } }
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" } }
         end
       end
     end
@@ -89,26 +90,18 @@ class UserCreationTest < ActionDispatch::IntegrationTest
     new_email = "newtester@osm.org"
     display_name = "new_tester"
 
-    assert_difference("User.count", 0) do
-      assert_difference("ActionMailer::Base.deliveries.size", 0) do
+    assert_difference("User.count", 1) do
+      assert_difference("ActionMailer::Base.deliveries.size", 1) do
         perform_enqueued_jobs do
           post "/user/new",
                :params => { :user => { :email => new_email,
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" } }
-        end
-      end
-    end
-
-    assert_redirected_to "/user/terms"
-
-    assert_difference("User.count") do
-      assert_difference("ActionMailer::Base.deliveries.size", 1) do
-        perform_enqueued_jobs do
-          post "/user/save",
-               :params => { :read_ct => 1, :read_tou => 1 }
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" } }
+          assert_response :redirect
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -138,33 +131,6 @@ class UserCreationTest < ActionDispatch::IntegrationTest
     assert_equal user, User.authenticate(:username => new_email, :password => "testtest")
   end
 
-  def test_user_create_no_tou_failure
-    new_email = "#newtester@osm.org"
-    display_name = "new_tester"
-
-    assert_difference("User.count", 0) do
-      assert_difference("ActionMailer::Base.deliveries.size", 0) do
-        perform_enqueued_jobs do
-          post "/user/new",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" } }
-        end
-      end
-    end
-
-    assert_redirected_to "/user/terms"
-
-    perform_enqueued_jobs do
-      post "/user/save"
-      assert_redirected_to "/user/terms"
-    end
-
-    ActionMailer::Base.deliveries.clear
-  end
-
   # Check that the user can successfully recover their password
   def test_lost_password_recovery_success
     # Open the lost password form
@@ -188,16 +154,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" },
                             :referer => referer }
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1, :read_tou => 1 }
+          assert_response(:redirect)
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -247,23 +208,14 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "openid",
                                        :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => password,
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "openid",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1, :read_tou => 1 }
-          assert_response :redirect
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -290,8 +242,9 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "openid",
                                        :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new")
@@ -325,23 +278,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "openid",
                                        :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" },
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" },
                             :referer => referer }
           assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "openid",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" },
-                            :read_ct => 1, :read_tou => 1 }
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -391,22 +336,13 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "google",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => password,
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "google", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "google")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "google",
-                                       :auth_uid => "123454321",
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1, :read_tou => 1 }
           assert_redirected_to welcome_path
           follow_redirect!
         end
@@ -434,7 +370,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "google",
                                        :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt_confirmation => "",
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "google", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "google")
@@ -469,23 +406,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "google",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" },
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" },
                             :referer => referer }
           assert_redirected_to auth_path(:provider => "google", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "google")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "google",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" },
-                            :read_ct => 1, :read_tou => 1 }
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -535,22 +464,13 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "facebook",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => password,
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "facebook")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "facebook",
-                                       :auth_uid => "123454321",
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1, :read_tou => 1 }
           assert_redirected_to welcome_path
           follow_redirect!
         end
@@ -578,7 +498,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "facebook",
                                        :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt_confirmation => "",
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "facebook")
@@ -611,23 +532,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "facebook",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" },
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" },
                             :referer => referer }
           assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "facebook")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "facebook",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" },
-                            :read_ct => 1, :read_tou => 1 }
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -677,22 +590,13 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "microsoft",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => password,
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "microsoft")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "microsoft",
-                                       :auth_uid => "123454321",
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1, :read_tou => 1 }
           assert_redirected_to welcome_path
           follow_redirect!
         end
@@ -720,7 +624,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "microsoft",
                                        :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt_confirmation => "",
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "microsoft")
@@ -753,23 +658,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "microsoft",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" },
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" },
                             :referer => referer }
           assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "microsoft")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "microsoft",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" },
-                            :read_ct => 1, :read_tou => 1 }
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -819,23 +716,13 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "github",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => password,
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "github", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "github")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "github",
-                                       :auth_uid => "123454321",
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1,
-                            :read_tou => 1 }
           assert_redirected_to welcome_path
           follow_redirect!
         end
@@ -863,7 +750,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "github",
                                        :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt_confirmation => "",
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "github", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "github")
@@ -896,24 +784,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "github",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" },
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" },
                             :referer => referer }
           assert_redirected_to auth_path(:provider => "github", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "github")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "github",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" },
-                            :read_ct => 1,
-                            :read_tou => 1 }
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
@@ -963,23 +842,13 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "wikipedia",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt => password,
+                                       :pass_crypt_confirmation => password,
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "wikipedia",
-                                       :auth_uid => "123454321",
-                                       :pass_crypt => password,
-                                       :pass_crypt_confirmation => password },
-                            :read_ct => 1,
-                            :read_tou => 1 }
           assert_redirected_to welcome_path
           follow_redirect!
         end
@@ -1007,7 +876,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :display_name => display_name,
                                        :auth_provider => "wikipedia",
                                        :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" } }
+                                       :pass_crypt_confirmation => "",
+                                       :consider_pd => "1" } }
           assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new")
@@ -1040,24 +910,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest
                                        :email_confirmation => new_email,
                                        :display_name => display_name,
                                        :auth_provider => "wikipedia",
-                                       :pass_crypt => "",
-                                       :pass_crypt_confirmation => "" },
+                                       :pass_crypt => "testtest",
+                                       :pass_crypt_confirmation => "testtest",
+                                       :consider_pd => "1" },
                             :referer => referer }
           assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new")
           post response.location
           assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new")
           follow_redirect!
-          assert_redirected_to "/user/terms"
-          post "/user/save",
-               :params => { :user => { :email => new_email,
-                                       :email_confirmation => new_email,
-                                       :display_name => display_name,
-                                       :auth_provider => "wikipedia",
-                                       :auth_uid => "http://localhost:1123/new.tester",
-                                       :pass_crypt => "testtest",
-                                       :pass_crypt_confirmation => "testtest" },
-                            :read_ct => 1,
-                            :read_tou => 1 }
+          assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name
           follow_redirect!
         end
       end
diff --git a/test/system/confirmation_resend_test.rb b/test/system/confirmation_resend_test.rb
index 31b9ed7bb..746338dbc 100644
--- a/test/system/confirmation_resend_test.rb
+++ b/test/system/confirmation_resend_test.rb
@@ -13,10 +13,6 @@ class ConfirmationResendSystemTest < ApplicationSystemTestCase
       fill_in "Confirm Password", :with => "testtest"
       click_on "Sign Up"
     end
-
-    check "I have read and agree to the above contributor terms"
-    check "I have read and agree to the Terms of Use"
-    click_on "Continue"
   end
 
   test "flash message should not contain raw html" do
diff --git a/test/system/user_signup_test.rb b/test/system/user_signup_test.rb
index e97046800..7e2c6ba54 100644
--- a/test/system/user_signup_test.rb
+++ b/test/system/user_signup_test.rb
@@ -8,25 +8,4 @@ class UserSignupTest < ApplicationSystemTestCase
 
     assert_content "Confirm Password"
   end
-
-  test "externally redirect when contributor terms declined" do
-    user = build(:user)
-
-    visit root_path
-    click_on "Sign Up"
-
-    within ".new_user" do
-      fill_in "Email", :with => user.email
-      fill_in "Email Confirmation", :with => user.email
-      fill_in "Display Name", :with => user.display_name
-      fill_in "Password", :with => "testtest"
-      fill_in "Confirm Password", :with => "testtest"
-      click_on "Sign Up"
-    end
-
-    assert_content "Contributor terms"
-    click_on "Cancel"
-
-    assert_current_path "https://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined"
-  end
 end