Add links to the ToU and include them in signup

.rubocop_todo.yml

@@ -73,7 +73,7 @@ Metrics/ParameterLists:
 
 # Offense count: 72
 Metrics/PerceivedComplexity:
-  Max: 23
+  Max: 25
 
 # Offense count: 6
 Naming/AccessorMethodName:

app/assets/javascripts/leaflet.map.js

@@ -15,27 +15,28 @@ L.OSM.Map = L.Map.extend({
 
     var copyright = I18n.t("javascripts.map.copyright", { copyright_url: "/copyright" });
     var donate = I18n.t("javascripts.map.donate_link_text", { donate_url: "https://donate.openstreetmap.org" });
+    var terms = I18n.t("javascripts.map.terms", { terms_url: "https://wiki.osmfoundation.org/wiki/Terms_of_Use" });
 
     this.baseLayers = [];
 
     this.baseLayers.push(new L.OSM.Mapnik({
-      attribution: copyright + " ♥ " + donate,
+      attribution: copyright + " ♥ " + donate + ". " + terms,
       code: "M",
       keyid: "mapnik",
       name: I18n.t("javascripts.map.base.standard")
     }));
 
     if (OSM.THUNDERFOREST_KEY) {
       this.baseLayers.push(new L.OSM.CycleMap({
-        attribution: copyright + ". Tiles courtesy of <a href='https://www.thunderforest.com/' target='_blank'>Andy Allan</a>",
+        attribution: copyright + ". Tiles courtesy of <a href='https://www.thunderforest.com/' target='_blank'>Andy Allan</a>. " + terms,
         apikey: OSM.THUNDERFOREST_KEY,
         code: "C",
         keyid: "cyclemap",
         name: I18n.t("javascripts.map.base.cycle_map")
       }));
 
       this.baseLayers.push(new L.OSM.TransportMap({
-        attribution: copyright + ". Tiles courtesy of <a href='https://www.thunderforest.com/' target='_blank'>Andy Allan</a>",
+        attribution: copyright + ". Tiles courtesy of <a href='https://www.thunderforest.com/' target='_blank'>Andy Allan</a>. " + terms,
         apikey: OSM.THUNDERFOREST_KEY,
         code: "T",
         keyid: "transportmap",
@@ -44,7 +45,7 @@ L.OSM.Map = L.Map.extend({
     }
 
     this.baseLayers.push(new L.OSM.HOT({
-      attribution: copyright + ". Tiles style by <a href='https://www.hotosm.org/' target='_blank'>Humanitarian OpenStreetMap Team</a> hosted by <a href='https://openstreetmap.fr/' target='_blank'>OpenStreetMap France</a>",
+      attribution: copyright + ". Tiles style by <a href='https://www.hotosm.org/' target='_blank'>Humanitarian OpenStreetMap Team</a> hosted by <a href='https://openstreetmap.fr/' target='_blank'>OpenStreetMap France</a>. " + terms,
       code: "H",
       keyid: "hot",
       name: I18n.t("javascripts.map.base.hot")

app/assets/javascripts/user.js

@@ -124,4 +124,12 @@ $(document).ready(function () {
     $("#contributorTerms").html("<img src='" + OSM.SEARCHING + "' />");
     $("#contributorTerms").load(url);
   });
+
+  $("#read_ct").on("click", function () {
+    $("#continue").prop("disabled", !($(this).prop("checked") && $("#read_tou").prop("checked")));
+  });
+
+  $("#read_tou").on("click", function () {
+    $("#continue").prop("disabled", !($(this).prop("checked") && $("#read_ct").prop("checked")));
+  });
 });

app/controllers/users_controller.rb

@@ -36,7 +36,7 @@ def terms
   def save
     @title = t "users.new.title"
 
-    if params[:decline]
+    if params[:decline] || !(params[:read_tou] && params[:read_ct])
       if current_user
         current_user.terms_seen = true
 
@@ -47,12 +47,15 @@ def save
         else
           redirect_to :action => :account, :display_name => current_user.display_name
         end
-      else
+      elsif params[:decline]
         redirect_to t("users.terms.declined")
+      else
+        redirect_to :action => :terms
       end
     elsif current_user
       unless current_user.terms_agreed?
         current_user.consider_pd = params[:user][:consider_pd]
+        current_user.tou_agreed = Time.now.getutc
         current_user.terms_agreed = Time.now.getutc
         current_user.terms_seen = true
 
@@ -73,6 +76,7 @@ def save
         current_user.creation_ip = request.remote_ip
         current_user.languages = http_accept_language.user_preferred_languages
         current_user.terms_agreed = Time.now.getutc
+        current_user.tou_agreed = Time.now.getutc
         current_user.terms_seen = true
 
         if current_user.auth_uid.blank?

app/models/user.rb

@@ -34,6 +34,7 @@
 #  image_content_type  :string
 #  auth_provider       :string
 #  home_tile           :integer
+#  tou_agreed          :datetime
 #
 # Indexes
 #

app/views/users/terms.html.erb

@@ -9,7 +9,12 @@
 
 <%= form_tag({ :action => "save" }, { :class => " inner22 standard-form fillL" }) do %>
   <!-- legale is <%= @legale %> -->
+  <p class="deemphasize"><%= t ".read and accept with tou" %></p>
+  <label class="standard-label">
+    <%= t ".heading_ct" %>
+  </label>
   <div class='form-row horizontal-list'>
+    <p class="deemphasize"><%= t ".contributor_terms_explain" %></p>
     <label class="standard-label">
       <%= t ".legale_select" %>
     </label>
@@ -28,27 +33,42 @@
     <%= render :partial => "terms" %>
   </div>
 
+  <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="user_consider_pd">
-      <%= check_box("user", "consider_pd") %>
-      <%= t ".consider_pd" %>
+    <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_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" %>
     </label>
-    <span class="minorNote">(<%= link_to(t(".consider_pd_why"), t(".consider_pd_why_url"), :target => :new) %>)</span>
 
     <%= hidden_field_tag("referer", h(params[:referer])) unless params[:referer].nil? %>
 
     <div class="buttons form-row inner20 clearfix">
-      <p class="deemphasize"><%= t ".read and accept" %></p>
-      <%= submit_tag(t(".agree"), :name => "agree", :id => "agree") %>
-      <%= 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>
 
-    <div class="deemphasize">
-      <p id="contributorGuidance">
-        <%= 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>
+    <label for="user_consider_pd">
+      <%= check_box("user", "consider_pd") %>
+      <%= t ".consider_pd" %>
+    </label>
+    <span class="minorNote">(<%= link_to(t(".consider_pd_why"), t(".consider_pd_why_url"), :target => :new) %>)</span>
+
   </div>
 <% end %>

config/locales/en.yml

@@ -1089,6 +1089,7 @@ en:
     partners_ucl: "UCL"
     partners_bytemark: "Bytemark Hosting"
     partners_partners: "partners"
+    tou: "Terms of Use"
     osm_offline: "The OpenStreetMap database is currently offline while essential database maintenance work is carried out."
     osm_read_only: "The OpenStreetMap database is currently in read-only mode while essential database maintenance work is carried out."
     donate: "Support OpenStreetMap by %{link} to the Hardware Upgrade Fund."
@@ -1299,7 +1300,7 @@ en:
         This site and many other related services are formally operated by the
         <a href='https://osmfoundation.org/'>OpenStreetMap Foundation</a> (OSMF)
         on behalf of the community. Use of all OSMF operated services is subject
-        to our <a href="https://wiki.openstreetmap.org/wiki/Acceptable_Use_Policy">
+        to our <a href="https://wiki.osmfoundation.org/wiki/Terms_of_Use">Terms of Use</a>, <a href="https://wiki.openstreetmap.org/wiki/Acceptable_Use_Policy">
         Acceptable Use Policies</a> and our <a href="https://wiki.osmfoundation.org/wiki/Privacy_Policy">Privacy Policy</a>
         <br>
         Please <a href='https://osmfoundation.org/Contact'>contact the OSMF</a>
@@ -2025,14 +2026,19 @@ en:
       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: https://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined
     terms:
-      title: "Contributor terms"
-      heading: "Contributor terms"
-      read and accept: "Please read the agreement and press the agree button to confirm that you accept the terms of this agreement for your existing and future contributions."
-      consider_pd: "In addition to the above agreement, I consider my contributions to be in the Public Domain"
+      title: "Terms"
+      heading: "Terms"
+      heading_ct: "Contributor terms"
+      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."
+      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."
@@ -2417,6 +2423,7 @@ en:
         title: "Layers"
       copyright: "© <a href='%{copyright_url}'>OpenStreetMap contributors</a>"
       donate_link_text: "<a class='donate-attr' href='%{donate_url}'>Make a Donation</a>"
+      terms: "<a href='%{terms_url}' target='_blank'>Website and API terms</a>"
     site:
       edit_tooltip: Edit the map
       edit_disabled_tooltip: Zoom in to edit the map

db/migrate/20181020114000_add_user_tou_agreed.rb

@@ -0,0 +1,5 @@
+class AddUserTouAgreed < ActiveRecord::Migration[5.1]
+  def change
+    add_column :users, :tou_agreed, :datetime
+  end
+end

test/controllers/users_controller_test.rb

@@ -221,7 +221,7 @@ def test_new_success
     assert_difference "User.count", 1 do
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -245,7 +245,7 @@ def test_new_duplicate_email
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -262,7 +262,7 @@ def test_new_duplicate_email_uppercase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -279,7 +279,7 @@ def test_new_duplicate_name
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -296,7 +296,7 @@ def test_new_duplicate_name_uppercase
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -313,7 +313,7 @@ def test_new_blocked_domain
     assert_no_difference "User.count" do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         perform_enqueued_jobs do
-          post :save, :session => { :new_user => user }
+          post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -329,7 +329,8 @@ def test_save_referer_params
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
         perform_enqueued_jobs do
           post :save, :session => { :new_user => user,
-                                    :referer => "/edit?editor=id#map=1/2/3" }
+                                    :referer => "/edit?editor=id#map=1/2/3" },
+                      :params => { :read_ct => 1, :read_tou => 1 }
         end
       end
     end
@@ -637,7 +638,7 @@ def test_terms_not_seen_without_referer
     assert_response :success
     assert_template :terms
 
-    post :save, :params => { :user => { :consider_pd => true } }
+    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]
@@ -658,7 +659,7 @@ def test_terms_not_seen_with_referer
     assert_response :success
     assert_template :terms
 
-    post :save, :params => { :user => { :consider_pd => true }, :referer => "/test" }
+    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]