pr_labeler: refactor new_contributor_welcome code (#990)

* pr_labeler: add GlobalArgs.full_repo property

* pr_labeler: refactor new_contributor_welcome code

As of #69, the
pr_labeler responds with a welcome message when an issue or PR is opened
by a new contributor. It turns out this never actually worked properly.

The previous method that relied on Github's `author_association` flag
did not work with the app token that the pr_labeler uses. This refactors
the code to figure out whether a user is a new contributor by
searching the list of issues and PRs.

Fixes: #204

* pr_labeler: address potential race condition

hacking/pr_labeler/label.py

@@ -7,7 +7,7 @@
 import json
 import os
 import re
-from collections.abc import Collection
+from collections.abc import Callable, Collection
 from contextlib import suppress
 from functools import cached_property
 from pathlib import Path
@@ -36,6 +36,7 @@
     trim_blocks=True,
     undefined=StrictUndefined,
 )
+NEW_CONTRIBUTOR_LABEL = "new_contributor"
 
 IssueOrPrCtx = Union["IssueLabelerCtx", "PRLabelerCtx"]
 IssueOrPr = Union["github.Issue.Issue", "github.PullRequest.PullRequest"]
@@ -48,12 +49,12 @@ def log(ctx: IssueOrPrCtx, *args: object) -> None:
 
 
 def get_repo(
-    *, authed: bool = True, owner: str, repo: str
+    args: GlobalArgs, authed: bool = True
 ) -> tuple[github.Github, github.Repository.Repository]:
     gclient = github.Github(
         auth=github.Auth.Token(os.environ["GITHUB_TOKEN"]) if authed else None,
     )
-    repo_obj = gclient.get_repo(f"{owner}/{repo}")
+    repo_obj = gclient.get_repo(args.full_repo)
     return gclient, repo_obj
 
 
@@ -70,6 +71,11 @@ def get_event_info() -> dict[str, Any]:
 class GlobalArgs:
     owner: str
     repo: str
+    use_author_association: bool
+
+    @property
+    def full_repo(self) -> str:
+        return f"{self.owner}/{self.repo}"
 
 
 @dataclasses.dataclass()
@@ -79,6 +85,7 @@ class LabelerCtx:
     dry_run: bool
     event_info: dict[str, Any]
     issue: github.Issue.Issue
+    global_args: GlobalArgs
 
     TYPE: ClassVar[str]
 
@@ -211,24 +218,57 @@ def add_label_if_new(ctx: IssueOrPrCtx, labels: Collection[str] | str) -> None:
         ctx.member.add_to_labels(*labels)
 
 
-def new_contributor_welcome(ctx: IssueOrPrCtx) -> None:
+def is_new_contributor_assoc(ctx: IssueOrPrCtx) -> bool:
     """
-    Welcome a new contributor to the repo with a message and a label
+    Determine whether a user has previously contributed.
+    Requires authentication as a regular user and does not work with an app
+    token.
     """
-    # This contributor has already been welcomed!
-    if "new_contributor" in ctx.previously_labeled:
-        return
     author_association = ctx.event_member.get(
         "author_association", ctx.member.raw_data["author_association"]
     )
     log(ctx, "author_association is", author_association)
-    if author_association not in {
-        "FIRST_TIMER",
-        "FIRST_TIME_CONTRIBUTOR",
-    }:
+    return author_association in {"FIRST_TIMER", "FIRST_TIME_CONTRIBUTOR"}
+
+
+def is_new_contributor_manual(ctx: IssueOrPrCtx) -> bool:
+    """
+    Determine whether a user has previously opened an issue or PR in this repo
+    without needing special API access.
+    """
+    query_data = {
+        "repo": "ansible/ansible-documentation",
+        "author": ctx.issue.user.login,
+        # Avoid potential race condition where a new contributor opens multiple
+        # PRs or issues at once.
+        # Better to welcome twice than not at all.
+        "is": "closed",
+    }
+    issues = ctx.client.search_issues("", **query_data)
+    for issue in issues:
+        if issue.number != ctx.issue.number:
+            return False
+    return True
+
+
+def new_contributor_welcome(ctx: IssueOrPrCtx) -> None:
+    """
+    Welcome a new contributor to the repo with a message and a label
+    """
+    is_new_contributor: Callable[[IssueOrPrCtx], bool] = (
+        is_new_contributor_assoc
+        if ctx.global_args.use_author_association
+        else is_new_contributor_manual
+    )
+    if (
+        # Contributor has already been welcomed
+        NEW_CONTRIBUTOR_LABEL in ctx.previously_labeled
+        #
+        or not is_new_contributor(ctx)
+    ):
         return
     log(ctx, "Welcoming new contributor")
-    add_label_if_new(ctx, "new_contributor")
+    add_label_if_new(ctx, NEW_CONTRIBUTOR_LABEL)
     create_comment(ctx, get_data_file("docs_team_info.md"))
 
 
@@ -277,11 +317,17 @@ def warn_porting_guide_change(ctx: PRLabelerCtx) -> None:
 
 
 @APP.callback()
-def cb(*, click_ctx: typer.Context, owner: str = OWNER, repo: str = REPO):
+def cb(
+    *,
+    click_ctx: typer.Context,
+    owner: str = OWNER,
+    repo: str = REPO,
+    use_author_association: bool = False,
+):
     """
     Basic triager for ansible/ansible-documentation
     """
-    click_ctx.obj = GlobalArgs(owner, repo)
+    click_ctx.obj = GlobalArgs(owner, repo, use_author_association)
 
 
 @APP.command(name="pr")
@@ -300,9 +346,7 @@ def process_pr(
         dry_run = True
         authed = True
 
-    gclient, repo = get_repo(
-        authed=authed, owner=global_args.owner, repo=global_args.repo
-    )
+    gclient, repo = get_repo(global_args, authed)
     pr = repo.get_pull(pr_number)
     ctx = PRLabelerCtx(
         client=gclient,
@@ -311,6 +355,7 @@ def process_pr(
         dry_run=dry_run,
         event_info=get_event_info(),
         issue=pr.as_issue(),
+        global_args=global_args,
     )
     if not force_process_closed and pr.state != "open":
         log(ctx, "Refusing to process closed ticket")
@@ -337,16 +382,15 @@ def process_issue(
     if authed_dry_run:
         dry_run = True
         authed = True
-    gclient, repo = get_repo(
-        authed=authed, owner=global_args.owner, repo=global_args.repo
-    )
+    gclient, repo = get_repo(global_args, authed)
     issue = repo.get_issue(issue_number)
     ctx = IssueLabelerCtx(
         client=gclient,
         repo=repo,
         issue=issue,
         dry_run=dry_run,
         event_info=get_event_info(),
+        global_args=global_args,
     )
     if not force_process_closed and issue.state != "open":
         log(ctx, "Refusing to process closed ticket")