3604: Fix user_agreements behavior, update tests to expect redirects.

diff --git a/apps/workbench/app/assets/javascripts/user_agreements.js b/apps/workbench/app/assets/javascripts/user_agreements.js
new file mode 100644 (file)
index 0000000..688bd0b
--- /dev/null
+++ b/apps/workbench/app/assets/javascripts/user_agreements.js
@@ -0,0 +1,9 @@
+$('#open_user_agreement input[name="checked[]"]').on('click', function() {
+    var dialog = $('#open_user_agreement')[0]
+    $('input[type=submit]', dialog).prop('disabled',false);
+    $('input[name="checked[]"]', dialog).each(function(){
+        if(!this.checked) {
+            $('input[type=submit]', dialog).prop('disabled',true);
+        }
+    });
+});

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index de0e3438d42f5b39a4fd617eb2be742c8093e162..9963e2a13ce37c48369617e2fd568c4ebd744594 100644 (file)
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -512,19 +512,22 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  helper_method :unsigned_user_agreements
+  def unsigned_user_agreements
+    @signed_ua_uuids ||= UserAgreement.signatures.map &:head_uuid
+    @unsigned_user_agreements ||= UserAgreement.all.map do |ua|
+      if not @signed_ua_uuids.index ua.uuid
+        Collection.find(ua.uuid)
+      end
+    end.compact
+  end
+
   def check_user_agreements
     if current_user && !current_user.is_active
       if not current_user.is_invited
         return redirect_to inactive_users_path, return_to: request.fullpath
       end
-      signatures = UserAgreement.signatures
-      @signed_ua_uuids = UserAgreement.signatures.map &:head_uuid
-      @required_user_agreements = UserAgreement.all.map do |ua|
-        if not @signed_ua_uuids.index ua.uuid
-          Collection.find(ua.uuid)
-        end
-      end.compact
-      if @required_user_agreements.empty?
+      if unsigned_user_agreements.empty?
         # No agreements to sign. Perhaps we just need to ask?
         current_user.activate
         if !current_user.is_active

diff --git a/apps/workbench/app/views/user_agreements/index.html.erb b/apps/workbench/app/views/user_agreements/index.html.erb
index 49516eb67d48b646b46b743d5df178fa409e0f0c..d37360188dfee5a9f6fc366214ef35db7e6dddaf 100644 (file)
--- a/apps/workbench/app/views/user_agreements/index.html.erb
+++ b/apps/workbench/app/views/user_agreements/index.html.erb
@@ -1,27 +1,27 @@
 <% content_for :breadcrumbs do raw '<!-- -->' end %>
 
-<% n_files = @required_user_agreements.collect(&:files).flatten(1).count %>
+<% n_files = unsigned_user_agreements.collect(&:files).flatten(1).count %>
 <% content_for :page_title do %>
 <% if n_files == 1 %>
-<%= @required_user_agreements.first.files.first[1].sub(/\.[a-z]{3,4}$/,'') %>
+<%= unsigned_user_agreements.first.files.first[1].sub(/\.[a-z]{3,4}$/,'') %>
 <% else %>
 User agreements
 <% end %>
 <% end %>
 
-<%= form_for(@required_user_agreements.first, {url: {action: 'sign', controller: 'user_agreements'}, method: 'post'}) do |f| %>
+<%= form_for(unsigned_user_agreements.first, {url: {action: 'sign', controller: 'user_agreements'}, method: 'post'}) do |f| %>
 <%= hidden_field_tag :return_to, request.url %>
 <div id="open_user_agreement">
   <div class="alert alert-info">
     <strong>Please check <%= n_files > 1 ? 'each' : 'the' %> box below</strong> to indicate that you have read and accepted the user agreement<%= 's' if n_files > 1 %>.
   </div>
   <% if n_files == 1 and (Rails.configuration.show_user_agreement_inline rescue false) %>
-  <% ua = @required_user_agreements.first; file = ua.files.first %>
+  <% ua = unsigned_user_agreements.first; file = ua.files.first %>
   <object data="<%= url_for(controller: 'collections', action: 'show_file', uuid: ua.uuid, file: "#{file[0]}/#{file[1]}") %>" type="<%= Rack::Mime::MIME_TYPES[file[1].match(/\.\w+$/)[0]] rescue '' %>" width="100%" height="400px">
   </object>
   <% end %>
   <div>
-    <% @required_user_agreements.each do |ua| %>
+    <% unsigned_user_agreements.each do |ua| %>
     <% ua.files.each do |file| %>
     <%= f.label 'checked[]', class: 'checkbox inline' do %>
     <%= check_box_tag 'checked[]', "#{ua.uuid}/#{file[0]}/#{file[1]}", false %>
@@ -37,15 +37,3 @@ User agreements
   </div>
 </div>
 <% end %>
-
-<% content_for :footer_js do %>
-$('#open_user_agreement input[name="checked[]"]').on('click', function() {
-    var dialog = $('#open_user_agreement')[0]
-    $('input[type=submit]', dialog).prop('disabled',false);
-    $('input[name="checked[]"]', dialog).each(function(){
-        if(!this.checked) {
-            $('input[type=submit]', dialog).prop('disabled',true);
-        }
-    });
-});
-<% end %>

diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index ae0cad09e3917213d5f5a266370aed68666f95b0..01ccf186762e59e71192856044ed4d61302833e5 100644 (file)
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -1,13 +1,18 @@
 require 'test_helper'
 
 class ProjectsControllerTest < ActionController::TestCase
-  test "inactive user is asked to sign user agreements on front page" do
+  test "invited user is asked to sign user agreements on front page" do
     get :index, {}, session_for(:inactive)
-    assert_response :success
-    assert_not_empty assigns(:required_user_agreements),
-    "Inactive user did not have required_user_agreements"
-    assert_template 'user_agreements/index',
-    "Inactive user was not presented with a user agreement at the front page"
+    assert_response :redirect
+    assert_equal(user_agreements_url, @response.redirect_url,
+                 "Inactive user was not redirected to user_agreements page")
+  end
+
+  test "uninvited user is asked to wait for activation" do
+    get :index, {}, session_for(:inactive_uninvited)
+    assert_response :redirect
+    assert_equal(inactive_users_url, @response.redirect_url,
+                 "Uninvited user was not redirected to inactive user page")
   end
 
   [[:active, true],