Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/packages/github_api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ services:
factory: ['@Github\Client', api]
arguments: [repo]

Github\Api\GraphQL:
factory: ['@Github\Client', api]
arguments: [graphql]

Github\Api\Search:
factory: ['@Github\Client', api]
arguments: [search]
Expand Down
51 changes: 51 additions & 0 deletions src/Api/Issue/GithubIssueApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Api\Issue;

use App\Model\Repository;
use Github\Api\GraphQL;
use Github\Api\Issue;
use Github\Api\Issue\Comments;
use Github\Api\Search;
Expand All @@ -15,6 +16,7 @@ public function __construct(
private readonly Comments $issueCommentApi,
private readonly Issue $issueApi,
private readonly Search $searchApi,
private readonly GraphQL $graphqlApi,
private readonly string $botUsername,
) {
}
Expand Down Expand Up @@ -88,4 +90,53 @@ public function findStaleIssues(Repository $repository, \DateTimeImmutable $noUp
'desc',
]);
}

public function findBotComment(Repository $repository, int $issueNumber, string $search): ?string
{
$result = $this->graphqlApi->execute('
query($owner: String!, $repo: String!, $number: Int!) {
repository(owner: $owner, name: $repo) {
issueOrPullRequest(number: $number) {
... on Issue { comments(last: 100) { ...botCommentNodes } }
... on PullRequest { comments(last: 100) { ...botCommentNodes } }
}
}
}
fragment botCommentNodes on IssueCommentConnection {
nodes { id body isMinimized author { login } }
}
', [
'owner' => $repository->getVendor(),
'repo' => $repository->getName(),
'number' => $issueNumber,
]);

$nodes = $result['data']['repository']['issueOrPullRequest']['comments']['nodes'] ?? [];
foreach (array_reverse($nodes) as $comment) {
if ($comment['isMinimized'] ?? false) {
continue;
}

if ($this->botUsername !== ($comment['author']['login'] ?? null)) {
continue;
}

if (str_contains($comment['body'] ?? '', $search)) {
return $comment['id'];
}
}

return null;
}

public function minimizeComment(string $commentNodeId): void
{
$this->graphqlApi->execute('
mutation($subjectId: ID!) {
minimizeComment(input: {subjectId: $subjectId, classifier: OUTDATED}) {
minimizedComment { isMinimized }
}
}
', ['subjectId' => $commentNodeId]);
}
}
4 changes: 4 additions & 0 deletions src/Api/Issue/IssueApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,8 @@ public function findStaleIssues(Repository $repository, \DateTimeImmutable $noUp
* Close an issue and mark it as "not_planned".
*/
public function close(Repository $repository, int $issueNumber): void;

public function findBotComment(Repository $repository, int $issueNumber, string $search): ?string;

public function minimizeComment(string $commentNodeId): void;
}
9 changes: 9 additions & 0 deletions src/Api/Issue/NullIssueApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,13 @@ public function findStaleIssues(Repository $repository, \DateTimeImmutable $noUp
public function close(Repository $repository, int $issueNumber): void
{
}

public function findBotComment(Repository $repository, int $issueNumber, string $search): ?string
{
return null;
}

public function minimizeComment(string $commentNodeId): void
{
}
}
21 changes: 17 additions & 4 deletions src/Subscriber/AllowEditFromMaintainerSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,33 @@ public function __construct(
public function onPullRequest(GitHubEvent $event): void
{
$data = $event->getData();
if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) {
if (!in_array($data['action'], ['opened', 'ready_for_review', 'edited']) || ($data['pull_request']['draft'] ?? false)) {
return;
}

if ($data['repository']['full_name'] === $data['pull_request']['head']['repo']['full_name']) {
return;
}

$repository = $event->getRepository();
$pullRequestNumber = $data['pull_request']['number'];
$commentId = 'opened' !== $data['action']
? $this->commentsApi->findBotComment($repository, $pullRequestNumber, 'Allow edits from maintainer')
: null;

if ($data['pull_request']['maintainer_can_modify'] ?? true) {
if ($commentId) {
$this->commentsApi->minimizeComment($commentId);
}

return;
}

if ($data['repository']['full_name'] === $data['pull_request']['head']['repo']['full_name']) {
// Avoid duplicate comments
if ($commentId) {
return;
}

$repository = $event->getRepository();
$pullRequestNumber = $data['pull_request']['number'];
$this->commentsApi->commentOnIssue($repository, $pullRequestNumber, <<<TXT
It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Expand Down
15 changes: 14 additions & 1 deletion src/Subscriber/MismatchBranchDescriptionSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function __construct(
public function onPullRequest(GitHubEvent $event): void
{
$data = $event->getData();
if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) {
if (!in_array($data['action'], ['opened', 'ready_for_review', 'edited']) || ($data['pull_request']['draft'] ?? false)) {
return;
}

Expand All @@ -38,7 +38,20 @@ public function onPullRequest(GitHubEvent $event): void
}

$targetBranch = $data['pull_request']['base']['ref'];
$commentId = 'opened' !== $data['action']
? $this->issueApi->findBotComment($event->getRepository(), $number, 'seems your PR description refers to branch')
: null;

if ($targetBranch === $descriptionBranch) {
if ($commentId) {
$this->issueApi->minimizeComment($commentId);
}

return;
}

// Avoid duplicate comments
if ($commentId) {
return;
}

Expand Down
22 changes: 16 additions & 6 deletions src/Subscriber/UnsupportedBranchSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,11 @@ public function __construct(
public function onPullRequest(GitHubEvent $event): void
{
$data = $event->getData();
if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) {
if (!in_array($data['action'], ['opened', 'ready_for_review', 'edited']) || ($data['pull_request']['draft'] ?? false)) {
return;
}

$targetBranch = $data['pull_request']['base']['ref'];
if ($targetBranch === $data['repository']['default_branch']) {
return;
}

try {
$validBranches = $this->symfonyVersionProvider->getMaintainedVersions();
Expand All @@ -44,11 +41,24 @@ public function onPullRequest(GitHubEvent $event): void
return;
}

if (in_array($targetBranch, $validBranches)) {
$number = $data['pull_request']['number'];
$commentId = 'opened' !== $data['action']
? $this->issueApi->findBotComment($event->getRepository(), $number, 'target one of these branches instead')
: null;

if ($targetBranch === $data['repository']['default_branch'] || in_array($targetBranch, $validBranches)) {
if ($commentId) {
Comment thread
ayyoub-afwallah marked this conversation as resolved.
$this->issueApi->minimizeComment($commentId);
}

return;
}

// Avoid duplicate comments
if ($commentId) {
return;
}

$number = $data['pull_request']['number'];
$validBranchesString = implode(', ', $validBranches);
$this->issueApi->commentOnIssue($event->getRepository(), $number, <<<TXT
Hey!
Expand Down
60 changes: 60 additions & 0 deletions tests/Api/Issue/GithubIssueApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use App\Api\Issue\GithubIssueApi;
use App\Model\Repository;
use Github\Api\GraphQL;
use Github\Api\Issue;
use Github\Api\Issue\Comments;
use Github\Api\Search;
Expand Down Expand Up @@ -32,6 +33,8 @@ class GithubIssueApiTest extends TestCase

private Search|MockObject $searchApi;

private GraphQL|MockObject $graphqlApi;

private Repository $repository;

protected function setUp(): void
Expand All @@ -52,11 +55,16 @@ protected function setUp(): void
->disableOriginalConstructor()
->getMock();

$this->graphqlApi = $this->getMockBuilder(GraphQL::class)
->disableOriginalConstructor()
->getMock();

$this->api = new GithubIssueApi(
$this->resultPager,
$this->issueCommentApi,
$this->backendApi,
$this->searchApi,
$this->graphqlApi,
self::BOT_USERNAME
);

Expand Down Expand Up @@ -198,6 +206,58 @@ public function testOpenUpdatesExistingIssue()
$this->api->open($this->repository, $title, $body, $labels);
}

public function testFindBotCommentReturnsNodeId()
{
$this->graphqlApi->expects($this->once())
->method('execute')
->willReturn([
'data' => ['repository' => ['issueOrPullRequest' => ['comments' => ['nodes' => [
['id' => 'IC_user', 'body' => 'some comment', 'isMinimized' => false, 'author' => ['login' => 'user1']],
['id' => 'IC_bot123', 'body' => 'Hey! target one of these branches instead', 'isMinimized' => false, 'author' => ['login' => self::BOT_USERNAME]],
]]]]],
]);

$this->assertSame('IC_bot123', $this->api->findBotComment($this->repository, 1234, 'target one of these branches instead'));
}

public function testFindBotCommentReturnsNullWhenNotFound()
{
$this->graphqlApi->expects($this->once())
->method('execute')
->willReturn([
'data' => ['repository' => ['issueOrPullRequest' => ['comments' => ['nodes' => [
['id' => 'IC_user', 'body' => 'some comment', 'isMinimized' => false, 'author' => ['login' => 'user1']],
]]]]],
]);

$this->assertNull($this->api->findBotComment($this->repository, 1234, 'target one of these branches instead'));
}

public function testFindBotCommentSkipsMinimizedComments()
{
$this->graphqlApi->expects($this->once())
->method('execute')
->willReturn([
'data' => ['repository' => ['issueOrPullRequest' => ['comments' => ['nodes' => [
['id' => 'IC_bot123', 'body' => 'Hey! target one of these branches instead', 'isMinimized' => true, 'author' => ['login' => self::BOT_USERNAME]],
]]]]],
]);

$this->assertNull($this->api->findBotComment($this->repository, 1234, 'target one of these branches instead'));
}

public function testMinimizeComment()
{
$this->graphqlApi->expects($this->once())
->method('execute')
->with(
$this->stringContains('minimizeComment'),
['subjectId' => 'IC_bot123']
);

$this->api->minimizeComment('IC_bot123');
}

public function testFindStaleIssues()
{
$date = new \DateTimeImmutable('2022-01-01');
Expand Down
76 changes: 76 additions & 0 deletions tests/Subscriber/AllowEditFromMaintainerSubscriberTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php

namespace App\Tests\Subscriber;

use App\Api\Issue\NullIssueApi;
use App\Event\GitHubEvent;
use App\GitHubEvents;
use App\Model\Repository;
use App\Subscriber\AllowEditFromMaintainerSubscriber;
use PHPUnit\Framework\TestCase;
use Symfony\Component\EventDispatcher\EventDispatcher;

class AllowEditFromMaintainerSubscriberTest extends TestCase
{
private $issueApi;
private $repository;
private $dispatcher;

protected function setUp(): void
{
$this->issueApi = $this->createMock(NullIssueApi::class);
$this->repository = new Repository('carsonbot-playground', 'symfony', null);

$subscriber = new AllowEditFromMaintainerSubscriber($this->issueApi);

$this->dispatcher = new EventDispatcher();
$this->dispatcher->addSubscriber($subscriber);
}

public function testOnPullRequestEditedCleanup()
{
$this->issueApi->expects($this->once())
->method('findBotComment')
->with($this->repository, 1234, 'Allow edits from maintainer')
->willReturn('IC_kwNodeId666');

$this->issueApi->expects($this->once())
->method('minimizeComment')
->with('IC_kwNodeId666');

$event = new GitHubEvent([
'action' => 'edited',
'pull_request' => [
'number' => 1234,
'maintainer_can_modify' => true,
'head' => ['repo' => ['full_name' => 'contributor/symfony']],
],
'repository' => ['full_name' => 'fabpot/symfony', 'owner' => ['login' => 'fabpot'], 'name' => 'symfony'],
], $this->repository);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
}

public function testOnPullRequestEditedIdempotency()
{
$this->issueApi->expects($this->once())
->method('findBotComment')
->with($this->repository, 1234, 'Allow edits from maintainer')
->willReturn('IC_kwNodeId666');

$this->issueApi->expects($this->never())
->method('commentOnIssue');

$event = new GitHubEvent([
'action' => 'edited',
'pull_request' => [
'number' => 1234,
'maintainer_can_modify' => false,
'head' => ['repo' => ['full_name' => 'contributor/symfony']],
],
'repository' => ['full_name' => 'fabpot/symfony', 'owner' => ['login' => 'fabpot'], 'name' => 'symfony'],
], $this->repository);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
}
}
Loading
Loading