Skip to content

Commit

Permalink
fix(IssueManager): Fix when issue has no body
Browse files Browse the repository at this point in the history
[release:bugfix]
heitorpolidoro committed May 17, 2024
1 parent f8a7a13 commit 7365ad8
Showing 4 changed files with 53 additions and 97 deletions.
2 changes: 1 addition & 1 deletion src/helpers/issue_helper.py
Original file line number Diff line number Diff line change
@@ -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]]:
62 changes: 23 additions & 39 deletions src/managers/issue_manager.py
Original file line number Diff line number Diff line change
@@ -56,35 +56,34 @@ 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


@Config.call_if("issue_manager.handle_tasklist")
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):
existing_jobs[j.task] = j
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,25 +96,22 @@ 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)


@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,25 +297,21 @@ 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:
"""Update the progress in the issue comment."""
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,
5 changes: 3 additions & 2 deletions tests/helpers/test_issue_helper.py
Original file line number Diff line number Diff line change
@@ -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


81 changes: 26 additions & 55 deletions tests/managers/test_issue_manager.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 7365ad8

Please sign in to comment.