Phase 2 CTs implementation and tests, with config parameter
authorMatt Amos <zerebubuth@gmail.com>
Tue, 22 Mar 2011 10:26:10 +0000 (10:26 +0000)
committerTom Hughes <tom@compton.nu>
Sun, 3 Apr 2011 12:46:16 +0000 (13:46 +0100)
app/controllers/application_controller.rb
app/controllers/user_controller.rb
app/views/user/_terms.html.erb
app/views/user/terms.html.erb
config/example.application.yml
config/legales/GB.yml
config/locales/en.yml
db/migrate/20110322001319_add_terms_seen_to_user.rb [new file with mode: 0644]
test/fixtures/users.yml
test/functional/diary_entry_controller_test.rb
test/integration/user_terms_seen_test.rb [new file with mode: 0644]

index bc71f27..a22bc13 100644 (file)
@@ -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") 
index 6935af3..631c910 100644 (file)
@@ -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,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.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]))
@@ -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]
index 2c6b088..92b98ea 100644 (file)
@@ -1,10 +1,6 @@
 <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>
index 3b35851..5160716 100644 (file)
@@ -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") %>
-      <% end %>
+      <%= submit_tag(t('user.terms.decline'), :name => "decline", :id => "decline") %>
       <%= submit_tag(t('user.terms.agree'), :name => "agree", :id => "agree") %>
     </div>
   </p>
index 303c562..5037af1 100644 (file)
@@ -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
index 76ccdf5..2290316 100644 (file)
@@ -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."
index f8cbb6e..09b4838 100644 (file)
@@ -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."
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
@@ -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
index eb3c6ef..98517fd 100644 (file)
@@ -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
index 38c7bdd..b4ff824 100644 (file)
@@ -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
 
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
@@ -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
+
+