[ARVADOS] updated: d554f6e4b85728181946a016e08ae00f16c46c90
git at public.curoverse.com
git at public.curoverse.com
Thu Mar 5 18:57:24 EST 2015
Summary of changes:
.../app/assets/javascripts/report_issue.js | 2 +-
apps/workbench/test/integration/errors_test.rb | 38 +++++++++++-----------
.../test/integration/report_issue_test.rb | 5 +++
.../workbench/test/unit/arvados_api_client_test.rb | 4 +--
4 files changed, 27 insertions(+), 22 deletions(-)
discards 130511ce33222d8c4d5bea163c54e76fbf1c4e59 (commit)
discards 2b8a615808ba5fd0dc7cb80451622e28ab6e51a7 (commit)
discards 317782960f7d53f57ac4c186140d1a753089c3c3 (commit)
via d554f6e4b85728181946a016e08ae00f16c46c90 (commit)
via 630b52566620e906c2037bc498cdd36b93240bd6 (commit)
via 470e9c16d9c8713e4b0ba614ed5216ff1b47defa (commit)
via 878b0ff6d405dfcd4f3277ff7d9382af29db6c32 (commit)
via 4e98958d0eed70a8287d3e72d5256de483dbe721 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (130511ce33222d8c4d5bea163c54e76fbf1c4e59)
\
N -- N -- N (d554f6e4b85728181946a016e08ae00f16c46c90)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit d554f6e4b85728181946a016e08ae00f16c46c90
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 18:58:53 2015 -0500
5105: Really call the report_issue action, instead of fetching "/",
when the "send" button is clicked.
diff --git a/apps/workbench/app/assets/javascripts/report_issue.js b/apps/workbench/app/assets/javascripts/report_issue.js
index f3c323c..371dacc 100644
--- a/apps/workbench/app/assets/javascripts/report_issue.js
+++ b/apps/workbench/app/assets/javascripts/report_issue.js
@@ -8,7 +8,7 @@ $(document).
}
$('div').remove('.modal-footer-status');
- $.ajax('/').
+ $.ajax('/report_issue', {type: 'POST'}).
success(function(data, status, jqxhr) {
var $sendButton = $('#report-issue-submit');
$sendButton.html('Report sent');
diff --git a/apps/workbench/test/integration/errors_test.rb b/apps/workbench/test/integration/errors_test.rb
index 8198be4..32f16a6 100644
--- a/apps/workbench/test/integration/errors_test.rb
+++ b/apps/workbench/test/integration/errors_test.rb
@@ -103,6 +103,10 @@ class ErrorsTest < ActionDispatch::IntegrationTest
assert_no_selector 'a,button:not([disabled])', text: 'Send problem report'
assert_selector 'a,button', text: 'Cancel'
+ report = mock
+ report.expects(:deliver).returns true
+ IssueReporter.expects(:send_report).returns report
+
# enter a report text and click on report
find_field('report_issue_text').set 'my test report text'
click_button 'Send problem report'
diff --git a/apps/workbench/test/integration/report_issue_test.rb b/apps/workbench/test/integration/report_issue_test.rb
index 7d4058d..4a15851 100644
--- a/apps/workbench/test/integration/report_issue_test.rb
+++ b/apps/workbench/test/integration/report_issue_test.rb
@@ -59,6 +59,11 @@ class ReportIssueTest < ActionDispatch::IntegrationTest
# enter a report text and click on report
page.find_field('report_issue_text').set 'my test report text'
assert page.has_button?('Send problem report'), 'Send problem report button not enabled after entering text'
+
+ report = mock
+ report.expects(:deliver).returns true
+ IssueReporter.expects(:send_report).returns report
+
click_button 'Send problem report'
# ajax success updated button texts and added footer message
commit 630b52566620e906c2037bc498cdd36b93240bd6
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 18:35:31 2015 -0500
5105: Tidy up test case.
diff --git a/apps/workbench/test/integration/errors_test.rb b/apps/workbench/test/integration/errors_test.rb
index 1897a03..8198be4 100644
--- a/apps/workbench/test/integration/errors_test.rb
+++ b/apps/workbench/test/integration/errors_test.rb
@@ -87,40 +87,36 @@ class ErrorsTest < ActionDispatch::IntegrationTest
visit page_with_token("active")
- assert(page.has_text?(/fiddlesticks/i), 'Expected to be in error page')
+ assert_text 'fiddlesticks'
# reset api server base config to let the popup rendering to work
Rails.configuration.arvados_v1_base = original_arvados_v1_base
- # check the "Report problem" button
- assert page.has_link? 'Report problem', 'Report problem link not found'
-
click_link 'Report problem'
+
within '.modal-content' do
- assert page.has_text?('Report a problem'), 'Report a problem text not found'
- assert page.has_no_text?('Version / debugging info'), 'Version / debugging info is not expected'
- assert page.has_text?('Describe the problem'), 'Describe the problem text not found'
- assert page.has_text?('Send problem report'), 'Send problem report button text is not found'
- assert page.has_no_button?('Send problem report'), 'Send problem report button is not disabled before entering problem description'
- assert page.has_button?('Cancel'), 'Cancel button not found'
+ assert_text 'Report a problem'
+ assert_no_text 'Version / debugging info'
+ assert_text 'Describe the problem'
+ assert_text 'Send problem report'
+ # "Send" button should be disabled until text is entered
+ assert_no_selector 'a,button:not([disabled])', text: 'Send problem report'
+ assert_selector 'a,button', text: 'Cancel'
# enter a report text and click on report
- page.find_field('report_issue_text').set 'my test report text'
- assert page.has_button?('Send problem report'), 'Send problem report button not enabled after entering text'
+ find_field('report_issue_text').set 'my test report text'
click_button 'Send problem report'
# ajax success updated button texts and added footer message
- assert page.has_no_text?('Send problem report'), 'Found button - Send problem report'
- assert page.has_no_button?('Cancel'), 'Found button - Cancel'
- assert page.has_text?('Report sent'), 'No text - Report sent'
- assert page.has_button?('Close'), 'No button - Close'
- assert page.has_text?('Thanks for reporting this issue'), 'No text - Thanks for reporting this issue'
-
+ assert_no_selector 'a,button', text: 'Send problem report'
+ assert_no_selector 'a,button', text: 'Cancel'
+ assert_text 'Report sent'
+ assert_text 'Thanks for reporting this issue'
click_button 'Close'
end
# out of the popup now and should be back in the error page
- assert(page.has_text?(/fiddlesticks/i), 'Expected to be in error page after closing report issue popup')
+ assert_text 'fiddlesticks'
ensure
Rails.configuration.arvados_v1_base = original_arvados_v1_base
end
commit 470e9c16d9c8713e4b0ba614ed5216ff1b47defa
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 17:46:17 2015 -0500
5105: Treat not-logged-in AJAX requests as errors, instead of redirecting to login prompts.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index b52591b..e078641 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -441,19 +441,14 @@ class ApplicationController < ActionController::Base
end
def redirect_to_login
- respond_to do |f|
- f.html {
- if request.method.in? ['GET', 'HEAD']
- redirect_to arvados_api_client.arvados_login_url(return_to: strip_token_from_path(request.url))
- else
- flash[:error] = "Either you are not logged in, or your session has timed out. I can't automatically log you in and re-attempt this request."
- redirect_to :back
- end
- }
- f.json {
- @errors = ['You do not seem to be logged in. You did not supply an API token with this request, and your session (if any) has timed out.']
- self.render_error status: 422
- }
+ if request.xhr? or request.format.json?
+ @errors = ['You are not logged in. Most likely your session has timed out and you need to log in again.']
+ render_error status: 401
+ elsif request.method.in? ['GET', 'HEAD']
+ redirect_to arvados_api_client.arvados_login_url(return_to: strip_token_from_path(request.url))
+ else
+ flash[:error] = "Either you are not logged in, or your session has timed out. I can't automatically log you in and re-attempt this request."
+ redirect_to :back
end
false # For convenience to return from callbacks
end
@@ -594,7 +589,8 @@ class ApplicationController < ActionController::Base
end
end
- # Redirect to login/welcome if client provided expired API token (or none at all)
+ # 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
@@ -604,15 +600,26 @@ class ApplicationController < ActionController::Base
# log in" page instead of getting stuck in a redirect loop.
session.delete :arvados_api_token
redirect_to_login
+ elsif request.xhr?
+ # If we redirect to the welcome page, the browser will handle
+ # the 302 by itself and the client code will end up rendering
+ # the "welcome" page in some content area where it doesn't make
+ # sense. Instead, we send 401 ("authenticate and try again" or
+ # "display error", depending on how smart the client side is).
+ @errors = ['You are not logged in.']
+ render_error status: 401
else
redirect_to welcome_users_path(return_to: request.fullpath)
end
end
def ensure_current_user_is_admin
- unless current_user and current_user.is_admin
+ if not current_user
+ @errors = ['Not logged in']
+ render_error status: 401
+ elsif not current_user.is_admin
@errors = ['Permission denied']
- self.render_error status: 401
+ render_error status: 403
end
end
diff --git a/apps/workbench/test/integration/ajax_errors_test.rb b/apps/workbench/test/integration/ajax_errors_test.rb
new file mode 100644
index 0000000..de2dfc7
--- /dev/null
+++ b/apps/workbench/test/integration/ajax_errors_test.rb
@@ -0,0 +1,45 @@
+require 'integration_helper'
+
+class AjaxErrorsTest < ActionDispatch::IntegrationTest
+ setup do
+ # Regrettably...
+ need_selenium 'to assert_text in iframe'
+ end
+
+ test 'load pane with deleted session' do
+ # Simulate loading a page in browser-tab A, hitting "Log out" in
+ # browser-tab B, then returning to browser-tab A and choosing a
+ # different tab. (Automatic tab refreshes will behave similarly.)
+ visit page_with_token('active', '/projects/' + api_fixture('groups')['aproject']['uuid'])
+ ActionDispatch::Request::Session.any_instance.stubs(:[]).returns(nil)
+ click_link "Subprojects"
+ wait_for_ajax
+ assert_no_selector '.container-fluid .container-fluid'
+ assert_no_text 'If you have never used'
+ assert_text 'Reload tab'
+ page.driver.browser.switch_to.frame 0
+ assert_text 'You are not logged in.'
+ end
+
+ test 'load pane with expired token' do
+ # Similar to 'deleted session'. Here, the session cookie is still
+ # alive, but it contains a token which has expired. This uses a
+ # different code path because Workbench cannot detect that
+ # anything is amiss until it actually uses the token in an API
+ # request.
+ visit page_with_token('active', '/projects/' + api_fixture('groups')['aproject']['uuid'])
+ use_token :active_trustedclient do
+ # Go behind Workbench's back to expire the "active" token.
+ token = api_fixture('api_client_authorizations')['active']['api_token']
+ auth = ApiClientAuthorization.find(token)
+ auth.update_attributes(expires_at: '1999-12-31T23:59:59Z')
+ end
+ click_link "Subprojects"
+ wait_for_ajax
+ assert_no_selector '.container-fluid .container-fluid'
+ assert_no_text 'If you have never used'
+ assert_text 'Reload tab'
+ page.driver.browser.switch_to.frame 0
+ assert_text 'You are not logged in.'
+ end
+end
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 335fcd0..c2ff207 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -32,8 +32,13 @@ class ActiveSupport::TestCase
# in integration tests -- they do not yet inherit this setting
fixtures :all
def use_token token_name
+ was = Thread.current[:arvados_api_token]
auth = api_fixture('api_client_authorizations')[token_name.to_s]
Thread.current[:arvados_api_token] = auth['api_token']
+ if block_given?
+ yield
+ Thread.current[:arvados_api_token] = was
+ end
end
setup do
commit 878b0ff6d405dfcd4f3277ff7d9382af29db6c32
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 17:20:32 2015 -0500
5105: Move displaced comment.
diff --git a/apps/workbench/app/assets/javascripts/tab_panes.js b/apps/workbench/app/assets/javascripts/tab_panes.js
index 6565ea9..c67772d 100644
--- a/apps/workbench/app/assets/javascripts/tab_panes.js
+++ b/apps/workbench/app/assets/javascripts/tab_panes.js
@@ -106,8 +106,8 @@ $(document).on('arv:pane:reload', '[data-pane-content-url]', function(e) {
var content_url = $pane.attr('data-pane-content-url');
$.ajax(content_url, {dataType: 'html', type: 'GET', context: $pane}).
done(function(data, status, jqxhr) {
- // Preserve collapsed state
var $pane = this;
+ // Preserve collapsed state
var collapsable = {};
$(".collapse", this).each(function(i, c) {
collapsable[c.id] = $(c).hasClass('in');
commit 4e98958d0eed70a8287d3e72d5256de483dbe721
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 14:39:27 2015 -0500
5105: Add API stub helpers and basic ArvadosApiClient unit tests.
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 078190b..335fcd0 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -86,6 +86,32 @@ module ApiFixtureLoader
end
end
+module ApiMockHelpers
+ def self.included base
+ base.class_eval do
+ def stub_api_calls_with_body body, status_code=200
+ resp = mock
+ stubbed_client = ArvadosApiClient.new
+ stubbed_client.instance_eval do
+ resp.responds_like_instance_of HTTP::Message
+ resp.stubs(:content).returns body
+ resp.stubs(:status_code).returns status_code
+ @api_client = HTTPClient.new
+ @api_client.stubs(:post).returns resp
+ end
+ ArvadosApiClient.stubs(:new_or_current).returns(stubbed_client)
+ end
+
+ def stub_api_calls_with_invalid_json
+ stub_api_calls_with_body ']"omg,bogus"['
+ end
+ end
+ end
+end
+class ActiveSupport::TestCase
+ include ApiMockHelpers
+end
+
class ActiveSupport::TestCase
include ApiFixtureLoader
def session_for api_client_auth_name
diff --git a/apps/workbench/test/unit/arvados_api_client_test.rb b/apps/workbench/test/unit/arvados_api_client_test.rb
new file mode 100644
index 0000000..a58a1ee
--- /dev/null
+++ b/apps/workbench/test/unit/arvados_api_client_test.rb
@@ -0,0 +1,20 @@
+require 'test_helper'
+
+class ArvadosApiClientTest < ActiveSupport::TestCase
+ test 'successful stubbed api request' do
+ stub_api_calls_with_body '{"foo":"bar","baz":0}'
+ use_token :active
+ resp = ArvadosApiClient.new_or_current.api Link, ''
+ assert_equal Hash, resp.class
+ assert_equal 'bar', resp[:foo]
+ assert_equal 0, resp[:baz]
+ end
+
+ test 'exception if server returns non-JSON' do
+ stub_api_calls_with_invalid_json
+ assert_raises ArvadosApiClient::InvalidApiResponseException do
+ use_token :active
+ resp = ArvadosApiClient.new_or_current.api Link, ''
+ end
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list