details: http://www.bx.psu.edu/hg/galaxy/rev/267db48a2371 changeset: 2470:267db48a2371 user: Greg Von Kuster <greg@bx.psu.edu> date: Thu Jul 09 15:49:26 2009 -0400 description: Some history sharing cleanup and a fix for job error reports. 5 file(s) affected in this change: lib/galaxy/web/controllers/dataset.py lib/galaxy/web/controllers/history.py templates/history/list_shared.mako templates/history/share.mako test/functional/test_history_functions.py diffs (364 lines): diff -r 073c9a0b2467 -r 267db48a2371 lib/galaxy/web/controllers/dataset.py --- a/lib/galaxy/web/controllers/dataset.py Wed Jul 08 17:01:49 2009 -0400 +++ b/lib/galaxy/web/controllers/dataset.py Thu Jul 09 15:49:26 2009 -0400 @@ -17,7 +17,7 @@ ------------------------ This error report was sent from the Galaxy instance hosted on the server -"${remote_hostname}" +"${host}" ----------------------------------------------------------------------------- This is in reference to output dataset ${dataset_id}. ----------------------------------------------------------------------------- @@ -65,10 +65,10 @@ dataset = model.HistoryDatasetAssociation.get( id ) job = dataset.creating_job_associations[0].job # Get the name of the server hosting the Galaxy instance from which this report originated - remote_hostname = trans.request.remote_hostname + host = trans.request.host # Build the email message msg = MIMEText( string.Template( error_report_template ) - .safe_substitute( remote_hostname=remote_hostname, + .safe_substitute( host=host, dataset_id=dataset.id, email=email, message=message, diff -r 073c9a0b2467 -r 267db48a2371 lib/galaxy/web/controllers/history.py --- a/lib/galaxy/web/controllers/history.py Wed Jul 08 17:01:49 2009 -0400 +++ b/lib/galaxy/web/controllers/history.py Thu Jul 09 15:49:26 2009 -0400 @@ -310,8 +310,9 @@ email=email, send_to_err=send_to_err ) if params.get( 'share_button', False ): - can_change, cannot_change, no_change_needed, send_to_err = \ - self._populate_restricted( trans, user, histories, send_to_users, None, send_to_err ) + # 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 ) if can_change or cannot_change: return trans.fill_template( "/history/share.mako", histories=histories, @@ -319,7 +320,7 @@ send_to_err=send_to_err, can_change=can_change, cannot_change=cannot_change, - no_change_needed=no_change_needed ) + 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: @@ -335,7 +336,8 @@ user = trans.get_user() histories, send_to_users, send_to_err = self._get_histories_and_users( trans, user, id, email ) send_to_err = '' - can_change, cannot_change, no_change_needed, 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 = \ self._populate_restricted( trans, user, histories, send_to_users, action, send_to_err ) # Now that we've populated the can_change, cannot_change, and no_change_needed dictionaries, # we'll populate the histories_for_sharing dictionary from each of them. @@ -408,13 +410,13 @@ send_to_err += "%s is not a valid Galaxy user. " % email_address return histories, 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 + # This method will populate the histories_for_sharing dictionary with the users and # histories in other, eliminating histories that have already been shared with the # associated user. No security checking on datasets is performed. # If not empty, the histories_for_sharing dictionary looks like: # { userA: [ historyX, historyY ], userB: [ historyY ] } # other looks like: - # ## { userA: {historyX : [hda, hda], historyY : [hda]}, userB: {historyY : [hda]} } + # { userA: {historyX : [hda, hda], historyY : [hda]}, userB: {historyY : [hda]} } for send_to_user, history_dict in other.items(): for history in history_dict: # Make sure the current history has not already been shared with the current send_to_user @@ -430,20 +432,22 @@ elif history not in histories_for_sharing[ send_to_user ]: histories_for_sharing[ send_to_user ].append( history ) return histories_for_sharing, send_to_err - def _populate_restricted( self, trans, user, histories, send_to_users, action, send_to_err ): + def _populate_restricted( self, trans, user, histories, send_to_users, action, send_to_err, unique=False ): # The user may be attempting to share histories whose datasets cannot all be accessed by other users. # If this is the case, the user sharing the histories can: # 1) action=='public': choose to make the datasets public if he is permitted to do so # 2) action=='private': automatically create a new "sharing role" allowing protected # datasets to be accessed only by the desired users - # 3) action=='share_anyway': share only what can be shared when no permissions are changed - # 4) action=='no_share': Do not share anything - # In addition, the user may be sharing a history with a user with which the history was already shared - # and it will not be shared twice. - # This method will populate the can_change, cannot_change and no_change_needed dictionaries. + # This method will populate the can_change, cannot_change and no_change_needed dictionaries, which + # are used for either displaying to the user, letting them make 1 of the choices above, or sharing + # after the user has made a choice. They will be used for display if 'unique' is True, and will look + # like: {historyX : [hda, hda], historyY : [hda] } + # For sharing, they will look like: + # { userA: {historyX : [hda, hda], historyY : [hda]}, userB: {historyY : [hda]} } can_change = {} cannot_change = {} no_change_needed = {} + unique_no_change_needed = {} for history in histories: for send_to_user in send_to_users: # Make sure the current history has not already been shared with the current send_to_user @@ -456,7 +460,15 @@ # Only deal with datasets that have not been purged for hda in history.activatable_datasets: if trans.app.security_agent.dataset_is_public( hda.dataset ): - # Build the dict that will show the user what doesn't need to be changed + # The no_change_needed dictionary is a special case. If both of can_change + # and cannot_change are empty, no_change_needed will used for sharing. Otherwise + # unique_no_change_needed will be used for displaying, so we need to populate both. + # Build the dictionaries for display, containing unique histories only + if history not in unique_no_change_needed: + unique_no_change_needed[ history ] = [ hda ] + else: + unique_no_change_needed[ history ].append( hda ) + # Build the dictionaries for sharing if send_to_user not in no_change_needed: no_change_needed[ send_to_user ] = {} if history not in no_change_needed[ send_to_user ]: @@ -473,28 +485,40 @@ # The current user has authority to change permissions on the current dataset because # they have permission to manage permissions on the dataset and the dataset is not associated # with a library. - if send_to_user not in can_change: - # Build the set of histories / datasets on which the current user has authority - # to "manage permissions". - can_change[ send_to_user ] = {} - if history not in can_change[ send_to_user ]: - can_change[ send_to_user ][ history ] = [ hda ] + if unique: + # Build the dictionaries for display, containing unique histories only + if history not in can_change: + can_change[ history ] = [ hda ] + else: + can_change[ history ].append( hda ) else: - can_change[ send_to_user ][ history ].append( hda ) + # Build the dictionaries for sharing + if send_to_user not in can_change: + can_change[ send_to_user ] = {} + if history not in can_change[ send_to_user ]: + can_change[ send_to_user ][ history ] = [ hda ] + else: + can_change[ send_to_user ][ history ].append( hda ) else: if action in [ "private", "public" ]: - # Don't change stuff that the user doesn't have permission to change + # The user has made a choice, so 'unique' doesn't apply. Don't change stuff + # that the user doesn't have permission to change continue - #elif send_to_user not in cannot_change: - if send_to_user not in cannot_change: - # Build the set of histories / datasets on which the current user does - # not have authority to "manage permissions". - cannot_change[ send_to_user ] = {} - if history not in cannot_change[ send_to_user ]: - cannot_change[ send_to_user ][ history ] = [ hda ] + if unique: + # Build the dictionaries for display, containing unique histories only + if history not in cannot_change: + cannot_change[ history ] = [ hda ] + else: + cannot_change[ history ].append( hda ) else: - cannot_change[ send_to_user ][ history ].append( hda ) - return can_change, cannot_change, no_change_needed, send_to_err + # Build the dictionaries for sharing + if send_to_user not in cannot_change: + cannot_change[ send_to_user ] = {} + if history not in cannot_change[ send_to_user ]: + cannot_change[ send_to_user ][ history ] = [ hda ] + else: + cannot_change[ send_to_user ][ history ].append( hda ) + return can_change, cannot_change, no_change_needed, unique_no_change_needed, send_to_err def _share_histories( self, trans, user, send_to_err, histories={} ): # histories looks like: { userA: [ historyX, historyY ], userB: [ historyY ] } msg = "" @@ -605,6 +629,7 @@ if clone_choice == 'activatable': new_history = history.copy( name=name, target_user=user, activatable=True ) elif clone_choice == 'active': + name += " (active items only)" new_history = history.copy( name=name, target_user=user ) # Render the list view return trans.show_ok_message( 'Clone with name "%s" is now included in your list of stored histories.' % new_history.name ) diff -r 073c9a0b2467 -r 267db48a2371 templates/history/list_shared.mako --- a/templates/history/list_shared.mako Wed Jul 08 17:01:49 2009 -0400 +++ b/templates/history/list_shared.mako Thu Jul 09 15:49:26 2009 -0400 @@ -18,7 +18,7 @@ ${history.name} <a id="shared-${i}-popup" class="popup-arrow" style="display: none;">▼</a> <div popupmenu="shared-${i}-popup"> - <a class="action-button" href="${h.url_for( controller='history', action='clone', id=trans.security.encode_id( history.id ), clone_choice='activatable' )}">Clone</a> + <a class="action-button" href="${h.url_for( controller='history', action='clone', id=trans.security.encode_id( history.id ) )}">Clone</a> </div> </td> <td>${history.user.email}</td> diff -r 073c9a0b2467 -r 267db48a2371 templates/history/share.mako --- a/templates/history/share.mako Wed Jul 08 17:01:49 2009 -0400 +++ b/templates/history/share.mako Thu Jul 09 15:49:26 2009 -0400 @@ -64,35 +64,14 @@ <input type="hidden" name="id" value="${trans.security.encode_id( history.id )}"> %endfor %if no_change_needed: + ## no_change_needed looks like: {historyX : [hda, hda], historyY : [hda] } <div style="clear: both"></div> <div class="form-row"> <div class="donemessage"> The following datasets can be shared with ${email} with no changes </div> - </div> - ## no_change_needed looks like: - ## { userA: {historyX : [hda, hda], historyY : [hda]}, userB: {historyY : [hda]} } - <% - # TODO: move generation of these unique dictionaries to the history controller - # Build the list of unique histories and datasets - unique_stuff = {} - %> - %for user, history_dict in no_change_needed.items(): - %for history, hdas in history_dict.items(): - <% - if history in unique_stuff: - for hda in hdas: - if hda not in unique_stuff[ history ]: - unique_stuff[ history ].append( hda ) - else: - unique_stuff[ history ] = [] - for hda in hdas: - if hda not in unique_stuff[ history ]: - unique_stuff[ history ].append( hda ) - %> - %endfor - %endfor - %for history, hdas in unique_stuff.items(): + </div> + %for history, hdas in no_change_needed.items(): <div class="form-row"> <label>History</label> ${history.name} @@ -104,40 +83,22 @@ %for hda in hdas: <div class="form-row"> ${hda.name} + %if hda.deleted: + (deleted) + %endif </div> %endfor %endfor %endif %if can_change: + ## can_change looks like: {historyX : [hda, hda], historyY : [hda] } <div style="clear: both"></div> <div class="form-row"> <div class="warningmessage"> The following datasets can be shared with ${email} by updating their permissions </div> </div> - ## can_change looks like: - ## { userA: {historyX : [hda, hda], historyY : [hda]}, userB: {historyY : [hda]} } - <% - # TODO: move generation of these unique dictionaries to the history controller - # Build the list of unique histories and datasets - unique_stuff = {} - %> - %for user, history_dict in can_change.items(): - %for history, hdas in history_dict.items(): - <% - if history in unique_stuff: - for hda in hdas: - if hda not in unique_stuff[ history ]: - unique_stuff[ history ].append( hda ) - else: - unique_stuff[ history ] = [] - for hda in hdas: - if hda not in unique_stuff[ history ]: - unique_stuff[ history ].append( hda ) - %> - %endfor - %endfor - %for history, hdas in unique_stuff.items(): + %for history, hdas in can_change.items(): <div class="form-row"> <label>History</label> ${history.name} @@ -149,11 +110,15 @@ %for hda in hdas: <div class="form-row"> ${hda.name} + %if hda.deleted: + (deleted) + %endif </div> %endfor %endfor %endif %if cannot_change: + ## cannot_change looks like: {historyX : [hda, hda], historyY : [hda] } <div style="clear: both"></div> <div class="form-row"> <div class="errormessage"> @@ -161,29 +126,7 @@ change the permissions on them </div> </div> - ## cannot_change looks like: - ## { userA: {historyX : [hda, hda], historyY : [hda]}, userB: {historyY : [hda]} } - <% - # TODO: move generation of these unique dictionaries to the history controller - # Build the list of unique histories and datasets - unique_stuff = {} - %> - %for user, history_dict in can_change.items(): - %for history, hdas in history_dict.items(): - <% - if history in unique_stuff: - for hda in hdas: - if hda not in unique_stuff[ history ]: - unique_stuff[ history ].append( hda ) - else: - unique_stuff[ history ] = [] - for hda in hdas: - if hda not in unique_stuff[ history ]: - unique_stuff[ history ].append( hda ) - %> - %endfor - %endfor - %for history, hdas in unique_stuff.items(): + %for history, hdas in cannot_change.items(): <div class="form-row"> <label>History</label> ${history.name} @@ -195,10 +138,17 @@ %for hda in hdas: <div class="form-row"> ${hda.name} + %if hda.deleted: + (deleted) + %endif </div> %endfor %endfor %endif + <div class="toolFormTitle"></div> + <div class="form-row"> + Deleted items can be eliminated by the users with which you are sharing the history. + </div> <div class="form-row"> <label>How would you like to proceed?</label> </div> diff -r 073c9a0b2467 -r 267db48a2371 test/functional/test_history_functions.py --- a/test/functional/test_history_functions.py Wed Jul 08 17:01:49 2009 -0400 +++ b/test/functional/test_history_functions.py Thu Jul 09 15:49:26 2009 -0400 @@ -253,7 +253,7 @@ # Check list of histories to make sure shared history3 was cloned self.view_stored_active_histories( check_str="Clone of '%s'" % history3.name ) # Switch to the cloned history to make sure activatable datasets were cloned - self.switch_history( id=self.security.encode_id( history3_clone3.id ), name=history3_clone3.name ) + self.switch_history( id=self.security.encode_id( history3_clone3.id ) ) # Make sure the deleted datasets are NOT included in the cloned history try: self.check_history_for_string( 'This dataset has been deleted.', show_deleted=True )