Add accessibility-frontend-reviewers workflow

bot/code_review_bot/report/phabricator.py

@@ -13,6 +13,7 @@
 from code_review_bot.backend import BackendAPI
 from code_review_bot.report.base import Reporter
 from code_review_bot.revisions import PhabricatorRevision
+from code_review_bot.tasks.a11y_frontend import handle_a11y_review_group
 from code_review_bot.tasks.clang_tidy_external import ExternalTidyIssue
 from code_review_bot.tasks.coverage import CoverageIssue
 from code_review_bot.tools import treeherder
@@ -194,6 +195,9 @@ def publish(self, issues, revision, task_failures, notices, reviewers):
                     ],
                 )
 
+        # Handle the accessibility-frontend-reviewers group workflow.
+        handle_a11y_review_group(self.api, revision)
+
         # Use only new and publishable issues and patches
         # Avoid publishing a patch from a de-activated analyzer
         publishable_issues = [

bot/code_review_bot/tasks/a11y_frontend.py

@@ -0,0 +1,165 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import structlog
+
+logger = structlog.get_logger(__name__)
+
+A11Y_GROUP_SLUG = "accessibility-frontend-reviewers"
+
+# Used to detect whether the bot has already handled this revision,
+# by searching prior comments for this marker text.
+COMMENT_MARKER = "You've tagged the accessibility team for review."
+
+COMMENT_A11Y_REVIEW = """You've tagged the accessibility team for review.
+
+If you've already spoken to a team member about your request, please request review from the specific team member you spoke to.
+
+Otherwise, please do ALL of the following:
+  * Post a comment describing the specific accessibility behaviour you're looking for feedback about
+  * Attach screenshots showing behaviour before and after your patch (if this is a visual change)
+  * Post a description of screen reader output/behaviour before and after your patch (if this change affects assistive technology users)
+
+Once you have posted all of the above, please re-add the group. The group will not be removed again after you re-add it if the above conditions are met.
+"""
+
+
+def handle_a11y_review_group(api, revision):
+    """
+    Workflow for the #accessibility-frontend-reviewers group:
+
+    1. If the group is not currently tagged on the revision, do nothing.
+    2. If the group was added by an accessibility team member (i.e. a member of the
+       accessibility-frontend-reviewers project in Phabricator), leave it alone.
+    3. If this is the FIRST time a non-team-member has tagged the group (no prior bot comment):
+       - Post the instructions comment.
+       - Remove the group from reviewers.
+    4. If the author re-added the group after the bot removed it, check whether they
+       have followed the instructions by verifying at least one of:
+       - A comment was posted by the revision author after the bot's removal.
+       - An individual accessibility team member has been tagged as a reviewer.
+       If either condition holds, leave the group in place.
+       If neither holds, re-post the comment and remove the group.
+
+    Prior bot handling is detected by searching the revision's transaction history
+    for a comment authored by this bot that contains COMMENT_MARKER.
+    """
+    # Resolve the group's PHID and fetch the group's current member list.
+    data = api.search_projects(slugs=[A11Y_GROUP_SLUG], attachments={"members": True})
+    if not data or "phid" not in data[0]:
+        logger.warning(
+            "Unable to find PHID for accessibility reviewer group",
+            slug=A11Y_GROUP_SLUG,
+        )
+        return
+
+    group_phid = data[0]["phid"]
+    team_member_phids = {
+        m["phid"]
+        for m in data[0].get("attachments", {}).get("members", {}).get("members", [])
+    }
+
+    # Verify the group is tagged for the current revision.
+    revision_data = api.load_revision(
+        rev_id=revision.phabricator_id, attachments={"reviewers": True}
+    )
+    group_reviewer_entry = next(
+        (
+            r
+            for r in revision_data.get("attachments", {})
+            .get("reviewers", {})
+            .get("reviewers", [])
+            if r["reviewerPHID"] == group_phid
+        ),
+        None,
+    )
+
+    if group_reviewer_entry is None:
+        return
+
+    # If an accessibility team member added the group, leave the group assigned and exit the workflow.
+    actor_phid = group_reviewer_entry.get("actorPHID")
+    if actor_phid and actor_phid in team_member_phids:
+        logger.info(
+            "Accessibility reviewer group added by team member, taking no action",
+            revision_id=revision.phabricator_id,
+            actor_phid=actor_phid,
+        )
+        return
+
+    # Group is present and was added by a non-team-member. Search the revision's
+    # transaction history for a prior bot comment containing COMMENT_MARKER.
+    # If found, the bot has already handled this revision once (this is a re-add).
+    bot_phid = api.user["phid"]
+    all_reviewers = (
+        revision_data.get("attachments", {}).get("reviewers", {}).get("reviewers", [])
+    )
+    transactions = api.request(
+        "transaction.search", objectIdentifier=revision.phabricator_phid
+    )
+    transaction_data = transactions.get("data", [])
+
+    bot_comment_timestamp = None
+    for transaction in transaction_data:
+        if transaction.get("authorPHID") != bot_phid:
+            continue
+        for comment in transaction.get("comments", []):
+            if COMMENT_MARKER in comment.get("content", {}).get("raw", ""):
+                bot_comment_timestamp = transaction["dateCreated"]
+                break
+        if bot_comment_timestamp is not None:
+            break
+
+    if bot_comment_timestamp is not None:
+        # This is a re-add by a non-team-member (team member adds are handled above).
+        # Verify the author has followed the instructions before allowing the group to stay.
+
+        # Check 1: did the revision author post a comment after the bot's removal?
+        author_phid = revision_data["fields"]["authorPHID"]
+        has_followup_comment = any(
+            any(c.get("content", {}).get("raw") for c in tx.get("comments", []))
+            for tx in transaction_data
+            if tx.get("authorPHID") == author_phid
+            and tx.get("dateCreated", 0) > bot_comment_timestamp
+        )
+
+        # Check 2: has an individual accessibility team member been tagged?
+        current_reviewer_phids = {r["reviewerPHID"] for r in all_reviewers}
+        has_individual_team_reviewer = bool(current_reviewer_phids & team_member_phids)
+
+        if has_followup_comment or has_individual_team_reviewer:
+            logger.info(
+                "Accessibility reviewer group legitimately re-added, taking no action",
+                revision_id=revision.phabricator_id,
+                has_followup_comment=has_followup_comment,
+                has_individual_team_reviewer=has_individual_team_reviewer,
+            )
+            return
+
+        # Premature re-add: re-post the comment so the author knows why the group
+        # is being removed again, then remove it.
+        logger.info(
+            "Accessibility reviewer group re-added without required context, removing group again",
+            revision_id=revision.phabricator_id,
+        )
+        api.comment(revision.phabricator_id, COMMENT_A11Y_REVIEW)
+        api.edit_revision(
+            revision.phabricator_id,
+            [{"type": "reviewers.remove", "value": [group_phid]}],
+        )
+        return
+
+    # First time the group has been tagged by a non-team-member: post the
+    # instructions comment and remove the group.
+    logger.info(
+        "Accessibility reviewer group tagged for first time, posting comment and removing group",
+        revision_id=revision.phabricator_id,
+    )
+
+    api.comment(revision.phabricator_id, COMMENT_A11Y_REVIEW)
+
+    api.edit_revision(
+        revision.phabricator_id,
+        [{"type": "reviewers.remove", "value": [group_phid]}],
+    )

bot/tests/conftest.py

@@ -969,6 +969,14 @@ def search_projects(self, request):
             result = [
                 {"phid": "PHID-123456789-TGReviewers", "slug": "taskgraph-reviewers"}
             ]
+        elif params["constraints"]["slugs"] == ["accessibility-frontend-reviewers"]:
+            result = [
+                {
+                    "phid": "PHID-123456789-A11yReviewers",
+                    "slug": "accessibility-frontend-reviewers",
+                    "attachments": {"members": {"members": []}},
+                }
+            ]
 
         return (
             201,

bot/tests/test_a11y_frontend.py

@@ -0,0 +1,192 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from code_review_bot.tasks.a11y_frontend import (
+    A11Y_GROUP_SLUG,
+    COMMENT_A11Y_REVIEW,
+    COMMENT_MARKER,
+    handle_a11y_review_group,
+)
+
+GROUP_PHID = "PHID-PROJ-A11yReviewers"
+TEAM_MEMBER_PHID = "PHID-USER-TeamMember"
+NON_MEMBER_PHID = "PHID-USER-NonMember"
+BOT_PHID = "PHID-USER-Bot"
+REVISION_ID = 51
+REVISION_PHID = "PHID-DREV-test"
+
+
+@pytest.fixture
+def api():
+    mock = MagicMock()
+    mock.user = {"phid": BOT_PHID}
+    return mock
+
+
+@pytest.fixture
+def revision():
+    mock = MagicMock()
+    mock.phabricator_id = REVISION_ID
+    mock.phabricator_phid = REVISION_PHID
+    return mock
+
+
+def _group_project(members=None):
+    """Return a mock search_projects result for the a11y group."""
+    return [
+        {
+            "phid": GROUP_PHID,
+            "slug": A11Y_GROUP_SLUG,
+            "attachments": {
+                "members": {"members": [{"phid": p} for p in (members or [])]}
+            },
+        }
+    ]
+
+
+def _revision_with_reviewer(reviewer_phid, actor_phid, extra_reviewers=None):
+    """Return a mock load_revision result with the given reviewer."""
+    reviewers = [{"reviewerPHID": reviewer_phid, "actorPHID": actor_phid}]
+    for phid in extra_reviewers or []:
+        reviewers.append({"reviewerPHID": phid, "actorPHID": phid})
+    return {
+        "fields": {"authorPHID": NON_MEMBER_PHID},
+        "attachments": {"reviewers": {"reviewers": reviewers}},
+    }
+
+
+def _revision_without_reviewer():
+    """Return a mock load_revision result with no reviewers."""
+    return {
+        "fields": {"authorPHID": NON_MEMBER_PHID},
+        "attachments": {"reviewers": {"reviewers": []}},
+    }
+
+
+def _transactions(bot_comment_ts=None, author_comment_ts=None):
+    """Build a minimal transaction.search result."""
+    data = []
+    if bot_comment_ts is not None:
+        data.append(
+            {
+                "authorPHID": BOT_PHID,
+                "dateCreated": bot_comment_ts,
+                "comments": [{"content": {"raw": COMMENT_MARKER + " extra text"}}],
+            }
+        )
+    if author_comment_ts is not None:
+        data.append(
+            {
+                "authorPHID": NON_MEMBER_PHID,
+                "dateCreated": author_comment_ts,
+                "comments": [
+                    {"content": {"raw": "Here is my accessibility description."}}
+                ],
+            }
+        )
+    return {"data": data}
+
+
+def test_first_time_tagging(api, revision):
+    """First-time tag by a non-team-member: post comment and remove group."""
+    api.search_projects.return_value = _group_project(members=[])
+    api.load_revision.return_value = _revision_with_reviewer(
+        GROUP_PHID, NON_MEMBER_PHID
+    )
+    api.request.return_value = _transactions()
+
+    handle_a11y_review_group(api, revision)
+
+    api.comment.assert_called_once_with(REVISION_ID, COMMENT_A11Y_REVIEW)
+    api.edit_revision.assert_called_once_with(
+        REVISION_ID, [{"type": "reviewers.remove", "value": [GROUP_PHID]}]
+    )
+
+
+def test_team_member_tagging(api, revision):
+    """Group added by an accessibility team member: leave it in place."""
+    api.search_projects.return_value = _group_project(members=[TEAM_MEMBER_PHID])
+    api.load_revision.return_value = _revision_with_reviewer(
+        GROUP_PHID, TEAM_MEMBER_PHID
+    )
+
+    handle_a11y_review_group(api, revision)
+
+    api.comment.assert_not_called()
+    api.edit_revision.assert_not_called()
+
+
+def test_legitimate_readd_via_comment(api, revision):
+    """Re-add after the author posted a followup comment: group stays."""
+    api.search_projects.return_value = _group_project(members=[])
+    api.load_revision.return_value = _revision_with_reviewer(
+        GROUP_PHID, NON_MEMBER_PHID
+    )
+    # Bot commented at t=100, author commented at t=200 (after removal)
+    api.request.return_value = _transactions(bot_comment_ts=100, author_comment_ts=200)
+
+    handle_a11y_review_group(api, revision)
+
+    api.comment.assert_not_called()
+    api.edit_revision.assert_not_called()
+
+
+def test_legitimate_readd_via_individual_team_reviewer(api, revision):
+    """Re-add after the author tagged an individual team member: group stays."""
+    api.search_projects.return_value = _group_project(members=[TEAM_MEMBER_PHID])
+    # Group is tagged (by non-member), AND individual team member is also a reviewer
+    api.load_revision.return_value = _revision_with_reviewer(
+        GROUP_PHID, NON_MEMBER_PHID, extra_reviewers=[TEAM_MEMBER_PHID]
+    )
+    # Bot previously commented, but no author followup comment
+    api.request.return_value = _transactions(bot_comment_ts=100)
+
+    handle_a11y_review_group(api, revision)
+
+    api.comment.assert_not_called()
+    api.edit_revision.assert_not_called()
+
+
+def test_premature_readd(api, revision):
+    """Re-add without followup comment or individual team member: remove group again."""
+    api.search_projects.return_value = _group_project(members=[])
+    api.load_revision.return_value = _revision_with_reviewer(
+        GROUP_PHID, NON_MEMBER_PHID
+    )
+    # Bot previously commented, no author followup, no individual team reviewer
+    api.request.return_value = _transactions(bot_comment_ts=100)
+
+    handle_a11y_review_group(api, revision)
+
+    api.comment.assert_called_once_with(REVISION_ID, COMMENT_A11Y_REVIEW)
+    api.edit_revision.assert_called_once_with(
+        REVISION_ID, [{"type": "reviewers.remove", "value": [GROUP_PHID]}]
+    )
+
+
+def test_group_not_present(api, revision):
+    """Group not on the revision: do nothing."""
+    api.search_projects.return_value = _group_project(members=[])
+    api.load_revision.return_value = _revision_without_reviewer()
+
+    handle_a11y_review_group(api, revision)
+
+    api.comment.assert_not_called()
+    api.edit_revision.assert_not_called()
+    api.request.assert_not_called()
+
+
+def test_group_phid_not_found(api, revision):
+    """Project search returns nothing: log warning and do nothing."""
+    api.search_projects.return_value = []
+
+    handle_a11y_review_group(api, revision)
+
+    api.load_revision.assert_not_called()
+    api.comment.assert_not_called()
+    api.edit_revision.assert_not_called()