Phase 2 CTs implementation and tests, with config parameter

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index bc71f275f1e8607d25826963df89f2fe0ba5b9a9..a22bc13edd9500a804fb7d2e467e70534078f87c 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/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
       # 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") 
   end
 
   def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 6935af3bca5f3508fa7d2222a2f142975ebdc5b8..631c91035f0e8661a83513716efe436e72f2e80b 100644 (file)
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -24,7 +24,7 @@ class UserController < ApplicationController
 
     if request.xhr?
       render :update do |page|
 
     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'
       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]
     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
       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
         if @user.save
           flash[:notice] = t 'user.new.terms accepted'
         end


@@ -73,7 +76,8 @@ class UserController < ApplicationController
       @user.creation_ip = request.remote_ip
       @user.languages = request.user_preferred_languages
       @user.terms_agreed = Time.now.getutc
       @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
         Notifier.deliver_signup_confirm(@user, @user.tokens.create(:referer => params[:referer]))
       if @user.save
         flash[:notice] = t 'user.new.flash create success message', :email => @user.email
         Notifier.deliver_signup_confirm(@user, @user.tokens.create(:referer => params[:referer]))


@@ -214,10 +218,14 @@ class UserController < ApplicationController
         session[:user] = user.id
         session_expires_after 1.month if params[:remember_me]
 
         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.
         # 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]
           redirect_to user.blocked_on_view, :referer => params[:referer]
         elsif params[:referer]
           redirect_to params[:referer]

diff --git a/app/views/user/_terms.html.erb b/app/views/user/_terms.html.erb
index 2c6b0885cb2732608d6b1007dadc17789d55c177..92b98ea04ce2402a767ad5f29e55976c75a38233 100644 (file)
--- a/app/views/user/_terms.html.erb
+++ b/app/views/user/_terms.html.erb
@@ -1,10 +1,6 @@
 <p id="first">
   <%= @text['intro'] %>
 <p id="first">
   <%= @text['intro'] %>

-  <% if has_decline %>
-    <%= @text['next_with_decline'] %>
-  <% else %>
-    <%= @text['next_without_decline'] %>
-  <% end %>
+  <%= @text['next_with_decline'] %>

 </p>
 <ol>
   <li>
 </p>
 <ol>
   <li>

diff --git a/app/views/user/terms.html.erb b/app/views/user/terms.html.erb
index 3b35851991257ee55942dc0f352deafd4b50f5b5..5160716d16abf14aa713daf5639933e30fee6934 100644 (file)
--- a/app/views/user/terms.html.erb
+++ b/app/views/user/terms.html.erb
@@ -13,7 +13,7 @@
             :before => update_page do |page|
               page.replace_html 'contributorTerms', image_tag('searching.gif')
             end,
             :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) %>
           )
       %>
       <%= label_tag "legale_#{legale}", t('user.terms.legale_names.' + name) %>


@@ -22,7 +22,7 @@
 <% end %>
 
 <div id="contributorTerms">
 <% 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 %>
 </div>
 
 <% form_tag({:action => "save"}, { :id => "termsForm" }) do %>


@@ -41,9 +41,7 @@
       <%= hidden_field('user', 'pass_crypt_confirmation') %>
     <% end %>
     <div id="buttons">
       <%= hidden_field('user', 'pass_crypt_confirmation') %>
     <% end %>
     <div id="buttons">

-      <% if params[:user] %>
-        <%= submit_tag(t('user.terms.decline'), :name => "decline", :id => "decline") %>
-      <% end %>
+      <%= submit_tag(t('user.terms.decline'), :name => "decline", :id => "decline") %>

       <%= submit_tag(t('user.terms.agree'), :name => "agree", :id => "agree") %>
     </div>
   </p>
       <%= submit_tag(t('user.terms.agree'), :name => "agree", :id => "agree") %>
     </div>
   </p>

diff --git a/config/example.application.yml b/config/example.application.yml
index 303c5626a9f385b9c52e8a821d0b313fe1d0ab64..5037af1b0dd5a1971425f66ec71e9589c88cf9be 100644 (file)
--- a/config/example.application.yml
+++ b/config/example.application.yml
@@ -65,12 +65,15 @@ standard_settings: &standard_settings
   default_editor: "potlatch"
   # OAuth consumer key for Potlatch 2
   #potlatch2_key: ""
   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
 
 development:
   <<: *standard_settings
 
 production:
   <<: *standard_settings

+  require_terms_seen: false

 
 test:
   <<: *standard_settings
 
 test:
   <<: *standard_settings

diff --git a/config/legales/GB.yml b/config/legales/GB.yml
index 76ccdf5670715aaada6d6893d85817a999979ca2..2290316f40cc29e9f620362e12918129f0a93d6b 100644 (file)
--- a/config/legales/GB.yml
+++ b/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."
 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."
 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."

diff --git a/config/locales/en.yml b/config/locales/en.yml
index f8cbb6e374d0fb852f10dbb4f58df40220ebdc22..09b4838e7b850f1012d514f07600b35bee3e1b9c 100644 (file)
--- a/config/locales/en.yml
+++ b/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."
       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."
   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."

diff --git a/db/migrate/20110322001319_add_terms_seen_to_user.rb b/db/migrate/20110322001319_add_terms_seen_to_user.rb
new file mode 100644 (file)
index 0000000..8178c15
--- /dev/null
+++ b/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

diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml
index eb3c6ef6d590547fd78b029dd0be720a805a3749..98517fd2aa86f9b24134f0ce560027349c93324a 100644 (file)
--- a/test/fixtures/users.yml
+++ b/test/fixtures/users.yml
@@ -11,6 +11,7 @@ normal_user:
   home_lat: 12.1
   home_lon: 12.1
   home_zoom: 3
   home_lat: 12.1
   home_lon: 12.1
   home_zoom: 3

+  terms_seen: true

   
 public_user:
   id: 2
   
 public_user:
   id: 2


@@ -24,6 +25,7 @@ public_user:
   home_lat: 12
   home_lon: 12
   home_zoom: 12
   home_lat: 12
   home_lon: 12
   home_zoom: 12

+  terms_seen: true

   
 inactive_user:
   id: 3
   
 inactive_user:
   id: 3


@@ -37,6 +39,7 @@ inactive_user:
   home_lat: 123.4
   home_lon: 12.34
   home_zoom: 15
   home_lat: 123.4
   home_lon: 12.34
   home_zoom: 15

+  terms_seen: true

   
 second_public_user:
   id: 4
   
 second_public_user:
   id: 4


@@ -50,6 +53,7 @@ second_public_user:
   home_lat: 89
   home_lon: 87
   home_zoom: 12
   home_lat: 89
   home_lon: 87
   home_zoom: 12

+  terms_seen: true

 
 moderator_user:
   id: 5
 
 moderator_user:
   id: 5


@@ -59,6 +63,7 @@ moderator_user:
   creation_time: "2008-05-01 01:23:45"
   display_name: moderator
   data_public: true
   creation_time: "2008-05-01 01:23:45"
   display_name: moderator
   data_public: true

+  terms_seen: true

 
 administrator_user:
   id: 6
 
 administrator_user:
   id: 6


@@ -68,3 +73,14 @@ administrator_user:
   creation_time: "2008-05-01 01:23:45"
   display_name: administrator
   data_public: true
   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

diff --git a/test/functional/diary_entry_controller_test.rb b/test/functional/diary_entry_controller_test.rb
index 38c7bddcd50b89571de01e6745d2b67a0306a500..b4ff824bd25d71e357d6b18905498b9fa26d7c1d 100644 (file)
--- a/test/functional/diary_entry_controller_test.rb
+++ b/test/functional/diary_entry_controller_test.rb
@@ -1,7 +1,7 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class DiaryEntryControllerTest < ActionController::TestCase
 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
 
 
   include ActionView::Helpers::NumberHelper
 

diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb
new file mode 100644 (file)
index 0000000..fc42c23
--- /dev/null
+++ b/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
+
+