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

Ask feedback to author of community PRs #93

Merged
merged 4 commits into from
Oct 13, 2020
Merged
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 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. ;-)
3 changes: 3 additions & 0 deletions app/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,6 @@ services:

Github\Api\Repository\Contents:
factory: 'Github\Api\Repo:contents'
Github\Api\GraphQL:
factory: 'Github\Client:api'
arguments: [graphql]
5 changes: 5 additions & 0 deletions app/config/services_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ services:
# Integration tests shouldn't post comments to github
Psr\Log\NullLogger:

Tests\AppBundle\Organisations\FakeRepository:
decorates: AppBundle\Organizations\RepositoryInterface
arguments:
- ['%repository_username%']

Tests\AppBundle\Comments\FakeCommentApi:
decorates: AppBundle\Comments\CommentApiInterface
arguments:
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 @@ -57,6 +57,9 @@ public static function getSubscribedEvents(): array
'pullrequestevent_labeled' => [
['checkForMilestone', 255],
],
'pullrequestevent_closed' => [
['askForFeedback', 255],
],
];
}

Expand Down Expand Up @@ -225,4 +228,27 @@ public function checkForMilestone(GitHubEvent $gitHubEvent)
'status' => $missing ? 'not_found' : 'found',
]);
}

/**
* @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->getMerged()) {
return;
}

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

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

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

class Repository
class Repository implements RepositoryInterface
{
/**
* @var array list of teams (won't change during a request)
Expand All @@ -18,23 +19,23 @@ 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;
}

/**
* @return array
* {@inheritdoc}
*/
public function getTeams()
{
if (null === self::$teams) {
$teams = $this->organizationApi
->teams()
->all($this->repositoryUsername)
->all($this->repositoryOwner)
;

foreach ($teams as $team) {
Expand All @@ -47,7 +48,7 @@ public function getTeams()
}

/**
* @param string $teamName
* {@inheritdoc}
*/
public function getTeam(string $teamName)
{
Expand All @@ -57,9 +58,7 @@ public function getTeam(string $teamName)
}

/**
* @param string $teamName
*
* @return \Guzzle\Http\EntityBodyInterface|mixed|string
* {@inheritdoc}
*/
public function getTeamMembers(string $teamName)
{
Expand All @@ -74,4 +73,18 @@ public function getTeamMembers(string $teamName)
;
}
}

/**
* {@inheritdoc}
*/
public function isMember(string $userLogin)
{
try {
$this->organizationApi->members()->show($this->repositoryOwner, $userLogin);

return true;
} catch (RuntimeException $e) {
return false;
}
}
}
32 changes: 32 additions & 0 deletions src/AppBundle/Organizations/RepositoryInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace AppBundle\Organizations;

interface RepositoryInterface
{
/**
* @return array
*/
public function getTeams();

/**
* @param string $teamName
*/
public function getTeam(string $teamName);

/**
* @param string $teamName
*
* @return \Guzzle\Http\EntityBodyInterface|mixed|string
*/
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);
}
45 changes: 45 additions & 0 deletions src/AppBundle/PullRequests/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
use AppBundle\Commits\RepositoryInterface as CommitRepositoryInterface;
use AppBundle\Event\GitHubEvent;
use AppBundle\GithubDownloaderInterface;
use AppBundle\Organizations\RepositoryInterface as OrganizationRepository;
use AppBundle\PullRequests\RepositoryInterface as PullRequestRepositoryInterface;
use DateInterval;
use DateTime;
use PrestaShop\Github\Entity\PullRequest;
use PrestaShop\Github\Entity\User;
use PrestaShop\TranslationToolsBundle\Configuration;
Expand All @@ -21,9 +24,12 @@ class Listener
const TABLE_ERROR = 'PR_TABLE_DESCRIPTION_ERROR';
const COMMIT_ERROR = 'PR_COMMIT_NAME_ERROR';
const WORDING_TAG = 'PR_WORDING';
const FEEDBACK_COMMENT = 'PR_FEEDBACK_COMMENT';

Progi1984 marked this conversation as resolved.
Show resolved Hide resolved
const TRANS_CONFIG_FILE = '.t9n.yml';

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

/**
* @var CommentApiInterface
*/
Expand All @@ -44,6 +50,10 @@ class Listener
* @var RepositoryInterface
*/
private $repository;
/**
* @var OrganizationRepository
*/
private $organizationRepository;

/**
* @var GithubDownloaderInterface
Expand All @@ -65,6 +75,7 @@ public function __construct(
CommitRepositoryInterface $commitRepository,
ValidatorInterface $validator,
PullRequestRepositoryInterface $repository,
OrganizationRepository $organizationRepository,
GithubDownloaderInterface $githubDownloader,
ChainExtractor $chainExtractor,
LoggerInterface $logger,
Expand All @@ -75,6 +86,7 @@ public function __construct(
$this->logger = $logger;
$this->validator = $validator;
$this->repository = $repository;
$this->organizationRepository = $organizationRepository;
$this->githubDownloader = $githubDownloader;
$this->chainExtractor = $chainExtractor;
$this->cacheDir = $cacheDir;
Expand Down Expand Up @@ -284,6 +296,39 @@ public function checkForNewTranslations(PullRequest $pullRequest): bool
return false;
}

/**
* @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;
}

/**
* Get existing new translations comment.
*
Expand Down
45 changes: 44 additions & 1 deletion src/AppBundle/PullRequests/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

namespace AppBundle\PullRequests;

use AppBundle\Search\Repository as SearchRepository;
use AppBundle\Search\RepositoryInterface as SearchRepository;
use DateInterval;
use DateTime;
use Github\Api\Issue\Comments as KnpCommentsApi;
use Github\Exception\RuntimeException;
use PrestaShop\Github\Entity\Comment;
Expand Down Expand Up @@ -169,6 +171,47 @@ 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 = '{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of string should be store in a file 🤔
(Can be done in a new PR)

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 PrestaShop\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);
}
Loading