From 1be8d599e878c83305439a9b4d5273d72d9712d7 Mon Sep 17 00:00:00 2001
From: Simon Poole <simon@poole.ch>
Date: Tue, 2 Apr 2019 13:52:56 +0200
Subject: [PATCH] Add checkbox for contributor terms and improve text

---
 app/assets/javascripts/user.js            |  6 +++++-
 app/controllers/users_controller.rb       |  2 +-
 app/views/users/terms.html.erb            | 18 ++++++++++++------
 config/locales/en.yml                     |  9 +++++----
 test/controllers/users_controller_test.rb | 18 +++++++++---------
 5 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/app/assets/javascripts/user.js b/app/assets/javascripts/user.js
index 3803e180f..13941d6f3 100644
--- a/app/assets/javascripts/user.js
+++ b/app/assets/javascripts/user.js
@@ -125,7 +125,11 @@ $(document).ready(function () {
     $("#contributorTerms").load(url);
   });
 
+  $("#read_ct").on("click", function () { 
+    $("#continue").prop("disabled", !($(this).prop("checked") && $("#read_tou").prop("checked")));
+  });
+
   $("#read_tou").on("click", function () { 
-    $("#agree").prop("disabled", !$(this).prop("checked"));
+    $("#continue").prop("disabled", !($(this).prop("checked") && $("#read_ct").prop("checked")));
   });
 });
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 4b56dc033..e41330c4b 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -36,7 +36,7 @@ class UsersController < ApplicationController
   def save
     @title = t "users.new.title"
 
-    if params[:decline] || !params[:read_tou]
+    if params[:decline] || !(params[:read_tou] && params[:read_ct])
       if current_user
         current_user.terms_seen = true
 
diff --git a/app/views/users/terms.html.erb b/app/views/users/terms.html.erb
index ee74312f7..4e45671e1 100644
--- a/app/views/users/terms.html.erb
+++ b/app/views/users/terms.html.erb
@@ -33,29 +33,35 @@
     <%= render :partial => "terms" %>
   </div>
 
-  <div class="deemphasize">
-    <p id="contributorGuidance">
+  <div>
+    <p id="contributorGuidance" class="deemphasize">
       <%= raw t '.guidance',
                 :summary => 'https://www.osmfoundation.org/wiki/License/Contributor_Terms_Summary',
                 :translations => 'https://www.osmfoundation.org/wiki/License/Contributor_Terms/Informal_Translations' %>
     </p>
   </div>
+  <div class="form-row">
+    <label for="read_ct">
+      <%= check_box_tag 'read_ct' %>
+      <%= t '.read_ct' %>
+    </label>
+  </div>
 
   <label class="standard-label">
     <%= t 'layouts.tou' %>
   </label>
-  <p class="deemphasize"><%= t '.tou_explain' %></p>
+  <p class="deemphasize"><%= t '.tou_explain_html', :tou_link => link_to(t('layouts.tou'), "https://wiki.osmfoundation.org/wiki/Terms_of_Use", :target => :new) %></p>
   <div class="form-row">
     <label for="read_tou">
       <%= check_box_tag 'read_tou' %>
-      <%= t '.read_tou_html', :tou_link => link_to(t('layouts.tou'), "https://wiki.osmfoundation.org/wiki/Terms_of_Use", :target => :new) %>
+      <%= t '.read_tou' %>
     </label>
 
     <%= hidden_field_tag("referer", h(params[:referer])) unless params[:referer].nil? %>
 
     <div class="buttons form-row inner20 clearfix">
-      <%= submit_tag(t('.agree'), :name => "agree", :id => "agree", :disabled => true)  %>
-      <%= submit_tag(t('.decline'), :name => "decline", :id => "decline") %>
+      <%= submit_tag("Continue", :name => "continue", :id => "continue", :disabled => true)  %>
+      <%= submit_tag("Cancel", :name => "decline", :id => "decline") %>
     </div>
 
     <label for="user_consider_pd">
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 1d82e040f..ad1f68df0 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2030,15 +2030,16 @@ en:
       title: "Terms"
       heading: "Terms"
       heading_ct: "Contributor terms"
-      read and accept with tou: "Please read the contributor agreement and the terms of use, check the checkbox when done and then press the agree button to confirm that you accept the terms of both documents." 
+      read and accept with tou: "Please read the contributor agreement and the terms of use, check both checkboxes when done and then press the continue button."
       contributor_terms_explain: "This agreement governs the terms for your existing and future contributions."
-      tou_explain: "These terms govern the use of the website and other infrastructure provided by the OSMF."
-      read_tou_html: "I have read and agree to the %{tou_link}"
+      read_ct: "I have read and agree to the above contributor terms"
+      tou_explain_html: "These %{tou_link} govern the use of the website and other infrastructure provided by the OSMF. Please click on the link, read and aggree to the text."
+      read_tou: "I have read and agree to the Terms of Use"
       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://www.osmfoundation.org/wiki/License/Why_would_I_want_my_contributions_to_be_public_domain
       guidance: 'Information to help understand these terms: a <a href="%{summary}">human readable summary</a> and some <a href="%{translations}">informal translations</a>'
-      agree: Agree
+      continue: Continue
       declined: "https://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined"
       decline: "Decline"
       you need to accept or decline: "Please read and then either accept or decline the new Contributor Terms to continue."
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 0bc10885d..df2e7d8f5 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -221,7 +221,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_difference "User.count", 1 do
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }, :params => { :read_tou => 1 }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -245,7 +245,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }, :params => { :read_tou => 1 }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -262,7 +262,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }, :params => { :read_tou => 1 }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -279,7 +279,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }, :params => { :read_tou => 1 }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -296,7 +296,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }, :params => { :read_tou => 1 }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -313,7 +313,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }, :params => { :read_tou => 1 }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -330,7 +330,7 @@ class UsersControllerTest < ActionController::TestCase
         perform_enqueued_jobs do
           post :save, :session => { :new_user => user,
                                     :referer => "/edit?editor=id#map=1/2/3" },
-                      :params => { :read_tou => 1 }
+                      :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -638,7 +638,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_response :success
     assert_template :terms
 
-    post :save, :params => { :user => { :consider_pd => true }, :read_tou => 1 }
+    post :save, :params => { :user => { :consider_pd => true }, :read_ct => 1, :read_tou => 1 }
     assert_response :redirect
     assert_redirected_to :action => :account, :display_name => user.display_name
     assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
@@ -659,7 +659,7 @@ class UsersControllerTest < ActionController::TestCase
     assert_response :success
     assert_template :terms
 
-    post :save, :params => { :user => { :consider_pd => true }, :referer => "/test", :read_tou => 1 }
+    post :save, :params => { :user => { :consider_pd => true }, :referer => "/test", :read_ct => 1, :read_tou => 1 }
     assert_response :redirect
     assert_redirected_to "/test"
     assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
-- 
2.39.5