Skip to content
This repository has been archived by the owner on Jun 7, 2024. It is now read-only.

Commit

Permalink
Ask for feedback when a community PR is merged
Browse files Browse the repository at this point in the history
  • Loading branch information
atomiix committed Jun 23, 2020
1 parent 842fe48 commit 733829e
Show file tree
Hide file tree
Showing 10 changed files with 184 additions and 6 deletions.
4 changes: 4 additions & 0 deletions app/Resources/views/markdown/pr_merged_feedback.md.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<!-- PR_FEEDBACK_COMMENT -->
Contribution merged, congratulations!

Would you mind answering our [quick 1-minute survey](https://forms-prestashop.typeform.com/to/hNaeFP)? We would love to hear about your experience so far, it will help us improve our process for the community involved, like you. ;-)
4 changes: 4 additions & 0 deletions app/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@ services:

Github\Api\Issue\Labels:
factory: 'Github\Api\Issue:labels'

Github\Api\GraphQL:
factory: 'Github\Client:api'
arguments: [graphql]
1 change: 1 addition & 0 deletions app/config/services_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ services:
- '@Tests\AppBundle\Commits\FakeRepository'
- '@validator'
- '@AppBundle\PullRequests\Repository'
- '@AppBundle\Organizations\Repository'
- '@Psr\Log\NullLogger'

AppBundle\Issues\Listener:
Expand Down
26 changes: 26 additions & 0 deletions src/AppBundle/EventSubscriber/PullRequestSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public static function getSubscribedEvents(): array
['initBranchLabel', 254],
['initPullRequestTypeLabel', 254],
],
'pullrequestevent_closed' => [
['askForFeedback', 255],
],
];
}

Expand Down Expand Up @@ -206,4 +209,27 @@ public function removePullRequestValidationComment(GithubEvent $githubEvent)
]);
}
}

/**
* @param GitHubEvent $gitHubEvent
*
* Send a comment to an external contribution with a link to a feedback form
*/
public function askForFeedback(GitHubEvent $gitHubEvent)
{
$pullRequest = $gitHubEvent->getPullRequest();

if (!$pullRequest->isMerged()) {
return;
}

$success = $this->pullRequestsListener->askForFeedback($pullRequest);

if ($success) {
$gitHubEvent->addStatus([
'event' => 'pr_merged',
'action' => 'ask for feedback',
]);
}
}
}
27 changes: 23 additions & 4 deletions src/AppBundle/Organizations/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace AppBundle\Organizations;

use Github\Api\Organization;
use Github\Exception\RuntimeException;

class Repository
{
Expand All @@ -18,12 +19,12 @@ class Repository
/**
* @var string
*/
private $repositoryUsername;
private $repositoryOwner;

public function __construct(Organization $organizationApi, $repositoryUsername)
public function __construct(Organization $organizationApi, $repositoryOwner)
{
$this->organizationApi = $organizationApi;
$this->repositoryUsername = $repositoryUsername;
$this->repositoryOwner = $repositoryOwner;
}

/**
Expand All @@ -34,7 +35,7 @@ public function getTeams()
if (null === self::$teams) {
$teams = $this->organizationApi
->teams()
->all($this->repositoryUsername)
->all($this->repositoryOwner)
;

foreach ($teams as $team) {
Expand Down Expand Up @@ -74,4 +75,22 @@ public function getTeamMembers(string $teamName)
;
}
}

/**
* Check if a user is a member of the organisation.
*
* @param string $userLogin
*
* @return bool
*/
public function isMember(string $userLogin)
{
try {
$this->organizationApi->members()->show($this->repositoryOwner, $userLogin);

return true;
} catch (RuntimeException $e) {
return false;
}
}
}
46 changes: 46 additions & 0 deletions src/AppBundle/PullRequests/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

use AppBundle\Comments\CommentApiInterface;
use AppBundle\Commits\RepositoryInterface as CommitRepositoryInterface;
use AppBundle\Organizations\Repository as OrganizationRepository;
use AppBundle\PullRequests\RepositoryInterface as PullRequestRepositoryInterface;
use DateInterval;
use DateTime;
use Lpdigital\Github\Entity\PullRequest;
use Lpdigital\Github\Entity\User;
use Psr\Log\LoggerInterface;
Expand All @@ -15,6 +18,10 @@ class Listener
const PRESTONBOT_NAME = 'prestonBot';
const TABLE_ERROR = 'PR_TABLE_DESCRIPTION_ERROR';
const COMMIT_ERROR = 'PR_COMMIT_NAME_ERROR';
const FEEDBACK_COMMENT = 'PR_FEEDBACK_COMMENT';

const FEEDBACK_DELAY = 30; // days before asking feedback again

/**
* @var CommentApiInterface
*/
Expand All @@ -35,19 +42,25 @@ class Listener
* @var RepositoryInterface
*/
private $repository;
/**
* @var OrganizationRepository
*/
private $organizationRepository;

public function __construct(
CommentApiInterface $commentApi,
CommitRepositoryInterface $commitRepository,
ValidatorInterface $validator,
PullRequestRepositoryInterface $repository,
OrganizationRepository $organizationRepository,
LoggerInterface $logger
) {
$this->commentApi = $commentApi;
$this->commitRepository = $commitRepository;
$this->logger = $logger;
$this->validator = $validator;
$this->repository = $repository;
$this->organizationRepository = $organizationRepository;
}

/**
Expand Down Expand Up @@ -151,6 +164,39 @@ public function welcomePeople(PullRequest $pullRequest, User $sender)
return true;
}

/**
* @param PullRequest $pullRequest
*
* @return bool
*/
public function askForFeedback(PullRequest $pullRequest)
{
$author = $pullRequest->getUser()->getLogin();

if ($this->organizationRepository->isMember($author)) {
return false;
}

$now = new DateTime();
$since = $now->sub(DateInterval::createFromDateString(self::FEEDBACK_DELAY.' days'));
$response = $this->repository->getMergedFromWithCommentsFrom($author, self::PRESTONBOT_NAME, $since);
if (!empty($response['data']['search']['edges'])) {
foreach ($response['data']['search']['edges'] as $pr) {
foreach ($pr['node']['comments']['edges'] as $comment) {
if (self::PRESTONBOT_NAME === $comment['node']['author']['login']) {
if (false !== strpos($comment['node']['body'], self::FEEDBACK_COMMENT)) {
return false;
}
}
}
}
}

$this->commentApi->sendWithTemplate($pullRequest, 'markdown/pr_merged_feedback.md.twig', []);

return true;
}

/**
* Wrap the validation of commits.
*
Expand Down
44 changes: 44 additions & 0 deletions src/AppBundle/PullRequests/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace AppBundle\PullRequests;

use AppBundle\Search\Repository as SearchRepository;
use DateInterval;
use DateTime;
use Github\Api\Issue\Comments as KnpCommentsApi;
use Github\Exception\RuntimeException;
use Lpdigital\Github\Entity\Comment;
Expand Down Expand Up @@ -169,6 +171,48 @@ public function removeCommentsIfExists(PullRequest $pullRequest, $pattern, $user
return false;
}

/**
* {@inheritdoc}
*/
public function getMergedFromWithCommentsFrom(string $mergedFrom, string $commentedBy, ?DateTime $since = null)
{
if (null === $since) {
$since = (new DateTime())->sub(DateInterval::createFromDateString('30 days'));
}
$query = 'repo:'.$this->repositoryOwner.'/'.$this->repositoryName.' is:pr is:merged author:'.$mergedFrom.' commenter:'.$commentedBy.' merged:>='.$since->format('Y-m-d');
$search = '{
search(query: "'.$query.'", type: ISSUE, last: 100) {
edges {
node {
... on PullRequest {
id
title
author {
login
}
mergedAt
updatedAt
comments(last: 100) {
edges {
node {
body
author {
login
}
}
}
}
}
}
}
issueCount
}
}';

return $this->searchRepository->graphQL($search, ['query' => $query]);
}


/**
* @param $label
*
Expand Down
12 changes: 12 additions & 0 deletions src/AppBundle/PullRequests/RepositoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace AppBundle\PullRequests;

use DateTime;
use Lpdigital\Github\Entity\PullRequest;

interface RepositoryInterface
Expand Down Expand Up @@ -47,4 +48,15 @@ public function getCommentsByExpressionFrom(
* @param mixed $userLogin
*/
public function removeCommentsIfExists(PullRequest $pullRequest, $pattern, $userLogin);

/**
* Returns all merged pull requests of a specific user commented by another specific user.
*
* @param string $mergedFrom userLogin of the author of the PR
* @param string $commentedBy userLogin of the author of the comment we're looking for
* @param DateTime|null $since since what date should we get the PRs
*
* @return array
*/
public function getMergedFromWithCommentsFrom(string $mergedFrom, string $commentedBy, ?DateTime $since = null);
}
19 changes: 18 additions & 1 deletion src/AppBundle/Search/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace AppBundle\Search;

use Github\Api\GraphQL;
use Github\Api\Search;

/**
Expand All @@ -15,6 +16,10 @@ class Repository
* @var Search
*/
private $searchApi;
/**
* @var GraphQL
*/
private $graphQL;
/**
* @var string
*/
Expand All @@ -24,9 +29,10 @@ class Repository
*/
private $repositoryName;

public function __construct(Search $searchApi, string $repositoryOwner, string $repositoryName)
public function __construct(Search $searchApi, GraphQL $graphQL, string $repositoryOwner, string $repositoryName)
{
$this->searchApi = $searchApi;
$this->graphQL = $graphQL;
$this->repositoryOwner = $repositoryOwner;
$this->repositoryName = $repositoryName;
}
Expand All @@ -49,6 +55,17 @@ public function getPullRequests($filters = []): array
return $this->searchApi->issues($this->buildQuery($allFilters));
}

/**
* @param $query
* @param array $variables
*
* @return array
*/
public function graphQL($query, $variables = []): array
{
return $this->graphQL->execute($query, $variables);
}

/**
* @param $filters
*
Expand Down
7 changes: 6 additions & 1 deletion tests/AppBundle/Search/RepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Tests\AppBundle\Search;

use AppBundle\Search\Repository;
use Github\Api\GraphQL;
use Github\Api\Search;
use PHPUnit\Framework\TestCase;

Expand All @@ -13,17 +14,21 @@ class RepositoryTest extends TestCase
{
private $repository;
private $searchApiMock;
private $graphQL;

public function setUp()
{
$this->searchApiMock = $this->createMock(Search::class);
$this->repository = new Repository($this->searchApiMock, 'fakeUsername', 'fakeName');
$this->graphQL = $this->createMock(GraphQL::class);

$this->repository = new Repository($this->searchApiMock, $this->graphQL, 'fakeUsername', 'fakeName');
}

public function tearDown()
{
$this->repository = null;
$this->searchApiMock = null;
$this->graphQL = null;
}

/**
Expand Down

0 comments on commit 733829e

Please sign in to comment.