Phase 2 CTs implementation and tests, with config parameter

app/controllers/application_controller.rb

@@ -104,6 +104,14 @@ class ApplicationController < ActionController::Base
       # NOTE: need slightly more helpful message than this.
       render :text => t('application.setup_user_auth.blocked'), :status => :forbidden
     end
+    # if the user hasn't seen the contributor terms then don't
+    # allow editing - they have to go to the web site and see
+    # (but can decline) the CTs to continue.
+    if REQUIRE_TERMS_SEEN
+      unless @user.nil? or @user.terms_seen
+        render :text => t('application.setup_user_auth.need_to_see_terms'), :status => :forbidden
+      end
+    end
   end
 
   def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 

app/controllers/user_controller.rb

@@ -24,7 +24,7 @@ class UserController < ApplicationController
 
     if request.xhr?
       render :update do |page|
-        page.replace_html "contributorTerms", :partial => "terms", :locals => { :has_decline => params[:has_decline] }
+        page.replace_html "contributorTerms", :partial => "terms"
       end
     else
       @title = t 'user.terms.title'
@@ -53,11 +53,14 @@ class UserController < ApplicationController
     if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"})
       render :action => 'new'
     elsif params[:decline]
+      @user.terms_seen = true
+      @user.save
       redirect_to t('user.terms.declined')
     elsif @user
       if !@user.terms_agreed?
         @user.consider_pd = params[:user][:consider_pd]
         @user.terms_agreed = Time.now.getutc
+        @user.terms_seen = true
         if @user.save
           flash[:notice] = t 'user.new.terms accepted'
         end
@@ -73,6 +76,7 @@ class UserController < ApplicationController
       @user.creation_ip = request.remote_ip
       @user.languages = request.user_preferred_languages
       @user.terms_agreed = Time.now.getutc
+      @user.terms_seen = true
       
       if @user.save
         flash[:notice] = t 'user.new.flash create success message', :email => @user.email
@@ -214,10 +218,14 @@ class UserController < ApplicationController
         session[:user] = user.id
         session_expires_after 1.month if params[:remember_me]
 
+        # if the user hasn't seen the contributor terms then redirect them
+        # to that page.
+        if REQUIRE_TERMS_SEEN and not user.terms_seen
+          redirect_to :controller => 'user', :action => 'terms', :referer => params[:referer]
         # The user is logged in, if the referer param exists, redirect
         # them to that unless they've also got a block on them, in
         # which case redirect them to the block so they can clear it.
-        if user.blocked_on_view
+        elsif user.blocked_on_view
           redirect_to user.blocked_on_view, :referer => params[:referer]
         elsif params[:referer]
           redirect_to params[:referer]

app/views/user/_terms.html.erb

@@ -1,10 +1,6 @@
 <p id="first">
   <%= @text['intro'] %>
-  <% if has_decline %>
+  <%= @text['next_with_decline'] %>
-    <%= @text['next_with_decline'] %>
-  <% else %>
-    <%= @text['next_without_decline'] %>
-  <% end %>
 </p>
 <ol>
   <li>

app/views/user/terms.html.erb

@@ -13,7 +13,7 @@
             :before => update_page do |page|
               page.replace_html 'contributorTerms', image_tag('searching.gif')
             end,
-            :url => {:legale => legale, :has_decline => params.has_key?(:user)}
+            :url => {:legale => legale}
           )
       %>
       <%= label_tag "legale_#{legale}", t('user.terms.legale_names.' + name) %>
@@ -22,7 +22,7 @@
 <% end %>
 
 <div id="contributorTerms">
-  <%= render :partial => "terms", :locals => { :has_decline =>params.has_key?(:user) }  %>
+  <%= render :partial => "terms" %>
 </div>
 
 <% form_tag({:action => "save"}, { :id => "termsForm" }) do %>
@@ -41,9 +41,7 @@
       <%= hidden_field('user', 'pass_crypt_confirmation') %>
     <% end %>
     <div id="buttons">
-      <% if params[:user] %>
+      <%= submit_tag(t('user.terms.decline'), :name => "decline", :id => "decline") %>
-        <%= submit_tag(t('user.terms.decline'), :name => "decline", :id => "decline") %>
-      <% end %>
       <%= submit_tag(t('user.terms.agree'), :name => "agree", :id => "agree") %>
     </div>
   </p>

config/example.application.yml

@@ -65,12 +65,15 @@ standard_settings: &standard_settings
   default_editor: "potlatch"
   # OAuth consumer key for Potlatch 2
   #potlatch2_key: ""
+  # Whether to require users to view the CTs before continuing to edit...
+  require_terms_seen: true
 
 development:
   <<: *standard_settings
 
 production:
   <<: *standard_settings
+  require_terms_seen: false
 
 test:
   <<: *standard_settings

config/legales/GB.yml

@@ -1,6 +1,5 @@
 intro: "Thank you for your interest in contributing data and/or any other content (collectively, 'Contents') to the geo-database of the OpenStreetMap project (the 'Project'). This contributor agreement (the 'Agreement') is made between you ('You') and The OpenStreetMap Foundation ('OSMF') and clarifies the intellectual property rights in any Contents that You choose to submit to the Project."
 next_with_decline: "Please read the following terms and conditions carefully and click either the 'Accept' or 'Decline' button at the bottom to continue."
-next_without_decline: "Please read the following terms and conditions carefully. If you agree, click the 'Accept'  button at the bottom to continue.  Otherwise, use your browser's Back button or just close this page."
 section_1: "You agree to only add Contents for which You are the copyright holder (to the extent the Contents include any copyrightable elements). You represent and warrant that You are legally entitled to grant the licence in Section 2 below and that such licence does not violate any law, breach any contract, or, to the best of Your knowledge, infringe any third party’s rights. If You are not the copyright holder of the Contents, You represent and warrant that You have explicit permission from the rights holder to submit the Contents and grant the licence below."
 section_2: "Rights granted. Subject to Section 3 below, You hereby grant to OSMF a worldwide, royalty-free, non-exclusive, perpetual, irrevocable licence to do any act that is restricted by copyright over anything within the Contents, whether in the original medium or any other. These rights explicitly include commercial use, and do not exclude any field of endeavour. These rights include, without limitation, the right to sublicense the work through multiple tiers of sublicensees.  To the extent allowable under applicable local laws and copyright conventions, You also waive and/or agree not to assert against OSMF or its licensees any moral rights that You may have in the Contents."
 section_3: "OSMF agrees to use or sub-license Your Contents as part of a database and only under the terms of one of the following licences: ODbL 1.0 for the database and DbCL 1.0 for the individual contents of the database; CC-BY-SA 2.0; or another free and open licence. Which other free and open licence is chosen by a vote of the OSMF membership and approved by at least a 2/3 majority vote of active contributors."

config/locales/en.yml

@@ -1438,6 +1438,7 @@ en:
       cookies_needed: "You appear to have cookies disabled - please enable cookies in your browser before continuing."
     setup_user_auth:
       blocked: "Your access to the API has been blocked. Please log-in to the web interface to find out more."
+      need_to_see_terms: "Your access to the API is temporarily suspended. Please log-in to the web interface to view the Contributor Terms. You do not need to agree, but you must view them."
   oauth:
     oauthorize:
       request_access: "The application {{app_name}} is requesting access to your account. Please check whether you would like the application to have the following capabilities. You may choose as many or as few as you like."

db/migrate/20110322001319_add_terms_seen_to_user.rb

@@ -0,0 +1,13 @@
+class AddTermsSeenToUser < ActiveRecord::Migration
+  def self.up
+    add_column :users, :terms_seen, :boolean, :null => false, :default => false
+    
+    # best guess available is just that everyone who has agreed has
+    # seen the terms, and that noone else has.
+    User.update_all "terms_seen = (terms_agreed is not null)"
+  end
+
+  def self.down
+    remove_column :users, :terms_seen
+  end
+end

test/fixtures/users.yml

@@ -11,6 +11,7 @@ normal_user:
   home_lat: 12.1
   home_lon: 12.1
   home_zoom: 3
+  terms_seen: true
   
 public_user:
   id: 2
@@ -24,6 +25,7 @@ public_user:
   home_lat: 12
   home_lon: 12
   home_zoom: 12
+  terms_seen: true
   
 inactive_user:
   id: 3
@@ -37,6 +39,7 @@ inactive_user:
   home_lat: 123.4
   home_lon: 12.34
   home_zoom: 15
+  terms_seen: true
   
 second_public_user:
   id: 4
@@ -50,6 +53,7 @@ second_public_user:
   home_lat: 89
   home_lon: 87
   home_zoom: 12
+  terms_seen: true
 
 moderator_user:
   id: 5
@@ -59,6 +63,7 @@ moderator_user:
   creation_time: "2008-05-01 01:23:45"
   display_name: moderator
   data_public: true
+  terms_seen: true
 
 administrator_user:
   id: 6
@@ -68,3 +73,14 @@ administrator_user:
   creation_time: "2008-05-01 01:23:45"
   display_name: administrator
   data_public: true
+  terms_seen: true
+
+terms_not_seen_user:
+  id: 7
+  email: not_agreed@example.com
+  status: active
+  pass_crypt: <%= Digest::MD5.hexdigest('test') %>
+  creation_time: "2011-03-22 00:23:45"
+  display_name: not_agreed
+  data_public: true
+  terms_seen: false

test/functional/diary_entry_controller_test.rb

@@ -1,7 +1,7 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class DiaryEntryControllerTest < ActionController::TestCase
-  fixtures :users, :diary_entries, :diary_comments
+  fixtures :users, :diary_entries, :diary_comments, :languages
 
   include ActionView::Helpers::NumberHelper
 

test/integration/user_terms_seen_test.rb

@@ -0,0 +1,45 @@
+require File.dirname(__FILE__) + '/../test_helper'
+
+class UserTermsSeenTest < ActionController::IntegrationTest
+  fixtures :users
+
+  def auth_header(user, pass)
+    {"HTTP_AUTHORIZATION" => "Basic %s" % Base64.encode64("#{user}:#{pass}")}
+  end
+
+  def test_api_blocked
+    user = users(:terms_not_seen_user)
+
+    get "/api/#{API_VERSION}/user/details", nil, auth_header(user.display_name, "test")
+    assert_response :forbidden
+
+    # touch it so that the user has seen the terms
+    user.terms_seen = true
+    user.save
+
+    get "/api/#{API_VERSION}/user/details", nil, auth_header(user.display_name, "test")
+    assert_response :success
+  end
+
+  def test_terms_presented_at_login
+    user = users(:terms_not_seen_user)
+
+    # try to log in
+    get_via_redirect "/login"
+    assert_response :success
+    assert_template 'user/login'
+    post "/login", {'user[email]' => user.email, 'user[password]' => 'test', :referer => "/"}
+    assert_response :redirect
+    # but now we need to look at the terms
+    assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/"
+    follow_redirect!
+    assert_response :success
+
+    # don't agree to the terms, but hit decline
+    
+    # should be carried through to a normal login
+  end
+
+end
+
+