Address changes request

lean/commands/delete_project.py

@@ -12,8 +12,10 @@
 # limitations under the License.
 
 from pathlib import Path
+
 import click
-from lean.click import LeanCommand, PathParameter
+
+from lean.click import LeanCommand
 from lean.container import container
 
 
@@ -30,24 +32,30 @@ def delete_project(project: str) -> None:
     project_manager = container.project_manager()
     logger = container.logger()
 
+    project_id = None
+    try:
+        project_id = int(project)
+    except ValueError:
+        pass
+
     projects = []
     try:
-        projects = project_manager.get_projects_by_name_or_id(all_projects, project)
+        projects = project_manager.get_projects_by_name_or_id(all_projects, project_id or project)
     except RuntimeError:
-        # The project might only be local
-        logger.info(f"The project {project} was not found in the cloud. "
-                                f"It will be removed locally if it exists.")
-        pass
+        # If searching by id, we cannot try lo locate the project locally
+        if project_id is not None:
+            raise RuntimeError(f"The project with ID {project_id} was not found in the cloud.")
+
+        # The project might only be local, look for the path
+        logger.info(f"The project {project} was not found in the cloud. It will be removed locally if it exists.")
 
     full_project = next(iter(projects), None)
 
     if full_project is not None:
-        api_client = container.api_client()
         api_client.projects.delete(full_project.projectId)
 
     # Remove project locally
-    project_manager = container.project_manager()
     project_path = full_project.name if full_project is not None else project
-    project_manager.delete_project(project_path)
+    project_manager.delete_project(Path(project_path))
 
     logger.info(f"Successfully deleted project '{project_path}'")

lean/components/util/project_manager.py

@@ -18,7 +18,7 @@
 import sys
 from datetime import datetime, timezone
 from pathlib import Path
-from typing import List, Optional
+from typing import List, Optional, Union
 
 import pkg_resources
 
@@ -160,30 +160,65 @@ def create_new_project(self, project_dir: Path, language: QCLanguage) -> None:
     def delete_project(self, project_dir: Path) -> None:
         """Deletes a project directory.
 
-        :cloud_projects: all projects fetched from the cloud
+        Raises an error if the project directory does not exist.
+
         :param project_dir: the directory of the project to delete
         """
-        shutil.rmtree(project_dir)
+        if not self._directory_is_project(project_dir):
+            raise RuntimeError(f"Project directory {project_dir} is not a valid Lean project directory")
+
+        try:
+            shutil.rmtree(project_dir)
+        except FileNotFoundError:
+            raise RuntimeError(f"Failed to delete project. Could not find the specified path {project_dir}.")
 
-    def get_projects_by_name_or_id(self, cloud_projects: List[QCProject], project: Optional[str]) -> List[QCProject]:
+    def get_projects_by_name_or_id(self, cloud_projects: List[QCProject],
+                                   project: Optional[Union[str, int]]) -> List[QCProject]:
         """Returns a list of all the projects in the cloud that match the given name or id.
 
         :param cloud_projects: all projects fetched from the cloud
         :param project: the name or id of the project
+        :return: a list of all the projects in the cloud that match the given name or id
         """
         projects = []
+        search_by_id = isinstance(project, int)
 
         if project is not None:
-            project_path = Path(project).as_posix()
+            project_path = Path(project).as_posix() if not search_by_id else None
             projects = [p for p in cloud_projects
-                        if str(p.projectId) == project or Path(p.name).as_posix() == project_path]
+                        if search_by_id and p.projectId == project or
+                        not search_by_id and Path(p.name).as_posix() == project_path]
             if len(projects) == 0:
                 raise RuntimeError("No project with the given name or id exists in the cloud")
         else:
             projects = cloud_projects
 
         return projects
 
+    def _directory_is_project(self, project_dir: Path) -> bool:
+        """Checks if a directory is a project.
+
+        :param project_dir: the directory of the project to check
+        """
+        if not project_dir.exists():
+            return False
+
+        config_file = project_dir / "config.json"
+        if not config_file.exists():
+            return False
+
+        project_language = None
+        with open(config_file) as file:
+            project_language = json.load(file)["algorithm-language"]
+
+        if project_language is None:
+            return False
+
+        if project_language == "Python":
+            return (project_dir / "main.py").exists() and (project_dir / "research.ipynb").exists()
+
+        return (project_dir / "Main.cs").exists() and (project_dir / "research.ipynb").exists()
+
     def _generate_python_library_projects_config(self) -> None:
         """Generates the required configuration to enable autocomplete on Python library projects."""
         try:

tests/commands/test_delete_project.py

@@ -12,13 +12,15 @@
 # limitations under the License.
 
 from pathlib import Path
+from typing import List
 from unittest import mock
 
 import pytest
 from click.testing import CliRunner
 
 from lean.commands import lean
 from lean.components.api.project_client import ProjectClient
+from lean.models.api import QCProject
 from tests.test_helpers import create_fake_lean_cli_directory, create_api_project
 
 
@@ -35,13 +37,20 @@ def assert_project_does_not_exist(path: str) -> None:
     assert not project_dir.exists()
 
 
+def create_cloud_projects(count: int = 10) -> List[QCProject]:
+    return [create_api_project(i, f"Python Project {i}") for i in range(1, count + 1)]
+
+
 def test_delete_project_locally_that_does_not_have_cloud_counterpart() -> None:
     create_fake_lean_cli_directory()
 
     path = "Python Project"
     assert_project_exists(path)
 
-    with mock.patch.object(ProjectClient, 'get_all', return_value=[]) as mock_get_all,\
+    cloud_projects = create_cloud_projects()
+    assert not any(project.name == path for project in cloud_projects)
+
+    with mock.patch.object(ProjectClient, 'get_all', return_value=cloud_projects) as mock_get_all,\
          mock.patch.object(ProjectClient, 'delete', return_value=None) as mock_delete:
         result = CliRunner().invoke(lean, ["delete-project", path])
         assert result.exit_code == 0
@@ -51,14 +60,20 @@ def test_delete_project_locally_that_does_not_have_cloud_counterpart() -> None:
     assert_project_does_not_exist(path)
 
 
-@pytest.mark.parametrize("name_or_id", ["Python Project", "1"])
+@pytest.mark.parametrize("name_or_id", ["Python Project", "11"])
 def test_delete_project_deletes_in_cloud(name_or_id: str) -> None:
     create_fake_lean_cli_directory()
 
-    cloud_project = create_api_project(1, "Python Project")
-    assert_project_exists(cloud_project.name)
+    path = "Python Project"
+    assert_project_exists(path)
+
+    cloud_projects = create_cloud_projects(10)
+    assert not any(project.name == path for project in cloud_projects)
 
-    with mock.patch.object(ProjectClient, 'get_all', return_value=[cloud_project]) as mock_get_all,\
+    cloud_project = create_api_project(len(cloud_projects) + 1, path)
+    cloud_projects.append(cloud_project)
+
+    with mock.patch.object(ProjectClient, 'get_all', return_value=cloud_projects) as mock_get_all,\
          mock.patch.object(ProjectClient, 'delete', return_value=None) as mock_delete:
         result = CliRunner().invoke(lean, ["delete-project", name_or_id])
         assert result.exit_code == 0
@@ -74,11 +89,44 @@ def test_delete_project_aborts_when_path_does_not_exist() -> None:
     path = "Non Existing Project"
     assert_project_does_not_exist(path)
 
-    with mock.patch.object(ProjectClient, 'get_all', return_value=[]) as mock_get_all,\
+    with mock.patch.object(ProjectClient, 'get_all', return_value=create_cloud_projects()) as mock_get_all,\
          mock.patch.object(ProjectClient, 'delete', return_value=None) as mock_delete:
         result = CliRunner().invoke(lean, ["delete-project", path])
         assert result.exit_code != 0
 
     mock_get_all.assert_called_once()
     mock_delete.assert_not_called()
+
+
+def test_delete_project_aborts_when_path_is_no_a_valid_project() -> None:
+    create_fake_lean_cli_directory()
+
+    path = "Non Existing Project"
     assert_project_does_not_exist(path)
+
+    with mock.patch.object(ProjectClient, 'get_all', return_value=create_cloud_projects()) as mock_get_all,\
+         mock.patch.object(ProjectClient, 'delete', return_value=None) as mock_delete:
+        result = CliRunner().invoke(lean, ["delete-project", path])
+        assert result.exit_code != 0
+
+    mock_get_all.assert_called_once()
+    mock_delete.assert_not_called()
+
+
+def test_delete_project_by_id_aborts_when_not_found_in_cloud() -> None:
+    create_fake_lean_cli_directory()
+
+    path = "Python Project"
+    assert_project_exists(path)
+
+    cloud_projects = create_cloud_projects(10)
+    project_id = str(len(cloud_projects) + 1)
+
+    with mock.patch.object(ProjectClient, 'get_all', return_value=[]) as mock_get_all,\
+         mock.patch.object(ProjectClient, 'delete', return_value=None) as mock_delete:
+        result = CliRunner().invoke(lean, ["delete-project", project_id])
+        assert result.exit_code != 0
+
+    mock_get_all.assert_called_once()
+    mock_delete.assert_not_called()
+    assert_project_exists(path)