Update host tag rule condition editor

It's now possible to configure a list of tag IDs for a single tag
group. This reduces some situations where users had to configure
multiple rules when a rule should affect multiple choices of a
tag group.

In this step the tag condition input has been changed to be more
compact by default. This should improve the condition dialog usability,
especially for users with a larger number of tag groups.

CMK-2188

Change-Id: I9e6e8210d96c32938c4159406e444402a8b15b24

cmk/gui/plugins/wato/__init__.py

@@ -160,6 +160,7 @@
     ConfigHostname,
     SiteBackupJobs,
     HostTagCondition,
+    DictHostTagCondition,
     get_hostnames_from_checkboxes,
     get_hosts_from_checkboxes,
     get_check_information,

cmk/gui/plugins/wato/utils/__init__.py

@@ -66,6 +66,7 @@
     Transform,
     FixedValue,
     ListOf,
+    ListOfMultiple,
     RegExpUnicode,
     RegExp,
     TextUnicode,
@@ -1770,6 +1771,159 @@ def register_notification_parameters(scriptname, valuespec):
     notification_parameter_registry.register(parameter_class)
 
 
+class DictHostTagCondition(Transform):
+    def __init__(self, title, help_txt):
+        super(DictHostTagCondition, self).__init__(
+            ListOfMultiple(
+                title=title,
+                help=help_txt,
+                choices=self._get_tag_group_choices(),
+                add_label=_("Add tag condition"),
+                del_label=_("Remove tag condition"),
+            ),
+            forth=self._to_valuespec,
+            back=self._from_valuespec,
+        )
+
+    def _get_tag_group_choices(self):
+        choices = []
+        all_topics = config.tags.get_topic_choices()
+        tag_groups_by_topic = dict(config.tags.get_tag_groups_by_topic())
+        aux_tags_by_topic = dict(config.tags.get_aux_tags_by_topic())
+        for topic_id, _topic_title in all_topics:
+            for tag_group in tag_groups_by_topic.get(topic_id, []):
+                choices.append(self._get_tag_group_choice(tag_group))
+
+            for aux_tag in aux_tags_by_topic.get(topic_id, []):
+                choices.append(self._get_aux_tag_choice(aux_tag))
+
+        return choices
+
+    def _to_valuespec(self, host_tag_conditions):
+        valuespec_value = {}
+        for tag_group_id, tag_condition in host_tag_conditions.iteritems():
+            if isinstance(tag_condition, dict) and "$or" in tag_condition:
+                value = self._ored_tags_to_valuespec(tag_condition["$or"])
+            elif isinstance(tag_condition, dict) and "$nor" in tag_condition:
+                value = self._nored_tags_to_valuespec(tag_condition["$nor"])
+            else:
+                value = self._single_tag_to_valuespec(tag_condition)
+
+            valuespec_value[tag_group_id] = value
+
+        return valuespec_value
+
+    def _ored_tags_to_valuespec(self, tag_conditions):
+        return ("or", tag_conditions)
+
+    def _nored_tags_to_valuespec(self, tag_conditions):
+        return ("nor", tag_conditions)
+
+    def _single_tag_to_valuespec(self, tag_condition):
+        if isinstance(tag_condition, dict):
+            if "$ne" in tag_condition:
+                return ("is_not", tag_condition["$ne"])
+            raise NotImplementedError()
+        return ("is", tag_condition)
+
+    def _from_valuespec(self, valuespec_value):
+        tag_conditions = {}
+        for tag_group_id, (operator, operand) in valuespec_value.iteritems():
+            if operator in ["is", "is_not"]:
+                tag_group_value = self._single_tag_from_valuespec(operator, operand)
+            elif operator in ["or", "nor"]:
+                tag_group_value = {
+                    "$%s" % operator: operand,
+                }
+            else:
+                raise NotImplementedError()
+
+            tag_conditions[tag_group_id] = tag_group_value
+        return tag_conditions
+
+    def _single_tag_from_valuespec(self, operator, tag_id):
+        if operator == "is":
+            return tag_id
+        elif operator == "is_not":
+            return {"$ne": tag_id}
+        raise NotImplementedError()
+
+    def _get_tag_group_choice(self, tag_group):
+        tag_choices = tag_group.get_tag_choices()
+        tag_id_choice = ListOf(
+            valuespec=DropdownChoice(choices=tag_choices,),
+            style=ListOf.Style.FLOATING,
+            add_label=_("Add tag"),
+            del_label=_("Remove tag"),
+            magic="@@#!#@@",
+            movable=False,
+            validate=lambda value, varprefix: \
+                self._validate_tag_list(value, varprefix, tag_choices),
+        )
+
+        return (
+            tag_group.id,
+            CascadingDropdown(
+                label=tag_group.choice_title + " ",
+                title=tag_group.choice_title,
+                choices=[
+                    ("is", _("is"), DropdownChoice(choices=tag_choices)),
+                    ("is_not", _("is not"), DropdownChoice(choices=tag_choices)),
+                    ("or", _("one of"), tag_id_choice),
+                    ("nor", _("none of"), tag_id_choice),
+                ],
+                show_titles=False,
+                orientation="horizontal",
+                default_value=("is", tag_choices[0][0]),
+            ),
+        )
+
+    def _validate_tag_list(self, value, varprefix, tag_choices):
+        seen = set()
+        for tag_id in value:
+            if tag_id in seen:
+                raise MKUserError(
+                    varprefix,
+                    _("The tag '%s' is selected multiple times. A tag may be selected only once.") %
+                    dict(tag_choices)[tag_id])
+            seen.add(tag_id)
+
+    def _get_aux_tag_choice(self, aux_tag):
+        return (aux_tag.id,
+                Tuple(
+                    title=aux_tag.choice_title,
+                    elements=[
+                        FixedValue(_u(aux_tag.choice_title)),
+                        self._is_or_is_not(),
+                        FixedValue(
+                            aux_tag.id,
+                            title=_u(aux_tag.title),
+                            totext=_u(aux_tag.title),
+                        )
+                    ],
+                    show_titles=False,
+                    orientation="horizontal",
+                ))
+
+    def _tag_choice(self, tag_group):
+        return Tuple(
+            title=_u(tag_group.choice_title),
+            elements=[
+                self._is_or_is_not(),
+                DropdownChoice(choices=tag_group.get_tag_choices()),
+            ],
+            show_titles=False,
+            orientation="horizontal",
+        )
+
+    def _is_or_is_not(self):
+        return DropdownChoice(
+            choices=[
+                ("is", _("is")),
+                ("is_not", _("is not")),
+            ],)
+
+
 class HostTagCondition(ValueSpec):
     """ValueSpec for editing a tag-condition"""
 

cmk/gui/wato/pages/rulesets.py

@@ -76,6 +76,7 @@
     search_form,
     ConfigHostname,
     HostTagCondition,
+    DictHostTagCondition,
 )
 
 
@@ -1417,7 +1418,7 @@ def _to_valuespec(self, conditions):
         # type: (RuleConditions) -> dict
         explicit = {
             "folder_path": conditions.host_folder,
-            "host_tags": conditions.tag_list,
+            "host_tags": conditions.host_tags,
         }
 
         explicit_hosts = conditions.host_list
@@ -1463,20 +1464,12 @@ def _from_valuespec(self, explicit):
 
         return RuleConditions(
             host_folder=explicit["folder_path"],
-            host_tags=self._host_tags_from_valuespec(explicit["host_tags"]),
+            host_tags=explicit["host_tags"],
             host_name=self._condition_list_from_valuespec(
                 explicit.get("explicit_hosts"), is_service=False),
             service_description=service_description,
         )
 
-    # TODO: Change all HostTagCondition to produce a tag dictionary instead of the list
-    def _host_tags_from_valuespec(self, tags):
-        """Transform the host tag list of the valuespec to rule tag conditions"""
-        self.tuple_transformer = ruleset_matcher.RulesetToDictTransformer(
-            tag_to_group_map=ruleset_matcher.get_tag_to_group_map(config.tags))
-
-        return self.tuple_transformer.transform_host_tags(tags).get("host_tags", {})
-
     def _condition_list_from_valuespec(self, conditions, is_service):
         if conditions is None:
             return None
@@ -1511,11 +1504,11 @@ def _vs_folder(self):
         )
 
     def _vs_host_tag_condition(self):
-        return HostTagCondition(
+        return DictHostTagCondition(
             title=_("Host tags"),
-            help=_("The rule will only be applied to hosts fulfilling all "
-                   "of the host tag conditions listed here, even if they appear "
-                   "in the list of explicit host names."),
+            help_txt=_("The rule will only be applied to hosts fulfilling all "
+                       "of the host tag conditions listed here, even if they appear "
+                       "in the list of explicit host names."),
         )
 
     def _vs_explicit_hosts(self):
@@ -1609,16 +1602,29 @@ def render(self, rulespec, conditions):
     def _tag_conditions(self, conditions):
         # type: (RuleConditions) -> Generator
         for tag_spec in conditions.host_tags.itervalues():
-            is_not = isinstance(tag_spec, dict) and "$ne" in tag_spec
-            if is_not:
-                # mypy had some problem with this. Need to check type annotation
-                tag_id = tag_spec["$ne"]  # type: ignore
+            if isinstance(tag_spec, dict) and "$or" in tag_spec:
+                yield HTML(" <i>or</i> ").join(
+                    [self._single_tag_condition(sub_spec) for sub_spec in tag_spec["$or"]])
+            elif isinstance(tag_spec, dict) and "$nor" in tag_spec:
+                yield HTML(_("Neither") + " ") + HTML(" <i>nor</i> ").join(
+                    [self._single_tag_condition(sub_spec) for sub_spec in tag_spec["$nor"]])
+            else:
+                yield self._single_tag_condition(tag_spec)
+
+    def _single_tag_condition(self, tag_spec):
+        negate = False
+        if isinstance(tag_spec, dict):
+            if "$ne" in tag_spec:
+                negate = True
             else:
-                tag_id = tag_spec
+                raise NotImplementedError()
 
-            yield self._single_tag_condition(tag_id, is_not)
+        if negate:
+            # mypy had some problem with this. Need to check type annotation
+            tag_id = tag_spec["$ne"]  # type: ignore
+        else:
+            tag_id = tag_spec
 
-    def _single_tag_condition(self, tag_id, negate):
         tag = config.tags.get_tag_or_aux_tag(tag_id)
         if tag and tag.title:
             if not tag.is_aux_tag:

cmk/utils/tags.py

@@ -124,6 +124,12 @@ def get_dict_format(self):
             response["topic"] = self.topic
         return response
 
+    @property
+    def choice_title(self):
+        if self.topic:
+            return "%s / %s" % (self.topic, self.title)
+        return self.title
+
 
 class AuxTagList(object):
     def __init__(self):

web/htdocs/js/modules/valuespecs.js

@@ -22,6 +22,7 @@
 // to the Free Software Foundation, Inc., 51 Franklin St,  Fifth Floor,
 // Boston, MA 02110-1301 USA.
 
+import $ from "jquery";
 import * as utils from "utils";
 import * as popup_menu from "popup_menu";
 import * as ajax from "ajax";
@@ -541,6 +542,10 @@ export function listofmultiple_add(varprefix) {
 
     choice.options[choice.selectedIndex].disabled = true; // disable this choice
 
+    // Update select2 to make the disabled attribute be recognized by the dropdown
+    // (See https://github.com/select2/select2/issues/3347)
+    $(choice).select2();
+
     // make the filter visible
     var row = document.getElementById(varprefix + "_" + ident + "_row");
     utils.remove_class(row, "unused");
@@ -574,6 +579,10 @@ export function listofmultiple_del(varprefix, ident) {
         if (choice.children[i].value == ident)
             choice.children[i].disabled = false;
 
+    // Update select2 to make the disabled attribute be recognized by the dropdown
+    // (See https://github.com/select2/select2/issues/3347)
+    $(choice).select2();
+
     // Remove it from the list of active elements
     var active = document.getElementById(varprefix + "_active");
     var l = active.value.split(";");

web/htdocs/themes/facelift/scss/_main.scss

@@ -614,6 +614,10 @@ div.valuespec_listof_floating_container > table {
     float: left;
 }
 
+table.nform table.valuespec_listof div.valuespec_listof_floating_container > table > tbody > tr > td {
+    border-bottom: none;
+}
+
 table.valuespec_listof.regular > tbody > tr > td:first-child {
     padding-bottom: 5px;
 }