diff --git a/activity_browser/actions/calculation_setup/cs_duplicate.py b/activity_browser/actions/calculation_setup/cs_duplicate.py index 69a675a2e..92b944c99 100644 --- a/activity_browser/actions/calculation_setup/cs_duplicate.py +++ b/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) diff --git a/activity_browser/actions/calculation_setup/cs_new.py b/activity_browser/actions/calculation_setup/cs_new.py index 3e0d9878e..1b6196870 100644 --- a/activity_browser/actions/calculation_setup/cs_new.py +++ b/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 diff --git a/activity_browser/actions/calculation_setup/cs_rename.py b/activity_browser/actions/calculation_setup/cs_rename.py index 95a638810..8fa01760c 100644 --- a/activity_browser/actions/calculation_setup/cs_rename.py +++ b/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() diff --git a/tests/actions/test_calculation_setup_actions.py b/tests/actions/test_calculation_setup_actions.py index e8f40a946..2001291e6 100644 --- a/tests/actions/test_calculation_setup_actions.py +++ b/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