From 6121962df54d3f7448335e6b0ccc35a997c24d01 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Thu, 2 May 2024 17:21:19 +0530 Subject: [PATCH] fixup!: Update message, add tests --- openedx_webhooks/bot_comments.py | 35 +++++++-------- .../github_community_pr_comment.md.j2 | 11 +++-- tests/test_pull_request_opened.py | 44 +++++++++++++++++-- 3 files changed, 64 insertions(+), 26 deletions(-) diff --git a/openedx_webhooks/bot_comments.py b/openedx_webhooks/bot_comments.py index fef44800..2d49243f 100644 --- a/openedx_webhooks/bot_comments.py +++ b/openedx_webhooks/bot_comments.py @@ -5,21 +5,21 @@ import binascii import json import re +from collections import namedtuple from enum import Enum, auto -from typing import Dict +from typing import Dict, Literal import arrow -import yaml from flask import render_template from openedx_webhooks.info import ( get_jira_server_info, is_draft_pull_request, - pull_request_has_cla, read_github_file, + pull_request_has_cla, + get_catalog_info, ) from openedx_webhooks.types import JiraId, PrDict -from openedx_webhooks.utils import memoize_timed # Author association values for which we should consider the author new GITHUB_NEW_AUTHOR_ASSOCIATIONS = ( @@ -94,28 +94,24 @@ def is_comment_kind(kind: BotComment, text: str) -> bool: return any(snip in text for snip in BOT_COMMENT_INDICATORS[kind]) -@memoize_timed(minutes=30) -def get_repo_catalog(repo_full_name: str) -> dict: - """ - Get and load the catalog-info.yaml file for the repo. - """ - return yaml.safe_load(read_github_file(repo_full_name, "catalog-info.yaml", "")) +Lifecycle = Literal["experimental", "production", "deprecated"] +RepoSpec: (str | None, Lifecycle | None) = namedtuple('RepoSpec', ['owner', 'lifecycle']) -def _get_repo_owner(repo_full_name: str) -> str | None: +def _get_repo_spec(repo_full_name: str) -> RepoSpec: """ Get the owner of the repo from its catalog-info.yaml file. """ - repo_catalog = get_repo_catalog(repo_full_name) - if not repo_catalog: - return None - owner = repo_catalog["spec"]["owner"] - owner_type = "group" + catalog_info = get_catalog_info(repo_full_name) + if not catalog_info: + return RepoSpec(None, None) + owner = catalog_info["spec"].get("owner", "") + owner_type = None if ":" in owner: owner_type, owner = owner.split(":") if owner_type == "group": owner = f"openedx/{owner}" - return owner + return RepoSpec(owner, catalog_info["spec"]["lifecycle"]) def github_community_pr_comment(pull_request: PrDict) -> str: @@ -128,7 +124,7 @@ def github_community_pr_comment(pull_request: PrDict) -> str: * contain a link to our process documentation """ is_first_time = pull_request.get("author_association", None) in GITHUB_NEW_AUTHOR_ASSOCIATIONS - owner = _get_repo_owner(pull_request["base"]["repo"]["full_name"]) + spec = _get_repo_spec(pull_request["base"]["repo"]["full_name"]) return render_template( "github_community_pr_comment.md.j2", @@ -137,7 +133,8 @@ def github_community_pr_comment(pull_request: PrDict) -> str: is_draft=is_draft_pull_request(pull_request), is_merged=pull_request.get("merged", False), is_first_time=is_first_time, - owner=owner, + owner=spec.owner, + lifecycle=spec.lifecycle, ) diff --git a/openedx_webhooks/templates/github_community_pr_comment.md.j2 b/openedx_webhooks/templates/github_community_pr_comment.md.j2 index c0b92b1a..eb5da04d 100644 --- a/openedx_webhooks/templates/github_community_pr_comment.md.j2 +++ b/openedx_webhooks/templates/github_community_pr_comment.md.j2 @@ -1,4 +1,4 @@ -Thanks for the pull request, `@{{ user }}`! +Thanks for the pull request, @{{ user }}! ### What's next? @@ -70,8 +70,13 @@ Your PR is currently marked as a draft. After completing the steps above, updat {% if owner %} This repository is currently maintained by `@{{ owner }}`. Tag them in a comment and let them know that your changes are ready for review. -{% else %} -This repository is currently unmaintained. To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review: +{% else %} +{% if lifecycle == 'production' %} +This repository has no maintainer (yet). +{% else %} +This repository is currently unmaintained. +{% endif %} +To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review: 1. On the right-hand side of the PR, find the Contributions project, click the caret in the top right corner to expand it, and check the "Primary PM" field for the name of your PM. 2. Find their GitHub handle [here](https://openedx.atlassian.net/wiki/spaces/COMM/pages/3548807177/Community+Contributions+Project+Manager#Current-OSPR-Project-Managers). diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py index c20c98f0..a49a49c2 100644 --- a/tests/test_pull_request_opened.py +++ b/tests/test_pull_request_opened.py @@ -1,9 +1,11 @@ """Tests of tasks/github.py:pull_request_changed for opening pull requests.""" import textwrap +from unittest import mock import pytest +from openedx_webhooks import settings from openedx_webhooks.bot_comments import ( BotComment, is_comment_kind, @@ -16,10 +18,8 @@ CLA_STATUS_NO_CONTRIBUTIONS, CLA_STATUS_PRIVATE, ) -from openedx_webhooks import settings from openedx_webhooks.gh_projects import pull_request_projects from openedx_webhooks.tasks.github import pull_request_changed - from .helpers import check_issue_link_in_markdown # These tests should run when we want to test flaky GitHub behavior. @@ -75,6 +75,42 @@ def test_pr_in_nocontrib_repo_opened(fake_github, user): assert pull_request_projects(pr.as_json()) == set() +@pytest.mark.parametrize("owner,tag", [ + ("group:arch-bom", "@openedx/arch-bom"), + ("user:feanil", "@feanil"), + ("feanil", "@feanil"), +]) +@mock.patch("openedx_webhooks.bot_comments.get_catalog_info") +def test_pr_with_owner_repo_opened(get_catalog_info, fake_github, owner, tag): + get_catalog_info.return_value = { + 'spec': {'owner': owner, 'lifecycle': 'production'} + } + pr = fake_github.make_pull_request(owner="openedx", repo="edx-platform") + result = pull_request_changed(pr.as_json()) + assert not result.jira_issues + pr_comments = pr.list_comments() + assert len(pr_comments) == 1 + body = pr_comments[0].body + assert f"This repository is currently maintained by `{tag}`" in body + +@pytest.mark.parametrize("lifecycle", ["production", "deprecated", None]) +@mock.patch("openedx_webhooks.bot_comments.get_catalog_info") +def test_pr_without_owner_repo_opened(get_catalog_info, fake_github, lifecycle): + get_catalog_info.return_value = { + 'spec': {'lifecycle': lifecycle} + } if lifecycle else None + pr = fake_github.make_pull_request(owner="openedx", repo="edx-platform") + result = pull_request_changed(pr.as_json()) + assert not result.jira_issues + pr_comments = pr.list_comments() + assert len(pr_comments) == 1 + body = pr_comments[0].body + if lifecycle == "production": + assert f"This repository has no maintainer (yet)." in body + else: + assert f"This repository is currently unmaintained." in body + + def test_pr_opened_by_bot(fake_github): fake_github.make_user(login="some_bot", type="Bot") pr = fake_github.make_pull_request(user="some_bot") @@ -99,7 +135,7 @@ def test_external_pr_opened_no_cla(fake_github): assert len(pr_comments) == 1 body = pr_comments[0].body check_issue_link_in_markdown(body, None) - assert "Thanks for the pull request, `@new_contributor`!" in body + assert "Thanks for the pull request, @new_contributor!" in body assert is_comment_kind(BotComment.NEED_CLA, body) assert is_comment_kind(BotComment.WELCOME, body) @@ -132,7 +168,7 @@ def test_external_pr_opened_with_cla(fake_github): assert len(pr_comments) == 1 body = pr_comments[0].body check_issue_link_in_markdown(body, None) - assert "Thanks for the pull request, `@tusbar`!" in body + assert "Thanks for the pull request, @tusbar!" in body assert is_comment_kind(BotComment.WELCOME, body) assert not is_comment_kind(BotComment.NEED_CLA, body)