From f91d7f08a59927e056c758291a56230a12de2cc7 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 00:56:01 +0530 Subject: [PATCH 01/17] create new discussions --- .../PENDING_REVIEW_NOTIFICATION_TEMPLATE.md | 6 +- src/github_services.py | 237 ++++++++---------- src/main.py | 49 +--- 3 files changed, 108 insertions(+), 184 deletions(-) diff --git a/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md b/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md index 69060e6..897fe6a 100644 --- a/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md +++ b/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md @@ -1,8 +1,4 @@ -Hi {{ username }}, - -It looks like you're assigned to this PR, but haven't taken any action for at least 2 days: +The following PRs are blocked on your review: {{ pr_list }} -Please review and unassign yourself from the pending PRs as soon as possible. - To avoid these messages in the future, please bookmark [this link](https://github.com/pulls/assigned) and check it daily. Thanks! \ No newline at end of file diff --git a/src/github_services.py b/src/github_services.py index 59f2c27..19b2ea3 100644 --- a/src/github_services.py +++ b/src/github_services.py @@ -203,30 +203,17 @@ def get_pull_request_dict_with_timestamp( assignee['created_at'] = parser.parse(event['created_at']) return pr_dict - @check_token -def _get_discussion_data( +def _get_repository_id( org_name: str, repo_name: str, - discussion_category: str, - discussion_title: str, -) -> Tuple[str, int]: - """Fetch discussion data from api and return corresponding discussion id and - discussion number. - """ +) -> str: + """Fetch repository id from given org and repo and return the id.""" - # The following query is written in GraphQL and is being used to fetch the category - # ids and titles from the GitHub discussions. To learn more, check this out - # https://docs.github.com/en/graphql. query = """ query ($org_name: String!, $repository: String!) { repository(owner: $org_name, name: $repository) { - discussionCategories(first: 10) { - nodes { - id - name - } - } + id } } """ @@ -244,32 +231,33 @@ def _get_discussion_data( ) data = response.json() - category_id = None - discussion_categories = ( - data['data']['repository']['discussionCategories']['nodes']) - - for category in discussion_categories: - if category['name'] == discussion_category: - category_id = category['id'] - break + repository_id = ( + data['data']['repository']['id']) - if category_id is None: + if repository_id is None: raise builtins.BaseException( - f'{discussion_category} category is missing in GitHub Discussion.') + f'{org_name}/{repo_name} doesn\'t exist.') - # The following query is written in GraphQL and is being used to fetch discussions - # from a particular GitHub discussion category. This helps to find out the discussion - # where we want to comment. To learn more, check this out - # https://docs.github.com/en/graphql. + return repository_id +@check_token +def _get_category_id( + org_name: str, + repo_name: str, + discussion_category: str +) -> str: + """Fetch discussion category id from given category name and return the id.""" + + # The following query is written in GraphQL and is being used to fetch the category + # ids and titles from the GitHub discussions. To learn more, check this out + # https://docs.github.com/en/graphql. query = """ - query ($org_name: String!, $repository: String!, $category_id: ID!) { + query ($org_name: String!, $repository: String!) { repository(owner: $org_name, name: $repository) { - discussions(categoryId: $category_id, last:10) { + discussionCategories(first: 10) { nodes { id - title - number + name } } } @@ -278,8 +266,7 @@ def _get_discussion_data( variables = { 'org_name': org_name, - 'repository': repo_name, - 'category_id': category_id + 'repository': repo_name } response = requests.post( @@ -289,52 +276,49 @@ def _get_discussion_data( timeout=TIMEOUT_SECS ) data = response.json() - discussion_id = None - discussions = data['data']['repository']['discussions']['nodes'] + category_id = None + discussion_categories = ( + data['data']['repository']['discussionCategories']['nodes']) - for discussion in discussions: - if discussion['title'] == discussion_title: - discussion_id = discussion['id'] - discussion_number = discussion['number'] + for category in discussion_categories: + if category['name'] == discussion_category: + category_id = category['id'] break - if discussion_id is None: + if category_id is None: raise builtins.BaseException( - f'Discussion with title {discussion_title} not found, please create a ' - 'discussion with that title.') - - return discussion_id, discussion_number + f'{discussion_category} category is missing in GitHub Discussion.') + return category_id -def _get_past_time(days: int=60) -> str: - """Returns the subtraction of current time and the arg passed in days.""" - return ( - datetime.datetime.now( - datetime.timezone.utc) - datetime.timedelta(days=days)).strftime( - '%Y-%m-%dT%H:%M:%SZ') -def _get_old_comment_ids( +@check_token +def _get_discussion_ids( org_name: str, repo_name: str, - discussion_number: int + discussion_category: str, ) -> List[str]: - """Return the old comment ids.""" + """Fetch discussion data from api and return corresponding discussion id and + discussion number. + """ + + category_id = _get_category_id(org_name, repo_name, discussion_category) + + # The following query is written in GraphQL and is being used to fetch discussions + # from a particular GitHub discussion category. This helps to find out the discussion + # where we want to comment. To learn more, check this out + # https://docs.github.com/en/graphql. - # The following query is written in GraphQL and is being used to fetch the oldest 50 - # comments in a existing GitHub discussions. Assuming that, this workflow will run - # twice every week, we will may not have more than 50 comments to delete. To learn - # more, check this out https://docs.github.com/en/graphql. query = """ - query ($org_name: String!, $repository: String!, $discussion_number: Int!) { + query ($org_name: String!, $repository: String!, $category_id: ID!) { repository(owner: $org_name, name: $repository) { - discussion(number: $discussion_number) { - comments(first: 50) { - nodes { - id - createdAt - } + discussions(categoryId: $category_id, last:10) { + nodes { + id + title + number } } } @@ -344,7 +328,7 @@ def _get_old_comment_ids( variables = { 'org_name': org_name, 'repository': repo_name, - 'discussion_number': discussion_number + 'category_id': category_id } response = requests.post( @@ -353,44 +337,35 @@ def _get_old_comment_ids( headers=_get_request_headers(), timeout=TIMEOUT_SECS ) - - response.raise_for_status() data = response.json() - comment_ids: List[str] = [] - - discussion_comments = ( - data['data']['repository']['discussion']['comments']['nodes'] - ) - - # Delete comments posted before this time. - delete_comments_before_in_days = _get_past_time(DELETE_COMMENTS_BEFORE_IN_DAYS) + discussions = data['data']['repository']['discussions']['nodes'] + discussion_ids = [ + discussion['id'] for discussion in discussions if discussion['id'] is not None + ] - for comment in discussion_comments: - if comment['createdAt'] < delete_comments_before_in_days: - comment_ids.append(comment['id']) - else: - break - return comment_ids + if not discussion_ids: + logging.info('No existing discussions found') + return discussion_ids -def _delete_comment(comment_id: str) -> None: +def _delete_discussion(discussion_id: str) -> None: """Delete the GitHub Discussion comment related to the comment id.""" query = """ - mutation deleteComment($comment_id: ID!) { - deleteDiscussionComment(input: {id: $comment_id}) { - clientMutationId - comment { - bodyText + mutation deleteComment($id: ID!) { + deleteDiscussion(input: {id: $discussion_id}) { + clientMutationId, + discussion { + title } } } """ variables = { - 'comment_id': comment_id + 'discussion_id': discussion_id } response = requests.post( @@ -401,19 +376,36 @@ def _delete_comment(comment_id: str) -> None: ) response.raise_for_status() +@check_token +def delete_discussions( + org_name: str, + repo_name: str, + discussion_category: str, +) -> None: + """Delete all existing discussions""" -def _post_comment(discussion_id: str, message: str) -> None: - """Post the given message in an existing discussion.""" + discussion_ids = _get_discussion_ids( + org_name, repo_name, discussion_category) - # The following code is written in GraphQL and is being used to perform a mutation - # operation. More specifically, we are using it to comment in GitHub discussion to - # let reviewers know about some of their pending tasks. To learn more, check this out: - # https://docs.github.com/en/graphql. + for discussion_id in discussion_ids: + _delete_discussion(discussion_id) + +@check_token +def create_discussion( + org_name: str, + repo_name: str, + discussion_category: str, + discussion_title: str, + discussion_body: str +): + """Create new discussion with given title and body.""" + + category_id = _get_category_id(org_name, repo_name, discussion_category) + repo_id = _get_repository_id(org_name, repo_name) query = """ - mutation post_comment($discussion_id: ID!, $comment: String!) { - addDiscussionComment(input: {discussionId: $discussion_id, body: $comment}) { - clientMutationId - comment { + mutation createDiscussion($repo_id: ID!, $category_id: ID!, $title: String!, $body: String!) { + createDiscussion(input: {repositoryId: $repo_id, categoryId: $category_id, title: $title, body: $body}) { + discussion { id } } @@ -421,8 +413,10 @@ def _post_comment(discussion_id: str, message: str) -> None: """ variables = { - 'discussion_id': discussion_id, - 'comment': message + 'repo_id': repo_id, + 'category_id': category_id, + 'title': discussion_title, + 'body': discussion_body } response = requests.post( @@ -431,38 +425,5 @@ def _post_comment(discussion_id: str, message: str) -> None: headers=_get_request_headers(), timeout=TIMEOUT_SECS ) - response.raise_for_status() - - -@check_token -def delete_discussion_comments( - org_name: str, - repo_name: str, - discussion_category: str, - discussion_title: str -) -> None: - """Delete old comments from GitHub Discussion.""" - - _, discussion_number = _get_discussion_data( - org_name, repo_name, discussion_category, discussion_title) - - comment_ids = _get_old_comment_ids(org_name, repo_name, discussion_number) - - for comment_id in comment_ids: - _delete_comment(comment_id) - - -@check_token -def add_discussion_comments( - org_name: str, - repo_name: str, - discussion_category: str, - discussion_title: str, - message: str -) -> None: - """Add comments in an existing GitHub discussion.""" - - discussion_id, _ = _get_discussion_data( - org_name, repo_name, discussion_category, discussion_title) - _post_comment(discussion_id, message) + response.raise_for_status() diff --git a/src/main.py b/src/main.py index e1eada3..77e3a74 100644 --- a/src/main.py +++ b/src/main.py @@ -58,12 +58,11 @@ TEMPLATE_PATH = '.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md' -def generate_message(username: str, pr_list: str, template_path: str=TEMPLATE_PATH) -> str: +def generate_message(pr_list: str, template_path: str=TEMPLATE_PATH) -> str: """Generates message using the template provided in PENDING_REVIEW_NOTIFICATION_TEMPLATE.md. Args: - username: str. Reviewer username. pr_list: str. List of PRs not reviewed within the maximum waiting time. template_path: str. The template file path. @@ -79,44 +78,10 @@ def generate_message(username: str, pr_list: str, template_path: str=TEMPLATE_PA with open(template_path, 'r', encoding='UTF-8') as file: message = file.read() - message = re.sub(r'\{\{ *username *\}\}', '@' + username, message) message = re.sub(r'\{\{ *pr_list *\}\}', pr_list, message) return message - -def send_notification( - username: str, - pull_requests: List[github_domain.PullRequest], - org_name: str, - repo_name: str, - discussion_category: str, - discussion_title: str -) -> None: - """Sends notification on github-discussion. - - Args: - username: str. GitHub username of the reviewer. - pull_requests: List. List of pending PRs. - org_name: str. The GitHub org name. - repo_name: str. The GitHub repo name. - discussion_category: str. Category name of the discussion. - discussion_title: str. Discussion title. - """ - pr_list_messages: List[str] = [] - for pull_request in pull_requests: - assignee = pull_request.get_assignee(username) - assert assignee is not None - pr_list_messages.append( - f'- [#{pull_request.pr_number}]({pull_request.url}) [Waiting for the ' - f'last {assignee.get_waiting_time()}]') - - message = generate_message(username, '\n'.join(pr_list_messages), TEMPLATE_PATH) - - github_services.add_discussion_comments( - org_name, repo_name, discussion_category, discussion_title, message) - - def main(args: Optional[List[str]]=None) -> None: """The main function to execute the workflow. @@ -148,12 +113,14 @@ def main(args: Optional[List[str]]=None) -> None: reviewer_to_assigned_prs = github_services.get_prs_assigned_to_reviewers( org_name, repo_name, max_wait_hours) - github_services.delete_discussion_comments( - org_name, repo_name, discussion_category, discussion_title) + github_services.delete_discussions( + org_name, repo_name, discussion_category) - for reviewer_name, prs in reviewer_to_assigned_prs.items(): - send_notification( - reviewer_name, prs, org_name, repo_name, discussion_category, discussion_title) + for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): + discussion_title = f"Pending Reviews: @{reviewer_name}" + discussion_body = generate_message(pr_list) + github_services.create_discussion( + org_name, repo_name, discussion_category, discussion_title, discussion_body) if __name__ == '__main__': From 450f1b1e2148e202f3cbeb1a847f4154a3151d13 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 00:59:34 +0530 Subject: [PATCH 02/17] revert after testing is done --- src/main.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main.py b/src/main.py index 77e3a74..2453eb4 100644 --- a/src/main.py +++ b/src/main.py @@ -93,7 +93,9 @@ def main(args: Optional[List[str]]=None) -> None: """ parsed_args = PARSER.parse_args(args=args) - org_name, repo_name = parsed_args.repo.split('/') + # org_name, repo_name = parsed_args.repo.split('/') + org_name = "oppia" + repo_name = "oppia" discussion_category = parsed_args.category discussion_title = parsed_args.title max_wait_hours = parsed_args.max_wait_hours @@ -110,14 +112,18 @@ def main(args: Optional[List[str]]=None) -> None: github_services.init_service(parsed_args.token) + reviewer_to_assigned_prs = github_services.get_prs_assigned_to_reviewers( org_name, repo_name, max_wait_hours) + org_name ="SD-13" + repo_name = "test-pending-review-notifier" github_services.delete_discussions( org_name, repo_name, discussion_category) for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): - discussion_title = f"Pending Reviews: @{reviewer_name}" + # discussion_title = f"Pending Reviews: @{reviewer_name}" + discussion_title = "test-123" discussion_body = generate_message(pr_list) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From 69d0b8de381d86e377ed74e8ac5548cb24ba09c9 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 01:24:15 +0530 Subject: [PATCH 03/17] pass PRs as string to the template --- src/main.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main.py b/src/main.py index 2453eb4..fd5e657 100644 --- a/src/main.py +++ b/src/main.py @@ -97,7 +97,6 @@ def main(args: Optional[List[str]]=None) -> None: org_name = "oppia" repo_name = "oppia" discussion_category = parsed_args.category - discussion_title = parsed_args.title max_wait_hours = parsed_args.max_wait_hours # Raise error if any of the required arguments are not provided. @@ -124,7 +123,7 @@ def main(args: Optional[List[str]]=None) -> None: for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): # discussion_title = f"Pending Reviews: @{reviewer_name}" discussion_title = "test-123" - discussion_body = generate_message(pr_list) + discussion_body = generate_message('\n'.join(pr_list), TEMPLATE_PATH) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From a2ebd9a9cac357e21a5ea01316ee100481918923 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 01:31:14 +0530 Subject: [PATCH 04/17] trying to fix: TypeError: sequence item 0: expected str instance, PullRequest found --- src/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main.py b/src/main.py index fd5e657..5c7c922 100644 --- a/src/main.py +++ b/src/main.py @@ -123,7 +123,8 @@ def main(args: Optional[List[str]]=None) -> None: for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): # discussion_title = f"Pending Reviews: @{reviewer_name}" discussion_title = "test-123" - discussion_body = generate_message('\n'.join(pr_list), TEMPLATE_PATH) + PRs = '\n'.join(pr_list) + discussion_body = generate_message(PRs, TEMPLATE_PATH) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From 37a42e9301130ace0ffa9741276cc0e0069e31cd Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 01:36:04 +0530 Subject: [PATCH 05/17] converting PRs to str --- src/main.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main.py b/src/main.py index 5c7c922..b944f96 100644 --- a/src/main.py +++ b/src/main.py @@ -123,8 +123,7 @@ def main(args: Optional[List[str]]=None) -> None: for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): # discussion_title = f"Pending Reviews: @{reviewer_name}" discussion_title = "test-123" - PRs = '\n'.join(pr_list) - discussion_body = generate_message(PRs, TEMPLATE_PATH) + discussion_body = generate_message('\n'.join(str(pr) for pr in pr_list), TEMPLATE_PATH) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From b7fe720909b8b81c99332e7cb0ce0a83b813b566 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 01:43:38 +0530 Subject: [PATCH 06/17] debug deletion --- src/github_services.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/github_services.py b/src/github_services.py index 19b2ea3..a4dbf56 100644 --- a/src/github_services.py +++ b/src/github_services.py @@ -358,7 +358,7 @@ def _delete_discussion(discussion_id: str) -> None: deleteDiscussion(input: {id: $discussion_id}) { clientMutationId, discussion { - title + title } } } @@ -375,6 +375,8 @@ def _delete_discussion(discussion_id: str) -> None: timeout=TIMEOUT_SECS ) response.raise_for_status() + print() + print(response.json) @check_token def delete_discussions( @@ -387,6 +389,7 @@ def delete_discussions( discussion_ids = _get_discussion_ids( org_name, repo_name, discussion_category) + print(discussion_ids) for discussion_id in discussion_ids: _delete_discussion(discussion_id) From 0bc1a639c3bde05c24cc1a8760ed5844e2519491 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 01:51:02 +0530 Subject: [PATCH 07/17] correct variable name in deletion query --- src/github_services.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/github_services.py b/src/github_services.py index a4dbf56..b3ef818 100644 --- a/src/github_services.py +++ b/src/github_services.py @@ -354,7 +354,7 @@ def _delete_discussion(discussion_id: str) -> None: """Delete the GitHub Discussion comment related to the comment id.""" query = """ - mutation deleteComment($id: ID!) { + mutation deleteDiscussion($discussion_id: ID!) { deleteDiscussion(input: {id: $discussion_id}) { clientMutationId, discussion { @@ -375,8 +375,7 @@ def _delete_discussion(discussion_id: str) -> None: timeout=TIMEOUT_SECS ) response.raise_for_status() - print() - print(response.json) + @check_token def delete_discussions( @@ -389,7 +388,6 @@ def delete_discussions( discussion_ids = _get_discussion_ids( org_name, repo_name, discussion_category) - print(discussion_ids) for discussion_id in discussion_ids: _delete_discussion(discussion_id) From 11a4cba33162d78433adb196c60744393d4323fa Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 13:39:32 +0530 Subject: [PATCH 08/17] test title --- src/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.py b/src/main.py index b944f96..4b51b42 100644 --- a/src/main.py +++ b/src/main.py @@ -121,8 +121,8 @@ def main(args: Optional[List[str]]=None) -> None: org_name, repo_name, discussion_category) for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): - # discussion_title = f"Pending Reviews: @{reviewer_name}" - discussion_title = "test-123" + discussion_title = f"Pending Reviews: {reviewer_name}" + # discussion_title = "test-123" discussion_body = generate_message('\n'.join(str(pr) for pr in pr_list), TEMPLATE_PATH) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From 593f427c57d9a6373d66e305c73925144d745cf8 Mon Sep 17 00:00:00 2001 From: Sujay Date: Wed, 20 Nov 2024 13:45:43 +0530 Subject: [PATCH 09/17] revert testing tweaks --- src/main.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main.py b/src/main.py index 4b51b42..b6efddd 100644 --- a/src/main.py +++ b/src/main.py @@ -93,9 +93,7 @@ def main(args: Optional[List[str]]=None) -> None: """ parsed_args = PARSER.parse_args(args=args) - # org_name, repo_name = parsed_args.repo.split('/') - org_name = "oppia" - repo_name = "oppia" + org_name, repo_name = parsed_args.repo.split('/') discussion_category = parsed_args.category max_wait_hours = parsed_args.max_wait_hours @@ -111,18 +109,14 @@ def main(args: Optional[List[str]]=None) -> None: github_services.init_service(parsed_args.token) - reviewer_to_assigned_prs = github_services.get_prs_assigned_to_reviewers( org_name, repo_name, max_wait_hours) - org_name ="SD-13" - repo_name = "test-pending-review-notifier" github_services.delete_discussions( org_name, repo_name, discussion_category) for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): - discussion_title = f"Pending Reviews: {reviewer_name}" - # discussion_title = "test-123" + discussion_title = f"Pending Reviews: @{reviewer_name}" discussion_body = generate_message('\n'.join(str(pr) for pr in pr_list), TEMPLATE_PATH) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From c2d2ecc343d7615b432ad55d78f08e1213ca6815 Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 21 Nov 2024 23:51:25 +0530 Subject: [PATCH 10/17] requested changes and mypy fixes --- .../PENDING_REVIEW_NOTIFICATION_TEMPLATE.md | 6 ++++- src/github_services.py | 11 +++++---- src/main.py | 23 ++++++++++++++----- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md b/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md index 897fe6a..27c656a 100644 --- a/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md +++ b/.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md @@ -1,4 +1,8 @@ -The following PRs are blocked on your review: +Hi {{ username }}, + +The following PRs are currently blocked on your review: {{ pr_list }} +Please review and unassign yourself from the pending PRs as soon as possible, then mark this discussion thread as 'Done'. + To avoid these messages in the future, please bookmark [this link](https://github.com/pulls/assigned) and check it daily. Thanks! \ No newline at end of file diff --git a/src/github_services.py b/src/github_services.py index b3ef818..a09138a 100644 --- a/src/github_services.py +++ b/src/github_services.py @@ -231,7 +231,7 @@ def _get_repository_id( ) data = response.json() - repository_id = ( + repository_id: str = ( data['data']['repository']['id']) if repository_id is None: @@ -277,7 +277,7 @@ def _get_category_id( ) data = response.json() - category_id = None + category_id: Optional[str] = None discussion_categories = ( data['data']['repository']['discussionCategories']['nodes']) @@ -290,6 +290,7 @@ def _get_category_id( raise builtins.BaseException( f'{discussion_category} category is missing in GitHub Discussion.') + assert category_id is not None return category_id @@ -383,7 +384,7 @@ def delete_discussions( repo_name: str, discussion_category: str, ) -> None: - """Delete all existing discussions""" + """Delete all existing discussions in the given discussion category.""" discussion_ids = _get_discussion_ids( org_name, repo_name, discussion_category) @@ -398,8 +399,8 @@ def create_discussion( discussion_category: str, discussion_title: str, discussion_body: str -): - """Create new discussion with given title and body.""" +) -> None: + """Create a new discussion with the given title and body in the given discussion category.""" category_id = _get_category_id(org_name, repo_name, discussion_category) repo_id = _get_repository_id(org_name, repo_name) diff --git a/src/main.py b/src/main.py index b6efddd..3b14796 100644 --- a/src/main.py +++ b/src/main.py @@ -22,7 +22,7 @@ import os import re -from typing import List, Optional +from typing import DefaultDict, List, Optional from src import github_domain from src import github_services @@ -58,12 +58,13 @@ TEMPLATE_PATH = '.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md' -def generate_message(pr_list: str, template_path: str=TEMPLATE_PATH) -> str: +def generate_message(username: str, pull_requests: List[github_domain.PullRequest], template_path: str=TEMPLATE_PATH) -> str: """Generates message using the template provided in PENDING_REVIEW_NOTIFICATION_TEMPLATE.md. Args: - pr_list: str. List of PRs not reviewed within the maximum waiting time. + username: str. Reviewer username. + pr_list: List[github_domain.PullRequest]. List of PullRequest not reviewed within the maximum waiting time. template_path: str. The template file path. Returns: @@ -72,13 +73,23 @@ def generate_message(pr_list: str, template_path: str=TEMPLATE_PATH) -> str: Raises: Exception. Template file is missing in the given path. """ + pr_list_messages: List[str] = [] + for pull_request in pull_requests: + assignee = pull_request.get_assignee(username) + assert assignee is not None + pr_list_messages.append( + f'- [#{pull_request.pr_number}]({pull_request.url}) [Waiting for the ' + f'last {assignee.get_waiting_time()}]') + + if not os.path.exists(template_path): raise builtins.BaseException(f'Please add a template file at: {template_path}') message = '' with open(template_path, 'r', encoding='UTF-8') as file: message = file.read() - message = re.sub(r'\{\{ *pr_list *\}\}', pr_list, message) + message = re.sub(r'\{\{ *username *\}\}', '@' + username, message) + message = re.sub(r'\{\{ *pr_list *\}\}', '\n'.join(pr_list_messages), message) return message @@ -109,7 +120,7 @@ def main(args: Optional[List[str]]=None) -> None: github_services.init_service(parsed_args.token) - reviewer_to_assigned_prs = github_services.get_prs_assigned_to_reviewers( + reviewer_to_assigned_prs: DefaultDict[str, List[github_domain.PullRequest]] = github_services.get_prs_assigned_to_reviewers( org_name, repo_name, max_wait_hours) github_services.delete_discussions( @@ -117,7 +128,7 @@ def main(args: Optional[List[str]]=None) -> None: for reviewer_name, pr_list in reviewer_to_assigned_prs.items(): discussion_title = f"Pending Reviews: @{reviewer_name}" - discussion_body = generate_message('\n'.join(str(pr) for pr in pr_list), TEMPLATE_PATH) + discussion_body = generate_message(reviewer_name, pr_list, TEMPLATE_PATH) github_services.create_discussion( org_name, repo_name, discussion_category, discussion_title, discussion_body) From 6c794b4efb297a5bf0216425a11f97a0e7aef9d7 Mon Sep 17 00:00:00 2001 From: Sujay Date: Fri, 22 Nov 2024 00:07:11 +0530 Subject: [PATCH 11/17] fix linter --- src/github_services.py | 2 +- src/main.py | 14 ++++++++++---- tests/github_services_test.py | 9 +++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/github_services.py b/src/github_services.py index a09138a..ed5219c 100644 --- a/src/github_services.py +++ b/src/github_services.py @@ -21,7 +21,7 @@ import datetime import logging -from typing import Any, Callable, DefaultDict, Dict, List, Optional, Tuple, Union +from typing import Any, Callable, DefaultDict, Dict, List, Optional, Union from dateutil import parser import requests from src import github_domain diff --git a/src/main.py b/src/main.py index 3b14796..91f6362 100644 --- a/src/main.py +++ b/src/main.py @@ -58,13 +58,18 @@ TEMPLATE_PATH = '.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md' -def generate_message(username: str, pull_requests: List[github_domain.PullRequest], template_path: str=TEMPLATE_PATH) -> str: +def generate_message( + username: str, + pull_requests: List[github_domain.PullRequest], + template_path: str=TEMPLATE_PATH +) -> str: """Generates message using the template provided in PENDING_REVIEW_NOTIFICATION_TEMPLATE.md. Args: username: str. Reviewer username. - pr_list: List[github_domain.PullRequest]. List of PullRequest not reviewed within the maximum waiting time. + pr_list: List[github_domain.PullRequest]. List of PullRequest objects not reviewed within + the maximum waiting time. template_path: str. The template file path. Returns: @@ -120,8 +125,9 @@ def main(args: Optional[List[str]]=None) -> None: github_services.init_service(parsed_args.token) - reviewer_to_assigned_prs: DefaultDict[str, List[github_domain.PullRequest]] = github_services.get_prs_assigned_to_reviewers( - org_name, repo_name, max_wait_hours) + reviewer_to_assigned_prs: DefaultDict[str, List[github_domain.PullRequest]] = ( + github_services.get_prs_assigned_to_reviewers(org_name, repo_name, max_wait_hours) + ) github_services.delete_discussions( org_name, repo_name, discussion_category) diff --git a/tests/github_services_test.py b/tests/github_services_test.py index ec7a26d..2e596eb 100644 --- a/tests/github_services_test.py +++ b/tests/github_services_test.py @@ -291,6 +291,7 @@ def test_get_prs_assigned_to_reviewers(self) -> None: self.assertEqual(mock_request.call_count, 6) + def test_get_discussion_data(self) -> None: """Test _get_discussion_data.""" @@ -464,3 +465,11 @@ def test_add_discussion_comments(self) -> None: self.assertTrue(mock_response_2.assert_called) self.assertTrue(mock_response_3.assert_called) self.assertEqual(mock_post.call_count, 3) + + +_get_repository_id +_get_category_id +_get_discussion_ids +_delete_discussion +delete_discussions +create_discussion From 0eb4a722cbec2885bec65b2b8add8c1a5ea6fe97 Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 5 Dec 2024 01:40:35 +0530 Subject: [PATCH 12/17] fix: unit tests --- src/github_services.py | 2 - src/main.py | 13 +- tests/github_services_test.py | 230 ++++++++++++++-------------------- tests/main_test.py | 164 +++++++++++------------- 4 files changed, 176 insertions(+), 233 deletions(-) diff --git a/src/github_services.py b/src/github_services.py index ed5219c..34adc6d 100644 --- a/src/github_services.py +++ b/src/github_services.py @@ -293,8 +293,6 @@ def _get_category_id( assert category_id is not None return category_id - - @check_token def _get_discussion_ids( org_name: str, diff --git a/src/main.py b/src/main.py index 91f6362..1b0bc56 100644 --- a/src/main.py +++ b/src/main.py @@ -80,11 +80,12 @@ def generate_message( """ pr_list_messages: List[str] = [] for pull_request in pull_requests: - assignee = pull_request.get_assignee(username) - assert assignee is not None - pr_list_messages.append( - f'- [#{pull_request.pr_number}]({pull_request.url}) [Waiting for the ' - f'last {assignee.get_waiting_time()}]') + assignee: Optional[github_domain.Assignee] = pull_request.get_assignee(username) + + if assignee is not None: + pr_list_messages.append( + f'- [#{pull_request.pr_number}]({pull_request.url}) [Waiting for the ' + f'last {assignee.get_waiting_time()}]') if not os.path.exists(template_path): @@ -114,7 +115,7 @@ def main(args: Optional[List[str]]=None) -> None: max_wait_hours = parsed_args.max_wait_hours # Raise error if any of the required arguments are not provided. - required_args = ['max_wait_hours', 'discussion_category', 'discussion_title'] + required_args = ['max_wait_hours', 'discussion_category'] for arg in required_args: if arg is None: raise builtins.BaseException(f'Please provide {arg} argument.') diff --git a/tests/github_services_test.py b/tests/github_services_test.py index 2e596eb..660552f 100644 --- a/tests/github_services_test.py +++ b/tests/github_services_test.py @@ -62,71 +62,63 @@ def _get_past_time(self, hours: int=0) -> str: def setUp(self) -> None: self.org_name = 'orgName' self.repo_name = 'repo' - self.discussion_category = 'category' - self.discussion_title = 'title' - self.response_for_get_categories = { - "data": { - "repository": { - "discussionCategories": { - "nodes": [ - { - "id": "test_category_id_1", - "name": "test_category_name_1" - }, + self.discussion_category = 'test_category_name_1' + self.discussion_title = 'Pending Reviews: User-1' + self.discussion_body = 'body' + self.response_for_get_repository_id = { + 'data': { + 'repository': { + 'id': 'test_repository_id' + } + } + } + self.response_for_get_category_id = { + 'data': { + 'repository': { + 'discussionCategories': { + 'nodes': [ { - "id": "test_category_id_2", - "name": "test_category_name_2" + 'id': 'test_category_id_1', + 'name': 'test_category_name_1' + },{ + 'id': 'test_category_id_2', + 'name': 'test_category_name_2' } ] } } } } - self.response_for_get_discussion = { + self.response_for_get_discussion_ids = { 'data': { 'repository': { 'discussions': { 'nodes': [ { 'id': 'test_discussion_id_1', - 'title': 'test_discussion_title_1', - 'number': 12345 + 'title': 'Pending Reviews: User-1', + 'number': 65 } ] } } } } - self.response_for_get_old_comment_ids = { + self.response_for_delete_discussion = { 'data': { - 'repository': { + 'deleteDiscussion': { + 'clientMutationId': 'null', 'discussion': { - 'comments': { - 'nodes': [ - { - 'id': 'test_comment_id_2', - 'createdAt': '2022-05-05T11:44:00Z' - } - ] - } + 'title': 'Pending Reviews: User-1' } } } } - self.response_for_delete_comment = { + self.response_for_create_discussion = { 'data': { - 'deleteDiscussionComment': { - 'clientMutationId': 'test_id' - } - } - } - # Here we use type Any because this response is hard to annotate in a typedDict. - self.response_for_post_comment: Dict[str, Any] = { - 'data': { - 'addDiscussionComment': { - 'clientMutationId': 'test_id', - 'comment': { - 'id': 'test_discussion_id_1' + 'createDiscussion': { + 'discussion': { + 'id': 'D_kwDOJclmXc4AdCMs' } } } @@ -292,19 +284,15 @@ def test_get_prs_assigned_to_reviewers(self) -> None: self.assertEqual(mock_request.call_count, 6) - def test_get_discussion_data(self) -> None: - """Test _get_discussion_data.""" + def test_get_repository_id(self) -> None: + """Test _get_repository_id.""" - mock_response_for_get_categories = self.mock_post_requests( - self.response_for_get_categories) - mock_response_for_get_discussion = self.mock_post_requests( - self.response_for_get_discussion) + mock_response_for_get_repository_id = self.mock_post_requests( + self.response_for_get_repository_id) - self.assertTrue(mock_response_for_get_categories.assert_not_called) - self.assertTrue(mock_response_for_get_discussion.assert_not_called) + self.assertTrue(mock_response_for_get_repository_id.assert_not_called) mock_response = [ - mock_response_for_get_categories, - mock_response_for_get_discussion + mock_response_for_get_repository_id ] with requests_mock.Mocker() as mock_requests: @@ -313,22 +301,19 @@ def test_get_discussion_data(self) -> None: with mock.patch('requests.post', side_effect=mock_response) as mock_post: - mocked_response = github_services._get_discussion_data( + mocked_response = github_services._get_repository_id( self.org_name, - self.repo_name, - 'test_category_name_1', - 'test_discussion_title_1' + self.repo_name ) - self.assertTrue(mock_response_for_get_categories.assert_called_once) - self.assertTrue(mock_response_for_get_discussion.assert_called_once) - self.assertEqual(mock_post.call_count, 2) - self.assertEqual(mocked_response, ('test_discussion_id_1', 12345)) + self.assertTrue(mock_response_for_get_repository_id.assert_called_once) + self.assertEqual(mock_post.call_count, 1) + self.assertEqual(mocked_response, 'test_repository_id') - def test_get_old_comment_ids(self) -> None: - """Test _get_old_comment_ids.""" + def test_get_category_id(self) -> None: + """Test _get_category_id.""" mock_response = mock.Mock() - mock_response.json.return_value = self.response_for_get_old_comment_ids + mock_response.json.return_value = self.response_for_get_category_id self.assertTrue(mock_response.assert_not_called) with requests_mock.Mocker() as mock_requests: @@ -337,40 +322,51 @@ def test_get_old_comment_ids(self) -> None: with mock.patch('requests.post', side_effect=[mock_response]) as mock_post: - mocked_response = github_services._get_old_comment_ids( + mocked_response = github_services._get_category_id( self.org_name, self.repo_name, - 12345 + 'test_category_name_1' ) self.assertTrue(mock_response.assert_called_once) self.assertEqual(mock_post.call_count, 1) - self.assertEqual(mocked_response, ['test_comment_id_2']) + self.assertEqual(mocked_response, 'test_category_id_1') - def test_delete_comment(self) -> None: - """Test delete_comment.""" + def test_get_discussion_ids(self) -> None: + """Test _get_discussion_ids.""" - token = 'my_github_token' - github_services.init_service(token) - - mock_response = mock.Mock() - mock_response.json.return_value = self.response_for_delete_comment - self.assertTrue(mock_response.assert_not_called) + mock_response_1 = mock.Mock() + mock_response_1.json.return_value = self.response_for_get_category_id + mock_response_2 = mock.Mock() + mock_response_2.json.return_value = self.response_for_get_discussion_ids + self.assertTrue(mock_response_1.assert_not_called) + self.assertTrue(mock_response_2.assert_not_called) with requests_mock.Mocker() as mock_requests: self.mock_all_get_requests(mock_requests) - with mock.patch('requests.post', side_effect=[mock_response]) as mock_post: + with mock.patch('requests.post', side_effect=[ + mock_response_1, mock_response_2]) as mock_post: - github_services._delete_comment('test_comment_id_2') - self.assertTrue(mock_response.assert_called) - self.assertEqual(mock_post.call_count, 1) + mocked_response = github_services._get_discussion_ids( + self.org_name, + self.repo_name, + 'test_category_name_1' + ) + self.assertTrue(mock_response_1.assert_called_once) + self.assertTrue(mock_response_2.assert_called_once) + self.assertEqual(mock_post.call_count, 2) + self.assertEqual( + mocked_response, [ + 'test_discussion_id_1' + ] + ) - def test_post_comment(self) -> None: - """Test post comment.""" + def test_delete_discussion(self) -> None: + """Test _delete_discussion.""" mock_response = mock.Mock() - mock_response.json.return_value = self.response_for_post_comment + mock_response.json.return_value = self.response_for_delete_discussion self.assertTrue(mock_response.assert_not_called) with requests_mock.Mocker() as mock_requests: @@ -379,70 +375,40 @@ def test_post_comment(self) -> None: with mock.patch('requests.post', side_effect=[mock_response]) as mock_post: - github_services._post_comment( - 'test_discussion_id_1', - 'test_message' - ) + github_services._delete_discussion('test_discussion_id_1') self.assertTrue(mock_response.assert_called_once) self.assertEqual(mock_post.call_count, 1) - def test_delete_discussion_comments(self) -> None: - """Test delete_discussion_comments function.""" + def test_delete_discussions(self) -> None: + """Test _delete_discussions.""" - token = 'my_github_token' - github_services.init_service(token) - - mock_response_1 = mock.Mock() - mock_response_1.json.return_value = self.response_for_get_categories - - mock_response_2 = mock.Mock() - mock_response_2.json.return_value = self.response_for_get_discussion - - mock_response_3 = mock.Mock() - mock_response_3.json.return_value = self.response_for_get_old_comment_ids - - mock_response_4 = mock.Mock() - mock_response_4.json.return_value = self.response_for_delete_comment - - self.assertTrue(mock_response_1.assert_not_called) - self.assertTrue(mock_response_2.assert_not_called) - self.assertTrue(mock_response_3.assert_not_called) - self.assertTrue(mock_response_4.assert_not_called) + mock_response = mock.Mock() + mock_response.json.return_value = self.response_for_delete_discussion + self.assertTrue(mock_response.assert_not_called) + self.assertTrue(mock_response.assert_not_called) with requests_mock.Mocker() as mock_requests: self.mock_all_get_requests(mock_requests) - with mock.patch('requests.post', side_effect=[ - mock_response_1, mock_response_2, mock_response_3, mock_response_4]) as mock_post: + with mock.patch('requests.post', side_effect=[mock_response]) as mock_post: - github_services.delete_discussion_comments( - self.org_name, - self.repo_name, - 'test_category_name_1', - 'test_discussion_title_1' - ) - self.assertTrue(mock_response_1.assert_called) - self.assertTrue(mock_response_2.assert_called) - self.assertTrue(mock_response_3.assert_called) - self.assertTrue(mock_response_4.assert_called) - self.assertEqual(mock_post.call_count, 4) + github_services._delete_discussion('test_discussion_id_1') + self.assertTrue(mock_response.assert_called_once) + self.assertEqual(mock_post.call_count, 1) - def test_add_discussion_comments(self) -> None: - """Test discussion comments.""" + def test_create_discussion(self) -> None: + """Test create discussion.""" token = 'my_github_token' github_services.init_service(token) mock_response_1 = mock.Mock() - mock_response_1.json.return_value = self.response_for_get_categories - + mock_response_1.json.return_value = self.response_for_get_category_id mock_response_2 = mock.Mock() - mock_response_2.json.return_value = self.response_for_get_discussion - + mock_response_2.json.return_value = self.response_for_get_repository_id mock_response_3 = mock.Mock() - mock_response_3.json.return_value = self.response_for_post_comment - + mock_response_3.json.return_value = self.response_for_create_discussion self.assertTrue(mock_response_1.assert_not_called) self.assertTrue(mock_response_2.assert_not_called) self.assertTrue(mock_response_3.assert_not_called) @@ -454,22 +420,14 @@ def test_add_discussion_comments(self) -> None: with mock.patch('requests.post', side_effect=[ mock_response_1, mock_response_2, mock_response_3]) as mock_post: - github_services.add_discussion_comments( + github_services.create_discussion( self.org_name, self.repo_name, - 'test_category_name_1', - 'test_discussion_title_1', - 'test_message' + self.discussion_category, + self.discussion_title, + self.discussion_body ) self.assertTrue(mock_response_1.assert_called) self.assertTrue(mock_response_2.assert_called) self.assertTrue(mock_response_3.assert_called) self.assertEqual(mock_post.call_count, 3) - - -_get_repository_id -_get_category_id -_get_discussion_ids -_delete_discussion -delete_discussions -create_discussion diff --git a/tests/main_test.py b/tests/main_test.py index fbdeb13..0e2c2d7 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -23,6 +23,7 @@ from unittest import mock from src import github_services +from src import github_domain from src import main import requests @@ -42,13 +43,16 @@ def test_generate_message(self) -> None: file_data = mock.mock_open(read_data=self.test_template) template_path = '.github/PENDING_REVIEW_NOTIFICATION_TEMPLATE.md' with mock.patch('builtins.open', file_data): - pr_list = ( - '- [#123](https://githuburl.pull/123) [Waiting for the last 2 days, 8' - ' hours]') - response = main.generate_message('reviewerName1', pr_list, template_path) - expected_response = ( - '@reviewerName1\n- [#123](https://githuburl.pull/123) [Waiting for the last 2' - ' days, 8 hours]') + pull_requests = github_domain.PullRequest( + 'https://githuburl.pull/123', + 123, + 'user-1', + 'test-title', + [github_domain.Assignee('user-2', datetime.datetime.now())] + ) + response = main.generate_message('reviewerName1', [pull_requests], template_path) + expected_response = '@reviewerName1\n' + self.assertEqual(expected_response, response) def test_generate_message_raises_template_not_found_error(self) -> None: @@ -57,12 +61,12 @@ def test_generate_message_raises_template_not_found_error(self) -> None: file_data = mock.mock_open(read_data=self.test_template) template_path = 'invalid_path' with mock.patch('builtins.open', file_data): - pr_list = ( - '- [#123](https://githuburl.pull/123) [Waiting for the last 2 days, 8 ' - 'hours]') + pull_requests = github_domain.PullRequest( + 'https://githuburl.pull/123',123, 'user-1', 'test-title', [github_domain.Assignee('user-2', (datetime.datetime.now()))] + ) with self.assertRaisesRegex( builtins.BaseException, f'Please add a template file at: {template_path}'): - main.generate_message('reviewerName1', pr_list, template_path) + main.generate_message('reviewerName1', [pull_requests], template_path) class ModuleIntegrationTest(unittest.TestCase): @@ -80,69 +84,58 @@ def setUp(self) -> None: self.repo_name = 'repo' self.discussion_category = 'category' self.discussion_title = 'title' - self.response_for_get_categories = { - "data": { - "repository": { - "discussionCategories": { - "nodes": [ - { - "id": "test_category_id_1", - "name": "test_category_name_1" - }, + self.discussion_body = 'body' + self.response_for_get_repository_id = { + 'data': { + 'repository': { + 'id': 'test_repository_id' + } + } + } + self.response_for_get_category_ids = { + 'data': { + 'repository': { + 'discussionCategories': { + 'nodes': [ { - "id": "test_category_id_2", - "name": "test_category_name_2" + 'id': 'test_category_id_1', + 'name': 'test_category_name_1' } ] } } } } - self.response_for_get_discussion = { + self.response_for_get_discussion_ids = { 'data': { 'repository': { 'discussions': { 'nodes': [ { 'id': 'test_discussion_id_1', - 'title': 'test_discussion_title_1', - 'number': 12345 + 'title': 'Pending Reviews: User-1', + 'number': 65 } ] } } } } - self.response_for_get_old_comment_ids = { + self.response_for_delete_discussion = { 'data': { - 'repository': { + 'deleteDiscussion': { + 'clientMutationId': 'null', 'discussion': { - 'comments': { - 'nodes': [ - { - 'id': 'test_comment_id_2', - 'createdAt': '2022-05-05T11:44:00Z' - } - ] - } + 'title': 'Pending Reviews: User-1' } } } } - self.response_for_delete_comment = { - 'data': { - 'deleteDiscussionComment': { - 'clientMutationId': 'test_id' - } - } - } - # Here we use type Any because this response is hard to annotate in a typedDict. - self.response_for_post_comment: Dict[str, Any] = { + self.response_for_create_discussion = { 'data': { - 'addDiscussionComment': { - 'clientMutationId': 'test_id', - 'comment': { - 'id': 'test_discussion_id_1' + 'createDiscussion': { + 'discussion': { + 'id': 'D_kwDOJclmXc4AdCMs' } } } @@ -246,22 +239,30 @@ def mock_post_requests(self, response: Dict[str, Any]) -> mock.Mock: mocked_response.json.return_value = response return mocked_response - def test_executing_main_function_sends_notification(self) -> None: + def test_main_function(self) -> None: """Test main function to send notification.""" # Here we are mocking the POST requests that we will use in the test below. # and they are listed in the particular order they will be called. - post_requests_side_effect: List[mock.Mock] = [ - self.mock_post_requests(self.response_for_get_categories), - self.mock_post_requests(self.response_for_get_discussion), - self.mock_post_requests(self.response_for_get_old_comment_ids), - self.mock_post_requests(self.response_for_delete_comment), - self.mock_post_requests(self.response_for_get_categories), - self.mock_post_requests(self.response_for_get_discussion), - self.mock_post_requests(self.response_for_post_comment), - self.mock_post_requests(self.response_for_get_categories), - self.mock_post_requests(self.response_for_get_discussion), - self.mock_post_requests(self.response_for_post_comment) + post_requests_side_effect_1: List[mock.Mock] = [ + self.mock_post_requests(self.response_for_get_category_ids), + self.mock_post_requests(self.response_for_get_discussion_ids), + self.mock_post_requests(self.response_for_delete_discussion), + self.mock_post_requests(self.response_for_get_category_ids), + self.mock_post_requests(self.response_for_get_repository_id), + self.mock_post_requests(self.response_for_create_discussion), + ] + + post_requests_side_effect_2: List[mock.Mock] = [ + self.mock_post_requests(self.response_for_get_category_ids), + self.mock_post_requests(self.response_for_get_repository_id), + self.mock_post_requests(self.response_for_delete_discussion), + self.mock_post_requests(self.response_for_get_category_ids), + self.mock_post_requests(self.response_for_get_discussion_ids), + self.mock_post_requests(self.response_for_delete_discussion), + self.mock_post_requests(self.response_for_get_category_ids), + self.mock_post_requests(self.response_for_get_repository_id), + self.mock_post_requests(self.response_for_create_discussion), ] with requests_mock.Mocker() as mock_request: @@ -275,8 +276,7 @@ def test_executing_main_function_sends_notification(self) -> None: # to assert the response. with mock.patch( 'requests.post', side_effect=( - post_requests_side_effect + post_requests_side_effect - )) as mock_post: + post_requests_side_effect_1 + post_requests_side_effect_2)) as mock_post: self.assertEqual(mock_request.call_count, 0) self.assertEqual(mock_post.call_count, 0) @@ -286,68 +286,54 @@ def test_executing_main_function_sends_notification(self) -> None: main.main([ '--repo', 'orgName/repo', '--category', 'test_category_name_1', - '--title', 'test_discussion_title_1', + '--title', 'title', '--max-wait-hours', '20', '--token', 'githubTokenForApiRequest' ]) - response_for_get_categories = requests.post( + response_for_get_category_ids = requests.post( github_services.GITHUB_GRAPHQL_URL, timeout=( github_services.TIMEOUT_SECS)) - response_for_get_discussion = requests.post( + response_for_get_discussion_ids = requests.post( github_services.GITHUB_GRAPHQL_URL, timeout=( github_services.TIMEOUT_SECS)) - response_for_get_old_comment_ids = requests.post( + response_for_delete_discussion = requests.post( github_services.GITHUB_GRAPHQL_URL, timeout=( github_services.TIMEOUT_SECS)) - response_for_delete_comment = requests.post( + response_for_get_category_ids = requests.post( github_services.GITHUB_GRAPHQL_URL, timeout=( github_services.TIMEOUT_SECS)) - response_for_get_categories = requests.post( + response_for_get_repository_id = requests.post( github_services.GITHUB_GRAPHQL_URL, timeout=( github_services.TIMEOUT_SECS)) - response_for_get_discussion = requests.post( + response_for_create_discussion = requests.post( github_services.GITHUB_GRAPHQL_URL, timeout=( github_services.TIMEOUT_SECS)) - response_for_post_comment = requests.post( - github_services.GITHUB_GRAPHQL_URL, timeout=( - github_services.TIMEOUT_SECS)) - - self.assertEqual(mock_post.call_count, 17) + self.assertEqual(mock_post.call_count, 15) self.assertEqual(mock_request.call_count, 6) # Here we use MyPy ignore because the response is of Mock type and # Mock does not contain return_value attribute, so because of this MyPy throws an # error. Thus to avoid the error, we used ignore here. self.assertEqual( - response_for_get_categories.json.return_value, self.response_for_get_categories) # type: ignore[attr-defined] - # Here we use MyPy ignore because the response is of Mock type and - # Mock does not contain return_value attribute, so because of this MyPy throws an - # error. Thus to avoid the error, we used ignore here. - self.assertEqual( - response_for_get_discussion.json.return_value, self.response_for_get_discussion) # type: ignore[attr-defined] - # Here we use MyPy ignore because the response is of Mock type and - # Mock does not contain return_value attribute, so because of this MyPy throws an - # error. Thus to avoid the error, we used ignore here. - self.assertEqual( - response_for_get_old_comment_ids.json.return_value, self.response_for_get_old_comment_ids) # type: ignore[attr-defined] + response_for_get_category_ids.json.return_value, self.response_for_get_category_ids) # type: ignore[attr-defined] # Here we use MyPy ignore because the response is of Mock type and # Mock does not contain return_value attribute, so because of this MyPy throws an # error. Thus to avoid the error, we used ignore here. self.assertEqual( - response_for_delete_comment.json.return_value, self.response_for_delete_comment) # type: ignore[attr-defined] + response_for_get_discussion_ids.json.return_value, self.response_for_get_discussion_ids) # type: ignore[attr-defined] # Here we use MyPy ignore because the response is of Mock type and # Mock does not contain return_value attribute, so because of this MyPy throws an # error. Thus to avoid the error, we used ignore here. self.assertEqual( - response_for_get_categories.json.return_value, self.response_for_get_categories) # type: ignore[attr-defined] + response_for_delete_discussion.json.return_value, self.response_for_delete_discussion) # type: ignore[attr-defined] # Here we use MyPy ignore because the response is of Mock type and # Mock does not contain return_value attribute, so because of this MyPy throws an # error. Thus to avoid the error, we used ignore here. self.assertEqual( - response_for_get_discussion.json.return_value, self.response_for_get_discussion) # type: ignore[attr-defined] + response_for_get_repository_id.json.return_value, self.response_for_get_repository_id) # type: ignore[attr-defined] # Here we use MyPy ignore because the response is of Mock type and # Mock does not contain return_value attribute, so because of this MyPy throws an # error. Thus to avoid the error, we used ignore here. self.assertEqual( - response_for_post_comment.json.return_value, self.response_for_post_comment) # type: ignore[attr-defined] + response_for_create_discussion.json.return_value, self.response_for_create_discussion) # type: ignore[attr-defined] From 6b11a6b289ab4a2996f131c273e79acdc8190a89 Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 5 Dec 2024 01:43:50 +0530 Subject: [PATCH 13/17] testing: to not unnecessary mention --- src/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.py b/src/main.py index 1b0bc56..11a256c 100644 --- a/src/main.py +++ b/src/main.py @@ -94,7 +94,7 @@ def generate_message( with open(template_path, 'r', encoding='UTF-8') as file: message = file.read() - message = re.sub(r'\{\{ *username *\}\}', '@' + username, message) + message = re.sub(r'\{\{ *username *\}\}', + username, message) message = re.sub(r'\{\{ *pr_list *\}\}', '\n'.join(pr_list_messages), message) return message From b02cd03228314a788a2e73087d47188ffcd27e4c Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 5 Dec 2024 01:51:42 +0530 Subject: [PATCH 14/17] testing: change repo name --- src/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main.py b/src/main.py index 11a256c..684d8ea 100644 --- a/src/main.py +++ b/src/main.py @@ -130,6 +130,8 @@ def main(args: Optional[List[str]]=None) -> None: github_services.get_prs_assigned_to_reviewers(org_name, repo_name, max_wait_hours) ) + org_name = 'SD-13' + repo_name = 'test-pending-review-notifier' github_services.delete_discussions( org_name, repo_name, discussion_category) From 859f511db1a01f99a17402d0409ce63251507ea1 Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 5 Dec 2024 01:56:34 +0530 Subject: [PATCH 15/17] testing: final tweaks --- src/main.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main.py b/src/main.py index 684d8ea..3fbdf6b 100644 --- a/src/main.py +++ b/src/main.py @@ -110,7 +110,9 @@ def main(args: Optional[List[str]]=None) -> None: """ parsed_args = PARSER.parse_args(args=args) - org_name, repo_name = parsed_args.repo.split('/') + # org_name, repo_name = parsed_args.repo.split('/') + org_name = "oppia" + repo_name = "oppia" discussion_category = parsed_args.category max_wait_hours = parsed_args.max_wait_hours From c18db01e82e15631443f33c374fc7886aaabe7fd Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 5 Dec 2024 02:02:52 +0530 Subject: [PATCH 16/17] fix: syntax --- src/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.py b/src/main.py index 3fbdf6b..ddf6339 100644 --- a/src/main.py +++ b/src/main.py @@ -94,7 +94,7 @@ def generate_message( with open(template_path, 'r', encoding='UTF-8') as file: message = file.read() - message = re.sub(r'\{\{ *username *\}\}', + username, message) + message = re.sub(r'\{\{ *username *\}\}', username, message) message = re.sub(r'\{\{ *pr_list *\}\}', '\n'.join(pr_list_messages), message) return message From 19f2a10de05738a4fac38502acaaf573fad71557 Mon Sep 17 00:00:00 2001 From: Sujay Date: Thu, 5 Dec 2024 02:11:29 +0530 Subject: [PATCH 17/17] revert: testing tweaks --- src/main.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main.py b/src/main.py index ddf6339..1b0bc56 100644 --- a/src/main.py +++ b/src/main.py @@ -94,7 +94,7 @@ def generate_message( with open(template_path, 'r', encoding='UTF-8') as file: message = file.read() - message = re.sub(r'\{\{ *username *\}\}', username, message) + message = re.sub(r'\{\{ *username *\}\}', '@' + username, message) message = re.sub(r'\{\{ *pr_list *\}\}', '\n'.join(pr_list_messages), message) return message @@ -110,9 +110,7 @@ def main(args: Optional[List[str]]=None) -> None: """ parsed_args = PARSER.parse_args(args=args) - # org_name, repo_name = parsed_args.repo.split('/') - org_name = "oppia" - repo_name = "oppia" + org_name, repo_name = parsed_args.repo.split('/') discussion_category = parsed_args.category max_wait_hours = parsed_args.max_wait_hours @@ -132,8 +130,6 @@ def main(args: Optional[List[str]]=None) -> None: github_services.get_prs_assigned_to_reviewers(org_name, repo_name, max_wait_hours) ) - org_name = 'SD-13' - repo_name = 'test-pending-review-notifier' github_services.delete_discussions( org_name, repo_name, discussion_category)