[checkmk-commits] Simplify add_snapin(), move_snapin_before(), remove_snapin()

Module: check_mk Branch: master Commit: 32a060b65792d9dbdd4d6ec7edb00fe6cec5e5af URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=32a060b65792d9… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Thu Aug 23 14:08:39 2018 +0200 Simplify add_snapin(), move_snapin_before(), remove_snapin() These methods now directly work with UserSidebarSnapin instances instead of all dealing with the Snapin IDs. Instances are now fetched using get_snapin(). CMK-855 Change-Id: I26664c4afeb3726a5556a8343055ed59426a2764 --- cmk/gui/sidebar.py | 92 +++++++++++++++++++----------------------- tests/unit/web/test_sidebar.py | 46 ++++++++++++++++----- 2 files changed, 78 insertions(+), 60 deletions(-) diff --git a/cmk/gui/sidebar.py b/cmk/gui/sidebar.py index c15b82a..3945402 100644 --- a/cmk/gui/sidebar.py +++ b/cmk/gui/sidebar.py @@ -301,64 +301,36 @@ class UserSidebarConfig(object): self.snapins.append(snapin) - def move_snapin_before(self, snapin_id, other_id): - """Move the snapin with the given ID before the other given snapin - The other_id may be None. In this case the snapin is moved to the end. + def move_snapin_before(self, snapin, other): + # type: (UserSidebarSnapin, Union[UserSidebarSnapin,None]) -> None + """Move the given snapin before the other given snapin. + The other may be None. In this case the snapin is moved to the end. """ - # Get current state of snaping being moved (open, closed) - snap_to_move = None - for snapin in self.snapins: - if snapin.snapin_type.type_name() == snapin_id: - snap_to_move = snapin - break - - if not snap_to_move: - raise MKUserError(None, "Snapin being moved is not configured") - - # Build new config by removing snaping at current position - # and add before "other_id" or as last if other_id is not set - new_snapins = [] - for snapin in self.snapins: - if snapin.snapin_type.type_name() == snapin_id: - continue # remove at this position - - elif snapin.snapin_type.type_name() == other_id: - new_snapins.append(snap_to_move) + self.snapins.remove(snapin) - new_snapins.append(snapin) + try: + other_index = self.snapins.index(other) + self.snapins.insert(other_index, snapin) + except ValueError: + self.snapins.append(snapin) - if not other_id: # insert as last - new_snapins.append(snap_to_move) - del self.snapins[:] - self.snapins.extend(new_snapins) - - - def remove_snapin(self, snapin_id): + def remove_snapin(self, snapin): + # type: (UserSidebarSnapin) -> None """Remove the given snapin from the users sidebar""" - new_snapins = [] - for snapin in self.snapins: - if snapin.snapin_type.type_name() == snapin_id: - continue - - new_snapins.append(snapin) - del self.snapins[:] - self.snapins.extend(new_snapins) + self.snapins.remove(snapin) - def set_snapin_visibility(self, snapin_id, state): - """This toggles a snapin between it's visibility states - - Possible states are: open, closed - """ + def get_snapin(self, snapin_id): for snapin in self.snapins: if snapin.snapin_type.type_name() == snapin_id: - snapin.visible = state - break + return snapin + raise KeyError("Snapin %r does not exist" % snapin_id) @property def snapins(self): + # type: () -> List[UserSidebarSnapin] return self._config["snapins"] @@ -470,6 +442,9 @@ class UserSidebarSnapin(object): def __eq__(self, other): + if not isinstance(other, UserSidebarSnapin): + return NotImplemented + return ( self.snapin_type == other.snapin_type and self.visible == other.visible @@ -675,10 +650,17 @@ def ajax_openclose(): raise MKUserError("state", "Invalid state: %s" % state) user_config = UserSidebarConfig(config.user, config.sidebar) + + try: + snapin = user_config.get_snapin(snapin_id) + except KeyError: + return + if state == "off": - user_config.remove_snapin(snapin_id) + user_config.remove_snapin(snapin) else: - user_config.set_snapin_visibility(snapin_id, SnapinVisibility(state)) + snapin.visible = SnapinVisibility(state) + user_config.save() @@ -687,14 +669,22 @@ def move_snapin(): if not config.user.may("general.configure_sidebar"): return - snapin_name = html.var("name") - before_name = html.var("before") + snapin_id = html.var("name") + before_id = html.var("before") user_config = UserSidebarConfig(config.user, config.sidebar) + try: - user_config.move_snapin_before(snapin_name, before_name) - except MKUserError: + snapin = user_config.get_snapin(snapin_id) + except KeyError: return + + try: + before_snapin = user_config.get_snapin(before_id) + except KeyError: + before_snapin = None + + user_config.move_snapin_before(snapin, before_snapin) user_config.save() diff --git a/tests/unit/web/test_sidebar.py b/tests/unit/web/test_sidebar.py index ddf9d64..98f0bfb 100644 --- a/tests/unit/web/test_sidebar.py +++ b/tests/unit/web/test_sidebar.py @@ -42,7 +42,26 @@ def test_user_config_add_snapin(): assert user_config.snapins == [snapin] -(a)pytest.mark.parametrize("move,before,result"sult", [ +def test_user_config_get_snapin(): + user_config = sidebar.UserSidebarConfig(config.user, config.sidebar) + del user_config.snapins[:] + snapin = UserSidebarSnapin.from_snapin_type_id("tactical_overview") + user_config.add_snapin(snapin) + + assert user_config.get_snapin("tactical_overview") == snapin + + +def test_user_config_get_not_existing_snapin(): + user_config = sidebar.UserSidebarConfig(config.user, config.sidebar) + del user_config.snapins[:] + + with pytest.raises(KeyError) as e: + user_config.get_snapin("tactical_overview") + msg = "%s" % e + assert "does not exist" in msg + + +(a)pytest.mark.parametrize("move_id,before_id,result"sult", [ ("tactical_overview", "views", [ UserSidebarSnapin.from_snapin_type_id("admin"), UserSidebarSnapin.from_snapin_type_id("tactical_overview"), @@ -66,7 +85,7 @@ def test_user_config_add_snapin(): UserSidebarSnapin.from_snapin_type_id("admin"), ]), ]) -def test_user_config_move_snapin_before(mocker, move, before, result): +def test_user_config_move_snapin_before(mocker, move_id, before_id, result): user_config = sidebar.UserSidebarConfig(config.user, config.sidebar) del user_config.snapins[:] user_config.snapins.extend([ @@ -75,13 +94,22 @@ def test_user_config_move_snapin_before(mocker, move, before, result): UserSidebarSnapin.from_snapin_type_id("tactical_overview"), ]) - if result is None: - with pytest.raises(MKUserError) as e: - user_config.move_snapin_before(move, before) - assert "Snapin being moved is not configured" in "%s" % e - else: - user_config.move_snapin_before(move, before) - assert user_config.snapins == result + try: + move = user_config.get_snapin(move_id) + except KeyError, e: + if result is None: + assert "does not exist" in "%s" % e + return + else: + raise + + try: + before = user_config.get_snapin(before_id) + except KeyError: + before = None + + user_config.move_snapin_before(move, before) + assert user_config.snapins == result def test_load_default_config(monkeypatch):