Merge pull request #1386

* Methods deletion simplification and fix

* Action refactor

* Test updates

Author: mrvisscher

activity_browser/actions/method/method_delete.py

@@ -23,26 +23,20 @@ class MethodDelete(ABAction):
 
     @staticmethod
     @exception_dialogs
-    def run(methods: List[tuple], level: str):
-        # this action can handle only one selected method for now
-        selected_method = methods[0]
-
+    def run(methods: List[tuple]):
         # check whether we're dealing with a leaf or node. If it's a node, select all underlying methods for deletion
-        if level is not None and level != "leaf":
-            all_methods = [
-                bd.Method(method)
-                for method in bd.methods
-                if set(selected_method).issubset(method)
-            ]
+        all_methods = [bd.Method(method) for method in methods]
+
+        if len(all_methods) == 1:
+            warning_text = f"Are you sure you want to delete this method?\n\n{methods[0]}"
         else:
-            all_methods = [bd.Method(selected_method)]
+            warning_text = f"Are you sure you want to delete {len(all_methods)} methods?"
 
         # warn the user about the pending deletion
         warning = QtWidgets.QMessageBox.warning(
             application.main_window,
             "Deleting Method",
-            f"Are you sure you want to delete this method and possible underlying "
-            f"methods?\n\n{selected_method}",
+            warning_text,
             QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No,
             QtWidgets.QMessageBox.No,
         )

activity_browser/ui/tables/impact_categories.py

@@ -33,9 +33,7 @@ def __init__(self, parent=None):
         self.duplicate_method_action = actions.MethodDuplicate.get_QAction(
             self.selected_methods, None
         )
-        self.delete_method_action = actions.MethodDelete.get_QAction(
-            self.selected_methods, None
-        )
+        self.delete_method_action = actions.MethodDelete.get_QAction(self.selected_methods)
 
         self.connect_signals()
 
@@ -113,9 +111,7 @@ def __init__(self, parent=None):
         self.duplicate_method_action = actions.MethodDuplicate.get_QAction(
             self.selected_methods, self.tree_level
         )
-        self.delete_method_action = actions.MethodDelete.get_QAction(
-            self.selected_methods, self.tree_level
-        )
+        self.delete_method_action = actions.MethodDelete.get_QAction(self.selected_methods)
 
         self._connect_signals()
 
@@ -202,7 +198,7 @@ def selected_methods(self) -> Iterable:
             filter_on = ", ".join(tree_level[1]) + ", "
 
         methods = self.model.get_methods(filter_on)
-        return methods
+        return list(methods)
 
     def tree_level(self) -> tuple:
         """Return list of (tree level, content).

tests/actions/test_method_actions.py

@@ -116,8 +116,8 @@ def test_cf_uncertainty_remove(ab_app):
 
 def test_method_delete(ab_app, monkeypatch):
     method = ("A_methods", "methods", "method_to_delete")
-    branch = ("A_methods", "methods_to_delete")
-    branched_method = ("A_methods", "methods_to_delete", "method_to_delete")
+    dual_method_1 = ("A_methods", "methods_to_delete", "method_to_delete_one")
+    dual_method_2 = ("A_methods", "methods_to_delete", "method_to_delete_two")
 
     monkeypatch.setattr(
         QtWidgets.QMessageBox,
@@ -127,13 +127,15 @@ def test_method_delete(ab_app, monkeypatch):
 
     assert bd.projects.current == "default"
     assert method in bd.methods
-    assert branched_method in bd.methods
+    assert dual_method_1 in bd.methods
+    assert dual_method_2 in bd.methods
 
-    actions.MethodDelete.run([method], "leaf")
-    actions.MethodDelete.run([branch], "branch")
+    actions.MethodDelete.run([method])
+    actions.MethodDelete.run([dual_method_1, dual_method_2])
 
     assert method not in bd.methods
-    assert branched_method not in bd.methods
+    assert dual_method_1 not in bd.methods
+    assert dual_method_2 not in bd.methods
 
 
 def test_method_duplicate(ab_app, monkeypatch):