require terms by default, fixes CNVS-7330

they were required prior to our new settings refactors. this way we don't
need to go set it in prod

test plan:
1. terms should be required when registering at /register
2. terms should be required when activing an account (via course
   invitation email)
3. in the console, do Setting.set('terms_required', 'false')
4. terms should no longer be required on those pages

Change-Id: Idb13727839b847845cd4dacde4797e4e1571b42f
Reviewed-on: https://gerrit.instructure.com/22988
Reviewed-by: Mark Ericksen <marke@instructure.com>
Product-Review: Marc LeGendre <marc@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>

Author: jenseng

app/models/account.rb

@@ -238,7 +238,7 @@ def privacy_policy_url
   end
 
   def terms_required?
-    Setting.get('terms_required', false)
+    Setting.get('terms_required', 'true') == 'true'
   end
 
   def require_acceptance_of_terms?(user)

app/models/user.rb

@@ -1083,9 +1083,11 @@ def update_avatar_image(force_reload=false)
   end
 
   def record_acceptance_of_terms
-    if @require_acceptance_of_terms && @terms_of_use
-      preferences[:accepted_terms] = Time.now.utc
-    end
+    accept_terms if @require_acceptance_of_terms && @terms_of_use
+  end
+
+  def accept_terms
+    preferences[:accepted_terms] = Time.now.utc
   end
 
   def self.max_messages_per_day

app/views/communication_channels/confirm.html.erb

@@ -133,16 +133,18 @@ Which is you?", :count => @merge_opportunities.length %>
         </div>
       </div>
       <% end %>
-      <% if require_terms? %>
       <div class="control-group">
         <div class="controls">
-          <label for="user_terms_of_use">
-            <%= check_box :user, :terms_of_use %>
-            <%= agree_to_terms %>
-          </label>
+          <% if require_terms? %>
+            <label for="user_terms_of_use">
+              <%= check_box :user, :terms_of_use %>
+              <%= agree_to_terms %>
+            </label>
+          <% else %>
+            <a href="<%= @domain_root_account.privacy_policy_url %>" target="_blank"><%= t "#site.view_privacy_policy", "View Privacy Policy" %></a>
+          <% end %>
         </div>
       </div>
-      <% end %>
       <div class="control-group">
         <div class="controls">
           <% unless @merge_opportunities.empty? %>

spec/controllers/communication_channels_controller_spec.rb

@@ -114,6 +114,8 @@
     describe "open registration" do
       it "should show a pre-registered user the confirmation form" do
         user_with_pseudonym(:password => :autogenerate)
+        @user.accept_terms
+        @user.save
         @user.should be_pre_registered
 
         get 'confirm', :nonce => @cc.confirmation_code
@@ -126,6 +128,8 @@
 
       it "should finalize registration for a pre-registered user" do
         user_with_pseudonym(:password => :autogenerate)
+        @user.accept_terms
+        @user.save
         @user.should be_pre_registered
 
         post 'confirm', :nonce => @cc.confirmation_code, :register => 1, :pseudonym => {:password => 'asdfasdf', :password_confirmation => 'asdfasdf'}
@@ -152,6 +156,8 @@
         @account = Account.create!
         @course = Course.create!(:account => @account) { |c| c.workflow_state = 'available' }
         user_with_pseudonym(:account => @account, :password => :autogenerate)
+        @user.accept_terms
+        @user.save
         @enrollment = @course.enroll_user(@user)
         @pseudonym.account.should == @account
         @user.should be_pre_registered
@@ -190,6 +196,7 @@
       it "should show the confirm form for a creation_pending user" do
         course(:active_all => 1)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -205,6 +212,7 @@
       it "should register creation_pending user" do
         course(:active_all => 1)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -230,6 +238,7 @@
 
       it "should show the confirm form for a creation_pending user that's logged in (masquerading)" do
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         # not a full user session; just @current_user is set
@@ -243,6 +252,7 @@
 
       it "should register creation_pending user that's logged in (masquerading)" do
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         # not a full user session; just @current_user is set
@@ -267,6 +277,7 @@
         @account = Account.create!
         course(:active_all => 1, :account => @account)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -285,6 +296,7 @@
         @account = Account.create!
         course(:active_all => 1, :account => @account)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -311,6 +323,7 @@
       it "should prepare to register a creation_pending user in the correct account (admin)" do
         @account = Account.create!
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @account.add_user(@user)
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
@@ -327,6 +340,7 @@
       it "should register creation_pending user in the correct account (admin)" do
         @account = Account.create!
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @account.add_user(@user)
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
@@ -350,6 +364,7 @@
       it "should show the confirm form for old creation_pending users that have a pseudonym" do
         course(:active_all => 1)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -364,6 +379,7 @@
       it "should work for old creation_pending users that have a pseudonym" do
         course(:active_all => 1)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -391,6 +407,7 @@
         user_with_pseudonym(:active_all => 1, :username => 'jt@instructure.com')
         course(:active_all => 1)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -407,6 +424,7 @@
         user_with_pseudonym(:active_all => 1, :username => 'jt@instructure.com')
         course(:active_all => 1)
         user
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
         @enrollment = @course.enroll_student(@user)
@@ -562,6 +580,7 @@
 
       it "should accept an invitation when creating a new user" do
         course_with_student(:active_course => 1)
+        @user.accept_terms
         @user.update_attribute(:workflow_state, 'creation_pending')
         @cc = @user.communication_channels.create!(:path => 'jt@instructure.com')
 

spec/controllers/tours_controller_spec.rb

@@ -9,7 +9,7 @@
   end
 
   it "should add dismissed tours to user preferences" do
-    @user.preferences.should be_empty
+    @user.preferences[:dismissed_tours].should be_nil
     delete 'dismiss', :name => 'FakeTour'
     @user.reload
     @user.preferences[:dismissed_tours].should == {:fake_tour => 1}

spec/controllers/users_controller_spec.rb

@@ -275,14 +275,19 @@
         p.user.communication_channels.first.path.should == 'jacob@instructure.com'
       end
 
-      it "should validate acceptance of the terms if required" do
-        Setting.set('terms_required', true)
+      it "should validate acceptance of the terms" do
         post 'create', :pseudonym => { :unique_id => 'jacob@instructure.com' }, :user => { :name => 'Jacob Fugal' }
         response.status.should =~ /400 Bad Request/
         json = JSON.parse(response.body)
         json["errors"]["user"]["terms_of_use"].should be_present
       end
 
+      it "should not validate acceptance of the terms if not required" do
+        Setting.set('terms_required', 'false')
+        post 'create', :pseudonym => { :unique_id => 'jacob@instructure.com' }, :user => { :name => 'Jacob Fugal' }
+        response.should be_success
+      end
+
       it "should require email pseudonyms by default" do
         post 'create', :pseudonym => { :unique_id => 'jacob' }, :user => { :name => 'Jacob Fugal', :terms_of_use => '1' }
         response.status.should =~ /400 Bad Request/

spec/selenium/communication_channels_spec.rb

@@ -5,6 +5,7 @@
 
   context "confirm" do
     it "should register the user" do
+      Setting.set('terms_required', 'false')
       u1 = user_with_communication_channel(:user_state => 'creation_pending')
       get "/register/#{u1.communication_channel.confirmation_code}"
       set_value f('#pseudonym_password'), "asdfasdf"
@@ -15,7 +16,6 @@
     end
 
     it "should require the terms if configured to do so" do
-      Setting.set('terms_required', true)
       u1 = user_with_communication_channel(:user_state => 'creation_pending')
       get "/register/#{u1.communication_channel.confirmation_code}"
       f('input[name="user[terms_of_use]"]').should be_present
@@ -25,7 +25,6 @@
     end
 
     it "should not require the terms if the user has already accepted them" do
-      Setting.set('terms_required', true)
       u1 = user_with_communication_channel(:user_state => 'creation_pending')
       u1.preferences[:accepted_terms] = Time.now.utc
       u1.save
@@ -35,6 +34,8 @@
 
     it "should allow the user to edit the pseudonym if its already taken" do
       u1 = user_with_communication_channel(:username => 'asdf@qwerty.com', :user_state => 'creation_pending')
+      u1.accept_terms
+      u1.save
       # d'oh, now it's taken
       u2 = user_with_pseudonym(:username => 'asdf@qwerty.com', :active_user => true)
 

spec/selenium/terms_of_use_spec.rb

@@ -4,7 +4,6 @@
   it_should_behave_like "in-process server selenium tests"
 
   before do
-    Setting.set 'terms_required', true
     user_with_pseudonym(active_user: true)
   end
 

spec/selenium/users_spec.rb

@@ -197,6 +197,8 @@ def validate_login_info(user_id)
     end
 
     it "should not require terms if not configured to do so" do
+      Setting.set('terms_required', 'false')
+
       get '/register'
 
       %w{teacher student parent}.each do |type|
@@ -208,8 +210,6 @@ def validate_login_info(user_id)
     end
 
     it "should require terms if configured to do so" do
-      Setting.set('terms_required', true)
-
       get "/register"
 
       %w{teacher student parent}.each do |type|
@@ -237,6 +237,7 @@ def validate_login_info(user_id)
       f('#student_username').send_keys('student')
       f('#student_password').send_keys('asdfasdf')
       f('#student_password_confirmation').send_keys('asdfasdf')
+      f('input[name="user[terms_of_use]"]', form).click
 
       expect_new_page_load { form.submit }
       # confirm the user is authenticated into the dashboard
@@ -251,6 +252,7 @@ def validate_login_info(user_id)
       form = fj('.ui-dialog:visible form')
       f('#teacher_name').send_keys('teacher!')
       f('#teacher_email').send_keys('teacher@example.com')
+      f('input[name="user[terms_of_use]"]', form).click
 
       expect_new_page_load { form.submit }
       # confirm the user is authenticated into the dashboard
@@ -269,6 +271,7 @@ def validate_login_info(user_id)
       f('#parent_email').send_keys('parent@example.com')
       f('#parent_child_username').send_keys(@pseudonym.unique_id)
       f('#parent_child_password').send_keys('lolwut')
+      f('input[name="user[terms_of_use]"]', form).click
 
       expect_new_page_load { form.submit }
       # confirm the user is authenticated into the dashboard

spec/spec_helper.rb

@@ -270,7 +270,10 @@ def site_admin_user(opts={})
 
   def user(opts={})
     @user = User.create!(opts.slice(:name, :short_name))
-    @user.register! if opts[:active_user] || opts[:active_all]
+    if opts[:active_user] || opts[:active_all]
+      @user.accept_terms
+      @user.register!
+    end
     @user.update_attribute :workflow_state, opts[:user_state] if opts[:user_state]
     @user
   end