[openstreetmap/openstreetmap-website] Add links to the ToU and include them in signup (#2028)

Tom Hughes notifications at github.com
Wed Jan 9 21:40:55 UTC 2019


tomhughes requested changes on this pull request.

In addition to the inline comments, don't we need something on the server side to check that they actually agreed?

It looks to me like currently it relies on the submit button being disabled until javascript enables it when the checkbox is ticked but that leaves it open to somebody to hack a post of the form with `read_tou` unset and then claim they have never agreed...

> @@ -124,4 +124,9 @@ $(document).ready(function () {
     $("#contributorTerms").html("<img src='" + OSM.SEARCHING + "' />");
     $("#contributorTerms").load(url);
   });
+
+  $("#read_tou").on("click", function () { 
+    var agreebtn = document.getElementById('agree');
+    agreebtn.disabled=!this.checked;

I'd rather do this with jQuery like everything else - something like this I think:

```
$("#agree").prop("disabled", !$(this).prop("checked"))
```

>  
     <%= 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('.agree'), :name => "agree", :id => "agree", disabled: true)  %>

Please use the `=>` syntax for hashes.

>    <div class="form-row">
-    <label for="user_consider_pd">
-      <%= check_box('user', 'consider_pd') %>
-      <%= t '.consider_pd' %>
+    <label for="read_tou">
+      <%= check_box_tag 'read_tou',1,false %>

There's no need for the last two arguments - those are the default values.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/2028#pullrequestreview-190945024
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20190109/413adcef/attachment.html>


More information about the rails-dev mailing list