ui: fixed false name conflict after changing a rule's action

 - When the action of an auto-named rule was changed (e.g. reject to
   allow), the rule was renamed (reject-xxx -> allow-xxx), but the
   editor kept tracking the old name. Saving the rule again then
   wrongly reported "There's already a rule with this name", as it
   compared the renamed rule against itself.
 - Track the saved rule name instead of the one typed in the field,
   and stop mutating the rule object while deleting the old name.
 - Added a regression test.

ref: #1291

Author: redsies

ui/opensnitch/dialogs/ruleseditor/dialog.py

@@ -271,7 +271,11 @@ def cb_save_clicked(self):
         if self._old_rule_name is not None and self._old_rule_name != self.rule.name:
             self.delete_rule()
 
-        self._old_rule_name = rule_name
+        # use the saved rule name, not the one typed in the field: save_rule()
+        # may rename the rule (e.g. when the action of an auto-named rule
+        # changes). Otherwise _old_rule_name would lag behind and the next
+        # save would wrongly report a name conflict with the just saved rule.
+        self._old_rule_name = self.rule.name
 
         # after adding a new rule, we enter into EDIT mode, to allow further
         # changes without closing the dialog.
@@ -388,8 +392,6 @@ def delete_rule(self):
             # if the rule name has changed, we need to remove the old one
             if self._old_rule_name != self.rule.name:
                 node = nodes.get_node_addr(self)
-                old_rule = self.rule
-                old_rule.name = self._old_rule_name
                 if self.nodeApplyAllCheck.isChecked():
                     nid, noti = self._nodes.delete_rule(rule_name=self._old_rule_name, addr=None, callback=self._notification_callback)
                     self.notifications_sent[nid] = noti

ui/tests/dialogs/test_ruleseditor.py

@@ -286,6 +286,47 @@ def handle_dialog():
         records = self.rd._db.get_rule("www.test-renamed.com", node_addr)
         assert records.next() == True
 
+    def test_change_action_of_autonamed_rule(self, qtbot):
+        """ Regression: changing the action of an auto-named rule
+        (reject-xxx -> allow-xxx) renames it. The name tracking lagged
+        behind, so saving the rule a second time wrongly reported a name
+        conflict with the just renamed rule.
+        """
+        qtbot.addWidget(self.rd)
+        node = re_nodes.get_node_addr(self.rd)
+        # start from a clean state for the names used here
+        self.rd._db.delete_rule("reject-rename-host", node)
+        self.rd._db.delete_rule("allow-rename-host", node)
+
+        re_constants.WORK_MODE = re_constants.ADD_RULE
+        re_utils.reset_state(self.rd)
+        self.rd.statusLabel.setText("")
+        self.rd.ruleNameEdit.setText("reject-rename-host")
+        self.rd.dstHostCheck.setChecked(True)
+        self.rd.dstHostLine.setText("rename.example.com")
+        self.rd.actionRejectRadio.setChecked(True)
+
+        qtbot.mouseClick(self.rd.buttonBox.button(QtWidgets.QDialogButtonBox.StandardButton.Save), QtCore.Qt.MouseButton.LeftButton)
+        assert self.rd.statusLabel.text() == ""
+        assert self.rd._old_rule_name == "reject-rename-host"
+
+        # change the action: the rule gets renamed to allow-rename-host
+        self.rd.actionAllowRadio.setChecked(True)
+        qtbot.mouseClick(self.rd.buttonBox.button(QtWidgets.QDialogButtonBox.StandardButton.Save), QtCore.Qt.MouseButton.LeftButton)
+
+        assert self.rd.statusLabel.text() == ""
+        assert self.rd.ruleNameEdit.text() == "allow-rename-host"
+        # name tracking must follow the rename, not lag on the old name
+        assert self.rd._old_rule_name == "allow-rename-host"
+        assert self.rd.rule.name == "allow-rename-host"
+        assert self.rd._db.get_rule("reject-rename-host", node).next() == False
+        assert self.rd._db.get_rule("allow-rename-host", node).next() == True
+
+        # saving again must not report a name conflict with itself
+        qtbot.mouseClick(self.rd.buttonBox.button(QtWidgets.QDialogButtonBox.StandardButton.Save), QtCore.Qt.MouseButton.LeftButton)
+        assert self.rd.statusLabel.text() == ""
+        assert self.rd._db.get_rule("allow-rename-host", node).next() == True
+
     def test_durations(self, qtbot):
         """ Test adding new rule with action "deny".
         """