details: http://www.bx.psu.edu/hg/galaxy/rev/a2dee187b2b6 changeset: 2990:a2dee187b2b6 user: rc date: Mon Nov 09 16:15:52 2009 -0500 description: Fixed a checkbox bug in request & user_info Resolved bitbucket issue 221: now there is a reject option for an admin to move a request to an unsubmitted state diffstat: lib/galaxy/model/__init__.py | 6 +- lib/galaxy/web/controllers/admin.py | 3 +- lib/galaxy/web/controllers/requests.py | 6 +- lib/galaxy/web/controllers/requests_admin.py | 32 ++++++++++- lib/galaxy/web/controllers/user.py | 42 +++++++++++--- lib/galaxy/web/form_builder.py | 8 ++ templates/admin/requests/show_request.mako | 13 +++- templates/requests/show_request.mako | 2 +- templates/user/address.mako | 80 -------------------------- templates/user/info.mako | 46 ++++++++------ test/base/twilltestcase.py | 4 + test/functional/test_forms_and_requests.py | 12 ++++ test/functional/test_user_info.py | 29 ++++++--- 13 files changed, 154 insertions(+), 129 deletions(-) diffs (579 lines): diff -r 1612a5f4ed62 -r a2dee187b2b6 lib/galaxy/model/__init__.py --- a/lib/galaxy/model/__init__.py Mon Nov 09 15:50:12 2009 -0500 +++ b/lib/galaxy/model/__init__.py Mon Nov 09 16:15:52 2009 -0500 @@ -1170,7 +1170,7 @@ else: field_widget.add_option( option, option ) elif field[ 'type' ] == 'CheckboxField': - field_widget.checked = value + field_widget.set_checked( value ) if field[ 'required' ] == 'required': req = 'Required' else: @@ -1231,7 +1231,9 @@ self.values = form_values self.bar_code = bar_code def current_state(self): - return self.events[0].state + if self.events: + return self.events[0].state + return None class SampleState( object ): def __init__(self, name=None, desc=None, request_type=None): diff -r 1612a5f4ed62 -r a2dee187b2b6 lib/galaxy/web/controllers/admin.py --- a/lib/galaxy/web/controllers/admin.py Mon Nov 09 15:50:12 2009 -0500 +++ b/lib/galaxy/web/controllers/admin.py Mon Nov 09 16:15:52 2009 -0500 @@ -760,7 +760,6 @@ information, public username, reset password & other user information obtained during registration ''' - print 'KWD', kwd user_id = kwd.get( 'id', None ) if not user_id: message += "Invalid user id (%s) received" % str( user_id ) @@ -772,7 +771,7 @@ return trans.response.send_redirect( web.url_for( controller='user', action='show_info', user_id=user.id, - admin_view=True ) ) + admin_view=True, **kwd ) ) @web.expose @web.require_admin def user( self, trans, **kwd ): diff -r 1612a5f4ed62 -r a2dee187b2b6 lib/galaxy/web/controllers/requests.py --- a/lib/galaxy/web/controllers/requests.py Mon Nov 09 15:50:12 2009 -0500 +++ b/lib/galaxy/web/controllers/requests.py Mon Nov 09 16:15:52 2009 -0500 @@ -85,7 +85,7 @@ return self.__show_request(trans, id, kwargs.get('add_sample', False)) elif operation == "submit": id = trans.security.decode_id(kwargs['id']) - return self.__submit(trans, id) + return self.__submit_request(trans, id) elif operation == "delete": id = trans.security.decode_id(kwargs['id']) return self.__delete_request(trans, id) @@ -642,6 +642,8 @@ values.append('') else: values.append(int(value)) + elif field['type'] == 'CheckboxField': + values.append(CheckboxField.is_checked( params.get('field_%i' % index, '') )) else: values.append(util.restore_text(params.get('field_%i' % index, ''))) form_values = trans.app.model.FormValues(request_type.request_form, values) @@ -799,7 +801,7 @@ status='done', message='The request <b>%s</b> has been undeleted.' % request.name, **kwd) ) - def __submit(self, trans, id): + def __submit_request(self, trans, id): try: request = trans.sa_session.query( trans.app.model.Request ).get( id ) except: diff -r 1612a5f4ed62 -r a2dee187b2b6 lib/galaxy/web/controllers/requests_admin.py --- a/lib/galaxy/web/controllers/requests_admin.py Mon Nov 09 15:50:12 2009 -0500 +++ b/lib/galaxy/web/controllers/requests_admin.py Mon Nov 09 16:15:52 2009 -0500 @@ -37,6 +37,7 @@ operations = [ grids.GridOperation( "Submit", allow_multiple=False, condition=( lambda item: not item.deleted and item.unsubmitted() and item.samples ) ), grids.GridOperation( "Edit", allow_multiple=False, condition=( lambda item: not item.deleted ) ), + grids.GridOperation( "Reject", allow_multiple=False, condition=( lambda item: not item.deleted and item.submitted() ) ), grids.GridOperation( "Delete", allow_multiple=False, condition=( lambda item: not item.deleted and item.unsubmitted() ) ), grids.GridOperation( "Undelete", condition=( lambda item: item.deleted ) ), ] @@ -94,7 +95,7 @@ return self.__show_request(trans, id, status, message) elif operation == "submit": id = trans.security.decode_id(kwargs['id']) - return self.__submit(trans, id) + return self.__submit_request(trans, id) elif operation == "edit": id = trans.security.decode_id(kwargs['id']) return self.__edit_request(trans, id) @@ -104,6 +105,9 @@ elif operation == "undelete": id = trans.security.decode_id(kwargs['id']) return self.__undelete_request(trans, id) + elif operation == "reject": + id = trans.security.decode_id(kwargs['id']) + return self.__reject_request(trans, id) if 'show_filter' in kwargs.keys(): if kwargs['show_filter'] == 'All': self.request_grid.default_filter = {} @@ -226,7 +230,7 @@ status='done', message='The request <b>%s</b> has been undeleted.' % request.name, **kwd) ) - def __submit(self, trans, id): + def __submit_request(self, trans, id): try: request = trans.sa_session.query( trans.app.model.Request ).get( id ) except: @@ -582,6 +586,8 @@ values.append('') else: values.append(int(value)) + elif field['type'] == 'CheckboxField': + values.append(CheckboxField.is_checked( params.get('field_%i' % index, '') )) else: values.append(util.restore_text(params.get('field_%i' % index, ''))) form_values = trans.app.model.FormValues(request_type.request_form, values) @@ -690,6 +696,28 @@ action='list', show_filter=trans.app.model.Request.states.SUBMITTED, **kwd) ) + def __reject_request(self, trans, id): + try: + request = trans.sa_session.query( trans.app.model.Request ).get( id ) + except: + msg = "Invalid request ID" + log.warn( msg ) + return trans.response.send_redirect( web.url_for( controller='requests_admin', + action='list', + status='error', + message=msg, + **kwd) ) + # change request's submitted field + request.state = request.states.UNSUBMITTED + request.flush() + kwd = {} + kwd['id'] = trans.security.encode_id(request.id) + kwd['status'] = 'done' + kwd['message'] = 'The request <b>%s</b> is now unsubmitted.' % request.name + return trans.response.send_redirect( web.url_for( controller='requests_admin', + action='list', + show_filter=trans.app.model.Request.states.UNSUBMITTED, + **kwd) ) def __update_samples(self, request, **kwd): ''' This method retrieves all the user entered sample information and diff -r 1612a5f4ed62 -r a2dee187b2b6 lib/galaxy/web/controllers/user.py --- a/lib/galaxy/web/controllers/user.py Mon Nov 09 15:50:12 2009 -0500 +++ b/lib/galaxy/web/controllers/user.py Mon Nov 09 16:15:52 2009 -0500 @@ -168,7 +168,7 @@ username = util.restore_text( params.get('username', '') ) password = util.restore_text( params.get('password', '') ) confirm = util.restore_text( params.get('confirm', '') ) - subscribe = params.get('subscribe', False) + subscribe = CheckboxField.is_checked( params.get('subscribe', '') ) admin_view = params.get('admin_view', 'False') msg = util.restore_text( params.get( 'msg', '' ) ) messagetype = params.get( 'messagetype', 'done' ) @@ -241,10 +241,10 @@ # get all the user information forms user_info_forms = get_all_forms( trans, filter=dict(deleted=False), form_type=trans.app.model.FormDefinition.types.USER_INFO ) - # if there are no user forms available then there is nothing to save - if not len( user_info_forms ): - return if new_user: + # if there are no user forms available then there is nothing to save + if not len( user_info_forms ): + return user_info_type = params.get( 'user_info_select', 'none' ) try: user_info_form = trans.sa_session.query( trans.app.model.FormDefinition ).get(int(user_info_type)) @@ -254,7 +254,25 @@ msg='Invalid user information form id', messagetype='error') ) else: - user_info_form = user.values.form_definition + if user.values: + user_info_form = user.values.form_definition + else: + # user was created before any of the user_info forms were created + if len(user_info_forms) > 1: + # when there are multiple user_info forms and the user or admin + # can change the user_info form + user_info_type = params.get( 'user_info_select', 'none' ) + try: + user_info_form = trans.sa_session.query( trans.app.model.FormDefinition ).get(int(user_info_type)) + except: + return trans.response.send_redirect( web.url_for( controller='user', + action=action, + msg='Invalid user information form id', + messagetype='error') ) + else: + # when there is only one user_info form then there is no way + # to change the user_info form + user_info_form = user_info_forms[0] values = [] for index, field in enumerate(user_info_form.fields): if field['type'] == 'AddressField': @@ -278,13 +296,17 @@ values.append('') else: values.append(int(value)) + elif field['type'] == 'CheckboxField': + values.append(CheckboxField.is_checked( params.get('field_%i' % index, '') )) else: values.append(util.restore_text(params.get('field_%i' % index, ''))) - if new_user: + if new_user or not user.values: + # new user or existing form_values = trans.app.model.FormValues(user_info_form, values) form_values.flush() user.values = form_values - else: # editing the user info of an existing user + elif user.values: + # editing the user info of an existing user with existing user info user.values.content = values user.values.flush() user.flush() @@ -340,7 +362,7 @@ if user.values: selected_user_form_id = user.values.form_definition.id else: - selected_user_form_id = 'none' + selected_user_form_id = params.get( 'user_info_select', 'none' ) else: selected_user_form_id = params.get( 'user_info_select', 'none' ) # when there are more than one user information forms then show a select box @@ -428,10 +450,12 @@ addresses = [address for address in user.addresses if address.deleted] else: addresses = [address for address in user.addresses if not address.deleted] + user_info_forms = get_all_forms( trans, filter=dict(deleted=False), + form_type=trans.app.model.FormDefinition.types.USER_INFO ) return trans.fill_template( '/user/info.mako', user=user, admin_view=admin_view, user_info_select=user_info_select, user_info_form=user_info_form, widgets=widgets, - login_info=login_info, + login_info=login_info, user_info_forms=user_info_forms, addresses=addresses, show_filter=show_filter, msg=msg, messagetype=messagetype) @web.expose diff -r 1612a5f4ed62 -r a2dee187b2b6 lib/galaxy/web/form_builder.py --- a/lib/galaxy/web/form_builder.py Mon Nov 09 15:50:12 2009 -0500 +++ b/lib/galaxy/web/form_builder.py Mon Nov 09 16:15:52 2009 -0500 @@ -118,6 +118,14 @@ return True else: return False + def set_checked(self, value): + if type(value) == type('a'): + if value.lower() in [ "yes", "true", "on" ]: + self.checked = True + else: + self.checked = False + else: + self.checked = value class FileField(BaseField): """ diff -r 1612a5f4ed62 -r a2dee187b2b6 templates/admin/requests/show_request.mako --- a/templates/admin/requests/show_request.mako Mon Nov 09 15:50:12 2009 -0500 +++ b/templates/admin/requests/show_request.mako Mon Nov 09 16:15:52 2009 -0500 @@ -14,16 +14,23 @@ <ul class="manage-table-actions"> %if request.unsubmitted() and request.samples: <li> - <a class="action-button" confirm="More samples cannot be added to this request once it is submitted. Click OK to submit." href="${h.url_for( controller='requests_admin', action='submit_request', id=request.id)}"> + <a class="action-button" confirm="More samples cannot be added to this request once it is submitted. Click OK to submit." href="${h.url_for( controller='requests_admin', action='list', operation='Submit', id=trans.security.encode_id(request.id) )}"> <span>Submit request</span></a> </li> %endif + %if request.submitted(): + <li> + <a class="action-button" href="${h.url_for( controller='requests_admin', action='list', operation='Reject', id=trans.security.encode_id(request.id))}"> + <span>Reject request</span></a> + </li> + %endif %if request.submitted() and request.samples: <li> <a class="action-button" href="${h.url_for( controller='requests_admin', action='bar_codes', request_id=request.id)}"> <span>Bar codes</span></a> </li> %endif + </ul> @@ -76,7 +83,9 @@ <td> %if sample.request.unsubmitted(): Unsubmitted - %else: + %elif not sample.current_state(): + New + %else: <a href="${h.url_for( controller='requests_admin', action='show_events', sample_id=sample.id)}">${sample.current_state().name}</a> %endif </td> diff -r 1612a5f4ed62 -r a2dee187b2b6 templates/requests/show_request.mako --- a/templates/requests/show_request.mako Mon Nov 09 15:50:12 2009 -0500 +++ b/templates/requests/show_request.mako Mon Nov 09 16:15:52 2009 -0500 @@ -14,7 +14,7 @@ <ul class="manage-table-actions"> %if request.unsubmitted() and request.samples: <li> - <a class="action-button" confirm="More samples cannot be added to this request once it is submitted. Click OK to submit." href="${h.url_for( controller='requests', action='submit_request', id=request.id)}"> + <a class="action-button" confirm="More samples cannot be added to this request once it is submitted. Click OK to submit." href="${h.url_for( controller='requests', action='list', operation='Submit', id=trans.security.encode_id(request.id) )}"> <span>Submit request</span></a> </li> %endif diff -r 1612a5f4ed62 -r a2dee187b2b6 templates/user/address.mako --- a/templates/user/address.mako Mon Nov 09 15:50:12 2009 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,80 +0,0 @@ -<%inherit file="/base.mako"/> -<%namespace file="/message.mako" import="render_msg" /> - - -%if msg: - ${render_msg( msg, messagetype )} -%endif - -<h2>Manage Addresses</h2> -<ul class="manage-table-actions"> - <li> - <a class="action-button" href="${h.url_for( controller='user', action='new_address')}"> - <span>Add a new address</span></a> - <a class="action-button" href="${h.url_for( controller='user', action='index')}"> - <span>User preferences</span></a> - </li> -</ul> -%if not trans.user.addresses: - There are no addresses -%else: -<div class="grid-header"> - ##<span class="title">Filter:</span> - %for i, filter in enumerate( ['Active', 'Deleted', 'All'] ): - %if i > 0: - <span>|</span> - %endif - %if show_filter == filter: - <span class="filter"><a href="${h.url_for( controller='user', action='manage_addresses', show_filter=filter )}"><b>${filter}</b></a></span> - %else: - <span class="filter"><a href="${h.url_for( controller='user', action='manage_addresses', show_filter=filter )}">${filter}</a></span> - %endif - %endfor -</div> - - -%if not addresses: - <label>There are no addresses</label> -%else: - <div class="toolForm"> - <div class="toolFormBody"> - <% trans.sa_session.refresh( trans.user ) %> - <table class="grid"> - <tbody> - %for index, address in enumerate(addresses): - <tr class="libraryRow libraryOrFolderRow" id="libraryRow"> - <td> - <div class="form-row"> - <label>${address.desc}</label> - ${address.get_html()} - </div> - <div class="form-row"> - <ul class="manage-table-actions"> - <li> - %if not address.deleted: - <a class="action-button" href="${h.url_for( controller='user', action='edit_address', address_id=address.id, - short_desc=address.desc, - name=address.name, institution=address.institution, - address1=address.address, city=address.city, - state=address.state, postal_code=address.postal_code, - country=address.country, phone=address.phone)}"> - <span>Edit</span></a> - <a class="action-button" href="${h.url_for( controller='user', action='delete_address', address_id=address.id)}"> - <span>Delete</span></a> - %else: - <a class="action-button" href="${h.url_for( controller='user', action='undelete_address', address_id=address.id)}"> - <span>Undelete</span></a> - %endif - - </li> - </ul> - </div> - </td> - </tr> - %endfor - </tbody> - </table> - %endif - </div> - </div> -%endif \ No newline at end of file diff -r 1612a5f4ed62 -r a2dee187b2b6 templates/user/info.mako --- a/templates/user/info.mako Mon Nov 09 15:50:12 2009 -0500 +++ b/templates/user/info.mako Mon Nov 09 16:15:52 2009 -0500 @@ -76,28 +76,34 @@ <input type="submit" name="change_password_button" value="Save"> </div> </form> - %if user.values: - <form name="user_info" id="user_info" action="${h.url_for( controller='user', action='edit_info', user_id=user.id, admin_view=admin_view )}" method="post" > - <div class="toolFormTitle">User information</div> - %if user_info_form: - %for field in 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> - %endfor - %if not user_info_select: - <input type="hidden" name="user_info_select" value="${user_info_form.id}"/> - %endif - %endif + %if user.values or user_info_forms: + <form name="user_info" id="user_info" action="${h.url_for( controller='user', action='edit_info', user_id=user.id, admin_view=admin_view )}" method="post" > + <div class="toolFormTitle">User information</div> + %if user_info_select: <div class="form-row"> - <input type="submit" name="edit_user_info_button" value="Save"> + <label>User type</label> + ${user_info_select.get_html()} </div> - </form> + %endif + + %for field in 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> + %endfor + %if not user_info_select: + <input type="hidden" name="user_info_select" value="${user_info_form.id}"/> + %endif + + <div class="form-row"> + <input type="submit" name="edit_user_info_button" value="Save"> + </div> + </form> %endif <form name="user_info" id="user_info" action="${h.url_for( controller='user', action='new_address', user_id=user.id, admin_view=admin_view )}" method="post" > <div class="toolFormTitle">User Addresses</div> diff -r 1612a5f4ed62 -r a2dee187b2b6 test/base/twilltestcase.py --- a/test/base/twilltestcase.py Mon Nov 09 15:50:12 2009 -0500 +++ b/test/base/twilltestcase.py Mon Nov 09 16:15:52 2009 -0500 @@ -1265,6 +1265,10 @@ self.home() self.visit_url( "%s/requests/submit_request?id=%i" % ( self.url, request_id )) self.check_page_for_string( 'The request <b>%s</b> has been submitted.' % request_name ) + def reject_request( self, request_id, request_name ): + self.home() + self.visit_url( "%s/requests_admin/list?operation=Reject&id=%s" % ( self.url, self.security.encode_id( request_id ) )) + self.check_page_for_string( 'The request <b>%s</b> is now unsubmitted.' % request_name ) def add_bar_codes( self, request_id, request_name, bar_codes ): self.home() self.visit_url( "%s/requests_admin/bar_codes?request_id=%i" % (self.url, request_id) ) diff -r 1612a5f4ed62 -r a2dee187b2b6 test/functional/test_forms_and_requests.py --- a/test/functional/test_forms_and_requests.py Mon Nov 09 15:50:12 2009 -0500 +++ b/test/functional/test_forms_and_requests.py Mon Nov 09 16:15:52 2009 -0500 @@ -288,3 +288,15 @@ self.visit_url( '%s/requests_admin/list?show_filter=All' % self.url ) self.check_page_for_string( request_one.name ) self.check_page_for_string( request_two.name ) + def test_45_reject_request( self ): + self.logout() + self.login( email='test@bx.psu.edu' ) + self.reject_request( request_two.id, request_two.name ) + sa_session.refresh( request_two ) + # check if the request is showing in the 'unsubmitted' filter + self.home() + self.visit_url( '%s/requests_admin/list?show_filter=Unsubmitted' % self.url ) + self.check_page_for_string( request_two.name ) + # check if the request's state is now set to 'submitted' + assert request_two.state is not request_two.states.UNSUBMITTED, "The state of the request '%s' should be set to '%s'" \ + % ( request_two.name, request_two.states.UNSUBMITTED ) diff -r 1612a5f4ed62 -r a2dee187b2b6 test/functional/test_user_info.py --- a/test/functional/test_user_info.py Mon Nov 09 15:50:12 2009 -0500 +++ b/test/functional/test_user_info.py Mon Nov 09 16:15:52 2009 -0500 @@ -43,6 +43,10 @@ dict(name='Name of Organization', desc='', type='TextField', + required='optional'), + dict(name='Contact for feedback', + desc='', + type='CheckboxField', required='optional')] form_one = get_latest_form(form_one_name) self.form_add_field(form_one.id, form_one.name, form_one.desc, form_one.type, field_index=len(form_one.fields), fields=fields) @@ -65,6 +69,10 @@ dict(name='Name of Organization', desc='', type='TextField', + required='optional'), + dict(name='Contact for feedback', + desc='', + type='CheckboxField', required='optional')] form_two = get_latest_form(form_two_name) self.form_add_field(form_two.id, form_two.name, form_two.desc, form_two.type, field_index=len(form_one.fields), fields=fields) @@ -75,7 +83,7 @@ self.logout() # user a new user with 'Student' user info form form_one = get_latest_form(form_one_name) - user_info_values=['Educational', 'Penn State'] + user_info_values=['Educational', 'Penn State', True] self.create_user_with_info( 'test11@bx.psu.edu', 'testuser', 'test11', user_info_forms='multiple', user_info_form_id=form_one.id, @@ -83,8 +91,9 @@ self.home() self.visit_page( "user/show_info" ) self.check_page_for_string( "Manage User Information" ) - for value in user_info_values: - self.check_page_for_string( value ) + 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>' ) def test_010_user_reqistration_single_user_info_forms( self ): ''' Testing user registration with a single user info form ''' # lets delete the 'Researcher' user info form @@ -98,7 +107,7 @@ self.logout() # user a new user with 'Student' user info form form_one = get_latest_form(form_one_name) - user_info_values=['Educational', 'Penn State'] + user_info_values=['Educational', 'Penn State', True] self.create_user_with_info( 'test12@bx.psu.edu', 'testuser', 'test12', user_info_forms='single', user_info_form_id=form_one.id, @@ -106,8 +115,9 @@ self.home() self.visit_page( "user/show_info" ) self.check_page_for_string( "Manage User Information" ) - for value in user_info_values: - self.check_page_for_string( value ) + 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>' ) def test_015_edit_user_info( self ): """Testing editing user info as a regular user""" self.logout() @@ -122,7 +132,7 @@ self.logout() self.login( 'test@bx.psu.edu' ) form_one = get_latest_form(form_one_name) - user_info_values=['Educational', 'Penn State'] + user_info_values=['Educational', 'Penn State', True] self.create_user_with_info( 'test13@bx.psu.edu', 'testuser', 'test13', user_info_forms='single', user_info_form_id=form_one.id, @@ -136,8 +146,9 @@ self.visit_page( page ) self.check_page_for_string( 'Manage User Information' ) self.check_page_for_string( 'test13@bx.psu.edu' ) - for value in user_info_values: - self.check_page_for_string( value ) + 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>' ) # lets delete the 'Student' user info form self.login( 'test@bx.psu.edu' ) form_one_latest = get_latest_form(form_one_name)