# HG changeset patch -- Bitbucket.org # Project galaxy-dist # URL http://bitbucket.org/galaxy/galaxy-dist/overview # User Greg Von Kuster greg@bx.psu.edu # Date 1283450357 14400 # Node ID f09915c8da9404b8642b96ee81d89afdccde8414 # Parent 121678c8a483061c531c37c8d2e398ae1baa315a Library template improvements and miscellaneous fixes:

1. Add support for handling inherited library temnplates that include all supported field types ( AddressField, CheckboxField, SelectField, TextArea, TextField, WorkflowField ) to the upload form for library datasets. New userAddress objects can now be created on the upload forms.

2. Improve the UI for selecting tempaltes for library items - it is now clear that the displayed template is not useable since it is disabled and a message is displayed.

3. Automatically inherit template contents from parent for inherited templates.

4. Fix the AddressField UI to display the required fields for rendered html.

5. Add new test_library_templates.py functional test script.

.

--- /dev/null +++ b/test/functional/test_library_templates.py @@ -0,0 +1,219 @@ +from base.twilltestcase import * +from base.test_db_util import * + +class TestLibraryFeatures( TwillTestCase ): + def test_000_initiate_users( self ): + """Ensuring all required user accounts exist""" + self.logout() + self.login( email='test1@bx.psu.edu', username='regular-user1' ) + global regular_user1 + regular_user1 = get_user( 'test1@bx.psu.edu' ) + assert regular_user1 is not None, 'Problem retrieving user with email "test1@bx.psu.edu" from the database' + global regular_user1_private_role + regular_user1_private_role = get_private_role( regular_user1 ) + self.logout() + self.login( email='test2@bx.psu.edu', username='regular-user2' ) + global regular_user2 + regular_user2 = get_user( 'test2@bx.psu.edu' ) + assert regular_user2 is not None, 'Problem retrieving user with email "test2@bx.psu.edu" from the database' + global regular_user2_private_role + regular_user2_private_role = get_private_role( regular_user2 ) + self.logout() + self.login( email='test3@bx.psu.edu', username='regular-user3' ) + global regular_user3 + regular_user3 = get_user( 'test3@bx.psu.edu' ) + assert regular_user3 is not None, 'Problem retrieving user with email "test3@bx.psu.edu" from the database' + global regular_user3_private_role + regular_user3_private_role = get_private_role( regular_user3 ) + self.logout() + self.login( email='test@bx.psu.edu', username='admin-user' ) + global admin_user + admin_user = get_user( 'test@bx.psu.edu' ) + assert admin_user is not None, 'Problem retrieving user with email "test@bx.psu.edu" from the database' + global admin_user_private_role + admin_user_private_role = get_private_role( admin_user ) + def test_005_create_library_templates( self ): + """Testing creating several LibraryInformationTemplate form definitions""" + # Logged in as admin_user + for type in [ 'AddressField', 'CheckboxField', 'SelectField', 'TextArea', 'TextField', 'WorkflowField' ]: + form_desc = '%s description' % type + # Create form for library template + self.create_single_field_type_form_definition( name=type, + desc=form_desc, + formtype=galaxy.model.FormDefinition.types.LIBRARY_INFO_TEMPLATE, + field_type=type ) + # Get all of the new form definitions for later use + global AddressField_form + AddressField_form = get_form( 'AddressField' ) + global CheckboxField_form + CheckboxField_form = get_form( 'CheckboxField' ) + global SelectField_form + SelectField_form = get_form( 'SelectField' ) + global TextArea_form + TextArea_form = get_form( 'TextArea' ) + global TextField_form + TextField_form = get_form( 'TextField' ) + global WorkflowField_form + WorkflowField_form = get_form( 'WorkflowField' ) + def test_010_create_libraries( self ): + """Testing creating a new library for each template""" + # Logged in as admin_user + for index, form in enumerate( [ AddressField_form, CheckboxField_form, SelectField_form, TextArea_form, TextField_form, WorkflowField_form ] ): + name = 'library%s' % str( index + 1 ) + description = '%s description' % name + synopsis = '%s synopsis' % name + self.create_library( name=name, description=description, synopsis=synopsis ) + # Get the libraries for later use + global library1 + library1 = get_library( 'library1', 'library1 description', 'library1 synopsis' ) + global library2 + library2 = get_library( 'library2', 'library2 description', 'library2 synopsis' ) + global library3 + library3 = get_library( 'library3', 'library3 description', 'library3 synopsis' ) + global library4 + library4 = get_library( 'library4', 'library4 description', 'library4 synopsis' ) + global library5 + library5 = get_library( 'library5', 'library5 description', 'library5 synopsis' ) + global library6 + library6 = get_library( 'library6', 'library6 description', 'library6 synopsis' ) + def test_015_add_template_to_library1( self ): + """Testing add an inheritable template containing an AddressField to library1""" + # Logged in as admin_user + # Add a template to library1 + self.add_library_template( 'library_admin', + 'library', + self.security.encode_id( library1.id ), + self.security.encode_id( AddressField_form.id ), + AddressField_form.name ) + def test_020_add_folder_to_library1( self ): + """Testing adding a root folder to library1""" + # Logged in as admin_user + # Add a root folder to library1 + folder = library1.root_folder + name = "folder" + description = "folder description" + self.add_folder( 'library_admin', + self.security.encode_id( library1.id ), + self.security.encode_id( folder.id ), + name=name, + description=description ) + global folder1 + folder1 = get_folder( folder.id, name, description ) + def test_025_check_library1( self ): + """Checking library1 and its root folder""" + # Logged in as admin_user + self.browse_library( 'library_admin', + self.security.encode_id( library1.id ), + check_str1=folder1.name, + check_str2=folder1.description ) + # Make sure the template and contents were inherited to folder1 + self.folder_info( 'library_admin', + self.security.encode_id( folder1.id ), + self.security.encode_id( library1.id ), + check_str1=AddressField_form.name, + check_str2='This is an inherited template and is not required to be used with this folder' ) + def test_030_add_dataset_to_library1_root_folder( self ): + """ + Testing adding a new library dataset to library1's root folder, and adding a new UserAddress + on the upload form. + """ + # Logged in as admin_user + # The AddressField template should be inherited to the library dataset upload form. Passing + # the value 'new' should submit the form via refresh_on_change and allow new UserAddress information + # to be posted as part of the upload. + self.add_library_dataset( 'library_admin', + '1.bed', + self.security.encode_id( library1.id ), + self.security.encode_id( library1.root_folder.id ), + library1.root_folder.name, + file_type='bed', + dbkey='hg18', + root=True, + template_refresh_field_name='field_0', + template_refresh_field_contents='new', + field_0_short_desc='Office', + field_0_name='Dick', + field_0_institution='PSU', + field_0_address='32 O Street', + field_0_city='Anywhere', + field_0_state='AK', + field_0_postal_code='0000000', + field_0_country='USA' ) + global ldda1 + ldda1 = get_latest_ldda() + assert ldda1 is not None, 'Problem retrieving LibraryDatasetDatasetAssociation ldda1 from the database' + self.browse_library( 'library_admin', + self.security.encode_id( library1.id ), + check_str1='1.bed', + check_str2=admin_user.email ) + # Make sure the library template contents were correctly saved + self.ldda_edit_info( 'library_admin', + self.security.encode_id( library1.id ), + self.security.encode_id( library1.root_folder.id ), + self.security.encode_id( ldda1.id ), + ldda1.name, + check_str1='Dick' ) + def test_035_add_template_to_library2( self ): + """ Testing add an inheritable template containing an CheckboxField to library2""" + self.add_library_template( 'library_admin', + 'library', + self.security.encode_id( library2.id ), + self.security.encode_id( CheckboxField_form.id ), + CheckboxField_form.name ) + def test_040_add_template_to_library3( self ): + """ Testing add an inheritable template containing an SelectField to library3""" + self.add_library_template( 'library_admin', + 'library', + self.security.encode_id( library3.id ), + self.security.encode_id( SelectField_form.id ), + SelectField_form.name ) + def test_045_add_template_to_library4( self ): + """ Testing add an inheritable template containing an TextArea to library4""" + # Add an inheritable template to library4 + self.add_library_template( 'library_admin', + 'library', + self.security.encode_id( library4.id ), + self.security.encode_id( TextArea_form.id ), + TextArea_form.name ) + def test_050_add_template_to_library5( self ): + """ Testing add an inheritable template containing an TextField to library5""" + # Add an inheritable template to library5 + self.add_library_template( 'library_admin', + 'library', + self.security.encode_id( library5.id ), + self.security.encode_id( TextField_form.id ), + TextField_form.name ) + def test_055_add_template_to_library6( self ): + """ Testing add an inheritable template containing an WorkflowField to library6""" + # Add an inheritable template to library6 + self.add_library_template( 'library_admin', + 'library', + self.security.encode_id( library6.id ), + self.security.encode_id( WorkflowField_form.id ), + WorkflowField_form.name ) + def test_999_reset_data_for_later_test_runs( self ): + """Reseting data to enable later test runs to pass""" + # Logged in as admin_user + ################## + # Delete all form definitions + ################## + for form in [ AddressField_form, CheckboxField_form, SelectField_form, TextArea_form, TextField_form, WorkflowField_form ]: + self.mark_form_deleted( self.security.encode_id( form.form_definition_current.id ) ) + ################## + # Purge all libraries + ################## + for library in [ library1, library2, library3, library4, library5, library6 ]: + self.delete_library_item( 'library_admin', + self.security.encode_id( library.id ), + self.security.encode_id( library.id ), + library.name, + item_type='library' ) + self.purge_library( self.security.encode_id( library.id ), library.name ) + ################## + # Make sure all users are associated only with their private roles + ################## + for user in [ admin_user, regular_user1, regular_user2, regular_user3 ]: + refresh( user ) + if len( user.roles) != 1: + raise AssertionError( '%d UserRoleAssociations are associated with %s ( should be 1 )' % ( len( user.roles ), user.email ) ) + self.logout()

--- a/lib/galaxy/web/form_builder.py +++ b/lib/galaxy/web/form_builder.py @@ -4,12 +4,19 @@ Classes for generating HTML forms

import logging,sys from cgi import escape +from galaxy.util import restore_text + log = logging.getLogger(__name__)

class BaseField(object): def get_html( self, prefix="" ): """Returns the html widget corresponding to the parameter""" raise TypeError( "Abstract Method" ) + def get_disabled_str( self, disabled=False ): + if disabled: + return 'disabled="disabled"' + else: + return '' @staticmethod def form_field_types(): return ['TextField', 'TextArea', 'SelectField', 'CheckboxField', 'AddressField', 'WorkflowField'] @@ -31,9 +38,9 @@ class TextField(BaseField): self.name = name self.size = int( size or 10 ) self.value = value or "" - def get_html( self, prefix="" ): - return '<input type="text" name="%s%s" size="%d" value="%s">' \ - % ( prefix, self.name, self.size, escape(str(self.value), quote=True) ) + def get_html( self, prefix="", disabled=False ): + return '<input type="text" name="%s%s" size="%d" value="%s" %s>' \ + % ( prefix, self.name, self.size, escape( str( self.value ), quote=True ), self.get_disabled_str( disabled ) ) def set_size(self, size): self.size = int( size )

@@ -52,7 +59,7 @@ class PasswordField(BaseField): self.value = value or "" def get_html( self, prefix="" ): return '<input type="password" name="%s%s" size="%d" value="%s">' \ - % ( prefix, self.name, self.size, escape(str(self.value), quote=True) ) + % ( prefix, self.name, self.size, escape( str( self.value ), quote=True ) ) def set_size(self, size): self.size = int( size )

@@ -71,9 +78,9 @@ class TextArea(BaseField): self.rows = int(self.size[0]) self.cols = int(self.size[-1]) self.value = value or "" - def get_html( self, prefix="" ): - return '<textarea name="%s%s" rows="%d" cols="%d">%s</textarea>' \ - % ( prefix, self.name, self.rows, self.cols, escape( str( self.value ), quote=True ) ) + def get_html( self, prefix="", disabled=False ): + return '<textarea name="%s%s" rows="%d" cols="%d" %s>%s</textarea>' \ + % ( prefix, self.name, self.rows, self.cols, self.get_disabled_str( disabled ), escape( str( self.value ), quote=True ) ) def set_size(self, rows, cols): self.rows = rows self.cols = cols @@ -90,7 +97,7 @@ class CheckboxField(BaseField): def __init__( self, name, checked=None ): self.name = name self.checked = ( checked == True ) or ( isinstance( checked, basestring ) and ( checked.lower() in ( "yes", "true", "on" ) ) ) - def get_html( self, prefix="" ): + def get_html( self, prefix="", disabled=False ): if self.checked: checked_text = "checked" else: @@ -100,8 +107,8 @@ class CheckboxField(BaseField): # parsing the request, the value 'true' in the hidden field actually means it is NOT checked. # See the is_checked() method below. The prefix is necessary in each case to ensure functional # correctness when the param is inside a conditional. - return '<input type="checkbox" name="%s%s" value="true" %s><input type="hidden" name="%s%s" value="true">' \ - % ( prefix, self.name, checked_text, prefix, self.name ) + return '<input type="checkbox" name="%s%s" value="true" %s %s><input type="hidden" name="%s%s" value="true" %s>' \ + % ( prefix, self.name, checked_text, self.get_disabled_str( disabled ), prefix, self.name, self.get_disabled_str( disabled ) ) @staticmethod def is_checked( value ): if value == True: @@ -150,7 +157,7 @@ class HiddenField(BaseField): self.name = name self.value = value or "" def get_html( self, prefix="" ): - return '<input type="hidden" name="%s%s" value="%s">' % ( prefix, self.name, escape(str(self.value), quote=True) ) + return '<input type="hidden" name="%s%s" value="%s">' % ( prefix, self.name, escape( str( self.value ), quote=True ) )

class SelectField(BaseField): """ @@ -210,14 +217,14 @@ class SelectField(BaseField): self.refresh_on_change_text = '' def add_option( self, text, value, selected = False ): self.options.append( ( text, value, selected ) ) - def get_html( self, prefix="" ): + def get_html( self, prefix="", disabled=False ): if self.display == "checkboxes": - return self.get_html_checkboxes( prefix ) + return self.get_html_checkboxes( prefix, disabled ) elif self.display == "radio": - return self.get_html_radio( prefix ) + return self.get_html_radio( prefix, disabled ) else: - return self.get_html_default( prefix ) - def get_html_checkboxes( self, prefix="" ): + return self.get_html_default( prefix, disabled ) + def get_html_checkboxes( self, prefix="", disabled=False ): rval = [] ctr = 0 if len( self.options ) > 1: @@ -227,12 +234,14 @@ class SelectField(BaseField): if len(self.options) > 2 and ctr % 2 == 1: style = " class="odd_row"" if selected: - rval.append( '<div%s><input type="checkbox" name="%s%s" value="%s" checked>%s</div>' % ( style, prefix, self.name, escape(str(value), quote=True), text) ) + rval.append( '<div%s><input type="checkbox" name="%s%s" value="%s" checked %s>%s</div>' % \ + ( style, prefix, self.name, escape( str( value ), quote=True ), self.get_disabled_str( disabled ), text ) ) else: - rval.append( '<div%s><input type="checkbox" name="%s%s" value="%s">%s</div>' % ( style, prefix, self.name, escape(str(value), quote=True), text) ) + rval.append( '<div%s><input type="checkbox" name="%s%s" value="%s" %s>%s</div>' % \ + ( style, prefix, self.name, escape( str( value ), quote=True ), self.get_disabled_str( disabled ), text ) ) ctr += 1 return "\n".join( rval ) - def get_html_radio( self, prefix="" ): + def get_html_radio( self, prefix="", disabled=False ): rval = [] ctr = 0 for text, value, selected in self.options: @@ -241,11 +250,20 @@ class SelectField(BaseField): style = " class="odd_row"" if selected: selected_text = " checked" else: selected_text = "" - rval.append( '<div%s><input type="radio" name="%s%s"%s value="%s"%s>%s</div>' % ( style, prefix, self.name, self.refresh_on_change_text, escape(str(value), quote=True), selected_text, text ) ) + rval.append( '<div%s><input type="radio" name="%s%s"%s value="%s"%s %s>%s</div>' % \ + ( style, + prefix, + self.name, + self.refresh_on_change_text, + escape( str( value ), quote=True ), + selected_text, + self.get_disabled_str( disabled ), + text ) ) ctr += 1 return "\n".join( rval ) - def get_html_default( self, prefix="" ): - if self.multiple: multiple = " multiple" + def get_html_default( self, prefix="", disabled=False ): + if self.multiple: + multiple = " multiple" else: multiple = "" rval = [] last_selected_value = "" @@ -253,11 +271,13 @@ class SelectField(BaseField): if selected: selected_text = " selected" last_selected_value = value - else: selected_text = "" - rval.append( '<option value="%s"%s>%s</option>' % ( escape(str(value), quote=True), selected_text, text ) ) + else: + selected_text = "" + rval.append( '<option value="%s"%s>%s</option>' % ( escape( str( value ), quote=True ), selected_text, text ) ) if last_selected_value: - last_selected_value = ' last_selected_value="%s"' % escape(str(last_selected_value), quote=True) - rval.insert( 0, '<select name="%s%s"%s%s%s>' % ( prefix, self.name, multiple, self.refresh_on_change_text, last_selected_value ) ) + last_selected_value = ' last_selected_value="%s"' % escape( str( last_selected_value ), quote=True ) + rval.insert( 0, '<select name="%s%s"%s%s%s %s>' % \ + ( prefix, self.name, multiple, self.refresh_on_change_text, last_selected_value, self.get_disabled_str( disabled ), ) ) rval.append( '</select>' ) return "\n".join( rval ) def get_selected(self): @@ -381,61 +401,71 @@ class DrillDownField( BaseField ): class AddressField(BaseField): @staticmethod def fields(): - return [ ( "short_desc", "Short address description"), - ( "name", "Name" ), - ( "institution", "Institution" ), - ( "address", "Address" ), - ( "city", "City" ), - ( "state", "State/Province/Region" ), - ( "postal_code", "Postal Code" ), - ( "country", "Country" ), - ( "phone", "Phone" ) ] + return [ ( "short_desc", "Short address description", "Required" ), + ( "name", "Name", "Required" ), + ( "institution", "Institution", "Required" ), + ( "address", "Address", "Required" ), + ( "city", "City", "Required" ), + ( "state", "State/Province/Region", "Required" ), + ( "postal_code", "Postal Code", "Required" ), + ( "country", "Country", "Required" ), + ( "phone", "Phone", "" ) ] def __init__(self, name, user=None, value=None, params=None): self.name = name self.user = user self.value = value self.select_address = None self.params = params - def get_html(self): - from galaxy import util + def get_html( self, disabled=False ): address_html = '' add_ids = ['none'] if self.user: for a in self.user.addresses: - add_ids.append(str(a.id)) - add_ids.append('new') - self.select_address = SelectField(self.name, - refresh_on_change=True, - refresh_on_change_values=add_ids) + add_ids.append( str( a.id ) ) + add_ids.append( 'new' ) + self.select_address = SelectField( self.name, + refresh_on_change=True, + refresh_on_change_values=add_ids ) if self.value == 'none': - self.select_address.add_option('Select one', 'none', selected=True) + self.select_address.add_option( 'Select one', 'none', selected=True ) else: - self.select_address.add_option('Select one', 'none') + self.select_address.add_option( 'Select one', 'none' ) if self.user: for a in self.user.addresses: if not a.deleted: - if self.value == str(a.id): - self.select_address.add_option(a.desc, str(a.id), selected=True) - # display this address - address_html = '''<div class="form-row"> - %s - </div>''' % a.get_html() + if self.value == str( a.id ): + self.select_address.add_option( a.desc, str( a.id ), selected=True ) + # Display this address + address_html += ''' + <div class="form-row"> + %s + </div> + ''' % a.get_html() else: - self.select_address.add_option(a.desc, str(a.id)) + self.select_address.add_option( a.desc, str( a.id ) ) if self.value == 'new': - self.select_address.add_option('Add a new address', 'new', selected=True) - for field_name, label in self.fields(): - add_field = TextField(self.name+'_'+field_name, + self.select_address.add_option( 'Add a new address', 'new', selected=True ) + for field_name, label, help_text in self.fields(): + add_field = TextField( self.name + '_' + field_name, 40, - util.restore_text( self.params.get( self.name+'_'+field_name, '' ) )) - address_html += ''' <div class="form-row"> - <label>%s</label> + restore_text( self.params.get( self.name + '_' + field_name, '' ) ) ) + address_html += ''' + <div class="form-row"> + <label>%s</label> + %s + ''' % ( label, add_field.get_html( disabled=disabled ) ) + if help_text: + address_html += ''' + <div class="toolParamHelp" style="clear: both;"> %s </div> - ''' % (label, add_field.get_html()) + ''' % help_text + address_html += ''' + </div> + ''' else: - self.select_address.add_option('Add a new address', 'new') - return self.select_address.get_html()+address_html + self.select_address.add_option( 'Add a new address', 'new' ) + return self.select_address.get_html( disabled=disabled ) + address_html

class WorkflowField(BaseField): def __init__(self, name, user=None, value=None, params=None): @@ -444,21 +474,20 @@ class WorkflowField(BaseField): self.value = value self.select_workflow = None self.params = params - def get_html(self): - self.select_workflow = SelectField(self.name) + def get_html( self, disabled=False ): + self.select_workflow = SelectField( self.name ) if self.value == 'none': - self.select_workflow.add_option('Select one', 'none', selected=True) + self.select_workflow.add_option( 'Select one', 'none', selected=True ) else: - self.select_workflow.add_option('Select one', 'none') + self.select_workflow.add_option( 'Select one', 'none' ) if self.user: for a in self.user.stored_workflows: if not a.deleted: - if str(self.value) == str(a.id): - self.select_workflow.add_option(a.name, str(a.id), selected=True) + if str( self.value ) == str( a.id ): + self.select_workflow.add_option( a.name, str( a.id ), selected=True ) else: - self.select_workflow.add_option(a.name, str(a.id)) - return self.select_workflow.get_html() - + self.select_workflow.add_option( a.name, str( a.id ) ) + return self.select_workflow.get_html( disabled=disabled )

def get_suite(): """Get unittest suite for this module"""

--- a/lib/galaxy/web/base/controller.py +++ b/lib/galaxy/web/base/controller.py @@ -313,9 +313,9 @@ class UsesFormDefinitionWidgets: return True if isinstance( field[ 'widget' ], CheckboxField ) and field[ 'widget' ].checked: return True - if isinstance( field[ 'widget' ], WorkflowField ) and field[ 'widget' ].value not in [ 'none', 'None', None ]: + if isinstance( field[ 'widget' ], WorkflowField ) and str( field[ 'widget' ].value ).lower() not in [ 'none' ]: return True - if isinstance( field[ 'widget' ], AddressField ) and field[ 'widget' ].value not in [ 'none', 'None', None ]: + if isinstance( field[ 'widget' ], AddressField ) and str( field[ 'widget' ].value ).lower() not in [ 'none' ]: return True return False def clean_field_contents( self, widgets, **kwd ): @@ -332,9 +332,24 @@ class UsesFormDefinitionWidgets: field_value = widget.value field_contents.append( util.restore_text( field_value ) ) return field_contents + def field_param_values_ok( self, index, widget_type, **kwd ): + # Make sure required fields have contents, etc + # TODO: Add support for other field types ( e.g., WorkflowField, etc ) + params = util.Params( kwd ) + if widget_type == 'AddressField': + if not util.restore_text( params.get( 'field_%i_short_desc' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_name' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_institution' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_address' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_city' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_state' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_postal_code' % index, '' ) ) \ + or not util.restore_text( params.get( 'field_%i_country' % index, '' ) ): + return False + return True def save_widget_field( self, trans, field_obj, index, **kwd ): # Save a form_builder field object - # TODO: Add support for other field types ( e.g., WorkflowField ) + # TODO: Add support for other field types ( e.g., WorkflowField, etc ) params = util.Params( kwd ) if isinstance( field_obj, trans.model.UserAddress ): field_obj.desc = util.restore_text( params.get( 'field_%i_short_desc' % index, '' ) ) @@ -1227,6 +1242,7 @@ class Admin( object ): # - Dataset where HistoryDatasetAssociation.dataset_id = Dataset.id # - UserGroupAssociation where user_id == User.id # - UserRoleAssociation where user_id == User.id EXCEPT FOR THE PRIVATE ROLE + # - UserAddress where user_id == User.id # Purging Histories and Datasets must be handled via the cleanup_datasets.py script webapp = kwd.get( 'webapp', 'galaxy' ) id = kwd.get( 'id', None ) @@ -1271,6 +1287,9 @@ class Admin( object ): for ura in user.roles: if ura.role_id != private_role.id: trans.sa_session.delete( ura ) + # Delete UserAddresses + for address in user.addresses: + trans.sa_session.delete( address ) # Purge the user user.purged = True trans.sa_session.add( user )

--- a/templates/library/common/upload.mako +++ b/templates/library/common/upload.mako @@ -12,8 +12,32 @@ %><%def name="javascripts()"> + ${h.js("jquery.autocomplete", "autocomplete_tagging" )} ${parent.javascripts()} - ${h.js("jquery.autocomplete", "autocomplete_tagging" )} + <script type="text/javascript"> + $( function() { + $( "select[refresh_on_change='true']").change( function() { + var refresh = false; + var refresh_on_change_values = $( this )[0].attributes.getNamedItem( 'refresh_on_change_values' ) + if ( refresh_on_change_values ) { + refresh_on_change_values = refresh_on_change_values.value.split( ',' ); + var last_selected_value = $( this )[0].attributes.getNamedItem( 'last_selected_value' ); + for( i= 0; i < refresh_on_change_values.length; i++ ) { + if ( $( this )[0].value == refresh_on_change_values[i] || ( last_selected_value && last_selected_value.value == refresh_on_change_values[i] ) ){ + refresh = true; + break; + } + } + } + else { + refresh = true; + } + if ( refresh ){ + $( "#upload_library_dataset" ).submit(); + } + }); + }); + </script></%def><%def name="stylesheets()">

--- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -862,7 +862,8 @@ class Library( object ): # inherited is not applicable at the library level. The get_contents # param is passed by callers that are inheriting a template - these # are usually new library datsets for which we want to include template - # fields on the upload form, but not the contents of the inherited template. + # fields on the upload form, but not necessarily the contents of the + # inherited template saved for the parent. info_association, inherited = self.get_info_association() if info_association: template = info_association.template @@ -939,7 +940,8 @@ class LibraryFolder( object ): # See if we have any associated templates. The get_contents # param is passed by callers that are inheriting a template - these # are usually new library datsets for which we want to include template - # fields on the upload form. + # fields on the upload form, but not necessarily the contents of the + # inherited template saved for the parent. info_association, inherited = self.get_info_association() if info_association: if inherited: @@ -948,7 +950,12 @@ class LibraryFolder( object ): template = info_association.template # See if we have any field contents, but only if the info_association was # not inherited ( we do not want to display the inherited contents ). - if not inherited and get_contents: + # (gvk: 8/30/10) Based on conversations with Dan, we agreed to ALWAYS inherit + # contents. We'll use this behavior until we hear from the community that + # contents should not be inherited. If we don't hear anything for a while, + # eliminate the old commented out behavior. + #if not inherited and get_contents: + if get_contents: info = info_association.info if info: return template.get_widgets( trans.user, info.content ) @@ -1160,7 +1167,8 @@ class LibraryDatasetDatasetAssociation( # See if we have any associated templatesThe get_contents # param is passed by callers that are inheriting a template - these # are usually new library datsets for which we want to include template - # fields on the upload form. + # fields on the upload form, but not necessarily the contents of the + # inherited template saved for the parent. info_association, inherited = self.get_info_association() if info_association: if inherited: @@ -1169,7 +1177,12 @@ class LibraryDatasetDatasetAssociation( template = info_association.template # See if we have any field contents, but only if the info_association was # not inherited ( we do not want to display the inherited contents ). - if not inherited and get_contents: + # (gvk: 8/30/10) Based on conversations with Dan, we agreed to ALWAYS inherit + # contents. We'll use this behavior until we hear from the community that + # contents should not be inherited. If we don't hear anything for a while, + # eliminate the old commented out behavior. + #if not inherited and get_contents: + if get_contents: info = info_association.info if info: return template.get_widgets( trans.user, info.content )

--- a/test/functional/test_user_info.py +++ b/test/functional/test_user_info.py @@ -126,7 +126,7 @@ class TestUserInfo( TwillTestCase ): self.check_page_for_string( "Manage User Information" ) self.check_page_for_string( user_info_values[0] ) self.check_page_for_string( user_info_values[1] ) - self.check_page_for_string( '<input type="checkbox" name="field_2" value="true" checked>' ) + self.check_page_for_string( '<input type="checkbox" name="field_2" value="true" checked >' ) def test_015_user_reqistration_single_user_info_forms( self ): ''' Testing user registration with a single user info form ''' # Logged in as regular_user_11 @@ -155,7 +155,7 @@ class TestUserInfo( TwillTestCase ): self.check_page_for_string( "Manage User Information" ) self.check_page_for_string( user_info_values[0] ) self.check_page_for_string( user_info_values[1] ) - self.check_page_for_string( '<input type="checkbox" name="field_2" value="true" checked>' ) + self.check_page_for_string( '<input type="checkbox" name="field_2" value="true" checked >' ) def test_020_edit_user_info( self ): """Testing editing user info as a regular user""" # Logged in as regular_user_12 @@ -179,32 +179,23 @@ class TestUserInfo( TwillTestCase ): # Test editing the user info self.edit_user_info( ['Research', 'PSU'] ) def test_999_reset_data_for_later_test_runs( self ): + """Reseting data to enable later test runs to pass""" # Logged in as regular_user_12 self.logout() self.login( email=admin_user.email ) + ################## + # Mark all forms deleted + ################## + for form_name in [ form_one_name ]: + form = get_form( form_name ) + mark_form_deleted( form ) ############### - # Mark form_one as deleted ( form_two was marked deleted earlier ) + # Purge appropriate users ############### - form_latest = get_form( form_one_name ) - mark_form_deleted( form_latest ) - ############### - # Manually delete the test_user11 - ############### - self.mark_user_deleted( user_id=self.security.encode_id( regular_user11.id ), email=regular_user11.email ) - refresh( regular_user11 ) - self.purge_user( self.security.encode_id( regular_user11.id ), regular_user11.email ) - refresh( regular_user11 ) - # We should now only the the user and his private role - delete_user_roles( regular_user11 ) - delete_obj( regular_user11 ) - ############### - # Manually delete the test_user12 - ############### - refresh( regular_user12 ) - self.mark_user_deleted( user_id=self.security.encode_id( regular_user12.id ), email=regular_user12.email ) - refresh( regular_user12 ) - self.purge_user( self.security.encode_id( regular_user12.id ), regular_user12.email ) - refresh( regular_user12 ) - # We should now only the the user and his private role - delete_user_roles( regular_user12 ) - delete_obj( regular_user12 ) + for user in [ regular_user11, regular_user12 ]: + self.mark_user_deleted( user_id=self.security.encode_id( user.id ), email=user.email ) + refresh( user ) + self.purge_user( self.security.encode_id( user.id ), user.email ) + refresh( user ) + delete_user_roles( user ) + delete_obj( user )

--- a/templates/library/common/select_template.mako +++ b/templates/library/common/select_template.mako @@ -66,13 +66,18 @@ <input type="submit" name="add_template_button" value="Add template to ${item_desc}"/></div></form> - %if template_select_list.get_selected() != ('Select one', 'none'): - <div style="clear: both"></div> + </div> +</div> +<p/> +%if template_select_list.get_selected() != ('Select one', 'none'): + <div class="toolForm"> + <div class="toolFormTitle">Layout of selected template</div> + <div class="toolFormBody"><div class="form-row"> %for i, field in enumerate( widgets ): <div class="form-row"><label>${field[ 'label' ]}</label> - ${field[ 'widget' ].get_html()} + ${field[ 'widget' ].get_html( disabled=True )} <div class="toolParamHelp" style="clear: both;"> ${field[ 'helptext' ]} </div> @@ -80,6 +85,6 @@ </div> %endfor </div> - %endif + </div></div> -</div> +%endif

--- a/test/base/twilltestcase.py +++ b/test/base/twilltestcase.py @@ -1406,6 +1406,45 @@ class TwillTestCase( unittest.TestCase ) self.check_page_for_string( desc ) self.check_page_for_string( formtype ) self.home() + # Form stuff + def create_single_field_type_form_definition( self, name, desc, formtype, field_type ): + """ + Create a new form definition containing 1 field of a specified type ( AddressField, CheckboxField, SelectField, + TextArea, TextField, WorkflowField ). The form_type param value should not be 'Sequencing Sample Form,' use + create_form() above for that. + """ + self.home() + # Create a new form definition + self.visit_url( "%s/forms/new" % self.url ) + self.check_page_for_string( 'Create a new form definition' ) + tc.fv( "1", "name", name ) + tc.fv( "1", "description", desc ) + tc.fv( "1", "form_type_selectbox", formtype ) + tc.submit( "create_form_button" ) + # Add 1 AddressField to the new form definition + field_name = 'field_name_0' + field_contents = field_type + field_help_name = 'field_helptext_0' + field_help_contents = '%s help' % field_type + field_default = 'field_default_0' + field_default_contents = '%s default contents' % field_type + tc.fv( "1", field_name, field_contents ) + tc.fv( "1", field_help_name, field_help_contents ) + self.refresh_form( 'field_type_0', field_type ) + if field_type == 'SelectField': + # Add 2 options so our select list is functional + tc.submit( "addoption_0" ) + tc.fv( "1", "field_0_option_0", "One" ) + tc.submit( "addoption_0" ) + tc.fv( "1", "field_0_option_1", "Two" ) + tc.fv( "1", field_default, field_default_contents ) + tc.submit( "save_changes_button" ) + self.home() + self.visit_url( "%s/forms/manage" % self.url ) + self.check_page_for_string( name ) + self.check_page_for_string( desc ) + self.check_page_for_string( formtype ) + self.home() def edit_form( self, form_current_id, form_name, new_form_name="Form One's Name (Renamed)", new_form_desc="This is Form One's description (Re-described)"): """ Edit form details; name & description @@ -1459,6 +1498,15 @@ class TwillTestCase( unittest.TestCase ) check_str = "The form '%s' has been updated with the changes." % form_name self.check_page_for_string( check_str ) self.home() + def mark_form_deleted( self, form_id ): + """Mark a form_definition as deleted""" + self.home() + url = "%s/forms/manage?operation=delete&id=%s" % ( self.url, form_id ) + self.visit_url( url ) + check_str = "1 form(s) is deleted." + self.check_page_for_string( check_str ) + self.home() + # Requests stuff def check_request_grid(self, state, request_name, deleted=False): self.home() @@ -1603,7 +1651,11 @@ class TwillTestCase( unittest.TestCase )

# Library stuff def add_library_template( self, cntrller, item_type, library_id, form_id, form_name, folder_id=None, ldda_id=None ): - """Add a new info template to a library item""" + """ + Add a new info template to a library item - the template will ALWAYS BE SET TO INHERITABLE here. If you want to + dis-inherit your template, call the manage_library_template_inheritance() below immediately after you call this + method in your test code. + """ self.home() if item_type == 'library': url = "%s/library_common/add_template?cntrller=%s&item_type=%s&library_id=%s" % \ @@ -1616,11 +1668,31 @@ class TwillTestCase( unittest.TestCase ) ( self.url, cntrller, item_type, library_id, folder_id, ldda_id ) self.visit_url( url ) self.check_page_for_string ( "Select a template for the" ) - tc.fv( '1', 'form_id', form_id ) - tc.fv( '1', 'inherit', '1' ) - tc.submit( 'add_template_button' ) + self.refresh_form( "form_id", form_id ) + # For some unknown reason, twill barfs if the form number ( 1 ) is used in the following + # rather than the form anme ( select_template ), so we have to use the form name. + tc.fv( "select_template", "inheritable", '1' ) + tc.submit( "add_template_button" ) self.check_page_for_string = 'A template based on the form "%s" has been added to this' % form_name self.home() + def manage_library_template_inheritance( self, cntrller, item_type, library_id, folder_id=None, ldda_id=None, inheritable=True ): + # If inheritable is True, the item is currently inheritable. + self.home() + if item_type == 'library': + url = "%s/library_common/manage_template_inheritance?cntrller=%s&item_type=%s&library_id=%s" % \ + ( self.url, cntrller, item_type, library_id ) + elif item_type == 'folder': + url = "%s/library_common/manage_template_inheritance?cntrller=%s&item_type=%s&library_id=%s&folder_id=%s" % \ + ( self.url, cntrller, item_type, library_id, folder_id ) + elif item_type == 'ldda': + url = "%s/library_common/manage_template_inheritance?cntrller=%s&item_type=%s&library_id=%s&folder_id=%s&ldda_id=%s" % \ + ( self.url, cntrller, item_type, library_id, folder_id, ldda_id ) + self.visit_url( url ) + if inheritable: + self.check_page_for_string = 'will no longer be inherited to contained folders and datasets' + else: + self.check_page_for_string = 'will now be inherited to contained folders and datasets' + self.home() def browse_libraries_admin( self, deleted=False, check_str1='', check_str2='', not_displayed1='' ): self.visit_url( '%s/library_admin/browse_libraries?sort=name&f-description=All&f-name=All&f-deleted=%s' % ( self.url, str( deleted ) ) ) if check_str1: @@ -1718,13 +1790,14 @@ class TwillTestCase( unittest.TestCase ) self.home()

# Library folder stuff - def add_folder( self, controller, library_id, folder_id, name='Folder One', description='This is Folder One' ): + def add_folder( self, cntrller, library_id, folder_id, name='Folder One', description='This is Folder One' ): """Create a new folder""" self.home() - self.visit_url( "%s/library_common/create_folder?cntrller=%s&library_id=%s&parent_id=%s" % ( self.url, controller, library_id, folder_id ) ) + url = "%s/library_common/create_folder?cntrller=%s&library_id=%s&parent_id=%s" % ( self.url, cntrller, library_id, folder_id ) + self.visit_url( url ) self.check_page_for_string( 'Create a new folder' ) - tc.fv( "1", "name", name ) # form field 1 is the field named name... - tc.fv( "1", "description", description ) # form field 2 is the field named description... + tc.fv( "1", "name", name ) + tc.fv( "1", "description", description ) tc.submit( "new_folder_button" ) check_str = "The new folder named '%s' has been added to the data library." % name self.check_page_for_string( check_str ) @@ -1765,14 +1838,32 @@ class TwillTestCase( unittest.TestCase ) # Library dataset stuff def add_library_dataset( self, cntrller, filename, library_id, folder_id, folder_name, file_type='auto', dbkey='hg18', roles=[], message='', root=False, - template_field_name1='', template_field_contents1='', show_deleted='False', - upload_option='upload_file' ): + template_field_name1='', template_field_contents1='', + template_refresh_field_name='', template_refresh_field_contents='', + field_0_short_desc='', field_0_name='', field_0_institution='', + field_0_address='', field_0_city='', field_0_state='', field_0_postal_code='', + field_0_country='', show_deleted='False', upload_option='upload_file' ): """Add a dataset to a folder""" filename = self.get_filename( filename ) self.home() self.visit_url( "%s/library_common/upload_library_dataset?cntrller=%s&library_id=%s&folder_id=%s&upload_option=%s&message=%s" % \ ( self.url, cntrller, library_id, folder_id, upload_option, message.replace( ' ', '+' ) ) ) self.check_page_for_string( 'Upload files' ) + # A template containing an AddressField may be displayed on the upload form. + # If this is the case, we need to refresh the form with the passeed tmplate_field_name1. + if template_refresh_field_name and template_refresh_field_contents: + self.refresh_form( template_refresh_field_name, template_refresh_field_contents ) + tc.fv( "1", "field_0_short_desc", field_0_short_desc ) + tc.fv( "1", "field_0_name", field_0_name ) + tc.fv( "1", "field_0_institution", field_0_institution ) + tc.fv( "1", "field_0_address", field_0_address ) + tc.fv( "1", "field_0_city", field_0_city ) + tc.fv( "1", "field_0_state", field_0_state ) + tc.fv( "1", "field_0_postal_code", field_0_postal_code ) + tc.fv( "1", "field_0_country", field_0_country ) + # Add template field contents, if any... + if template_field_name1: + tc.fv( "1", template_field_name1, template_field_contents1 ) tc.fv( "1", "library_id", library_id ) tc.fv( "1", "folder_id", folder_id ) tc.fv( "1", "show_deleted", show_deleted ) @@ -1782,9 +1873,6 @@ class TwillTestCase( unittest.TestCase ) tc.fv( "1", "message", message.replace( '+', ' ' ) ) for role_id in roles: tc.fv( "1", "roles", role_id ) - # Add template field contents, if any... - if template_field_name1: - tc.fv( "1", template_field_name1, template_field_contents1 ) tc.submit( "runtool_btn" ) if root: check_str = "Added 1 datasets to the library '%s' (each is selected)." % folder_name

--- a/templates/library/common/common.mako +++ b/templates/library/common/common.mako @@ -22,7 +22,7 @@ has_contents = True label = field[ 'label' ] value = 'checked' - elif isinstance( field[ 'widget' ], WorkflowField ) and field[ 'widget' ].value not in [ 'none', 'None', None ]: + elif isinstance( field[ 'widget' ], WorkflowField ) and str( field[ 'widget' ].value ).lower() not in [ 'none' ]: has_contents = True label = field[ 'label' ] widget = field[ 'widget' ] @@ -35,7 +35,7 @@ else: # If we didn't find the selected workflow option above, we'll just print the value value = field[ 'widget' ].value - elif isinstance( field[ 'widget' ], AddressField ) and field[ 'widget' ].value not in [ 'none', 'None', None ]: + elif isinstance( field[ 'widget' ], AddressField ) and str( field[ 'widget' ].value ).lower() not in [ 'none' ]: has_contents = True widget = field[ 'widget' ] address = trans.sa_session.query( trans.model.UserAddress ).get( int( widget.value ) ) @@ -69,71 +69,77 @@ else: can_modify = False %> - %if widgets: - %if editable and can_modify: - <p/> - <div class="toolForm"> - <div class="toolFormTitle"> - %if inherited: - Other information <i>- this is an inherited template and is not required to be used with this ${item_type}</i> - %else: - Other information - %endif - %if info_association and not inherited and can_modify: + %if editable and can_modify: + <p/> + <div class="toolForm"> + <div class="toolFormTitle">Other information + <a id="item-${item.id}-popup" class="popup-arrow" style="display: none;">&#9660;</a> + <div popupmenu="item-${item.id}-popup"> + %if info_association and inherited and can_modify: ## "inherited" will be true only if the info_association is not associated with the current item, - ## in which case we do not want to render the following popup menu. - <a id="item-${item.id}-popup" class="popup-arrow" style="display: none;">&#9660;</a> - <div popupmenu="item-${item.id}-popup"> - <a class="action-button" href="${h.url_for( controller='library_common', action='edit_template', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Edit template</a> - <a class="action-button" href="${h.url_for( controller='library_common', action='delete_template', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Delete template</a> - %if item_type not in [ 'ldda', 'library_dataset' ]: - %if info_association.inheritable: - <a class="action-button" href="${h.url_for( controller='library_common', action='manage_template_inheritance', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Dis-inherit template</a> - %else: - <a class="action-button" href="${h.url_for( controller='library_common', action='manage_template_inheritance', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Inherit template</a> - %endif + ## which means that the currently display template has not yet been saved for the current item. + <a class="action-button" href="${h.url_for( controller='library_common', action='add_template', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Select a different template</a> + %elif info_association and not inherited and can_modify: + <a class="action-button" href="${h.url_for( controller='library_common', action='edit_template', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Edit template</a> + <a class="action-button" href="${h.url_for( controller='library_common', action='delete_template', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Delete template</a> + %if item_type not in [ 'ldda', 'library_dataset' ]: + %if info_association.inheritable: + <a class="action-button" href="${h.url_for( controller='library_common', action='manage_template_inheritance', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Dis-inherit template</a> + %else: + <a class="action-button" href="${h.url_for( controller='library_common', action='manage_template_inheritance', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}">Inherit template</a> %endif - </div> + %endif %endif </div> - <div class="toolFormBody"> - <form name="edit_info" id="edit_info" action="${h.url_for( controller='library_common', action='edit_template_info', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}" method="post"> - %for i, field in enumerate( widgets ): - <div class="form-row"> - <label>${field[ 'label' ]}</label> - ${field[ 'widget' ].get_html()} - <div class="toolParamHelp" style="clear: both;"> - ${field[ 'helptext' ]} - </div> - <div style="clear: both"></div> + </div> + <div class="toolFormBody"> + %if inherited: + <div class="form-row"> + <font color="red"> + <b> + This is an inherited template and is not required to be used with this ${item_type}. You can + <a href="${h.url_for( controller='library_common', action='add_template', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}"><font color="red">select a different template</font></a> + or fill in the desired fields and save this one. This template will not be assocaiated with this ${item_type} until you click the Save button. + </b> + </font> + </div> + %endif + <form name="edit_info" id="edit_info" action="${h.url_for( controller='library_common', action='edit_template_info', cntrller=cntrller, item_type=item_type, library_id=library_id, folder_id=folder_id, ldda_id=ldda_id, show_deleted=show_deleted )}" method="post"> + %for i, field in enumerate( widgets ): + <div class="form-row"> + <label>${field[ 'label' ]}</label> + ${field[ 'widget' ].get_html()} + <div class="toolParamHelp" style="clear: both;"> + ${field[ 'helptext' ]} </div> - %endfor - <div class="form-row"> - <input type="submit" name="edit_info_button" value="Save"/> + <div style="clear: both"></div></div> - </form> - </div> + %endfor + <div class="form-row"> + <input type="submit" name="edit_info_button" value="Save"/> + </div> + </form></div> - <p/> - %elif widget_fields_have_contents: - <p/> - <div class="toolForm"> - <div class="toolFormTitle">Other information about ${item.name}</div> - <div class="toolFormBody"> - %for i, field in enumerate( widgets ): - ${render_template_field( field )} - %endfor - </div> + </div> + <p/> + %elif widget_fields_have_contents: + <p/> + <div class="toolForm"> + <div class="toolFormTitle">Other information about ${item.name}</div> + <div class="toolFormBody"> + %for i, field in enumerate( widgets ): + ${render_template_field( field )} + %endfor </div> - <p/> - %endif + </div> + <p/> %endif </%def><%def name="render_upload_form( cntrller, upload_option, action, library_id, folder_id, replace_dataset, file_formats, dbkeys, widgets, roles, history, show_deleted )"><% import os, os.path %> %if upload_option in [ 'upload_file', 'upload_directory', 'upload_paths' ]: - <div class="toolForm" id="upload_library_dataset"> + <div class="toolForm" id="upload_library_dataset_tool_form"> %if upload_option == 'upload_directory': <div class="toolFormTitle">Upload a directory of files</div> %elif upload_option == 'upload_paths': @@ -142,7 +148,7 @@ <div class="toolFormTitle">Upload files</div> %endif <div class="toolFormBody"> - <form name="upload_library_dataset" action="${action}" enctype="multipart/form-data" method="post"> + <form name="upload_library_dataset" id="upload_library_dataset" action="${action}" enctype="multipart/form-data" method="post"><input type="hidden" name="tool_id" value="upload1"/><input type="hidden" name="tool_state" value="None"/><input type="hidden" name="cntrller" value="${cntrller}"/>

--- a/lib/galaxy/web/controllers/library_common.py +++ b/lib/galaxy/web/controllers/library_common.py @@ -7,6 +7,7 @@ from galaxy.util.json import to_json_str from galaxy.tools.actions import upload_common from galaxy.model.orm import * from galaxy.util.streamball import StreamBall +from galaxy.web.form_builder import AddressField, CheckboxField, SelectField, TextArea, TextField, WorkflowField import logging, tempfile, zipfile, tarfile, os, sys

if sys.version_info[:2] < ( 2, 6 ): @@ -943,7 +944,7 @@ class LibraryCommon( BaseController, Use # Check to see if the user selected roles to associate with the DATASET_ACCESS permission # on the dataset that would cause accessibility issues. roles = params.get( 'roles', False ) - error = None + error = False if upload_option == 'upload_paths' and not trans.app.config.allow_library_path_paste: error = True message = '"allow_library_path_paste" is not defined in the Galaxy configuration file' @@ -966,11 +967,41 @@ class LibraryCommon( BaseController, Use status='error' ) )

else: - # See if we have any inherited templates, but do not inherit contents. + # See if we have any inherited templates. info_association, inherited = folder.get_info_association( inherited=True ) if info_association and info_association.inheritable: template_id = str( info_association.template.id ) - widgets = folder.get_template_widgets( trans, get_contents=False ) + widgets = folder.get_template_widgets( trans, get_contents=True ) + processed_widgets = [] + # The list of widgets may include an AddressField which we need to save if it is new + for index, widget_dict in enumerate( widgets ): + widget = widget_dict[ 'widget' ] + if isinstance( widget, AddressField ): + value = util.restore_text( params.get( 'field_%i' % index, '' ) ) + if value == 'new': + if self.field_param_values_ok( index, 'AddressField', **kwd ): + # Save the new address + address = trans.app.model.UserAddress( user=trans.user ) + self.save_widget_field( trans, address, index, **kwd ) + widget.value = str( address.id ) + widget_dict[ 'widget' ] = widget + processed_widgets.append( widget_dict ) + # FIXME: ( hack ) It is now critical to update the value of 'field_%i', replacing the string + # 'new' with the new address id. This is necessary because the upload_dataset() + # method below calls the handle_library_params() method, which does not parse the + # widget fields, it instead pulls form values from kwd. See the FIXME comments in the + # handle_library_params() method... + kwd[ 'field_%i' % index ] = str( address.id ) + else: + # The invalid address won't be saved, but we cannot dispaly error + # messages on the upload form due to the ajax upload already occurring. + # When we re-engineer the upload process ( currently under way ), we + # will be able to check the form values before the ajax upload occurs + # in the background. For now, we'll do nothing... + pass + else: + processed_widgets.append( widget_dict ) + widgets = processed_widgets else: template_id = 'None' widgets = [] @@ -1039,10 +1070,34 @@ class LibraryCommon( BaseController, Use show_deleted=show_deleted, message=util.sanitize_text( message ), status=status ) ) - # See if we have any inherited templates, but do not inherit contents. + # Note: if the upload form was submitted due to refresh_on_demand for a form field, we cannot re-populate + # the field for the selected file ( files_0|file_data ) if the user selected one. This is because the value + # attribute of the html input file type field is typically ignored by browsers as a security precaution. + + # See if we have any inherited templates. info_association, inherited = folder.get_info_association( inherited=True ) if info_association and info_association.inheritable: - widgets = folder.get_template_widgets( trans, get_contents=False ) + widgets = folder.get_template_widgets( trans, get_contents=True ) + # Handle form submission via refresh_on_change by keeping the contents of widget fields + populated_widgets = [] + for index, widget_dict in enumerate( widgets ): + widget = widget_dict[ 'widget' ] + if isinstance( widget, AddressField ): + value = util.restore_text( params.get( 'field_%i' % index, '' ) ) + if value: + if value == 'new': + # Adding a new address + widget.value = value + widget_dict[ 'widget' ] = widget + elif value == 'none': + widget.value = '' + widget_dict[ 'widget' ] = widget + else: + # An existing address object was selected + address_obj = trans.sa_session.query( trans.app.model.UserAddress ).get( int( value ) ) + widget_dict[ 'widget' ] = address_obj + populated_widgets.append( widget_dict ) + widgets = populated_widgets else: widgets = [] upload_option = params.get( 'upload_option', 'upload_file' ) @@ -1132,6 +1187,8 @@ class LibraryCommon( BaseController, Use message = '"allow_library_path_paste" is not defined in the Galaxy configuration file' # Some error handling should be added to this method. try: + # FIXME: instead of passing params here ( chiech have been process by util.Params(), the original kwd + # should be passed so that complex objects that may have been included in the initial request remain. library_bunch = upload_common.handle_library_params( trans, params, folder_id, replace_dataset ) except: response_code = 500 @@ -2174,10 +2231,23 @@ class LibraryCommon( BaseController, Use value = util.restore_text( params.get( 'field_%i' % index, '' ) ) if value == 'new': if params.get( 'edit_info_button', False ): - # Save the new address - address = trans.app.model.UserAddress( user=trans.user ) - self.save_widget_field( trans, address, index, **kwd ) - widget.value = str( address.id ) + if self.field_param_values_ok( index, 'AddressField', **kwd ): + # Save the new address + address = trans.app.model.UserAddress( user=trans.user ) + self.save_widget_field( trans, address, index, **kwd ) + widget.value = str( address.id ) + else: + message = 'Required fields are missing contents.' + return trans.response.send_redirect( web.url_for( controller='library_common', + action=action, + cntrller=cntrller, + use_panels=use_panels, + library_id=library_id, + folder_id=folder_id, + id=id, + show_deleted=show_deleted, + message=util.sanitize_text( message ), + status='error' ) ) else: # Form was submitted via refresh_on_change widget.value = 'new'

--- a/templates/library/common/library_dataset_info.mako +++ b/templates/library/common/library_dataset_info.mako @@ -90,6 +90,5 @@ %endif

%if widgets: - ## Templates are not currently supported for library_datasets, only the associated ldda. ${render_template_fields( cntrller, 'library_dataset', library_id, widgets, widget_fields_have_contents, info_association=None, inherited=False, editable=False )} %endif

--- a/lib/galaxy/tools/actions/upload_common.py +++ b/lib/galaxy/tools/actions/upload_common.py @@ -35,6 +35,9 @@ def persist_uploads( params ): params['files'] = new_files return params def handle_library_params( trans, params, folder_id, replace_dataset=None ): + # FIXME: the received params has already been parsed by util.Params() by the time it reaches here, + # so no complex objects remain. This is not good because it does not allow for those objects to be + # manipulated here. The receivd params should be the original kwd from the initial request. library_bunch = util.bunch.Bunch() library_bunch.replace_dataset = replace_dataset library_bunch.message = params.get( 'message', '' ) @@ -42,9 +45,8 @@ def handle_library_params( trans, params library_bunch.template_field_contents = [] template_id = params.get( 'template_id', None ) library_bunch.folder = trans.sa_session.query( trans.app.model.LibraryFolder ).get( trans.security.decode_id( folder_id ) ) - # We are inheriting the folder's info_association, so we did not - # receive any inherited contents, but we may have redirected here - # after the user entered template contents ( due to errors ). + # We are inheriting the folder's info_association, so we may have received inherited contents or we may have redirected + # here after the user entered template contents ( due to errors ). if template_id not in [ None, 'None' ]: library_bunch.template = trans.sa_session.query( trans.app.model.FormDefinition ).get( template_id ) for field_index in range( len( library_bunch.template.fields ) ):

--- a/lib/galaxy/web/controllers/tool_runner.py +++ b/lib/galaxy/web/controllers/tool_runner.py @@ -179,6 +179,8 @@ class ToolRunner( BaseController ): replace_dataset = trans.sa_session.query( trans.app.model.LibraryDataset ).get( trans.security.decode_id( replace_id ) ) else: replace_dataset = None + # FIXME: instead of passing params here ( chiech have been process by util.Params(), the original kwd + # should be passed so that complex objects that may have been included in the initial request remain. library_bunch = upload_common.handle_library_params( trans, nonfile_params, nonfile_params.folder_id, replace_dataset ) else: library_bunch = None