kallithea Changeset - 8e5450cd4686

Changeset - 8e5450cd4686

Parent rev.

Child rev.

[Not reviewed]

default

0 2 0

Thomas De Schampheleire - 8 years ago 2018-05-20 22:51:13
thomas.de_schampheleire@nokia.com

admin: hooks: only flash 'Updated hooks' if there are changes

Clicking 'Save' on the hook administration page currently always renders the
flash message 'Updated hooks' even if nothing was changed.
This may be particularly confusing when the action you intended to do got an
error, e.g. adding a hook that already exists, adding a builtin hook, ...

Instead, compare the old and new value when editing a hook, and only save
and create the flash if they are different.

For this to be work correctly in test, the old value needs to be passed as
well like in the real situation, otherwise the 'zip' operation will return
an empty list.

2 files changed with 5 insertions and 2 deletions:

kallithea/controllers/admin/settings.py

kallithea/tests/functional/test_admin_settings.py

0 comments (0 inline, 0 general)

kallithea/controllers/admin/settings.py

➞

Show inline comments

@@ @@ -320,98 +320,100 @@ class SettingsController(BaseController) @@
                                 'html', body=test_body)
             recipients = [test_email] if test_email else None
             tasks.send_email(recipients, test_email_subj,
                              test_email_txt_body, test_email_html_body)
             h.flash(_('Send email task created'), category='success')
             raise HTTPFound(location=url('admin_settings_email'))
         defaults = Setting.get_app_settings()
         defaults.update(self._get_hg_ui_settings())
         import kallithea
         c.ini = kallithea.CONFIG
         return htmlfill.render(
             render('admin/settings/settings.html'),
             defaults=defaults,
             encoding="UTF-8",
             force_defaults=False)
     @HasPermissionAnyDecorator('hg.admin')
     def settings_hooks(self):
         c.active = 'hooks'
         if request.POST:
             if c.visual.allow_custom_hooks_settings:
                 ui_key = request.POST.get('new_hook_ui_key')
                 ui_value = request.POST.get('new_hook_ui_value')
                 hook_id = request.POST.get('hook_id')
                 try:
                     ui_key = ui_key and ui_key.strip()
                     if ui_key in (x.ui_key for x in Ui.get_custom_hooks()):
                         h.flash(_('Hook already exists'), category='error')
                     elif ui_key in (x.ui_key for x in Ui.get_builtin_hooks()):
                         h.flash(_('Builtin hooks are read-only. Please use another hook name.'), category='error')
                     elif ui_value and ui_key:
                         Ui.create_or_update_hook(ui_key, ui_value)
                         h.flash(_('Added new hook'), category='success')
                     elif hook_id:
                         Ui.delete(hook_id)
                         Session().commit()
                     # check for edits
                     update = False
                     _d = request.POST.dict_of_lists()
                     for k, v in zip(_d.get('hook_ui_key', []),
                                     _d.get('hook_ui_value_new', [])):
                     for k, v, ov in zip(_d.get('hook_ui_key', []),
                                         _d.get('hook_ui_value_new', []),
                                         _d.get('hook_ui_value', [])):
                         if v != ov:
                         Ui.create_or_update_hook(k, v)
                         update = True
                     if update:
                         h.flash(_('Updated hooks'), category='success')
                     Session().commit()
                 except Exception:
                     log.error(traceback.format_exc())
                     h.flash(_('Error occurred during hook creation'),
                             category='error')
                 raise HTTPFound(location=url('admin_settings_hooks'))
         defaults = Setting.get_app_settings()
         defaults.update(self._get_hg_ui_settings())
         c.hooks = Ui.get_builtin_hooks()
         c.custom_hooks = Ui.get_custom_hooks()
         return htmlfill.render(
             render('admin/settings/settings.html'),
             defaults=defaults,
             encoding="UTF-8",
             force_defaults=False)
     @HasPermissionAnyDecorator('hg.admin')
     def settings_search(self):
         c.active = 'search'
         if request.POST:
             repo_location = self._get_hg_ui_settings()['paths_root_path']
             full_index = request.POST.get('full_index', False)
             tasks.whoosh_index(repo_location, full_index)
             h.flash(_('Whoosh reindex task scheduled'), category='success')
             raise HTTPFound(location=url('admin_settings_search'))
         defaults = Setting.get_app_settings()
         defaults.update(self._get_hg_ui_settings())
         return htmlfill.render(
             render('admin/settings/settings.html'),
             defaults=defaults,
             encoding="UTF-8",
             force_defaults=False)
     @HasPermissionAnyDecorator('hg.admin')
     def settings_system(self):
         c.active = 'system'

kallithea/tests/functional/test_admin_settings.py

➞

Show inline comments

@@ @@ -4,96 +4,97 @@ from kallithea.model.db import Setting, @@
 from kallithea.tests.base import *
 from kallithea.tests.fixture import Fixture
 fixture = Fixture()
 class TestAdminSettingsController(TestController):
     def test_index_main(self):
         self.log_user()
         response = self.app.get(url('admin_settings'))
     def test_index_mapping(self):
         self.log_user()
         response = self.app.get(url('admin_settings_mapping'))
     def test_index_global(self):
         self.log_user()
         response = self.app.get(url('admin_settings_global'))
     def test_index_visual(self):
         self.log_user()
         response = self.app.get(url('admin_settings_visual'))
     def test_index_email(self):
         self.log_user()
         response = self.app.get(url('admin_settings_email'))
     def test_index_hooks(self):
         self.log_user()
         response = self.app.get(url('admin_settings_hooks'))
     def test_create_custom_hook(self):
         self.log_user()
         response = self.app.post(url('admin_settings_hooks'),
                                 params=dict(new_hook_ui_key='test_hooks_1',
                                             new_hook_ui_value='cd %s' % TESTS_TMP_PATH,
                                             _authentication_token=self.authentication_token()))
         self.checkSessionFlash(response, 'Added new hook')
         response = response.follow()
         response.mustcontain('test_hooks_1')
         response.mustcontain('cd %s' % TESTS_TMP_PATH)
     def test_edit_custom_hook(self):
         self.log_user()
         response = self.app.post(url('admin_settings_hooks'),
                                 params=dict(hook_ui_key='test_hooks_1',
                                             hook_ui_value='old_value_of_hook_1',
                                             hook_ui_value_new='new_value_of_hook_1',
                                             _authentication_token=self.authentication_token()))
         response = response.follow()
         response.mustcontain('test_hooks_1')
         response.mustcontain('new_value_of_hook_1')
     def test_add_existing_custom_hook(self):
         self.log_user()
         response = self.app.post(url('admin_settings_hooks'),
                                 params=dict(new_hook_ui_key='test_hooks_1',
                                             new_hook_ui_value='attempted_new_value',
                                             _authentication_token=self.authentication_token()))
         self.checkSessionFlash(response, 'Hook already exists')
         response = response.follow()
         response.mustcontain('test_hooks_1')
         response.mustcontain('new_value_of_hook_1')
     def test_create_custom_hook_delete(self):
         self.log_user()
         response = self.app.post(url('admin_settings_hooks'),
                                 params=dict(new_hook_ui_key='test_hooks_2',
                                             new_hook_ui_value='cd %s2' % TESTS_TMP_PATH,
                                             _authentication_token=self.authentication_token()))
         self.checkSessionFlash(response, 'Added new hook')
         response = response.follow()
         response.mustcontain('test_hooks_2')
         response.mustcontain('cd %s2' % TESTS_TMP_PATH)
         hook_id = Ui.get_by_key('hooks', 'test_hooks_2').ui_id
         ## delete
         self.app.post(url('admin_settings_hooks'),
                         params=dict(hook_id=hook_id, _authentication_token=self.authentication_token()))
         response = self.app.get(url('admin_settings_hooks'))
         response.mustcontain(no=['test_hooks_2'])
         response.mustcontain(no=['cd %s2' % TESTS_TMP_PATH])
     def test_add_existing_builtin_hook(self):
         self.log_user()
         response = self.app.post(url('admin_settings_hooks'),
                                 params=dict(new_hook_ui_key='changegroup.update',
                                             new_hook_ui_value='attempted_new_value',
                                             _authentication_token=self.authentication_token()))
         self.checkSessionFlash(response, 'Builtin hooks are read-only')
         response = response.follow()

0 comments (0 inline, 0 general)