From dc8736851eaa9e9fc80b6060c0e04c4dee82a463 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Oct 2025 12:00:47 +0000 Subject: [PATCH 1/3] Initial plan From f7e00f9b8ebc16621d8caa4a51fe9fbb54d169bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Oct 2025 12:06:20 +0000 Subject: [PATCH 2/3] Implement retry logic and pre-population for calculation setup dialogs Co-authored-by: mrvisscher <103424764+mrvisscher@users.noreply.github.com> --- .../actions/calculation_setup/cs_duplicate.py | 14 +- .../actions/calculation_setup/cs_new.py | 15 +- .../actions/calculation_setup/cs_rename.py | 16 ++- .../actions/test_calculation_setup_actions.py | 131 ++++++++++++++++++ 4 files changed, 166 insertions(+), 10 deletions(-) diff --git a/activity_browser/actions/calculation_setup/cs_duplicate.py b/activity_browser/actions/calculation_setup/cs_duplicate.py index 69a675a2e..fd743e3d5 100644 --- a/activity_browser/actions/calculation_setup/cs_duplicate.py +++ b/activity_browser/actions/calculation_setup/cs_duplicate.py @@ -34,14 +34,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..154e30904 100644 --- a/activity_browser/actions/calculation_setup/cs_new.py +++ b/activity_browser/actions/calculation_setup/cs_new.py @@ -48,14 +48,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 +77,19 @@ 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..2f043d752 100644 --- a/activity_browser/actions/calculation_setup/cs_rename.py +++ b/activity_browser/actions/calculation_setup/cs_rename.py @@ -24,24 +24,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..640fecc37 100644 --- a/tests/actions/test_calculation_setup_actions.py +++ b/tests/actions/test_calculation_setup_actions.py @@ -77,3 +77,134 @@ 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 that CSNew retries with the same name when a 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 that CSNew cancels properly when user cancels after duplicate error.""" + 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 that CSDuplicate retries with the same name when a duplicate is 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 that CSRename retries with the same name when a duplicate is 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 that 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 From 9c88e3d09758d46c722a3f1601fa056b9fc3ec86 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Oct 2025 12:09:14 +0000 Subject: [PATCH 3/3] Fix linting issues in calculation setup actions and tests Co-authored-by: mrvisscher <103424764+mrvisscher@users.noreply.github.com> --- .../actions/calculation_setup/cs_duplicate.py | 7 +- .../actions/calculation_setup/cs_new.py | 27 +++-- .../actions/calculation_setup/cs_rename.py | 7 +- .../actions/test_calculation_setup_actions.py | 108 +++++++++++------- 4 files changed, 91 insertions(+), 58 deletions(-) diff --git a/activity_browser/actions/calculation_setup/cs_duplicate.py b/activity_browser/actions/calculation_setup/cs_duplicate.py index fd743e3d5..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 diff --git a/activity_browser/actions/calculation_setup/cs_new.py b/activity_browser/actions/calculation_setup/cs_new.py index 154e30904..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 @@ -80,9 +86,10 @@ def run(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. + 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( diff --git a/activity_browser/actions/calculation_setup/cs_rename.py b/activity_browser/actions/calculation_setup/cs_rename.py index 2f043d752..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 diff --git a/tests/actions/test_calculation_setup_actions.py b/tests/actions/test_calculation_setup_actions.py index 640fecc37..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) @@ -80,111 +77,135 @@ def test_cs_rename(monkeypatch, basic_database): def test_cs_new_retry_on_duplicate(monkeypatch, basic_database): - """Test that CSNew retries with the same name when a duplicate is entered.""" + """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)) - + + 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 that CSNew cancels properly when user cancels after duplicate error.""" + """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)) - + + 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 that CSDuplicate retries with the same name when a duplicate is entered.""" + """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)) - + + 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 that CSRename retries with the same name when a duplicate is entered.""" + """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)) - + + 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 @@ -192,19 +213,22 @@ def mock_getText(*args, **kwargs): def test_cs_rename_prepopulates_old_name(monkeypatch, basic_database): - """Test that CSRename pre-populates the dialog with the old name.""" + """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)) - + + 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