[ARVADOS] created: 199e61ebf83ae9ee11d8da46adb4a8897d885504

git at public.curoverse.com git at public.curoverse.com
Sun Aug 17 16:41:28 EDT 2014


        at  199e61ebf83ae9ee11d8da46adb4a8897d885504 (commit)


commit 199e61ebf83ae9ee11d8da46adb4a8897d885504
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 16:40:46 2014 -0400

    3604: Fix tests, and restore ability to view user agreement before completing profile.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index bea72a4..f59cb95 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -4,7 +4,8 @@ class CollectionsController < ApplicationController
   skip_before_filter(:find_object_by_uuid,
                      only: [:provenance, :show_file, :show_file_links])
   # We depend on show_file to display the user agreement:
-  skip_before_filter :check_user_agreements, only: [:show_file]
+  skip_before_filter :check_user_agreements, only: :show_file
+  skip_before_filter :check_user_profile, only: :show_file
 
   RELATION_LIMIT = 5
 
diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
index 4745d1b..f86c50a 100644
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ b/apps/workbench/test/functional/collections_controller_test.rb
@@ -170,7 +170,7 @@ class CollectionsControllerTest < ActionController::TestCase
       uuid: ua_collection['uuid'],
       file: ua_collection['manifest_text'].match(/ \d+:\d+:(\S+)/)[1]
     }, session_for(:inactive)
-    assert_nil(assigns(:required_user_agreements),
+    assert_nil(assigns(:unsigned_user_agreements),
                "Did not skip check_user_agreements filter " +
                "when showing the user agreement.")
     assert_response :success
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index 01ccf18..d76430c 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -4,14 +4,16 @@ class ProjectsControllerTest < ActionController::TestCase
   test "invited user is asked to sign user agreements on front page" do
     get :index, {}, session_for(:inactive)
     assert_response :redirect
-    assert_equal(user_agreements_url, @response.redirect_url,
+    assert_match(/^#{Regexp.escape(user_agreements_url)}\b/,
+                 @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,
+    assert_match(/^#{Regexp.escape(inactive_users_url)}\b/,
+                 @response.redirect_url,
                  "Uninvited user was not redirected to inactive user page")
   end
 
diff --git a/apps/workbench/test/integration/application_layout_test.rb b/apps/workbench/test/integration/application_layout_test.rb
index 8a0ebc3..bb1f7e3 100644
--- a/apps/workbench/test/integration/application_layout_test.rb
+++ b/apps/workbench/test/integration/application_layout_test.rb
@@ -190,14 +190,14 @@ class ApplicationLayoutTest < ActionDispatch::IntegrationTest
       end
     end
 
-    assert page.has_text? profile_message[0,25]
+    assert page.has_text? profile_message.gsub(/<.*?>/,'')[0,25]
     assert page.has_text? required_field_title
     page.find_field('user[prefs][:profile][:'+required_field_key+']').set 'value to fill required field'
 
     click_button "Save profile"
     # profile saved and in profile page now with success
     assert page.has_text? 'Thank you for filling in your profile'
-    click_button 'Access Arvados Workbench'
+    click_link 'Back to work'
 
     # profile saved and in home page now
     assert page.has_text? 'My projects'

commit 7d258618cbfa7b071d81b6bf7fa18c344d2b5f87
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 15:26:53 2014 -0400

    3604: Fix up profile and redirect handling.

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 9963e2a..b626361 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -14,7 +14,7 @@ class ApplicationController < ActionController::Base
   around_filter :require_thread_api_token, except: ERROR_ACTIONS
   before_filter :accept_uuid_as_id_param, except: ERROR_ACTIONS
   before_filter :check_user_agreements, except: ERROR_ACTIONS
-  before_filter :check_user_profile, except: [:update_profile] + ERROR_ACTIONS
+  before_filter :check_user_profile, except: ERROR_ACTIONS
   before_filter :check_user_notifications, except: ERROR_ACTIONS
   before_filter :load_filters_and_paging_params, except: ERROR_ACTIONS
   before_filter :find_object_by_uuid, except: [:index, :choose] + ERROR_ACTIONS
@@ -501,7 +501,7 @@ class ApplicationController < ActionController::Base
       session.delete :arvados_api_token
       redirect_to_login
     else
-      redirect_to welcome_users_path, return_to: request.fullpath
+      redirect_to welcome_users_path(return_to: request.fullpath)
     end
   end
 
@@ -525,7 +525,7 @@ class ApplicationController < ActionController::Base
   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
+        return redirect_to inactive_users_path(return_to: request.fullpath)
       end
       if unsigned_user_agreements.empty?
         # No agreements to sign. Perhaps we just need to ask?
@@ -536,7 +536,7 @@ class ApplicationController < ActionController::Base
         end
       end
       if !current_user.is_active
-        redirect_to user_agreements_path, return_to: request.fullpath
+        redirect_to user_agreements_path(return_to: request.fullpath)
       end
     end
     true
@@ -550,7 +550,7 @@ class ApplicationController < ActionController::Base
     end
 
     if missing_required_profile?
-      render 'users/profile'
+      redirect_to profile_user_path(current_user.uuid, return_to: request.fullpath)
     end
     true
   end
diff --git a/apps/workbench/app/controllers/users_controller.rb b/apps/workbench/app/controllers/users_controller.rb
index 1fae7d2..eb07fc3 100644
--- a/apps/workbench/app/controllers/users_controller.rb
+++ b/apps/workbench/app/controllers/users_controller.rb
@@ -1,6 +1,7 @@
 class UsersController < ApplicationController
   skip_around_filter :require_thread_api_token, only: :welcome
-  skip_before_filter :check_user_agreements, only: :inactive
+  skip_before_filter :check_user_agreements, only: [:welcome, :inactive]
+  skip_before_filter :check_user_profile, only: [:welcome, :inactive, :profile]
   skip_before_filter :find_object_by_uuid, only: [:welcome, :activity, :storage]
   before_filter :ensure_current_user_is_admin, only: [:sudo, :unsetup, :setup]
 
@@ -16,6 +17,10 @@ class UsersController < ApplicationController
     end
   end
 
+  def profile
+    params[:offer_return_to] ||= params[:return_to]
+  end
+
   def activity
     @breadcrumb_page_name = nil
     @users = User.limit(params[:limit] || 1000).all
diff --git a/apps/workbench/app/views/users/profile.html.erb b/apps/workbench/app/views/users/profile.html.erb
index 2c40fef..aab8843 100644
--- a/apps/workbench/app/views/users/profile.html.erb
+++ b/apps/workbench/app/views/users/profile.html.erb
@@ -3,14 +3,7 @@
     current_user_profile = current_user.prefs[:profile]
     show_save_button = false
 
-    profile_message = Rails.configuration.user_profile_form_message ? Rails.configuration.user_profile_form_message : 'You can manage your profile using this page. Any feilds in red are required and missing. Please fill in those fields before you can accesse Arvados Workbench.'
-
-    missing_required = missing_required_profile?
-
-    profile_url = '/users/'+current_user.uuid+'/profile'
-    target = request.url.partition('?target=')[-1]
-    target = request.url if target.empty?
-    return_to_url = (request.url.ends_with? profile_url) ? profile_url : profile_url+'?target='+target
+    profile_message = Rails.configuration.user_profile_form_message
 %>
 
 <div>
@@ -21,24 +14,19 @@
           </h4>
         </div>
         <div class="panel-body">
-          <% if !missing_required && params.andand.keys.include?('target') %>
-            <div class="rounded" style="border-width: 1px; border-style: dotted; border-color: lightgray;">
-              <p style="margin: 8px;">Thank you for filling in your profile. If you are done updating your profile,
-                 you can now access Arvados Workbench by clicking on this button.
-                  <form action="<%=target%>">
-                    <input style="margin-left: 8px;" class="btn btn-primary" type="submit" value="Access Arvados Workbench">
-                  </form>
-              </p>
+          <% if !missing_required_profile? && params[:offer_return_to] %>
+            <div class="alert alert-success">
+              <p>Thank you for filling in your profile. <%= link_to 'Back to work!', params[:offer_return_to], class: 'btn btn-sm btn-primary' %></p>
             </div>
           <% else %>
-            <div class="rounded" style="border-width: 1px; border-style: dotted; border-color: lightgray;">
-              <p style="margin: 8px;"> <%=raw(profile_message)%> </p>
+            <div class="alert alert-info">
+              <p><%=raw(profile_message)%></p>
             </div>
           <% end %>
 
-          <div class="rounded" style="border-width: 1px; border-style: dotted; border-color: lightgray;">
-            <%= form_tag "/users/#{current_user.uuid}", {method: 'patch', id: 'save_profile_form', name: 'save_profile_form', class: 'form-horizontal'} do %>
-              <%= hidden_field_tag :return_to, return_to_url %>
+            <%= form_for current_user, html: {id: 'save_profile_form', name: 'save_profile_form', class: 'form-horizontal'} do %>
+              <%= hidden_field_tag :offer_return_to, params[:offer_return_to] %>
+              <%= hidden_field_tag :return_to, profile_user_path(current_user.uuid, offer_return_to: params[:offer_return_to]) %>
               <div class="form-group">
                   <label for="email" class="col-sm-3 control-label"> Email </label>
                   <div class="col-sm-8">
@@ -102,7 +90,6 @@
                 </div>
               <% end %>
             <% end %>
-          </div>
         </div>
     </div>
 </div>
diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 3b4c2c0..45337e8 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -130,4 +130,4 @@ common:
 
   # Use "user_profile_form_message" to configure the message you want to display in
   # the profile page. If this is not provided, a default message will be displayed.
-  user_profile_form_message: Welcome to Arvados. Please fill in all required fields before you can access Arvados Workbench. Missing required fields are in <span style="color:red">red</span>.
+  user_profile_form_message: Welcome to Arvados. All <span style="color:red">required fields</span> must be completed before you can proceed.
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 7ae95ca..158a8a4 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -180,6 +180,9 @@ class User < ArvadosModel
     Link.destroy_all(link_class: 'signature',
                      tail_uuid: self.uuid)
 
+    # delete user preferences (including profile)
+    self.prefs = {}
+
     # mark the user as inactive
     self.is_active = false
     self.save!

commit e7b360fd7acfaa3428b774389baf9626f7ff026c
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 14:34:26 2014 -0400

    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
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 de0e343..9963e2a 100644
--- 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 49516eb..d373601 100644
--- 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 ae0cad0..01ccf18 100644
--- 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],

commit cf233d57cea2795891a40d4ecd2fdde492ab87b3
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 14:10:20 2014 -0400

    3604: Add test for redirect to welcome page

diff --git a/apps/workbench/test/functional/users_controller_test.rb b/apps/workbench/test/functional/users_controller_test.rb
index 86778df..e8ee10f 100644
--- a/apps/workbench/test/functional/users_controller_test.rb
+++ b/apps/workbench/test/functional/users_controller_test.rb
@@ -23,4 +23,10 @@ class UsersControllerTest < ActionController::TestCase
     assert_nil assigns(:my_jobs)
     assert_nil assigns(:my_ssh_keys)
   end
+
+  test "show welcome page if no token provided" do
+    get :index, {}
+    assert_response :redirect
+    assert_match /\/users\/welcome/, @response.redirect_url
+  end
 end

commit 59522bd2b1a628e090d89f71d491b76a0047fdf5
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 14:10:08 2014 -0400

    3604: Verify permission cache behavior in unsetup test

diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index a448d1a..90399ca 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -734,6 +734,9 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
 
     verify_link_existence response_user['uuid'], response_user['email'],
           false, false, false, false, false
+
+    assert_equal([], User.find_by_uuid(users(:active).uuid).groups_i_can(:read),
+                 "active user can still read some groups after being deactivated")
   end
 
   test "setup user with send notification param false and verify no email" do

commit 201663b6f49d3f1fa95a574dccdbf63c500d59d4
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 14:05:11 2014 -0400

    3604: Remove test for persistent/cache switch on wrong page.

diff --git a/apps/workbench/test/integration/collections_test.rb b/apps/workbench/test/integration/collections_test.rb
index 8657aaa..5606fc4 100644
--- a/apps/workbench/test/integration/collections_test.rb
+++ b/apps/workbench/test/integration/collections_test.rb
@@ -13,7 +13,7 @@ class CollectionsTest < ActionDispatch::IntegrationTest
     page.assert_no_selector "div[data-persistent-state='#{oldstate}']"
   end
 
-  ['/collections', '/users/welcome'].each do |path|
+  ['/collections'].each do |path|
     test "Flip persistent switch at #{path}" do
       Capybara.current_driver = Capybara.javascript_driver
       uuid = api_fixture('collections')['foo_file']['uuid']

commit ff024b6b09112e89205d01370cdab19b49d6bd50
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 14:02:19 2014 -0400

    3604: Do not check for notifications if user is not even activated.

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 8883e82..de0e343 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -589,15 +589,6 @@ class ApplicationController < ActionController::Base
     }
   }
 
-  #@@notification_tests.push lambda { |controller, current_user|
-  #  Job.limit(1).where(created_by: current_user.uuid).each do
-  #    return nil
-  #  end
-  #  return lambda { |view|
-  #    view.render partial: 'notifications/jobs_notification'
-  #  }
-  #}
-
   @@notification_tests.push lambda { |controller, current_user|
     Collection.limit(1).where(created_by: current_user.uuid).each do
       return nil
@@ -622,7 +613,7 @@ class ApplicationController < ActionController::Base
     @notification_count = 0
     @notifications = []
 
-    if current_user
+    if current_user.andand.is_active
       @showallalerts = false
       @@notification_tests.each do |t|
         a = t.call(self, current_user)

commit 8221eaf272a7781af6ccc5672e8bfba54283fe43
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 13:59:13 2014 -0400

    3604: Fix stale permission cache by using destroy instead of delete.

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 64e0d09..7ae95ca 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -153,47 +153,32 @@ class User < ArvadosModel
   # delete user signatures, login, repo, and vm perms, and mark as inactive
   def unsetup
     # delete oid_login_perms for this user
-    oid_login_perms = Link.where(tail_uuid: self.email,
-                                 link_class: 'permission',
-                                 name: 'can_login')
-    oid_login_perms.each do |perm|
-      Link.delete perm
-    end
+    Link.destroy_all(tail_uuid: self.email,
+                     link_class: 'permission',
+                     name: 'can_login')
 
     # delete repo_perms for this user
-    repo_perms = Link.where(tail_uuid: self.uuid,
-                            link_class: 'permission',
-                            name: 'can_manage')
-    repo_perms.each do |perm|
-      Link.delete perm
-    end
+    Link.destroy_all(tail_uuid: self.uuid,
+                     link_class: 'permission',
+                     name: 'can_manage')
 
     # delete vm_login_perms for this user
-    vm_login_perms = Link.where(tail_uuid: self.uuid,
-                                link_class: 'permission',
-                                name: 'can_login')
-    vm_login_perms.each do |perm|
-      Link.delete perm
-    end
+    Link.destroy_all(tail_uuid: self.uuid,
+                     link_class: 'permission',
+                     name: 'can_login')
 
     # delete "All users' group read permissions for this user
     group = Group.where(name: 'All users').select do |g|
       g[:uuid].match /-f+$/
     end.first
-    group_perms = Link.where(tail_uuid: self.uuid,
-                             head_uuid: group[:uuid],
-                             link_class: 'permission',
-                             name: 'can_read')
-    group_perms.each do |perm|
-      Link.delete perm
-    end
+    Link.destroy_all(tail_uuid: self.uuid,
+                     head_uuid: group[:uuid],
+                     link_class: 'permission',
+                     name: 'can_read')
 
     # delete any signatures by this user
-    signed_uuids = Link.where(link_class: 'signature',
-                              tail_uuid: self.uuid)
-    signed_uuids.each do |sign|
-      Link.delete sign
-    end
+    Link.destroy_all(link_class: 'signature',
+                     tail_uuid: self.uuid)
 
     # mark the user as inactive
     self.is_active = false

commit 1293b79e382a0e256818c06ad7c1d5a10ff20bb5
Author: Tom Clegg <tom at curoverse.com>
Date:   Sun Aug 17 13:50:05 2014 -0400

    3604: Fix theme support for welcome page

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index bbc2a82..8883e82 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -490,7 +490,7 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  # Reroute this request if an API token is unavailable.
+  # Redirect to login/welcome if client provided expired API token (or none at all)
   def require_thread_api_token
     if Thread.current[:arvados_api_token]
       yield
@@ -501,7 +501,7 @@ class ApplicationController < ActionController::Base
       session.delete :arvados_api_token
       redirect_to_login
     else
-      render 'users/welcome'
+      redirect_to welcome_users_path, return_to: request.fullpath
     end
   end
 
@@ -515,7 +515,7 @@ class ApplicationController < ActionController::Base
   def check_user_agreements
     if current_user && !current_user.is_active
       if not current_user.is_invited
-        return render 'users/inactive'
+        return redirect_to inactive_users_path, return_to: request.fullpath
       end
       signatures = UserAgreement.signatures
       @signed_ua_uuids = UserAgreement.signatures.map &:head_uuid
@@ -533,7 +533,7 @@ class ApplicationController < ActionController::Base
         end
       end
       if !current_user.is_active
-        render 'user_agreements/index'
+        redirect_to user_agreements_path, return_to: request.fullpath
       end
     end
     true
diff --git a/apps/workbench/app/controllers/users_controller.rb b/apps/workbench/app/controllers/users_controller.rb
index 67b51a9..1fae7d2 100644
--- a/apps/workbench/app/controllers/users_controller.rb
+++ b/apps/workbench/app/controllers/users_controller.rb
@@ -1,11 +1,18 @@
 class UsersController < ApplicationController
-  skip_before_filter :find_object_by_uuid, :only => [:welcome, :activity, :storage]
+  skip_around_filter :require_thread_api_token, only: :welcome
+  skip_before_filter :check_user_agreements, only: :inactive
+  skip_before_filter :find_object_by_uuid, only: [:welcome, :activity, :storage]
   before_filter :ensure_current_user_is_admin, only: [:sudo, :unsetup, :setup]
 
   def welcome
     if current_user
-      params[:action] = 'home'
-      home
+      redirect_to (params[:return_to] || '/')
+    end
+  end
+
+  def inactive
+    if current_user.andand.is_invited
+      redirect_to (params[:return_to] || '/')
     end
   end
 
diff --git a/apps/workbench/app/views/users/inactive.html.erb b/apps/workbench/app/views/users/inactive.html.erb
index 5f825e4..832b580 100644
--- a/apps/workbench/app/views/users/inactive.html.erb
+++ b/apps/workbench/app/views/users/inactive.html.erb
@@ -16,6 +16,10 @@
         An administrator must activate your account before you can get
         any further.
 
+      </p><p>
+
+        <%= link_to 'Retry', (params[:return_to] || '/'), class: 'btn btn-primary' %>
+
       </p>
     </div>
   </div>
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index 4ef2661..dfd6821 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -27,6 +27,7 @@ ArvadosWorkbench::Application.routes.draw do
     get 'choose', :on => :collection
     get 'home', :on => :member
     get 'welcome', :on => :collection
+    get 'inactive', :on => :collection
     get 'activity', :on => :collection
     get 'storage', :on => :collection
     post 'sudo', :on => :member

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list