1 new commit in galaxy-central: https://bitbucket.org/galaxy/galaxy-central/commits/5ac9a9c3d39a/ Changeset: 5ac9a9c3d39a User: carlfeberhard Date: 2015-01-30 21:35:12+00:00 Summary: Managers: add a 'user_manager' in BaseController; History: allow 'history/share' to accept user ids, add browser tests for same; Browser tests: make api urls absolute and so usable in web-based tests Affected #: 4 files diff -r e93367302c182b66dc70503a4e02c05a2f0c19ff -r 5ac9a9c3d39a1bd2c260cc793a6a765ed2f9bfbb lib/galaxy/web/base/controller.py --- a/lib/galaxy/web/base/controller.py +++ b/lib/galaxy/web/base/controller.py @@ -40,6 +40,7 @@ from galaxy.managers import tags from galaxy.managers import workflows from galaxy.managers import base as managers_base +from galaxy.managers import users from galaxy.datatypes.metadata import FileParameter from galaxy.tools.parameters import visit_input_values from galaxy.tools.parameters.basic import DataToolParameter @@ -69,6 +70,7 @@ """Initialize an interface for application 'app'""" self.app = app self.sa_session = app.model.context + self.user_manager = users.UserManager( app ) def get_toolbox(self): """Returns the application toolbox""" diff -r e93367302c182b66dc70503a4e02c05a2f0c19ff -r 5ac9a9c3d39a1bd2c260cc793a6a765ed2f9bfbb lib/galaxy/webapps/galaxy/controllers/history.py --- a/lib/galaxy/webapps/galaxy/controllers/history.py +++ b/lib/galaxy/webapps/galaxy/controllers/history.py @@ -761,7 +761,9 @@ histories=histories, email=email, send_to_err=send_to_err ) - histories, send_to_users, send_to_err = self._get_histories_and_users( trans, user, id, email ) + + histories = self._get_histories( trans, id ) + send_to_users, send_to_err = self._get_users( trans, user, email ) if not send_to_users: if not send_to_err: send_to_err += "%s is not a valid Galaxy user. %s" % ( email, err_msg ) @@ -769,14 +771,21 @@ histories=histories, email=email, send_to_err=send_to_err ) + if params.get( 'share_button', False ): + # The user has not yet made a choice about how to share, so dictionaries will be built for display can_change, cannot_change, no_change_needed, unique_no_change_needed, send_to_err = \ self._populate_restricted( trans, user, histories, send_to_users, None, send_to_err, unique=True ) + send_to_err += err_msg if cannot_change and not no_change_needed and not can_change: send_to_err = "The histories you are sharing do not contain any datasets that can be accessed by the users with which you are sharing." - return trans.fill_template( "/history/share.mako", histories=histories, email=email, send_to_err=send_to_err ) + return trans.fill_template( "/history/share.mako", + histories=histories, + email=email, + send_to_err=send_to_err ) + if can_change or cannot_change: return trans.fill_template( "/history/share.mako", histories=histories, @@ -785,12 +794,18 @@ can_change=can_change, cannot_change=cannot_change, no_change_needed=unique_no_change_needed ) + if no_change_needed: return self._share_histories( trans, user, send_to_err, histories=no_change_needed ) + elif not send_to_err: # User seems to be sharing an empty history send_to_err = "You cannot share an empty history. " - return trans.fill_template( "/history/share.mako", histories=histories, email=email, send_to_err=send_to_err ) + + return trans.fill_template( "/history/share.mako", + histories=histories, + email=email, + send_to_err=send_to_err ) @web.expose @web.require_login( "share restricted histories with other users" ) @@ -807,7 +822,8 @@ share_button=True ) ) user = trans.get_user() user_roles = user.all_roles() - histories, send_to_users, send_to_err = self._get_histories_and_users( trans, user, id, email ) + histories = self._get_histories( trans, id ) + send_to_users, send_to_err = self._get_users( trans, user, email ) send_to_err = '' # The user has made a choice, so dictionaries will be built for sharing can_change, cannot_change, no_change_needed, unique_no_change_needed, send_to_err = \ @@ -856,31 +872,49 @@ histories_for_sharing[ send_to_user ].append( history ) return self._share_histories( trans, user, send_to_err, histories=histories_for_sharing ) - def _get_histories_and_users( self, trans, user, id, email ): - if not id: + def _get_histories( self, trans, ids ): + if not ids: # Default to the current history - id = trans.security.encode_id( trans.history.id ) - id = galaxy.util.listify( id ) + ids = trans.security.encode_id( trans.history.id ) + ids = galaxy.util.listify( ids ) + histories = [] + for history_id in ids: + histories.append( self.history_manager.get_owned( trans, self.decode_id( history_id ), trans.user ) ) + return histories + + def _get_users( self, trans, user, emails_or_ids ): + send_to_users = [] send_to_err = "" - histories = [] - for history_id in id: - histories.append( self.history_manager.get_owned( trans, self.decode_id( history_id ), trans.user ) ) - send_to_users = [] - for email_address in galaxy.util.listify( email ): - email_address = email_address.strip() - if email_address: - if email_address == user.email: - send_to_err += "You cannot send histories to yourself. " - else: - send_to_user = trans.sa_session.query( trans.app.model.User ) \ - .filter( and_( trans.app.model.User.table.c.email==email_address, - trans.app.model.User.table.c.deleted==False ) ) \ - .first() - if send_to_user: - send_to_users.append( send_to_user ) - else: - send_to_err += "%s is not a valid Galaxy user. " % email_address - return histories, send_to_users, send_to_err + for string in galaxy.util.listify( emails_or_ids ): + string = string.strip() + if not string: + continue + + send_to_user = None + if '@' in string: + email_address = string + send_to_user = self.user_manager.by_email( trans, email_address, + filters=[ trans.app.model.User.table.c.deleted == False ] ) + + else: + user_id = string + try: + decoded_user_id = self.decode_id( string ) + send_to_user = self.user_manager.by_id( trans, decoded_user_id ) + if send_to_user.deleted: + send_to_user = None + #TODO: in an ideal world, we would let this bubble up to web.expose which would handle it + except exceptions.MalformedId: + send_to_user = None + + if not send_to_user: + send_to_err += "%s is not a valid Galaxy user. " % string + elif send_to_user == user: + send_to_err += "You cannot send histories to yourself. " + else: + send_to_users.append( send_to_user ) + + return send_to_users, send_to_err def _populate( self, trans, histories_for_sharing, other, send_to_err ): # This method will populate the histories_for_sharing dictionary with the users and @@ -1466,7 +1500,6 @@ trans.set_history( history ) return self.history_serializer.serialize_to_view( trans, history, view='detailed' ) except exceptions.MessageException, msg_exc: - print type( msg_exc ) trans.response.status = msg_exc.err_code.code return { 'err_msg': msg_exc.err_msg, 'err_code': msg_exc.err_code.code } diff -r e93367302c182b66dc70503a4e02c05a2f0c19ff -r 5ac9a9c3d39a1bd2c260cc793a6a765ed2f9bfbb test/casperjs/history-share-tests.js --- /dev/null +++ b/test/casperjs/history-share-tests.js @@ -0,0 +1,227 @@ +var require = patchRequire( require ), + spaceghost = require( 'spaceghost' ).fromCasper( casper ), + xpath = require( 'casper' ).selectXPath, + utils = require( 'utils' ), + format = utils.format; + +spaceghost.test.begin( 'Testing the user share form for histories', 0, function suite( test ){ + spaceghost.start(); + + // =================================================================== globals and helpers + var email = spaceghost.user.getRandomEmail(), + password = '123456'; + if( spaceghost.fixtureData.testUser ){ + email = spaceghost.fixtureData.testUser.email; + password = spaceghost.fixtureData.testUser.password; + spaceghost.info( 'Will use fixtureData.testUser: ' + email ); + } + var email2 = spaceghost.user.getRandomEmail(), + password2 = '123456'; + if( spaceghost.fixtureData.testUser2 ){ + email2 = spaceghost.fixtureData.testUser2.email; + password2 = spaceghost.fixtureData.testUser2.password; + } + + var shareLink = 'a[href^="/history/share?"]', + shareSubmit = 'input[name="share_button"]', + firstUserShareButton = '#user-0-popup', + shareHistoryId = null, + shareUserId = null; + + function fromUserSharePage( fn ){ + spaceghost.then( function(){ + this.openHomePage( function(){ + this.historyoptions.clickOption( 'Share or Publish' ); + }); + this.waitForNavigation( 'history/sharing', function(){ + this.jumpToMain( function(){ + this.click( shareLink ); + }); + }); + this.waitForNavigation( 'history/share', function(){ + this.jumpToMain( function(){ + fn.call( this ); + }); + }); + }); + } + + function thenSwitchUser( email, password ){ + spaceghost.then( function(){ + spaceghost.user.logout(); + spaceghost.user.loginOrRegisterUser( email, password ); + }); + return spaceghost; + } + + function thenShareWithUser( comment, emailOrId, thenFn ){ + spaceghost.then( function(){ + fromUserSharePage( function(){ + this.test.comment( comment ); + this.fill( 'form#share', { + email : emailOrId + }); + // strangely, casper's submit=true flag doesn't work well here - need to manually push the button + this.click( shareSubmit ); + }); + spaceghost.then( function(){ + this.jumpToMain( function(){ + thenFn.call( this ); + }); + }); + }); + return spaceghost; + } + + // =================================================================== TESTS + // create user 1 and the test/target history + spaceghost.user.loginOrRegisterUser( email, password ).openHomePage( function(){ + shareHistoryId = this.api.histories.index()[0].id; + this.info( 'shareHistoryId: ' + shareHistoryId ); + }); + spaceghost.then( function(){ + // can't share an empty history (for some reason) + this.api.tools.thenUpload( shareHistoryId, { filepath: '../../test-data/1.bed' }); + }); + + // create user 2 and make sure they can't access the history right now + thenSwitchUser( email2, password2 ).openHomePage( function(){ + shareUserId = this.api.users.index()[0].id; + this.info( 'shareUserId: ' + shareUserId ); + + this.test.comment( 'user2 should not have access to test history' ); + this.api.assertRaises( function(){ + this.api.histories.show( shareHistoryId ); + }, 403, 'History is not accessible by user', 'show failed with error' ); + }); + + thenSwitchUser( email, password ); + thenShareWithUser( "should NOT work: share using non-existant user email", 'chunkylover53(a)aol.com', function(){ + this.test.assertExists( '.errormessage', 'found error message' ); + this.test.assertSelectorHasText( '.errormessage', 'is not a valid Galaxy user', 'wording is good' ); + }); + thenShareWithUser( "should NOT work: share using current user email", email, function(){ + this.test.assertExists( '.errormessage', 'found error message' ); + this.test.assertSelectorHasText( '.errormessage', 'You cannot send histories to yourself', 'wording is good' ); + }); + thenShareWithUser( "should work: share using email", email2, function(){ + this.test.assertExists( firstUserShareButton, 'found user share button' ); + this.test.assertSelectorHasText( firstUserShareButton, email2, 'share button text is email2' ); + }); + + // user 2 can now access the history + thenSwitchUser( email2, password2 ).openHomePage( function(){ + this.test.comment( 'user 2 can now access the history' ); + this.test.assert( !!this.api.histories.show( shareHistoryId ).id ); + }); + + + // remove share + thenSwitchUser( email, password ).thenOpen( spaceghost.baseUrl + '/history/sharing', function(){ + this.jumpToMain( function(){ + this.click( firstUserShareButton ); + this.wait( 100, function(){ + this.click( 'a[href^="/history/sharing?unshare_user"]' ); + }); + }); + }); + spaceghost.then( function(){ + this.test.assertDoesntExist( firstUserShareButton, 'no user share button seen' ); + }); + + thenSwitchUser( email2, password2 ).openHomePage( function(){ + this.test.comment( 'user2 should not have access to test history (again)' ); + this.api.assertRaises( function(){ + this.api.histories.show( shareHistoryId ); + }, 403, 'History is not accessible by user', 'show failed with error' ); + }); + + + // should NOT work: share using malformed id + thenSwitchUser( email, password ); + thenShareWithUser( "should NOT work: share using malformed id", '1234xyz', function(){ + this.test.assertExists( '.errormessage', 'found error message' ); + this.test.assertSelectorHasText( '.errormessage', 'is not a valid Galaxy user', 'wording is good' ); + }); + //spaceghost.then( function(){ + // // test user share using email + // fromUserSharePage( function(){ + // this.test.comment( 'should NOT work: share using malformed id' ); + // this.fill( '#share', { + // email : '1234xyz' + // }); + // this.click( shareSubmit ); + // }); + // spaceghost.then( function(){ + // this.jumpToMain( function(){ + // this.test.assertExists( '.errormessage', 'found error message' ); + // this.test.assertSelectorHasText( '.errormessage', 'is not a valid Galaxy user', 'wording is good' ); + // }); + // }); + //}); + + // should NOT work: share using current user id + spaceghost.then( function(){ + var currUserId = spaceghost.api.users.index()[0].id; + thenShareWithUser( "should NOT work: share using current user id", currUserId, function(){ + this.test.assertExists( '.errormessage', 'found error message' ); + this.test.assertSelectorHasText( '.errormessage', + 'You cannot send histories to yourself', 'wording is good' ); + }); + }); + //// should NOT work: share using current user id + //spaceghost.then( function(){ + // var currUserId = spaceghost.api.users.index()[0].id; + // // test user share using email + // fromUserSharePage( function(){ + // this.test.comment( 'should NOT work: share using current user id' ); + // this.debug( 'currUserId: ' + currUserId ); + // this.fill( 'form#share', { + // email : currUserId + // }); + // this.click( shareSubmit ); + // }); + // spaceghost.then( function(){ + // this.jumpToMain( function(){ + // this.test.assertExists( '.errormessage', 'found error message' ); + // this.test.assertSelectorHasText( '.errormessage', + // 'You cannot send histories to yourself', 'wording is good' ); + // }); + // }); + //}); + + spaceghost.then( function(){ + thenShareWithUser( "should work: share using id", shareUserId, function(){ + this.test.assertExists( firstUserShareButton, 'found user share button' ); + this.test.assertSelectorHasText( firstUserShareButton, email2, 'share button text is email2' ); + }); + }); + //// should work: share using id + //spaceghost.then( function(){ + // // test user share using email + // fromUserSharePage( function(){ + // this.test.comment( 'should work: share using id' ); + // this.fill( '#share', { + // email : shareUserId + // }); + // this.click( shareSubmit ); + // }); + // spaceghost.then( function(){ + // this.jumpToMain( function(){ + // this.test.assertExists( firstUserShareButton, 'found user share button' ); + // this.test.assertSelectorHasText( firstUserShareButton, email2, 'share button text is email2' ); + // }); + // }); + //}); + + // user 2 can now access the history + thenSwitchUser( email2, password2 ).openHomePage( function(){ + this.test.comment( 'user 2 can now access the history' ); + this.test.assert( !!this.api.histories.show( shareHistoryId ).id ); + }); + + /* + */ + // =================================================================== + spaceghost.run( function(){ test.done(); }); +}); diff -r e93367302c182b66dc70503a4e02c05a2f0c19ff -r 5ac9a9c3d39a1bd2c260cc793a6a765ed2f9bfbb test/casperjs/modules/api.js --- a/test/casperjs/modules/api.js +++ b/test/casperjs/modules/api.js @@ -187,7 +187,7 @@ // ------------------------------------------------------------------- ConfigurationAPI.prototype.urlTpls = { - index : 'api/configuration' + index : '/api/configuration' }; ConfigurationAPI.prototype.index = function index( deleted ){ @@ -209,12 +209,12 @@ // ------------------------------------------------------------------- HistoriesAPI.prototype.urlTpls = { - index : 'api/histories', - show : 'api/histories/%s', - create : 'api/histories', - delete_ : 'api/histories/%s', - undelete: 'api/histories/deleted/%s/undelete', - update : 'api/histories/%s', + index : '/api/histories', + show : '/api/histories/%s', + create : '/api/histories', + delete_ : '/api/histories/%s', + undelete: '/api/histories/deleted/%s/undelete', + update : '/api/histories/%s', }; HistoriesAPI.prototype.index = function index( deleted ){ @@ -292,10 +292,10 @@ // ------------------------------------------------------------------- HDAAPI.prototype.urlTpls = { - index : 'api/histories/%s/contents', - show : 'api/histories/%s/contents/%s', - create : 'api/histories/%s/contents', - update : 'api/histories/%s/contents/%s' + index : '/api/histories/%s/contents', + show : '/api/histories/%s/contents/%s', + create : '/api/histories/%s/contents', + update : '/api/histories/%s/contents/%s' }; HDAAPI.prototype.index = function index( historyId, ids ){ @@ -375,9 +375,9 @@ // ------------------------------------------------------------------- ToolsAPI.prototype.urlTpls = { - index : 'api/tools', - show : 'api/tools/%s', - create : 'api/tools' + index : '/api/tools', + show : '/api/tools/%s', + create : '/api/tools' }; ToolsAPI.prototype.index = function index( in_panel, trackster ){ @@ -651,14 +651,14 @@ // ------------------------------------------------------------------- WorkflowsAPI.prototype.urlTpls = { - index : 'api/workflows', - show : 'api/workflows/%s', + index : '/api/workflows', + show : '/api/workflows/%s', // run a workflow - create : 'api/workflows', - update : 'api/workflows/%s', + create : '/api/workflows', + update : '/api/workflows/%s', - upload : 'api/workflows/upload', // POST - download: 'api/workflows/download/%s' // GET + upload : '/api/workflows/upload', // POST + download: '/api/workflows/download/%s' // GET }; WorkflowsAPI.prototype.index = function index(){ @@ -712,12 +712,12 @@ // ------------------------------------------------------------------- //NOTE: lots of admin only functionality in this section UsersAPI.prototype.urlTpls = { - index : 'api/users', - show : 'api/users/%s', - create : 'api/users', - delete_ : 'api/users/%s', - undelete: 'api/users/deleted/%s/undelete', - update : 'api/users/%s' + index : '/api/users', + show : '/api/users/%s', + create : '/api/users', + delete_ : '/api/users/%s', + undelete: '/api/users/deleted/%s/undelete', + update : '/api/users/%s' }; UsersAPI.prototype.index = function index( deleted ){ @@ -795,12 +795,12 @@ // ------------------------------------------------------------------- VisualizationsAPI.prototype.urlTpls = { - index : 'api/visualizations', - show : 'api/visualizations/%s', - create : 'api/visualizations', - //delete_ : 'api/visualizations/%s', - //undelete: 'api/visualizations/deleted/%s/undelete', - update : 'api/visualizations/%s' + index : '/api/visualizations', + show : '/api/visualizations/%s', + create : '/api/visualizations', + //delete_ : '/api/visualizations/%s', + //undelete: '/api/visualizations/deleted/%s/undelete', + update : '/api/visualizations/%s' }; VisualizationsAPI.prototype.index = function index(){ Repository URL: https://bitbucket.org/galaxy/galaxy-central/ -- This is a commit notification from bitbucket.org. You are receiving this because you have the service enabled, addressing the recipient of this email.