Mercurial > bitten > bitten-test
changeset 631:0c11c58d1985
0.6dev: Switch to use warnings in admin instead of raising error pages. Also adds some notices on successful actions. Closes #413.
| author | osimons |
|---|---|
| date | Tue, 11 Aug 2009 23:49:59 +0000 |
| parents | 042c8b49ce7f |
| children | 01c9848950d5 |
| files | bitten/admin.py bitten/tests/admin.py |
| diffstat | 2 files changed, 79 insertions(+), 66 deletions(-) [+] |
line wrap: on
line diff
--- a/bitten/admin.py +++ b/bitten/admin.py @@ -14,7 +14,7 @@ from trac.core import * from trac.admin import IAdminPanelProvider -from trac.web.chrome import add_stylesheet, add_script +from trac.web.chrome import add_stylesheet, add_script, add_warning, add_notice from bitten.model import BuildConfig, TargetPlatform from bitten.recipe import Recipe, InvalidRecipeError @@ -110,6 +110,7 @@ platform_id = None if config_name: # Existing build config + warnings = [] if platform_id or ( # Editing or creating one of the config's target platforms req.method == 'POST' and 'new' in req.args): @@ -146,12 +147,18 @@ if req.method == 'POST': if 'remove' in req.args: # Remove selected platforms self._remove_platforms(req) + add_notice(req, "Target Platform(s) Removed.") req.redirect(req.abs_href.admin(cat, page, config.name)) elif 'save' in req.args: # Save this build config - self._update_config(req, config) + warnings = self._update_config(req, config) - req.redirect(req.abs_href.admin(cat, page)) + if not warnings: + add_notice(req, "Configuration Saved.") + req.redirect(req.abs_href.admin(cat, page, config.name)) + + for warning in warnings: + add_warning(req, warning) # Prepare template variables data['config'] = { @@ -218,7 +225,13 @@ req.perm.assert_permission('BUILD_CREATE') config = BuildConfig(self.env) - self._update_config(req, config) + warnings = self._update_config(req, config) + if warnings: + if len(warnings) == 1: + raise TracError(warnings[0], 'Add Configuration') + else: + raise TracError('Errors: %s' % ' '.join(warnings), + 'Add Configuration') return config def _remove_configs(self, req): @@ -238,14 +251,15 @@ db.commit() def _update_config(self, req, config): + warnings = [] req.perm.assert_permission('BUILD_MODIFY') name = req.args.get('name') if not name: - raise TracError('Missing required field "name"', 'Missing Field') - if not re.match(r'^[\w.-]+$', name): - raise TracError('The field "name" may only contain letters, ' - 'digits, periods, or dashes.', 'Invalid Field') + warnings.append('Missing required field "name".') + if name and not re.match(r'^[\w.-]+$', name): + warnings.append('The field "name" may only contain letters, ' + 'digits, periods, or dashes.') path = req.args.get('path', '') repos = self.env.get_repository(req.authname) @@ -254,22 +268,21 @@ node = repos.get_node(path, max_rev) assert node.isdir, '%s is not a directory' % node.path except (AssertionError, TracError), e: - raise TracError(unicode(e), 'Invalid Repository Path') + warnings.append('Invalid Repository Path "%s".' % path) if req.args.get('min_rev'): try: repos.get_node(path, req.args.get('min_rev')) except TracError, e: - raise TracError(unicode(e), 'Invalid Oldest Revision') + warnings.append('Invalid Oldest Revision: %s.' % unicode(e)) recipe_xml = req.args.get('recipe', '') if recipe_xml: try: Recipe(xmlio.parse(recipe_xml)).validate() except xmlio.ParseError, e: - raise TracError('Failure parsing recipe: %s' % e, - 'Invalid Recipe') + warnings.append('Failure parsing recipe: %s.' % unicode(e)) except InvalidRecipeError, e: - raise TracError(unicode(e), 'Invalid Recipe') + warnings.append('Invalid Recipe: %s.' % unicode(e)) config.name = name config.path = repos.normalize_path(path) @@ -279,10 +292,14 @@ config.label = req.args.get('label', config.name) config.description = req.args.get('description', '') + if warnings: # abort + return warnings + if config.exists: config.update() else: config.insert() + return [] def _create_platform(self, req, config_name): req.perm.assert_permission('BUILD_MODIFY')
--- a/bitten/tests/admin.py +++ b/bitten/tests/admin.py @@ -328,7 +328,8 @@ def test_process_add_config_no_name(self): req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), - args={'add': ''}) + chrome={'warnings': []}, href=Href('/'), + authname='joe', args={'add': ''}) provider = BuildConfigurationsAdminPageProvider(self.env) try: @@ -336,11 +337,12 @@ self.fail('Expected TracError') except TracError, e: - self.assertEqual('Missing required field "name"', e.message) - self.assertEqual('Missing Field', e.title) + self.assertEqual('Missing required field "name".', e.message) + self.assertEqual('Add Configuration', e.title) def test_process_add_config_invalid_name(self): req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), + chrome={'warnings': []}, href=Href('/'), authname='joe', args={'add': '', 'name': 'no spaces allowed'}) provider = BuildConfigurationsAdminPageProvider(self.env) @@ -351,7 +353,7 @@ except TracError, e: self.assertEqual('The field "name" may only contain letters, ' 'digits, periods, or dashes.', e.message) - self.assertEqual('Invalid Field', e.title) + self.assertEqual('Add Configuration', e.title) def test_new_config_submit_with_invalid_path(self): req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), @@ -360,7 +362,7 @@ def get_node(path, rev=None): raise TracError('No such node') - self.repos = Mock(get_node=get_node) + self.repos.get_node = get_node provider = BuildConfigurationsAdminPageProvider(self.env) try: @@ -368,8 +370,7 @@ self.fail('Expected TracError') except TracError, e: - self.assertEqual('No such node', e.message) - self.assertEqual('Invalid Repository Path', e.title) + self.failUnless('Invalid Repository Path' in e.message) def test_process_add_config_no_perms(self): BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', @@ -486,10 +487,10 @@ raise RequestDone req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), abs_href=Href('http://example.org/'), redirect=redirect, - authname='joe', args={ - 'save': '', 'name': 'foo', 'label': 'Foobar', - 'description': 'Thanks for all the fish!' - }) + authname='joe', chrome={'warnings': [], 'notices': []}, + href=Href('/'), + args={'save': '', 'name': 'foo', 'label': 'Foobar', + 'description': 'Thanks for all the fish!'}) provider = BuildConfigurationsAdminPageProvider(self.env) try: @@ -497,7 +498,8 @@ self.fail('Expected RequestDone') except RequestDone: - self.assertEqual('http://example.org/admin/bitten/configs', + self.assertEqual(['Configuration Saved.'], req.chrome['notices']) + self.assertEqual('http://example.org/admin/bitten/configs/foo', redirected_to[0]) config = BuildConfig.fetch(self.env, name='foo') self.assertEqual('Foobar', config.label) @@ -508,90 +510,81 @@ active=True).insert() req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), - args={'save': ''}) + args={'save': '', 'name': ''}, authname='joe', + chrome={'warnings':[]}, href=Href('/')) provider = BuildConfigurationsAdminPageProvider(self.env) - try: - provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - self.fail('Expected TracError') + provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - except TracError, e: - self.assertEqual('Missing required field "name"', e.message) - self.assertEqual('Missing Field', e.title) + self.failUnless(req.chrome['warnings'], "No warnings?") + self.assertEquals(['Missing required field "name".'], + req.chrome['warnings']) def test_process_update_config_invalid_name(self): BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', active=True).insert() req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), - args={'save': '', 'name': 'no spaces allowed'}) + args={'save': '', 'name': 'no spaces allowed'}, + authname='joe', chrome={'warnings': []}, + href=Href('/')) provider = BuildConfigurationsAdminPageProvider(self.env) - try: - provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - self.fail('Expected TracError') + provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - except TracError, e: - self.assertEqual('The field "name" may only contain letters, ' - 'digits, periods, or dashes.', e.message) - self.assertEqual('Invalid Field', e.title) + self.failUnless(req.chrome['warnings'], "No warnings?") + self.assertEquals(req.chrome['warnings'], ['The field "name" may ' \ + 'only contain letters, digits, periods, or dashes.']) def test_process_update_config_invalid_path(self): BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', active=True).insert() req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), - authname='joe', + authname='joe', chrome={'warnings': []}, href=Href('/'), args={'save': '', 'name': 'foo', 'path': 'invalid/path'}) def get_node(path, rev=None): raise TracError('No such node') - self.repos = Mock(get_node=get_node) + self.repos.get_node = get_node provider = BuildConfigurationsAdminPageProvider(self.env) - try: - provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - self.fail('Expected TracError') - - except TracError, e: - self.assertEqual('No such node', e.message) - self.assertEqual('Invalid Repository Path', e.title) + provider.render_admin_panel(req, 'bitten', 'configs', 'foo') + + self.failUnless(req.chrome['warnings'], "No warnings?") + self.assertEquals(req.chrome['warnings'], + ['Invalid Repository Path "invalid/path".']) def test_process_update_config_non_wellformed_recipe(self): BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', active=True).insert() req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), - authname='joe', + authname='joe', chrome={'warnings': []}, href=Href('/'), args={'save': '', 'name': 'foo', 'recipe': 'not_xml'}) provider = BuildConfigurationsAdminPageProvider(self.env) - try: - provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - self.fail('Expected TracError') + provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - except TracError, e: - self.assertEqual('Failure parsing recipe: syntax error: line 1, ' - 'column 0', e.message) - self.assertEqual('Invalid Recipe', e.title) + self.failUnless(req.chrome['warnings'], "No warnings?") + self.assertEquals(['Failure parsing recipe: syntax error: line 1, ' \ + 'column 0.'], req.chrome['warnings']) def test_process_update_config_invalid_recipe(self): BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', active=True).insert() req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), - authname='joe', + authname='joe', chrome={'warnings': []}, href=Href('/'), args={'save': '', 'name': 'foo', 'recipe': '<build><step /></build>'}) provider = BuildConfigurationsAdminPageProvider(self.env) - try: - provider.render_admin_panel(req, 'bitten', 'configs', 'foo') - self.fail('Expected TracError') - - except TracError, e: - self.assertEqual('Steps must have an "id" attribute', e.message) - self.assertEqual('Invalid Recipe', e.title) + provider.render_admin_panel(req, 'bitten', 'configs', 'foo') + + self.failUnless(req.chrome['warnings'], "No warnings?") + self.assertEquals(req.chrome['warnings'], + ['Invalid Recipe: Steps must have an "id" attribute.']) def test_process_new_platform_no_name(self): BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', @@ -648,7 +641,8 @@ raise RequestDone req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), abs_href=Href('http://example.org/'), redirect=redirect, - authname='joe', + authname='joe', chrome={'warnings': [], 'notices': []}, + href=Href('/'), args={'remove': '', 'sel': str(platform.id)}) provider = BuildConfigurationsAdminPageProvider(self.env) @@ -657,6 +651,8 @@ self.fail('Expected RequestDone') except RequestDone: + self.assertEquals(['Target Platform(s) Removed.'], + req.chrome['notices']) self.assertEqual('http://example.org/admin/bitten/configs/foo', redirected_to[0]) platforms = list(TargetPlatform.select(self.env, config='foo'))