From 8b12abd5bb1b96567ab882a3aca0780d0e4af67a Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Tue, 22 Mar 2011 10:26:10 +0000 Subject: [PATCH] Phase 2 CTs implementation and tests, with config parameter --- app/controllers/application_controller.rb | 8 ++++ app/controllers/user_controller.rb | 14 ++++-- app/views/user/_terms.html.erb | 6 +-- app/views/user/terms.html.erb | 8 ++-- config/example.application.yml | 3 ++ config/legales/GB.yml | 1 - config/locales/en.yml | 1 + .../20110322001319_add_terms_seen_to_user.rb | 13 ++++++ test/fixtures/users.yml | 16 +++++++ .../functional/diary_entry_controller_test.rb | 2 +- test/integration/user_terms_seen_test.rb | 45 +++++++++++++++++++ 11 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20110322001319_add_terms_seen_to_user.rb create mode 100644 test/integration/user_terms_seen_test.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bc71f275f..a22bc13ed 100644 --- 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 + # 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") diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 6935af3bc..631c91035 100644 --- 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| - 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] diff --git a/app/views/user/_terms.html.erb b/app/views/user/_terms.html.erb index 2c6b0885c..92b98ea04 100644 --- a/app/views/user/_terms.html.erb +++ b/app/views/user/_terms.html.erb @@ -1,10 +1,6 @@

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

  1. diff --git a/app/views/user/terms.html.erb b/app/views/user/terms.html.erb index 3b3585199..5160716d1 100644 --- 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, - :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 %>
    - <%= render :partial => "terms", :locals => { :has_decline =>params.has_key?(:user) } %> + <%= render :partial => "terms" %>
    <% form_tag({:action => "save"}, { :id => "termsForm" }) do %> @@ -41,9 +41,7 @@ <%= hidden_field('user', 'pass_crypt_confirmation') %> <% end %>
    - <% 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") %>

    diff --git a/config/example.application.yml b/config/example.application.yml index 303c5626a..5037af1b0 100644 --- 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: "" + # 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 diff --git a/config/legales/GB.yml b/config/legales/GB.yml index 76ccdf567..2290316f4 100644 --- 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." -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." diff --git a/config/locales/en.yml b/config/locales/en.yml index f8cbb6e37..09b4838e7 100644 --- 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." + 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 index 000000000..8178c1592 --- /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 eb3c6ef6d..98517fd2a 100644 --- 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 + 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 diff --git a/test/functional/diary_entry_controller_test.rb b/test/functional/diary_entry_controller_test.rb index 38c7bddcd..b4ff824bd 100644 --- 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 - 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 index 000000000..fc42c2353 --- /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 + + -- 2.39.5