Improve calculation setup dialogs with retry and pre-population

activity_browser/actions/calculation_setup/cs_duplicate.py

@@ -12,9 +12,10 @@
 
 class CSDuplicate(ABAction):
     """
-    ABAction to duplicate a calculation setup. Prompts the user for a new name. Returns if the user cancels, or if a CS
-    with the same name is already present within the project. If all is right, instructs the CalculationSetupController
-    to duplicate the CS.
+    ABAction to duplicate a calculation setup. Prompts the user for a new
+    name. Returns if the user cancels, or if a CS with the same name is
+    already present within the project. If all is right, instructs the
+    CalculationSetupController to duplicate the CS.
     """
 
     icon = qicons.copy
@@ -34,14 +35,22 @@ def run(cs_name: str):
         if not ok or not new_name:
             return
 
-        # throw error if the name is already present, and return
-        if new_name in bd.calculation_setups:
+        # throw error if the name is already present, and retry with the same name
+        while new_name in bd.calculation_setups:
             QtWidgets.QMessageBox.warning(
                 application.main_window,
                 "Not possible",
                 "A calculation setup with this name already exists.",
             )
-            return
+            new_name, ok = QtWidgets.QInputDialog.getText(
+                application.main_window,
+                f"Duplicate '{cs_name}'",
+                "Name of the duplicated calculation setup:" + " " * 10,
+                text=new_name,
+            )
+            # return if the user cancels or gives no name
+            if not ok or not new_name:
+                return
 
         bd.calculation_setups[new_name] = bd.calculation_setups[cs_name].copy()
         signals.calculation_setup_selected.emit(new_name)

activity_browser/actions/calculation_setup/cs_new.py

@@ -16,20 +16,26 @@ class CSNew(ABAction):
     """
     Create a new Calculation Setup.
 
-    This method prompts the user for a name for the new Calculation Setup (CS) if not provided.
-    It validates the name to ensure it is unique within the project and creates a new CS
-    with the specified functional units and impact categories.
+    This method prompts the user for a name for the new Calculation Setup
+    (CS) if not provided. It validates the name to ensure it is unique
+    within the project and creates a new CS with the specified functional
+    units and impact categories.
 
     Args:
-        name (str, optional): The name of the new Calculation Setup. If not provided, the user is prompted.
-        functional_units (list[dict[tuple | int | bd.Node, float]], optional): A list of functional units to include in the CS.
-        impact_categories (list[tuple], optional): A list of impact categories to include in the CS.
+        name (str, optional): The name of the new Calculation Setup. If not
+            provided, the user is prompted.
+        functional_units (list[dict[tuple | int | bd.Node, float]],
+            optional): A list of functional units to include in the CS.
+        impact_categories (list[tuple], optional): A list of impact
+            categories to include in the CS.
 
     Returns:
-        None: Returns early if the user cancels, provides no name, or if the name already exists.
+        None: Returns early if the user cancels, provides no name, or if
+            the name already exists.
 
     Raises:
-        None: This method does not raise exceptions but logs errors and shows warnings for invalid inputs.
+        None: This method does not raise exceptions but logs errors and
+            shows warnings for invalid inputs.
     """
 
     icon = qicons.add
@@ -48,14 +54,17 @@ def run(name: str = None,
         if not name:
             return
 
-        # throw error if the name is already present, and return
-        if name in bd.calculation_setups:
+        # throw error if the name is already present, and retry with the same name
+        while name in bd.calculation_setups:
             QtWidgets.QMessageBox.warning(
                 application.main_window,
                 "Not possible",
                 "A calculation setup with this name already exists.",
             )
-            return
+            name = CSNew.get_cs_name(default_name=name)
+            # return if the user cancels or gives no name
+            if not name:
+                return
 
         inv = functional_units or []
         for i, fu in enumerate(inv):
@@ -74,15 +83,20 @@ def run(name: str = None,
         actions.CSOpen.run(name)
 
     @staticmethod
-    def get_cs_name() -> str | None:
+    def get_cs_name(default_name: str = "") -> str | None:
         """
         Prompt the user for a name for the new calculation setup.
+
+        Args:
+            default_name (str, optional): Default name to pre-populate in
+                the dialog.
         """
         # prompt the user to give a name for the new calculation setup
         name, ok = QtWidgets.QInputDialog.getText(
             application.main_window,
             "Create new calculation setup",
             "Name of new calculation setup:" + " " * 10,
+            text=default_name,
         )
 
         # return if the user cancels or gives no name

activity_browser/actions/calculation_setup/cs_rename.py

@@ -12,9 +12,10 @@
 
 class CSRename(ABAction):
     """
-    ABAction to rename a calculation setup. Prompts the user for a new name. Returns if the user cancels, or if a CS
-    with the same name is already present within the project. If all is right, instructs the CalculationSetupController
-    to rename the CS.
+    ABAction to rename a calculation setup. Prompts the user for a new name.
+    Returns if the user cancels, or if a CS with the same name is already
+    present within the project. If all is right, instructs the
+    CalculationSetupController to rename the CS.
     """
 
     icon = qicons.edit
@@ -24,24 +25,34 @@ class CSRename(ABAction):
     @exception_dialogs
     def run(cs_name: str, new_name: str = None):
         # prompt the user to give a name for the new calculation setup
+        # pre-populate with the old name as actual text (not placeholder)
         new_name, ok = QtWidgets.QInputDialog.getText(
             application.main_window,
             f"Rename '{cs_name}'",
             "New name of this calculation setup:" + " " * 10,
+            text=cs_name,
         )
 
         # return if the user cancels or gives no name
         if not ok or not new_name:
             return
 
-        # throw error if the name is already present, and return
-        if new_name in bd.calculation_setups:
+        # throw error if the name is already present, and retry with the same name
+        while new_name in bd.calculation_setups:
             QtWidgets.QMessageBox.warning(
                 application.main_window,
                 "Not possible",
                 "A calculation setup with this name already exists.",
             )
-            return
+            new_name, ok = QtWidgets.QInputDialog.getText(
+                application.main_window,
+                f"Rename '{cs_name}'",
+                "New name of this calculation setup:" + " " * 10,
+                text=new_name,
+            )
+            # return if the user cancels or gives no name
+            if not ok or not new_name:
+                return
 
         # instruct the CalculationSetupController to rename the CS to the new name
         bd.calculation_setups[new_name] = bd.calculation_setups[cs_name].copy()

tests/actions/test_calculation_setup_actions.py

@@ -1,12 +1,9 @@
-import pytest
 import bw2data as bd
-from bw2data.errors import BW2Exception
 from qtpy import QtWidgets
 
 from activity_browser import actions
 
 
-
 def test_cs_delete(monkeypatch, basic_database):
     monkeypatch.setattr(
         QtWidgets.QMessageBox, "warning", staticmethod(lambda *args, **kwargs: True)
@@ -77,3 +74,161 @@ def test_cs_rename(monkeypatch, basic_database):
 
     assert cs_name not in bd.calculation_setups
     assert renamed_cs in bd.calculation_setups
+
+
+def test_cs_new_retry_on_duplicate(monkeypatch, basic_database):
+    """Test CSNew retries with same name when duplicate is entered."""
+    cs_name = "basic_calculation_setup"
+    new_cs = "cs_that_is_new"
+
+    # Simulate user first entering an existing name, then a new name
+    call_count = [0]
+
+    def mock_getText(*args, **kwargs):
+        call_count[0] += 1
+        if call_count[0] == 1:
+            return (cs_name, True)  # First attempt - duplicate
+        else:
+            return (new_cs, True)  # Second attempt - unique name
+
+    monkeypatch.setattr(
+        QtWidgets.QInputDialog, "getText", staticmethod(mock_getText)
+    )
+    monkeypatch.setattr(
+        QtWidgets.QMessageBox, "warning",
+        staticmethod(lambda *args, **kwargs: True)
+    )
+
+    assert cs_name in bd.calculation_setups
+    assert new_cs not in bd.calculation_setups
+
+    actions.CSNew.run()
+
+    assert cs_name in bd.calculation_setups
+    assert new_cs in bd.calculation_setups
+    assert call_count[0] == 2  # Dialog should have been shown twice
+
+
+def test_cs_new_cancel_on_retry(monkeypatch, basic_database):
+    """Test CSNew cancels properly when user cancels after duplicate."""
+    cs_name = "basic_calculation_setup"
+
+    # Simulate user first entering an existing name, then canceling
+    call_count = [0]
+
+    def mock_getText(*args, **kwargs):
+        call_count[0] += 1
+        if call_count[0] == 1:
+            return (cs_name, True)  # First attempt - duplicate
+        else:
+            return ("", False)  # Second attempt - cancel
+
+    monkeypatch.setattr(
+        QtWidgets.QInputDialog, "getText", staticmethod(mock_getText)
+    )
+    monkeypatch.setattr(
+        QtWidgets.QMessageBox, "warning",
+        staticmethod(lambda *args, **kwargs: True)
+    )
+
+    initial_setups = set(bd.calculation_setups.keys())
+
+    actions.CSNew.run()
+
+    # No new setup should be created
+    assert set(bd.calculation_setups.keys()) == initial_setups
+    assert call_count[0] == 2  # Dialog should have been shown twice
+
+
+def test_cs_duplicate_retry_on_duplicate(monkeypatch, basic_database):
+    """Test CSDuplicate retries with same name when duplicate entered."""
+    cs_name = "basic_calculation_setup"
+    duplicate_name = "cs_duplicate"
+
+    # Simulate user first entering an existing name, then a new name
+    call_count = [0]
+
+    def mock_getText(*args, **kwargs):
+        call_count[0] += 1
+        if call_count[0] == 1:
+            return (cs_name, True)  # First attempt - duplicate
+        else:
+            return (duplicate_name, True)  # Second attempt - unique name
+
+    monkeypatch.setattr(
+        QtWidgets.QInputDialog, "getText", staticmethod(mock_getText)
+    )
+    monkeypatch.setattr(
+        QtWidgets.QMessageBox, "warning",
+        staticmethod(lambda *args, **kwargs: True)
+    )
+
+    assert cs_name in bd.calculation_setups
+    assert duplicate_name not in bd.calculation_setups
+
+    actions.CSDuplicate.run(cs_name)
+
+    assert cs_name in bd.calculation_setups
+    assert duplicate_name in bd.calculation_setups
+    assert call_count[0] == 2  # Dialog should have been shown twice
+
+
+def test_cs_rename_retry_on_duplicate(monkeypatch, basic_database):
+    """Test CSRename retries with same name when duplicate entered."""
+    cs_name = "basic_calculation_setup"
+    existing_name = "another_setup"
+    renamed_cs = "cs_renamed"
+
+    # Create another setup to test duplicate detection
+    bd.calculation_setups[existing_name] = {"inv": [], "ia": []}
+
+    # Simulate user first entering an existing name, then a new name
+    call_count = [0]
+
+    def mock_getText(*args, **kwargs):
+        call_count[0] += 1
+        if call_count[0] == 1:
+            return (existing_name, True)  # First attempt - duplicate
+        else:
+            return (renamed_cs, True)  # Second attempt - unique name
+
+    monkeypatch.setattr(
+        QtWidgets.QInputDialog, "getText", staticmethod(mock_getText)
+    )
+    monkeypatch.setattr(
+        QtWidgets.QMessageBox, "warning",
+        staticmethod(lambda *args, **kwargs: True)
+    )
+
+    assert cs_name in bd.calculation_setups
+    assert existing_name in bd.calculation_setups
+    assert renamed_cs not in bd.calculation_setups
+
+    actions.CSRename.run(cs_name)
+
+    assert cs_name not in bd.calculation_setups
+    assert existing_name in bd.calculation_setups
+    assert renamed_cs in bd.calculation_setups
+    assert call_count[0] == 2  # Dialog should have been shown twice
+
+
+def test_cs_rename_prepopulates_old_name(monkeypatch, basic_database):
+    """Test CSRename pre-populates the dialog with the old name."""
+    cs_name = "basic_calculation_setup"
+    renamed_cs = "cs_renamed"
+
+    captured_kwargs = []
+
+    def mock_getText(*args, **kwargs):
+        captured_kwargs.append(kwargs)
+        return (renamed_cs, True)
+
+    monkeypatch.setattr(
+        QtWidgets.QInputDialog, "getText", staticmethod(mock_getText)
+    )
+
+    actions.CSRename.run(cs_name)
+
+    # Check that the text parameter was set to the old name
+    assert len(captured_kwargs) == 1
+    assert captured_kwargs[0].get('text') == cs_name