diff --git a/src/helpers/issue_helper.py b/src/helpers/issue_helper.py index 3ac5cae..7c24dac 100644 --- a/src/helpers/issue_helper.py +++ b/src/helpers/issue_helper.py @@ -10,7 +10,7 @@ def has_tasklist(issue_body: str) -> bool: """Return if the issue has a tasklist""" - return bool(re.search(r"- \[(.)] (.*)", issue_body)) + return bool(issue_body) and bool(re.search(r"- \[(.)] (.*)", issue_body)) def get_tasklist(issue_body: str) -> list[tuple[str, bool]]: diff --git a/src/managers/issue_manager.py b/src/managers/issue_manager.py index 1a59b94..ad81de7 100644 --- a/src/managers/issue_manager.py +++ b/src/managers/issue_manager.py @@ -56,10 +56,12 @@ def get_or_create_issue_job(event: IssuesEvent) -> IssueJob: @Config.call_if("issue_manager.enabled") def manage(event: IssuesEvent) -> Optional[IssueJob]: """Manage an issue or they task list.""" - if isinstance(event, (IssueOpenedEvent, IssueEditedEvent)): - return handle_task_list(event) - if isinstance(event, IssueClosedEvent): - close_sub_tasks(event) + issue = event.issue + if issue_helper.has_tasklist(issue.body): + if isinstance(event, (IssueOpenedEvent, IssueEditedEvent)): + return handle_task_list(event) + if isinstance(event, IssueClosedEvent): + close_sub_tasks(event) return None @@ -67,9 +69,7 @@ def manage(event: IssuesEvent) -> Optional[IssueJob]: def handle_task_list(event: IssuesEvent) -> Optional[IssueJob]: """Handle the task list of an issue.""" issue = event.issue - if not (tasklist := issue_helper.get_tasklist(issue.body or "")): - return None - issue_job = get_or_create_issue_job(event) + tasklist = issue_helper.get_tasklist(issue.body) existing_jobs = {} created_issues = {} for j in JobService.filter(original_issue_url=issue.url): @@ -77,14 +77,13 @@ def handle_task_list(event: IssuesEvent) -> Optional[IssueJob]: if j.issue_ref: created_issues[j.issue_ref] = j jobs = [] + for task, checked in tasklist: if task in existing_jobs: continue # issue created in a previous run if created_issue := created_issues.get(task): - JobService.update( - created_issue, checked=checked, job_status=JobStatus.PENDING - ) + JobService.update(created_issue, checked=checked, job_status=JobStatus.PENDING) else: jobs.append( Job( @@ -97,15 +96,14 @@ def handle_task_list(event: IssuesEvent) -> Optional[IssueJob]: if jobs: JobService.insert_many(jobs) + issue_job = get_or_create_issue_job(event) if issue_job.issue_job_status == IssueJobStatus.DONE: IssueJobService.update(issue_job, issue_job_status=IssueJobStatus.PENDING) return issue_job @lru_cache -def _cached_get_auth( - hook_installation_target_id: int, installation_id: int -) -> github.Auth: +def _cached_get_auth(hook_installation_target_id: int, installation_id: int) -> github.Auth: """Get the auth for the given installation, cached.""" return _get_auth(hook_installation_target_id, installation_id) @@ -113,9 +111,7 @@ def _cached_get_auth( @lru_cache def _get_gh(hook_installation_target_id: int, installation_id: int) -> github.Github: """Get the Github object for the given installation, cached""" - return github.Github( - auth=_cached_get_auth(hook_installation_target_id, installation_id) - ) + return github.Github(auth=_cached_get_auth(hook_installation_target_id, installation_id)) @lru_cache @@ -147,9 +143,7 @@ def _get_repository(issue_job: IssueJob, repository_name: str) -> Repository: @lru_cache -def _instantiate_github_class( - clazz: type[T], hook_installation_target_id: int, installation_id: int, url: str -) -> T: +def _instantiate_github_class(clazz: type[T], hook_installation_target_id: int, installation_id: int, url: str) -> T: """Instantiate a Github class, cached""" return clazz( requester=_get_requester(hook_installation_target_id, installation_id), @@ -212,9 +206,7 @@ def _get_repository_url_and_title(issue_job: IssueJob, task: str) -> tuple[str, def process_pending_jobs(issue_job: IssueJob) -> NoReturn: """Process the pending jobs separating what is a job to create an issue from a job to update an issue""" - for job in JobService.filter( - original_issue_url=issue_job.issue_url, job_status=JobStatus.PENDING - ): + for job in JobService.filter(original_issue_url=issue_job.issue_url, job_status=JobStatus.PENDING): task = job.task if job.issue_ref or is_issue_ref(task): issue_ref = job.issue_ref or task @@ -242,9 +234,7 @@ def process_pending_jobs(issue_job: IssueJob) -> NoReturn: def process_update_issue_status(issue_job: IssueJob) -> NoReturn: """Process the update issue status jobs.""" - for job in JobService.filter( - original_issue_url=issue_job.issue_url, job_status=JobStatus.UPDATE_ISSUE_STATUS - ): + for job in JobService.filter(original_issue_url=issue_job.issue_url, job_status=JobStatus.UPDATE_ISSUE_STATUS): issue = _instantiate_github_class( Issue, issue_job.hook_installation_target_id, @@ -262,9 +252,7 @@ def process_update_issue_status(issue_job: IssueJob) -> NoReturn: @Config.call_if("issue_manager.create_issues_from_tasklist") def process_create_issue(issue_job: IssueJob) -> NoReturn: """Process the create issue status jobs.""" - for job in JobService.filter( - original_issue_url=issue_job.issue_url, job_status=JobStatus.CREATE_ISSUE - ): + for job in JobService.filter(original_issue_url=issue_job.issue_url, job_status=JobStatus.CREATE_ISSUE): repository = _instantiate_github_class( Repository, issue_job.hook_installation_target_id, @@ -309,9 +297,11 @@ def close_issue_if_all_checked(issue_job: IssueJob) -> NoReturn: issue_job.installation_id, issue_job.issue_url, ) - tasklist = issue_helper.get_tasklist(issue.body) - if tasklist and all(checked for _, checked in tasklist): - issue.edit(state="closed") + issue_body = issue.body + if issue_helper.has_tasklist(issue_body): + tasklist = issue_helper.get_tasklist(issue_body) + if tasklist and all(checked for _, checked in tasklist): + issue.edit(state="closed") def process_update_progress(issue_job: IssueJob) -> NoReturn: @@ -319,15 +309,9 @@ def process_update_progress(issue_job: IssueJob) -> NoReturn: if issue_job.issue_job_status == IssueJobStatus.DONE: comment = "Job's done" else: - done = len( - JobService.filter( - original_issue_url=issue_job.issue_url, job_status=JobStatus.DONE - ) - ) + done = len(JobService.filter(original_issue_url=issue_job.issue_url, job_status=JobStatus.DONE)) total = len(JobService.filter(original_issue_url=issue_job.issue_url)) - comment = ( - f"Analyzing the tasklist [{done}/{total}]\n{markdown_progress(done, total)}" - ) + comment = f"Analyzing the tasklist [{done}/{total}]\n{markdown_progress(done, total)}" issue = _instantiate_github_class( Issue, issue_job.hook_installation_target_id, diff --git a/tests/helpers/test_issue_helper.py b/tests/helpers/test_issue_helper.py index 0f947bd..605a051 100644 --- a/tests/helpers/test_issue_helper.py +++ b/tests/helpers/test_issue_helper.py @@ -13,9 +13,10 @@ def _task_list_data(): + # "issue_body, expected_tasks, assert_fail_message" return { "argvalues": [ - ("", [], "Empty string should not have a tasklist"), + (None, [], "Empty string should not have a tasklist"), ( "This is a sample issue description.", [], @@ -62,7 +63,7 @@ def test_has_tasklist(issue_body, expected, assert_fail_message): ) def test_get_tasklist(issue_body, expected_tasks, assert_fail_message): """Test get_tasklist with various inputs using parametrize""" - result = get_tasklist(issue_body) + result = get_tasklist(issue_body or "") assert result == expected_tasks, assert_fail_message diff --git a/tests/managers/test_issue_manager.py b/tests/managers/test_issue_manager.py index 5add011..1e59054 100644 --- a/tests/managers/test_issue_manager.py +++ b/tests/managers/test_issue_manager.py @@ -53,9 +53,7 @@ def issue_helper(request): (True, True), ], ) -def test_get_or_create_issue_job( - issue_job_service_output, issue_job_exists, event, issue, issue_helper, issue_job -): +def test_get_or_create_issue_job(issue_job_service_output, issue_job_exists, event, issue, issue_helper, issue_job): if issue_job_exists: IssueJobService.insert_one(issue_job) # Now we can call the function and check its output @@ -77,24 +75,23 @@ def test_get_or_create_issue_job( @pytest.mark.parametrize( - "event, handle_task_list_called, close_sub_tasks_called", + "event, handle_task_list_called, close_sub_tasks_called, has_task_list", [ - (IssueOpenedEvent, True, False), - (IssueEditedEvent, True, False), - (IssueClosedEvent, False, True), - (None, False, False), # Any other type of event + (IssueOpenedEvent, True, False, True), + (IssueOpenedEvent, False, False, False), + (IssueEditedEvent, True, False, True), + (IssueClosedEvent, False, True, True), + (IssueClosedEvent, False, False, False), + (None, False, False, True), # Any other type of event ], ) -def test_manage( - event, - handle_task_list_called, - close_sub_tasks_called, -): +def test_manage(event, handle_task_list_called, close_sub_tasks_called, has_task_list, issue_helper): + issue_helper.has_tasklist.return_value = has_task_list with ( patch("src.managers.issue_manager.handle_task_list") as handle_task_list_mock, patch("src.managers.issue_manager.close_sub_tasks") as close_sub_tasks_mock, ): - manage(Mock(spec=event)) + manage(Mock(spec=event, issue=Mock())) assert handle_task_list_mock.called == handle_task_list_called assert close_sub_tasks_mock.called == close_sub_tasks_called @@ -123,11 +120,6 @@ def test_manage( ], IssueJobStatus.DONE, ], - [ - [], - [], - None, - ], [ [("task1", False), ("task2", False)], [ @@ -141,21 +133,17 @@ def test_manage( "All new tasks", "Existing tasks and add new task", "Editing issue, with new task", - "No task list", "No new task in task list", ], ) -def test_handle_task_list( - event, issue, tasks, existing_tasks: list[dict], issue_job_status, issue_helper -): +def test_handle_task_list(event, issue, tasks, existing_tasks: list[dict], issue_job_status, issue_helper): for existing_task in existing_tasks: JobService.insert_one(Job(original_issue_url=event.issue.url, **existing_task)) + issue_helper.has_tasklist.return_value = bool(tasks) issue_helper.get_tasklist.return_value = tasks issue_job = Mock(issue_job_status=issue_job_status) - with patch( - "src.managers.issue_manager.get_or_create_issue_job", return_value=issue_job - ): + with patch("src.managers.issue_manager.get_or_create_issue_job", return_value=issue_job): result = handle_task_list(event) if tasks: assert result == issue_job @@ -213,12 +201,8 @@ def test_handle_task_list( "owner/repo with task title", ], ) -def test_get_title_and_repository_url( - task, expected_url, expected_title, issue_job, get_repository_return, github -): - with patch( - "src.managers.issue_manager._get_repository", return_value=get_repository_return - ): +def test_get_title_and_repository_url(task, expected_url, expected_title, issue_job, get_repository_return, github): + with patch("src.managers.issue_manager._get_repository", return_value=get_repository_return): result_url, result_title = _get_repository_url_and_title(issue_job, task) assert result_url == expected_url assert result_title == expected_title @@ -270,9 +254,7 @@ def test_set_jobs_to_done(issue_job): ), ] JobService.insert_many(jobs) - with patch( - "src.managers.issue_manager.process_update_progress" - ) as process_update_progress_mock: + with patch("src.managers.issue_manager.process_update_progress") as process_update_progress_mock: set_jobs_to_done(jobs, issue_job) process_update_progress_mock.assert_called_once_with(issue_job) @@ -346,14 +328,10 @@ def test_process_jobs(issue_job_status, expected_return, issue_job): "Is issue ref (just #num)", ], ) -def test_process_pending_jobs( - task, expected_job_update_values, _get_repository_return, issue_job, github -): +def test_process_pending_jobs(task, expected_job_update_values, _get_repository_return, issue_job, github): github.Github().get_repo.return_value = _get_repository_return - JobService.insert_one( - Job(original_issue_url=issue_job.issue_url, task=task, checked=False) - ) + JobService.insert_one(Job(original_issue_url=issue_job.issue_url, task=task, checked=False)) with patch( "src.managers.issue_manager._get_repository", return_value=_get_repository_return, @@ -385,11 +363,7 @@ def test_process_update_issue_status(issue_state, checked, issue_job, final_job_ ) ) issue = Mock(state=issue_state) - with ( - patch( - "src.managers.issue_manager._instantiate_github_class", return_value=issue - ), - ): + with (patch("src.managers.issue_manager._instantiate_github_class", return_value=issue),): if final_job_status == JobStatus.ERROR: issue.edit.side_effect = UnknownObjectException(0) process_update_issue_status(issue_job) @@ -458,15 +432,18 @@ def test_process_update_issue_body(issue_job): [[("task1", False), ("task2", False)], False], [[("task1", True), ("task2", True)], True], [[("task1", True), ("task2", False)], False], + [[], False], ], ids=[ "All opened", "All closed", "1 closed", + "No tasks", ], ) def test_close_issue_if_all_checked(tasks, issue_job, issue_helper, should_close): issue = Mock() + issue_helper.has_tasklist.return_value = bool(tasks) issue_helper.get_tasklist.return_value = tasks with ( @@ -482,9 +459,7 @@ def test_close_issue_if_all_checked(tasks, issue_job, issue_helper, should_close issue.edit.assert_not_called() -@pytest.mark.parametrize( - "issue_job_status", [IssueJobStatus.DONE, IssueJobStatus.RUNNING] -) +@pytest.mark.parametrize("issue_job_status", [IssueJobStatus.DONE, IssueJobStatus.RUNNING]) def test_process_update_progress(issue_job, issue_job_status, issue_helper): issue_job.issue_job_status = issue_job_status issue = Mock() @@ -557,15 +532,11 @@ def instantiate_github_class_mock(_clazz, _hook_id, _installlation_id, issue_url call(ANY, ANY, ANY, "https://api.github.com/repos/owner/repo/issues/1"), call(ANY, ANY, ANY, "repository.url/issues/2"), call(ANY, ANY, ANY, "repository.url/issues/0"), - call( - ANY, ANY, ANY, "https://api.github.com/repos/owner/error/issues/3" - ), + call(ANY, ANY, ANY, "https://api.github.com/repos/owner/error/issues/3"), ] ) for issue in issues: if issue.state == "closed": issue.edit.assert_not_called() else: - issue.edit.assert_called_once_with( - state="closed", state_reason=event.issue.state_reason - ) + issue.edit.assert_called_once_with(state="closed", state_reason=event.issue.state_reason)