From 25bb50536071831fd3ffdea1345e18e191154cc5 Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:12:29 +0200 Subject: [PATCH 1/7] Add symfony validator --- composer.json | 1 + composer.lock | 98 +++++++++++++++++++++++++++++++++- config/packages/validator.yaml | 13 +++++ symfony.lock | 12 +++++ 4 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 config/packages/validator.yaml diff --git a/composer.json b/composer.json index 81d5b95..a454d86 100644 --- a/composer.json +++ b/composer.json @@ -21,6 +21,7 @@ "symfony/property-info": "~6.3.0", "symfony/runtime": "~6.3.0", "symfony/twig-bundle": "~6.3.0", + "symfony/validator": "~6.3.0", "symfony/webhook": "~6.3.0", "symfony/yaml": "~6.3.0" }, diff --git a/composer.lock b/composer.lock index 920a104..089430a 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1f3aa70229f6d682407a60a970dfbb0d", + "content-hash": "6862e9758aad75b80be89b50498ca5e1", "packages": [ { "name": "doctrine/annotations", @@ -3458,6 +3458,102 @@ ], "time": "2024-01-23T14:35:58+00:00" }, + { + "name": "symfony/validator", + "version": "v6.3.12", + "source": { + "type": "git", + "url": "https://github.com/symfony/validator.git", + "reference": "5e3ac975cc36d22db979225c587eed3d1f172bb8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/validator/zipball/5e3ac975cc36d22db979225c587eed3d1f172bb8", + "reference": "5e3ac975cc36d22db979225c587eed3d1f172bb8", + "shasum": "" + }, + "require": { + "php": ">=8.1", + "symfony/deprecation-contracts": "^2.5|^3", + "symfony/polyfill-ctype": "~1.8", + "symfony/polyfill-mbstring": "~1.0", + "symfony/polyfill-php83": "^1.27", + "symfony/translation-contracts": "^2.5|^3" + }, + "conflict": { + "doctrine/annotations": "<1.13", + "doctrine/lexer": "<1.1", + "symfony/dependency-injection": "<5.4", + "symfony/expression-language": "<5.4", + "symfony/http-kernel": "<5.4", + "symfony/intl": "<5.4", + "symfony/property-info": "<5.4", + "symfony/translation": "<5.4.35|>=6.0,<6.3.12|>=6.4,<6.4.3", + "symfony/yaml": "<5.4" + }, + "require-dev": { + "doctrine/annotations": "^1.13|^2", + "egulias/email-validator": "^2.1.10|^3|^4", + "symfony/cache": "^5.4|^6.0", + "symfony/config": "^5.4|^6.0", + "symfony/console": "^5.4|^6.0", + "symfony/dependency-injection": "^5.4|^6.0", + "symfony/expression-language": "^5.4|^6.0", + "symfony/finder": "^5.4|^6.0", + "symfony/http-client": "^5.4|^6.0", + "symfony/http-foundation": "^5.4|^6.0", + "symfony/http-kernel": "^5.4|^6.0", + "symfony/intl": "^5.4|^6.0", + "symfony/mime": "^5.4|^6.0", + "symfony/property-access": "^5.4|^6.0", + "symfony/property-info": "^5.4|^6.0", + "symfony/translation": "^5.4.35|~6.3.12|^6.4.3", + "symfony/yaml": "^5.4|^6.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Validator\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides tools to validate values", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/validator/tree/v6.3.12" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2024-01-29T14:46:07+00:00" + }, { "name": "symfony/var-dumper", "version": "v6.3.12", diff --git a/config/packages/validator.yaml b/config/packages/validator.yaml new file mode 100644 index 0000000..0201281 --- /dev/null +++ b/config/packages/validator.yaml @@ -0,0 +1,13 @@ +framework: + validation: + email_validation_mode: html5 + + # Enables validator auto-mapping support. + # For instance, basic validation constraints will be inferred from Doctrine's metadata. + #auto_mapping: + # App\Entity\: [] + +when@test: + framework: + validation: + not_compromised_password: false diff --git a/symfony.lock b/symfony.lock index bb1f47a..df103b4 100644 --- a/symfony.lock +++ b/symfony.lock @@ -151,6 +151,18 @@ "templates/base.html.twig" ] }, + "symfony/validator": { + "version": "6.3", + "recipe": { + "repo": "github.com/symfony/recipes", + "branch": "main", + "version": "5.3", + "ref": "c32cfd98f714894c4f128bb99aa2530c1227603c" + }, + "files": [ + "config/packages/validator.yaml" + ] + }, "symfony/webhook": { "version": "6.3", "recipe": { From 11a39f2ae3b7822ba164e0586f409c0fd0ef1dd6 Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:13:32 +0200 Subject: [PATCH 2/7] Improve PullRequest and PullRequestRepository --- .../Aggregate/PullRequest/PullRequest.php | 56 +++++++- .../PullRequestRepositoryInterface.php | 7 + .../Adapter/InMemoryPullRequestRepository.php | 13 ++ .../Adapter/RestPullRequestRepository.php | 122 +++++++++++++----- 4 files changed, 167 insertions(+), 31 deletions(-) diff --git a/src/PullRequest/Domain/Aggregate/PullRequest/PullRequest.php b/src/PullRequest/Domain/Aggregate/PullRequest/PullRequest.php index 4e383bc..45a25da 100644 --- a/src/PullRequest/Domain/Aggregate/PullRequest/PullRequest.php +++ b/src/PullRequest/Domain/Aggregate/PullRequest/PullRequest.php @@ -15,6 +15,8 @@ private function __construct( private array $labels, private array $approvals, private string $targetBranch, + private string $bodyDescription = '', + private ?int $milestoneNumber = null, ) { } @@ -22,9 +24,9 @@ private function __construct( * @param string[] $labels * @param Approval[] $approvals */ - public static function create(PullRequestId $id, array $labels, array $approvals, string $targetBranch): self + public static function create(PullRequestId $id, array $labels, array $approvals, string $targetBranch, string $bodyDescription = '', ?int $milestoneNumber = null): self { - return new self($id, $labels, $approvals, $targetBranch); + return new self($id, $labels, $approvals, $targetBranch, $bodyDescription, $milestoneNumber); } public function getId(): PullRequestId @@ -83,4 +85,54 @@ public function getTargetBranch(): string { return $this->targetBranch; } + + public function getBodyDescription(): string + { + return $this->bodyDescription; + } + + public function getMilestoneNumber(): ?int + { + return $this->milestoneNumber; + } + + public function addLabelsByDescription(PullRequestDescription $description): void + { + // Remove some labels in labels list + $this->labels = array_diff($this->labels, [ + 'develop', + '8.1.x', + 'Bug fix', + 'Improvement', + 'Feature', + 'Refactoring', + 'BC break', + ]); + + // Add label for branch + if ($description->getBranch()) { + $this->labels[] = $description->getBranch(); + } + + // Add label for PR type + if ($description->getType()) { + $mapLabelTypes = [ + 'bug fix' => 'Bug fix', + 'improvement' => 'Improvement', + 'new feature' => 'Feature', + 'refacto' => 'Refactoring', + ]; + $this->labels[] = $mapLabelTypes[$description->getType()]; + } + + // Add label if BC break declared + if ($description->isBCBreak()) { + $this->labels[] = 'BC break'; + } + } + + public function isQAValidated(): bool + { + return in_array('QA ✔️', $this->labels); + } } diff --git a/src/PullRequest/Domain/Gateway/PullRequestRepositoryInterface.php b/src/PullRequest/Domain/Gateway/PullRequestRepositoryInterface.php index 5ea0212..5658fa7 100644 --- a/src/PullRequest/Domain/Gateway/PullRequestRepositoryInterface.php +++ b/src/PullRequest/Domain/Gateway/PullRequestRepositoryInterface.php @@ -5,6 +5,7 @@ use App\PullRequest\Domain\Aggregate\PullRequest\PullRequest; use App\PullRequest\Domain\Aggregate\PullRequest\PullRequestDiff; use App\PullRequest\Domain\Aggregate\PullRequest\PullRequestId; +use Symfony\Component\Validator\ConstraintViolationListInterface; interface PullRequestRepositoryInterface { @@ -21,4 +22,10 @@ public function getDiff(PullRequestId $pullRequestId): PullRequestDiff; public function addTranslationsComment(PullRequestId $pullRequestId, array $newTranslations, array $newDomains): void; public function addWelcomeComment(PullRequestId $pullRequestId, string $contributor): void; + + public function addTableDescriptionErrorsComment(PullRequestId $pullRequestId, ConstraintViolationListInterface $errors, bool $isLinkedIssuesNeeded): void; + + public function removeTableDescriptionErrorsComment(PullRequestId $pullRequestId): void; + + public function addMissingMilestoneComment(PullRequestId $pullRequestId): void; } diff --git a/src/PullRequest/Infrastructure/Adapter/InMemoryPullRequestRepository.php b/src/PullRequest/Infrastructure/Adapter/InMemoryPullRequestRepository.php index 85eb89d..93f7997 100644 --- a/src/PullRequest/Infrastructure/Adapter/InMemoryPullRequestRepository.php +++ b/src/PullRequest/Infrastructure/Adapter/InMemoryPullRequestRepository.php @@ -9,6 +9,7 @@ use App\PullRequest\Domain\Aggregate\PullRequest\PullRequestId; use App\PullRequest\Domain\Gateway\PullRequestRepositoryInterface; use Symfony\Component\DependencyInjection\Attribute\When; +use Symfony\Component\Validator\ConstraintViolationListInterface; #[When(env: 'test')] class InMemoryPullRequestRepository implements PullRequestRepositoryInterface @@ -58,4 +59,16 @@ public function addTranslationsComment(PullRequestId $pullRequestId, array $newT public function addWelcomeComment(PullRequestId $pullRequestId, string $contributor): void { } + + public function addTableDescriptionErrorsComment(PullRequestId $pullRequestId, ConstraintViolationListInterface $errors, bool $isLinkedIssuesNeeded): void + { + } + + public function removeTableDescriptionErrorsComment(PullRequestId $pullRequestId): void + { + } + + public function addMissingMilestoneComment(PullRequestId $pullRequestId): void + { + } } diff --git a/src/PullRequest/Infrastructure/Adapter/RestPullRequestRepository.php b/src/PullRequest/Infrastructure/Adapter/RestPullRequestRepository.php index 996536a..718117c 100644 --- a/src/PullRequest/Infrastructure/Adapter/RestPullRequestRepository.php +++ b/src/PullRequest/Infrastructure/Adapter/RestPullRequestRepository.php @@ -9,6 +9,7 @@ use App\PullRequest\Domain\Aggregate\PullRequest\PullRequestDiff; use App\PullRequest\Domain\Aggregate\PullRequest\PullRequestId; use App\PullRequest\Domain\Gateway\PullRequestRepositoryInterface; +use Symfony\Component\Validator\ConstraintViolationListInterface; use Symfony\Contracts\HttpClient\HttpClientInterface; use Twig\Environment; @@ -34,7 +35,9 @@ static function (array $label): string { $responseArr['labels'] ), $this->getApprovals($pullRequestId), - $responseArr['base']['ref'] + $responseArr['base']['ref'], + $responseArr['body'], + $responseArr['milestone']['number'] ?? null, ); } @@ -88,25 +91,20 @@ public function getDiff(PullRequestId $pullRequestId): PullRequestDiff public function addTranslationsComment(PullRequestId $pullRequestId, array $newTranslations, array $newDomains): void { // First, we need to check if the comment already exists. - $t9nCommentIdExist = false; + $alreadyExistComment = $this->getExistingComment($pullRequestId, ''); $validatedWordings = []; - $comments = $this->githubClient->request('GET', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/'.$pullRequestId->pullRequestNumber.'/comments')->toArray(); // if we have found one PR WORDING comment, in all comments: - foreach ($comments as $comment) { - if (false !== strpos($comment['body'], '')) { - $t9nCommentIdExist = $comment['id']; - // We need to determine if the comment is already validated with a tick. - $matches = []; - preg_match_all('/^- (?:\[(x| )\].*)?\s*`(.*)`((?:\s{4,}- \[.\] .*)+)/mi', $comment['body'], $matches, PREG_SET_ORDER); - foreach ($matches as $match) { - $wordings = []; - preg_match_all('/^\s+- \[x\] `(.*)`/mi', $match[3], $wordings, PREG_SET_ORDER); - foreach ($wordings as $wording) { - $validatedWordings[$match[2]][] = $wording[1]; - } + if ($alreadyExistComment) { + // We need to determine if some wording is already validated with a tick. + $matches = []; + preg_match_all('/^- (?:\[(x| )\].*)?\s*`(.*)`((?:\s{4,}- \[.\] .*)+)/mi', $alreadyExistComment['body'], $matches, PREG_SET_ORDER); + foreach ($matches as $match) { + $wordings = []; + preg_match_all('/^\s+- \[x\] `(.*)`/mi', $match[3], $wordings, PREG_SET_ORDER); + foreach ($wordings as $wording) { + $validatedWordings[$match[2]][] = $wording[1]; } - break; } } @@ -118,9 +116,9 @@ public function addTranslationsComment(PullRequestId $pullRequestId, array $newT ]); // Finally, we need to add or edit the comment to the PR. - if ($t9nCommentIdExist) { + if ($alreadyExistComment) { // Edit $t9nCommentIdExist comment - $this->githubClient->request('PATCH', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/comments/'.$t9nCommentIdExist, [ + $this->githubClient->request('PATCH', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/comments/'.$alreadyExistComment['id'], [ 'json' => ['body' => $comment], ]); } else { @@ -134,22 +132,88 @@ public function addTranslationsComment(PullRequestId $pullRequestId, array $newT public function addWelcomeComment(PullRequestId $pullRequestId, string $contributor): void { // First, we need to check if the comment already exists, and if we have already welcomed the contributor, we do nothing. + $alreadyExistComment = $this->getExistingComment($pullRequestId, ''); + + // If the comment does not exist, we need to add it. + if (!$alreadyExistComment) { + // Then we need to format the new comment with the new contributor. + $welcomeComment = $this->twig->render('pr_comments/welcome.html.twig', [ + 'contributor' => $contributor, + ]); + + // We add the comment to the PR. + $this->githubClient->request('POST', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/'.$pullRequestId->pullRequestNumber.'/comments', [ + 'json' => ['body' => $welcomeComment], + ]); + } + } + + public function addTableDescriptionErrorsComment(PullRequestId $pullRequestId, ConstraintViolationListInterface $errors, bool $isLinkedIssuesNeeded): void + { + // First, we need to check if the comment already exists + $alreadyExistComment = $this->getExistingComment($pullRequestId, ''); + + // Then we need to format the new comment with the new translations keys. + $comment = $this->twig->render('pr_comments/table_errors.html.twig', [ + 'errors' => $errors, + 'linkedIssuesNeeded' => $isLinkedIssuesNeeded, + ]); + + // Finally, we need to add or edit the comment to the PR. + if ($alreadyExistComment) { + // Edit $t9nCommentIdExist comment + $this->githubClient->request('PATCH', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/comments/'.$alreadyExistComment['id'], [ + 'json' => ['body' => $comment], + ]); + } else { + // add new comment + $this->githubClient->request('POST', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/'.$pullRequestId->pullRequestNumber.'/comments', [ + 'json' => ['body' => $comment], + ]); + } + } + + public function removeTableDescriptionErrorsComment(PullRequestId $pullRequestId): void + { + // First, we need to check if the comment already exists + $alreadyExistComment = $this->getExistingComment($pullRequestId, ''); + + // If the comment exists, we remove it. + if ($alreadyExistComment) { + $this->githubClient->request('DELETE', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/comments/'.$alreadyExistComment['id']); + } + } + + public function addMissingMilestoneComment(PullRequestId $pullRequestId): void + { + // First, we need to check if the comment already exists + $alreadyExistComment = $this->getExistingComment($pullRequestId, ''); + + // If the comment does not exist, we need to add it. + if (!$alreadyExistComment) { + $comment = $this->twig->render('pr_comments/missing_milestone.html.twig'); + $this->githubClient->request('POST', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/'.$pullRequestId->pullRequestNumber.'/comments', [ + 'json' => ['body' => $comment], + ]); + } + } + + /** + * @return ?array{ + * id: string, + * body: string + * } + */ + private function getExistingComment(PullRequestId $pullRequestId, string $commentMarker): ?array + { $comments = $this->githubClient->request('GET', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/'.$pullRequestId->pullRequestNumber.'/comments')->toArray(); foreach ($comments as $comment) { - if (str_contains($comment['body'], '')) { - return; + if (str_contains($comment['body'], $commentMarker)) { + return $comment; } } - // Then we need to format the new comment with the new contributor. - $welcomeComment = $this->twig->render('pr_comments/welcome.html.twig', [ - 'contributor' => $contributor, - ]); - - // We add the comment to the PR. - $this->githubClient->request('POST', '/repos/'.$pullRequestId->repositoryOwner.'/'.$pullRequestId->repositoryName.'/issues/'.$pullRequestId->pullRequestNumber.'/comments', [ - 'json' => ['body' => $welcomeComment], - ]); + return null; } } From 50286367f89b228fb8cbf3e28f12e2a9b3086a3e Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:13:52 +0200 Subject: [PATCH 3/7] Add PullRequestDescription --- .../PullRequest/PullRequestDescription.php | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 src/PullRequest/Domain/Aggregate/PullRequest/PullRequestDescription.php diff --git a/src/PullRequest/Domain/Aggregate/PullRequest/PullRequestDescription.php b/src/PullRequest/Domain/Aggregate/PullRequest/PullRequestDescription.php new file mode 100644 index 0000000..7f77203 --- /dev/null +++ b/src/PullRequest/Domain/Aggregate/PullRequest/PullRequestDescription.php @@ -0,0 +1,151 @@ + Every detail helps: versions, browser/server configuration, specific module/theme, etc. Feel free to add more information below this table.'; + public const TYPES_AVAILABLE = ['bug fix', 'improvement', 'new feature', 'refacto']; + public const CATEGORIES_AVAILABLE = ['FO', 'BO', 'CO', 'IN', 'WS', 'TE', 'LO', 'ME', 'PM']; + public const TEMPLATE_HOW_TO_TEST = 'Indicate how to verify that this change works as expected.'; + public const TEMPLATE_UI_TESTS = 'Please run UI tests and paste here the link to the run. [Read this page to know why and how to use this tool.](https://devdocs.prestashop-project.org/8/contribute/contribution-guidelines/ui-tests/).'; + + public function __construct( + private string $bodyContent + ) { + } + + #[Assert\NotBlank(message: 'The `branch` should be `develop` or `8.1.x`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))')] + public function getBranch(): ?string + { + $branch = $this->extractWithRegex('Branch'); + if (in_array($branch, self::TARGET_BRANCH_AVAILABLE)) { + return $branch; + } + + return null; + } + + #[Assert\NotBlank(message: 'The `description` shouldn\'t be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#description))')] + public function getDescription(): ?string + { + $description = $this->extractWithRegex('Description'); + if (self::TEMPLATE_DESCRIPTION !== $description) { + return $description; + } + + return null; + } + + #[Assert\NotBlank(message: 'The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))')] + public function getType(): ?string + { + $type = strtolower($this->extractWithRegex('Type')); + if (in_array($type, self::TYPES_AVAILABLE)) { + return $type; + } + + return null; + } + + #[Assert\NotBlank(message: 'The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))')] + public function getCategory(): ?string + { + $category = strtoupper($this->extractWithRegex('Category')); + if (in_array($category, self::CATEGORIES_AVAILABLE)) { + return $category; + } + + return null; + } + + #[Assert\NotNull(message: 'The `BC breaks` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#bc-breaks))')] + public function isBcBreak(): ?bool + { + $bcBreak = strtolower($this->extractWithRegex('BC breaks')); + if (in_array($bcBreak, ['yes', 'no'])) { + return 'yes' === $bcBreak; + } + + return null; + } + + #[Assert\NotNull(message: 'The `deprecations` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#deprecations))')] + public function isDeprecated(): ?bool + { + $deprecations = strtolower($this->extractWithRegex('Deprecations')); + if (in_array($deprecations, ['yes', 'no'])) { + return 'yes' === $deprecations; + } + + return null; + } + + #[Assert\NotBlank(message: 'The `How to test` shouldn\'t be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#how-to-test))')] + public function getHowToTest(): ?string + { + $howToTest = $this->extractWithRegex('How to test'); + if (self::TEMPLATE_HOW_TO_TEST !== $howToTest) { + return $howToTest; + } + + return null; + } + + public function getUITests(): ?string + { + $uiTests = $this->extractWithRegex('UI Tests'); + if (self::TEMPLATE_UI_TESTS !== $uiTests) { + return $uiTests; + } + + return null; + } + + /** + * @return string[] + */ + public function getIssuesFixed(): array + { + $issuesFixed = $this->extractWithRegex('Fixed issue or discussion'); + + preg_match_all('/(?:#[0-9]+|https:\/\/github.com\/.*\/.*\/issues\/[0-9]+|https:\/\/github.com\/.*\/.*\/discussions\/[0-9]+)/', $issuesFixed, $matches); + + return array_pop($matches); + } + + /** + * @return string[] + */ + public function getPrRelated(): array + { + $prRelated = $this->extractWithRegex('Related PRs'); + + preg_match_all('/(?:#[0-9]+|https:\/\/github.com\/.*\/.*\/pull\/[0-9]+)/', $prRelated, $matches); + + return array_pop($matches); + } + + public function getSponsor(): ?string + { + return $this->extractWithRegex('Sponsor company'); + } + + public function isLinkedIssuesNeeded(): bool + { + return in_array($this->getType(), ['bug fix', 'new feature']) && !in_array($this->getCategory(), ['TE', 'ME', 'PM']); + } + + private function extractWithRegex(string $field, string $patternValue = '[^|]*'): string + { + $regex = sprintf('~(?:\|?\s*%s\??\s+\|\s*)(%s)\s*~', $field, $patternValue); + preg_match($regex, $this->bodyContent, $matches); + + return isset($matches[1]) ? trim($matches[1]) : ''; + } +} From 9416bf364b03cfd4f79bb2f5379e9da16bd58102 Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:14:31 +0200 Subject: [PATCH 4/7] Add command to check table description for PRs --- .../Command/CheckTableDescriptionCommand.php | 15 ++++++ .../CheckTableDescriptionCommandHandler.php | 53 +++++++++++++++++++ templates/pr_comments/table_errors.html.twig | 23 ++++++++ 3 files changed, 91 insertions(+) create mode 100644 src/PullRequest/Application/Command/CheckTableDescriptionCommand.php create mode 100644 src/PullRequest/Application/CommandHandler/CheckTableDescriptionCommandHandler.php create mode 100644 templates/pr_comments/table_errors.html.twig diff --git a/src/PullRequest/Application/Command/CheckTableDescriptionCommand.php b/src/PullRequest/Application/Command/CheckTableDescriptionCommand.php new file mode 100644 index 0000000..71aaaf6 --- /dev/null +++ b/src/PullRequest/Application/Command/CheckTableDescriptionCommand.php @@ -0,0 +1,15 @@ +repositoryOwner || 'PrestaShop' !== $command->repositoryName) { + return; + } + + // Retrieve the PullRequest object + $prId = new PullRequestId($command->repositoryOwner, $command->repositoryName, $command->pullRequestNumber); + $pullRequest = $this->prRepository->find($prId); + if (null === $pullRequest) { + throw new PullRequestNotFoundException(); + } + + // Create PulLRequestDescription object with the description + $prDescription = new PullRequestDescription($pullRequest->getBodyDescription()); + + // We check the description with the validator + $errors = $this->validator->validate($prDescription); + + // If we have some errors, we need to add (or edit) the comment about this errors + if (count($errors) > 0 || $prDescription->isLinkedIssuesNeeded()) { + $this->prRepository->addTableDescriptionErrorsComment($prId, $errors, $prDescription->isLinkedIssuesNeeded()); + } else { + $this->prRepository->removeTableDescriptionErrorsComment($prId); + } + + // Then, we had the labels to the PR by PullRequestDescription object + $pullRequest->addLabelsByDescription($prDescription); + $this->prRepository->update($pullRequest); + } +} diff --git a/templates/pr_comments/table_errors.html.twig b/templates/pr_comments/table_errors.html.twig new file mode 100644 index 0000000..2c4afc4 --- /dev/null +++ b/templates/pr_comments/table_errors.html.twig @@ -0,0 +1,23 @@ + +Hi, thanks for this contribution! + +{% if errors|length > 0 %} +I found some issues with the Pull Request description: + +{% for error in errors %} +* {{ error.message }} +{% endfor %} + +Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much! +{% endif %} +{% if linkedIssuesNeeded %} +{% if errors|length > 0 %} +### About linked issues +{% endif %} +Please consider opening an issue before submitting a Pull Request: +* If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already. +* It can help trigger a discussion about the best implementation path before a single line of code is written. +* It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention. +{% endif %} + +(Note: this is an automated message, but answering it will reach a real human) From 6f6878f50b57dc573ce70900da66717951f3a9ac Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:14:50 +0200 Subject: [PATCH 5/7] Add checking milestone in PRs --- .../Command/CheckMilestoneCommand.php | 15 ++++++++ .../CheckMilestoneCommandHandler.php | 38 +++++++++++++++++++ .../pr_comments/missing_milestone.html.twig | 2 + 3 files changed, 55 insertions(+) create mode 100644 src/PullRequest/Application/Command/CheckMilestoneCommand.php create mode 100644 src/PullRequest/Application/CommandHandler/CheckMilestoneCommandHandler.php create mode 100644 templates/pr_comments/missing_milestone.html.twig diff --git a/src/PullRequest/Application/Command/CheckMilestoneCommand.php b/src/PullRequest/Application/Command/CheckMilestoneCommand.php new file mode 100644 index 0000000..7b02271 --- /dev/null +++ b/src/PullRequest/Application/Command/CheckMilestoneCommand.php @@ -0,0 +1,15 @@ +repositoryOwner || 'PrestaShop' !== $command->repositoryName) { + return; + } + + // Retrieve the PullRequest object + $prId = new PullRequestId($command->repositoryOwner, $command->repositoryName, $command->pullRequestNumber); + $pullRequest = $this->prRepository->find($prId); + if (null === $pullRequest) { + throw new PullRequestNotFoundException(); + } + + // We check if the milestone is set + if (null === $pullRequest->getMilestoneNumber() && $pullRequest->isQAValidated()) { + $this->prRepository->addMissingMilestoneComment($prId); + } + } +} diff --git a/templates/pr_comments/missing_milestone.html.twig b/templates/pr_comments/missing_milestone.html.twig new file mode 100644 index 0000000..bbd52c3 --- /dev/null +++ b/templates/pr_comments/missing_milestone.html.twig @@ -0,0 +1,2 @@ + +QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge. \ No newline at end of file From 5102ba64056e67dadd3c1a78852aaf6f9117c21e Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:15:44 +0200 Subject: [PATCH 6/7] Add CheckTableDescription and CheckMilestone in strategies --- .../Command/PullRequestEditedStrategy.php | 64 +++++++++++++++++++ .../Command/PullRequestLabeledStrategy.php | 29 ++++++--- .../PullRequestReadyForReviewStrategy.php | 8 ++- 3 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestEditedStrategy.php diff --git a/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestEditedStrategy.php b/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestEditedStrategy.php new file mode 100644 index 0000000..4033c8e --- /dev/null +++ b/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestEditedStrategy.php @@ -0,0 +1,64 @@ + + */ + public function createCommandsFromPayload(array $payload): array + { + $repoOwner = $payload['pull_request']['base']['repo']['owner']['login']; + $repoName = $payload['pull_request']['base']['repo']['name']; + $prNumber = (string) $payload['pull_request']['number']; + + return [ + new CheckTableDescriptionCommand( + $repoOwner, + $repoName, + $prNumber, + ), + ]; + } +} diff --git a/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestLabeledStrategy.php b/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestLabeledStrategy.php index 2bb639c..b8d6393 100644 --- a/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestLabeledStrategy.php +++ b/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestLabeledStrategy.php @@ -4,6 +4,7 @@ namespace App\Shared\Infrastructure\Factory\CommandFactory\Strategy\Command; +use App\PullRequest\Application\Command\CheckMilestoneCommand; use App\PullRequestDashboard\Application\Command\MovePullRequestCardToColumnByLabelCommand; use App\Shared\Infrastructure\Factory\CommandFactory\CommandStrategyInterface; @@ -42,16 +43,28 @@ public function supports(string $eventType, array $payload): bool * } * } $payload * - * @return MovePullRequestCardToColumnByLabelCommand[] + * @return array */ public function createCommandsFromPayload(array $payload): array { - return [new MovePullRequestCardToColumnByLabelCommand( - $this->pullRequestDashboardNumber, - $payload['pull_request']['base']['repo']['owner']['login'], - $payload['pull_request']['base']['repo']['name'], - (string) $payload['pull_request']['number'], - (string) $payload['label']['name'], - )]; + $repoOwner = $payload['pull_request']['base']['repo']['owner']['login']; + $repoName = $payload['pull_request']['base']['repo']['name']; + $prNumber = (string) $payload['pull_request']['number']; + $labelName = (string) $payload['label']['name']; + + return [ + new MovePullRequestCardToColumnByLabelCommand( + $this->pullRequestDashboardNumber, + $repoOwner, + $repoName, + $prNumber, + $labelName, + ), + new CheckMilestoneCommand( + $repoOwner, + $repoName, + $prNumber, + ), + ]; } } diff --git a/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestReadyForReviewStrategy.php b/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestReadyForReviewStrategy.php index 6e9e784..74e9d07 100644 --- a/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestReadyForReviewStrategy.php +++ b/src/Shared/Infrastructure/Factory/CommandFactory/Strategy/Command/PullRequestReadyForReviewStrategy.php @@ -4,6 +4,7 @@ namespace App\Shared\Infrastructure\Factory\CommandFactory\Strategy\Command; +use App\PullRequest\Application\Command\CheckTableDescriptionCommand; use App\PullRequest\Application\Command\CheckTranslationsCommand; use App\PullRequest\Application\Command\WelcomeNewContributorCommand; use App\PullRequestDashboard\Application\Command\MovePullRequestCardToColumnByLabelCommand; @@ -57,7 +58,7 @@ public function supports(string $eventType, array $payload): bool * }, * } $payload * - * @return array + * @return array */ public function createCommandsFromPayload(array $payload): array { @@ -67,6 +68,11 @@ public function createCommandsFromPayload(array $payload): array $contributor = $payload['pull_request']['user']['login']; return [ + new CheckTableDescriptionCommand( + $repoOwner, + $repoName, + $prNumber, + ), new WelcomeNewContributorCommand( $repoOwner, $repoName, From 3235c9bf4fea0497b528a564654420538f3fe3ba Mon Sep 17 00:00:00 2001 From: boherm Date: Tue, 28 May 2024 10:16:25 +0200 Subject: [PATCH 7/7] Add tests for CheckTableDescription, CheckMilestone and WelcomeNewContributor --- .../CheckMilestoneCommandHandlerTest.php | 116 ++++ ...heckTableDescriptionCommandHandlerTest.php | 640 ++++++++++++++++++ ...elcomeNewContributorCommandHandlerTest.php | 111 +++ .../Webhook/GithubWebhookTest.php | 43 ++ 4 files changed, 910 insertions(+) create mode 100644 tests/PullRequest/Application/CommandHandler/CheckMilestoneCommandHandlerTest.php create mode 100644 tests/PullRequest/Application/CommandHandler/CheckTableDescriptionCommandHandlerTest.php create mode 100644 tests/PullRequest/Application/CommandHandler/WelcomeNewContributorCommandHandlerTest.php diff --git a/tests/PullRequest/Application/CommandHandler/CheckMilestoneCommandHandlerTest.php b/tests/PullRequest/Application/CommandHandler/CheckMilestoneCommandHandlerTest.php new file mode 100644 index 0000000..4d4906a --- /dev/null +++ b/tests/PullRequest/Application/CommandHandler/CheckMilestoneCommandHandlerTest.php @@ -0,0 +1,116 @@ +prRepository = $this->getMockBuilder(InMemoryPullRequestRepository::class) + ->onlyMethods(['addMissingMilestoneComment']) + ->getMock(); + $this->checkMilestoneCommandHandler = new CheckMilestoneCommandHandler($this->prRepository); + } + + /** + * @param string[] $originalLabels + * + * @dataProvider provideTestHandle + */ + public function testHandle(PullRequestId $pullRequestId, array $originalLabels, ?int $milestoneNumber, bool $expectedComment): void + { + $this->prRepository->feed([ + PullRequest::create( + id: $pullRequestId, + labels: $originalLabels, + approvals: [], + targetBranch: 'main', + milestoneNumber: $milestoneNumber + ), + ]); + + // @phpstan-ignore-next-line + $this->prRepository + ->expects($expectedComment ? $this->once() : $this->never()) + ->method('addMissingMilestoneComment') + ->with($pullRequestId); + + $this->checkMilestoneCommandHandler->__invoke(new CheckMilestoneCommand( + repositoryOwner: $pullRequestId->repositoryOwner, + repositoryName: $pullRequestId->repositoryName, + pullRequestNumber: $pullRequestId->pullRequestNumber, + )); + } + + /** + * @return array|PullRequestId|bool|int|null>> + */ + public static function provideTestHandle(): array + { + return [ + [ + new PullRequestId( + repositoryOwner: 'fake', + repositoryName: 'fake', + pullRequestNumber: 'fake' + ), + ['QA ✔️'], + null, + false, + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'pullRequestNumber' + ), + ['QA ✔️'], + 123, + false, + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'pullRequestNumber' + ), + ['QA ✔️'], + null, + true, + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'pullRequestNumber' + ), + ['Waiting for QA'], + null, + false, + ], + ]; + } + + public function testPullRequestNotFound(): void + { + $this->expectException(PullRequestNotFoundException::class); + $this->checkMilestoneCommandHandler->__invoke(new CheckMilestoneCommand( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + )); + } +} diff --git a/tests/PullRequest/Application/CommandHandler/CheckTableDescriptionCommandHandlerTest.php b/tests/PullRequest/Application/CommandHandler/CheckTableDescriptionCommandHandlerTest.php new file mode 100644 index 0000000..900c07b --- /dev/null +++ b/tests/PullRequest/Application/CommandHandler/CheckTableDescriptionCommandHandlerTest.php @@ -0,0 +1,640 @@ +prRepository = $this->getMockBuilder(InMemoryPullRequestRepository::class) + ->onlyMethods(['addTableDescriptionErrorsComment']) + ->getMock(); + /** @var ValidatorInterface $validator */ + $validator = $this->getContainer()->get(ValidatorInterface::class); + $this->validator = $validator; + $this->checkTableDescriptionCommandHandler = new CheckTableDescriptionCommandHandler($this->prRepository, $this->validator); + } + + /** + * @dataProvider provideTestHandle + * + * @param string[] $expectedErrors + * @param string[] $expectedNotErrors + * @param string[] $expectedLabels + */ + public function testHandle(PullRequestId $pullRequestId, string $bodyDescription, array $expectedErrors, array $expectedNotErrors, bool $linkedIssuesNeeded, bool $expectedComment, array $expectedLabels): void + { + // We feed the PR repository with a PR + $this->prRepository->feed([ + PullRequest::create( + id: $pullRequestId, + labels: [], + approvals: [], + targetBranch: 'main', + bodyDescription: $bodyDescription, + ), + ]); + + // We check errors on the description after validation + $prDescription = new PullRequestDescription($bodyDescription); + $errors = $this->validator->validate($prDescription); + $errorsMessages = array_map(fn (ConstraintViolationInterface $error) => $error->getMessage(), iterator_to_array($errors)); + foreach ($expectedErrors as $expectedError) { + $this->assertContains($expectedError, $errorsMessages); + } + foreach ($expectedNotErrors as $expectedNotError) { + $this->assertNotContains($expectedNotError, $errorsMessages); + } + + // We check if issue linked needed + $this->assertEquals($linkedIssuesNeeded, $prDescription->isLinkedIssuesNeeded()); + + // Then, we check if the comment is added or not. + // @phpstan-ignore-next-line + $this->prRepository + ->expects($expectedComment ? $this->once() : $this->never()) + ->method('addTableDescriptionErrorsComment'); + + $this->checkTableDescriptionCommandHandler->__invoke(new CheckTableDescriptionCommand( + repositoryOwner: $pullRequestId->repositoryOwner, + repositoryName: $pullRequestId->repositoryName, + pullRequestNumber: $pullRequestId->pullRequestNumber, + )); + + // We check the automatic labels in PR + /** @var PullRequest $pr */ + $pr = $this->prRepository->find($pullRequestId); + $this->assertEquals($expectedLabels, $pr->getLabels()); + } + + /** + * @return array|PullRequestId|bool|string>> + */ + public static function provideTestHandle(): array + { + return [ + [ + new PullRequestId( + repositoryOwner: 'fake', + repositoryName: 'fake', + pullRequestNumber: 'fake' + ), + '', + [], + [], + false, + false, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '', + [ + 'The `branch` should be `develop` or `8.1.x`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))', + "The `description` shouldn't be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#description))", + 'The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))', + 'The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))', + 'The `BC breaks` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#bc-breaks))', + 'The `deprecations` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#deprecations))', + "The `How to test` shouldn't be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#how-to-test))", + ], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + " +| Questions | Answers +| ----------------- | ------------------------------------------------------- +| Branch? | develop / 8.1.x +| Description? | Please be specific when describing the PR.
Every detail helps: versions, browser/server configuration, specific module/theme, etc. Feel free to add more information below this table. +| Type? | bug fix / improvement / new feature / refacto +| Category? | FO / BO / CO / IN / WS / TE / LO / ME / PM / see explanations at https://devdocs.prestashop-project.org/8/contribute/contribution-guidelines/pull-requests/#type--category +| BC breaks? | yes / no +| Deprecations? | yes / no +| How to test? | Indicate how to verify that this change works as expected. +| UI Tests | Please run UI tests and paste here the link to the run. [Read this page to know why and how to use this tool.](https://devdocs.prestashop-project.org/8/contribute/contribution-guidelines/ui-tests/). +| Fixed issue or discussion? | Fixes #{issue number here}, Fixes #{another issue number here}, Fixes https://github.com/PrestaShop/PrestaShop/discussions/ {discussion number here} +| Related PRs | If theme, autoupgrade or other module change is needed to make this change work, provide a link to related PRs here. +| Sponsor company | Your company or customer's name goes here (if applicable). +", + [ + 'The `branch` should be `develop` or `8.1.x`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))', + "The `description` shouldn't be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#description))", + 'The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))', + 'The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))', + 'The `BC breaks` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#bc-breaks))', + 'The `deprecations` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#deprecations))', + "The `How to test` shouldn't be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#how-to-test))", + ], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Branch? | fake', + ['The `branch` should be `develop` or `8.1.x`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Branch? | develop', + [], + ['The `branch` should be `develop` or `8.1.x`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + ['develop'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Branch? | 8.1.x', + [], + ['The `branch` should be `develop` or `8.1.x`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + ['8.1.x'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Description? | This is a fake description', + [], + ['The `description` shouldn\'t be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#description))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Type? | fake', + ['The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Type? | bug fix', + [], + ['The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + true, + true, + ['Bug fix'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Type? | improvement', + [], + ['The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + ['Improvement'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Type? | new feature', + [], + ['The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + true, + true, + ['Feature'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Type? | refacto', + [], + ['The `type` should be one of these: `new feature`, `improvement`, `bug fix` or `refacto`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + ['Refactoring'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | fake', + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | FO', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | BO', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | CO', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | IN', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | WS', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | TE', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | LO', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | ME', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Category? | PM', + [], + ['The `category` should be one of these: `FO`, `BO`, `CO`, `IN`, `WS`, `TE`, `LO`, `ME` or `PM`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#branch))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| BC breaks? | fake', + ['The `BC breaks` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#bc-breaks))'], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| BC breaks? | yes', + [], + ['The `BC breaks` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#bc-breaks))'], + false, + true, + ['BC break'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| BC breaks? | no', + [], + ['The `BC breaks` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#bc-breaks))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Deprecations? | fake', + ['The `deprecations` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#deprecations))'], + [], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Deprecations? | yes', + [], + ['The `deprecations` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#deprecations))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| Deprecations? | no', + [], + ['The `deprecations` should be `yes` or `no`. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#deprecations))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + '| How to test? | Fake', + [], + ['The `How to test` shouldn\'t be empty. ([Read explanation](https://devdocs.prestashop-project.org/9/contribute/contribution-guidelines/pull-requests/#how-to-test))'], + false, + true, + [], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | bug fix + | Category? | TE + ', + [], + [], + false, + true, + ['Bug fix'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | bug fix + | Category? | ME + ', + [], + [], + false, + true, + ['Bug fix'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | bug fix + | Category? | PM + ', + [], + [], + false, + true, + ['Bug fix'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | bug fix + | Category? | BO + ', + [], + [], + true, + true, + ['Bug fix'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | new feature + | Category? | TE + ', + [], + [], + false, + true, + ['Feature'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | new feature + | Category? | ME + ', + [], + [], + false, + true, + ['Feature'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | new feature + | Category? | PM + ', + [], + [], + false, + true, + ['Feature'], + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + ), + ' + | Type? | new feature + | Category? | BO + ', + [], + [], + true, + true, + ['Feature'], + ], + ]; + } + + public function testPullRequestNotFound(): void + { + $this->expectException(PullRequestNotFoundException::class); + $this->checkTableDescriptionCommandHandler->__invoke(new CheckTableDescriptionCommand( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'fake' + )); + } +} diff --git a/tests/PullRequest/Application/CommandHandler/WelcomeNewContributorCommandHandlerTest.php b/tests/PullRequest/Application/CommandHandler/WelcomeNewContributorCommandHandlerTest.php new file mode 100644 index 0000000..8e07ebb --- /dev/null +++ b/tests/PullRequest/Application/CommandHandler/WelcomeNewContributorCommandHandlerTest.php @@ -0,0 +1,111 @@ +prRepository = $this->getMockBuilder(InMemoryPullRequestRepository::class) + ->onlyMethods(['addWelcomeComment']) + ->getMock(); + + $this->committerRepository = $this->getMockBuilder(InMemoryCommitterRepository::class) + ->onlyMethods(['isNewContributor']) + ->getMock(); + + $this->welcomeNewContributorCommandHandler = new WelcomeNewContributorCommandHandler($this->committerRepository, $this->prRepository); + } + + /** + * @dataProvider provideTestHandle + */ + public function testHandle(PullRequestId $pullRequestId, bool $newContributor, bool $expectedComment): void + { + $this->prRepository->feed([ + PullRequest::create( + id: $pullRequestId, + labels: [], + approvals: [], + targetBranch: 'main', + ), + ]); + + // @phpstan-ignore-next-line + $this->prRepository + ->expects($expectedComment ? $this->once() : $this->never()) + ->method('addWelcomeComment') + ->with($pullRequestId, 'fakeContributor'); + + // @phpstan-ignore-next-line + $this->committerRepository + ->method('isNewContributor') + ->willReturn($newContributor); + + $this->welcomeNewContributorCommandHandler->__invoke(new WelcomeNewContributorCommand( + repositoryOwner: $pullRequestId->repositoryOwner, + repositoryName: $pullRequestId->repositoryName, + pullRequestNumber: $pullRequestId->pullRequestNumber, + contributor: 'fakeContributor', + )); + } + + /** + * @return array> + */ + public static function provideTestHandle(): array + { + return [ + [ + new PullRequestId( + repositoryOwner: 'fake', + repositoryName: 'fake', + pullRequestNumber: 'fake' + ), + false, + false, + ], + [ + new PullRequestId( + repositoryOwner: 'fake', + repositoryName: 'fake', + pullRequestNumber: 'fake' + ), + true, + true, + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'pullRequestNumber' + ), + false, + false, + ], + [ + new PullRequestId( + repositoryOwner: 'PrestaShop', + repositoryName: 'PrestaShop', + pullRequestNumber: 'pullRequestNumber' + ), + true, + true, + ], + ]; + } +} diff --git a/tests/Shared/Infrastructure/Webhook/GithubWebhookTest.php b/tests/Shared/Infrastructure/Webhook/GithubWebhookTest.php index 834b7ab..ffeef7e 100644 --- a/tests/Shared/Infrastructure/Webhook/GithubWebhookTest.php +++ b/tests/Shared/Infrastructure/Webhook/GithubWebhookTest.php @@ -5,6 +5,8 @@ namespace App\Tests\Shared\Infrastructure\Webhook; use App\PullRequest\Application\Command\AddLabelByApprovalCountCommand; +use App\PullRequest\Application\Command\CheckMilestoneCommand; +use App\PullRequest\Application\Command\CheckTableDescriptionCommand; use App\PullRequest\Application\Command\CheckTranslationsCommand; use App\PullRequest\Application\Command\RequestChangesCommand; use App\PullRequest\Application\Command\WelcomeNewContributorCommand; @@ -187,6 +189,11 @@ public static function successfulExecutionProvider(): array pullRequestNumber: '123', label: 'documentation', ), + new CheckMilestoneCommand( + repositoryOwner: 'owner', + repositoryName: 'repo', + pullRequestNumber: '123', + ), ], ], [ @@ -210,6 +217,11 @@ public static function successfulExecutionProvider(): array } }', [ + new CheckTableDescriptionCommand( + repositoryOwner: 'owner', + repositoryName: 'repo', + pullRequestNumber: '123', + ), new WelcomeNewContributorCommand( repositoryOwner: 'owner', repositoryName: 'repo', @@ -270,6 +282,11 @@ public static function successfulExecutionProvider(): array } }', [ + new CheckTableDescriptionCommand( + repositoryOwner: 'owner', + repositoryName: 'repo', + pullRequestNumber: '123', + ), new WelcomeNewContributorCommand( repositoryOwner: 'owner', repositoryName: 'repo', @@ -462,6 +479,32 @@ public static function successfulExecutionProvider(): array ), ], ], + [ + 'pull_request', + '{ + "action": "edited", + "pull_request": { + "base": { + "repo": { + "name": "repo", + "owner": { + "login": "owner" + } + } + }, + "state": "open", + "draft": false, + "number": 123 + } + }', + [ + new CheckTableDescriptionCommand( + repositoryOwner: 'owner', + repositoryName: 'repo', + pullRequestNumber: '123', + ), + ], + ], ]; }