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

Make relatedTicket mandatory in the PR's description #94

Merged
merged 4 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 2 additions & 4 deletions app/Resources/views/markdown/pr_table_errors.md.twig
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
<!-- PR_TABLE_DESCRIPTION_ERROR -->
Hi!
Hi, thanks for this contribution!

Your pull request description seems to be incomplete or malformed:

{% for error in errors %}
* {{ error.message }}
{% endfor %}

Would you mind completing the contribution table ? This would help us understand how interesting your contribution is.

Thank you!
Would you mind completing it? This would help us understand how interesting your contribution is, thank you very much!

(note: this is an automated message, but answering it will reach a real human )
6 changes: 4 additions & 2 deletions src/AppBundle/PullRequests/BodyParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function getBody()
*/
public function getDescription()
{
$regex = sprintf(self::DEFAULT_PATTERN, 'Description', '.+');
$regex = sprintf(self::DEFAULT_PATTERN, 'Description', '[^|]+');

return $this->extractWithRegex($regex);
}
Expand Down Expand Up @@ -142,11 +142,13 @@ public function isARefacto()
}

/**
* @Assert\NotBlank(message = "Each pull request should have a ticket linked. Please link it to an existing issue or create a new issue so that it can be reviewed and tested.")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @Assert\NotBlank(message = "Each pull request should have a ticket linked. Please link it to an existing issue or create a new issue so that it can be reviewed and tested.")
* @Assert\NotBlank(message = "Each pull request should have a ticket linked. Please link it to an existing issue or [create a new issue](https://github.com/PrestaShop/PrestaShop/issues/new/choose) so that it can be reviewed and tested.")

*
* @return string
*/
public function getRelatedTicket()
{
$regex = sprintf(self::DEFAULT_PATTERN, 'Fixed ticket', '.+');
$regex = sprintf(self::DEFAULT_PATTERN, 'Fixed ticket', '?:.*(#[0-9]+)');

return $this->extractWithRegex($regex);
}
Expand Down
16 changes: 15 additions & 1 deletion tests/AppBundle/PullRequests/BodyParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public function testGetTypeForBugFix()

public function testGetTicket()
{
$this->assertSame('http://forge.prestashop.com/browse/TEST-1234', $this->bodyParser->getRelatedTicket());
$this->assertSame('#1234', $this->bodyParser->getRelatedTicket());
}

public function testRepeatBodParserTestsWithSpaces()
Expand All @@ -100,4 +100,18 @@ public function testRepeatBodParserTestsWithSpaces()
$this->testIsDeprecated();
$this->testIsBackwardCompatible();
}

public function testGetEmptyDescription()
{
$this->bodyParser = new BodyParser(file_get_contents(__DIR__.'/../../Resources/PullRequestBody/missing_description.txt'));

$this->assertSame($this->bodyParser->getType(), 'bug fix');
$this->assertContains($this->bodyParser->getType(), $this->bodyParser->getValidTypes());
$this->assertFalse($this->bodyParser->isAFeature());
$this->assertFalse($this->bodyParser->isAnImprovement());
$this->assertTrue($this->bodyParser->isABugFix());
$this->assertFalse($this->bodyParser->isASmallFix());
$this->assertFalse($this->bodyParser->isARefacto());
$this->assertEmpty($this->bodyParser->getDescription());
}
}
64 changes: 64 additions & 0 deletions tests/AppBundle/PullRequests/ListenerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

namespace Tests\AppBundle\PullRequests;

use AppBundle\PullRequests\BodyParser;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\ValidatorBuilder;

class ListenerTest extends TestCase
{

private $validator;

public function setUp()
{
$this->validator = (new ValidatorBuilder())
->enableAnnotationMapping()
->getValidator();
}

/**
* @dataProvider getTests
*
* @param $description
* @param $expected
*/
public function testDescriptions($descriptionFilename, $expected)
{
$body = file_get_contents(__DIR__.'/../../Resources/PullRequestBody/'.$descriptionFilename);
$bodyParser = new BodyParser($body);

$validations = $this->validator->validate($bodyParser);
$this->assertEquals(count($expected), count($validations));
foreach ($validations as $validation) {
$this->assertContains($validation->getPropertyPath(), $expected);
}
}

public function getTests()
{
return [
'Valid description' => [
'bug_fix.txt',
[],
],
'Missing description' => [
'missing_description.txt',
['description'],
],
'Invalid type' => [
'invalid_type.txt',
['type'],
],
'Invalid category' => [
'invalid_category.txt',
['category'],
],
'No related ticked' => [
'no_related_ticket.txt',
['relatedTicket'],
],
];
}
}
2 changes: 1 addition & 1 deletion tests/Resources/PullRequestBody/bug_fix.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Please take the time to edit the "Answers" rows with the necessary information:-
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | http://forge.prestashop.com/browse/TEST-1234
| Fixed ticket? | #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->
Expand Down
2 changes: 1 addition & 1 deletion tests/Resources/PullRequestBody/feature.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Please take the time to edit the "Answers" rows with the necessary information:-
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | http://forge.prestashop.com/browse/TEST-1234
| Fixed ticket? | #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->
Expand Down
2 changes: 1 addition & 1 deletion tests/Resources/PullRequestBody/improvement.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Please take the time to edit the "Answers" rows with the necessary information:-
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | http://forge.prestashop.com/browse/TEST-1234
| Fixed ticket? | #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->
Expand Down
23 changes: 23 additions & 0 deletions tests/Resources/PullRequestBody/invalid_category.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Thank you for contributing to the PrestaShop project!

Please take the time to edit the "Answers" rows with the necessary information:-->

| Questions | Answers
| ------------- | -------------------------------------------------------
| Branch? | develop
| Description? | Such a great description
| Type? | bug fix
| Category? | AZ
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->

#### Important guidelines

* Make sure [your local branch is up to date](https://help.github.com/articles/syncing-a-fork/) before commiting your changes!
* Your code MUST respect [our Coding Standards](http://doc.prestashop.com/display/PS16/Coding+Standards) (for code written in PHP, JavaScript, HTML/CSS/Smarty/Twig, SQL)!
* Your commit name MUST respect our [naming convention](http://doc.prestashop.com/display/PS16/How+to+write+a+commit+message)!

23 changes: 23 additions & 0 deletions tests/Resources/PullRequestBody/invalid_type.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Thank you for contributing to the PrestaShop project!

Please take the time to edit the "Answers" rows with the necessary information:-->

| Questions | Answers
| ------------- | -------------------------------------------------------
| Branch? | develop
| Description? | Such a great description
| Type? |
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->

#### Important guidelines

* Make sure [your local branch is up to date](https://help.github.com/articles/syncing-a-fork/) before commiting your changes!
* Your code MUST respect [our Coding Standards](http://doc.prestashop.com/display/PS16/Coding+Standards) (for code written in PHP, JavaScript, HTML/CSS/Smarty/Twig, SQL)!
* Your commit name MUST respect our [naming convention](http://doc.prestashop.com/display/PS16/How+to+write+a+commit+message)!

23 changes: 23 additions & 0 deletions tests/Resources/PullRequestBody/missing_description.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Thank you for contributing to the PrestaShop project!

Please take the time to edit the "Answers" rows with the necessary information:-->

| Questions | Answers
| ------------- | -------------------------------------------------------
| Branch? | develop
| Description? |
| Type? | bug fix
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->

#### Important guidelines

* Make sure [your local branch is up to date](https://help.github.com/articles/syncing-a-fork/) before commiting your changes!
* Your code MUST respect [our Coding Standards](http://doc.prestashop.com/display/PS16/Coding+Standards) (for code written in PHP, JavaScript, HTML/CSS/Smarty/Twig, SQL)!
* Your commit name MUST respect our [naming convention](http://doc.prestashop.com/display/PS16/How+to+write+a+commit+message)!

23 changes: 23 additions & 0 deletions tests/Resources/PullRequestBody/no_related_ticket.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Thank you for contributing to the PrestaShop project!

Please take the time to edit the "Answers" rows with the necessary information:-->

| Questions | Answers
| ------------- | -------------------------------------------------------
| Branch? | develop
| Description? | Such a great description
| Type? | bug fix
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? |
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->

#### Important guidelines

* Make sure [your local branch is up to date](https://help.github.com/articles/syncing-a-fork/) before commiting your changes!
* Your code MUST respect [our Coding Standards](http://doc.prestashop.com/display/PS16/Coding+Standards) (for code written in PHP, JavaScript, HTML/CSS/Smarty/Twig, SQL)!
* Your commit name MUST respect our [naming convention](http://doc.prestashop.com/display/PS16/How+to+write+a+commit+message)!

2 changes: 1 addition & 1 deletion tests/Resources/PullRequestBody/with_spaces.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Please take the time to edit the "Answers" rows with the necessary information:-
| Category? | BO
| BC breaks? | no
| Deprecations? | yes
| Fixed ticket? | http://forge.prestashop.com/browse/TEST-1234
| Fixed ticket? | This fixes #1234
| How to test? | To test it, launch unit tests

<!-- Click the form's "Preview button" to make sure the table is functional in GitHub. Thank you! -->
Expand Down