SDK-4212 Introduced a logic for an empty PR (#240)

* SDK-4212 empty PR * SDK-4212 update steps executor * SDK-4212 update command * SDK-4212 add test * SDK-4212 update functionality * SDK-4212 fix test * SDK-4212 add tests * SDK-4212 add empty PR
spryker-sdk · Nov 9, 2023 · 4c00f86 · 4c00f86
1 parent e657080
commit 4c00f86
Show file tree

Hide file tree

Showing 13 changed files with 483 additions and 11 deletions.
diff --git a/config/Upgrade/services.yaml b/config/Upgrade/services.yaml
@@ -134,6 +134,8 @@ services:
         class: Upgrade\Application\Strategy\Composer\ComposerStrategy
         arguments:
             - '@composer.step_executor'
+            - '@create_empty_pr.step_executor'
+            - '@monolog.logger'
 
     release_app.step_executor:
         class: Upgrade\Application\Executor\StepExecutor
@@ -154,10 +156,21 @@ services:
             - - '@feature_dev_master.fixer'
               - '@feature_package.fixer'
 
+    create_empty_pr.step_executor:
+        class: Upgrade\Application\Executor\StepExecutor
+        arguments:
+            - '@monolog.logger'
+            - - '@create_branch.step'
+              - '@push_changes.step'
+              - '@create_pr.step'
+              - '@checkout.step'
+
     release_app.strategy:
         class: Upgrade\Application\Strategy\ReleaseApp\ReleaseAppStrategy
         arguments:
-            -  '@release_app.step_executor'
+            - '@release_app.step_executor'
+            - '@create_empty_pr.step_executor'
+            - '@monolog.logger'
 
     Upgrade\Application\Strategy\StrategyResolver:
         class: Upgrade\Application\Strategy\StrategyResolver

diff --git a/src/Upgrade/Application/Dto/StepsResponseDto.php b/src/Upgrade/Application/Dto/StepsResponseDto.php
@@ -97,6 +97,11 @@ class StepsResponseDto extends ResponseDto
      */
     protected int $currentReleaseGroupId = self::UNDEFINED_RELEASE_GROUP_ID;
 
+    /**
+     * @var bool
+     */
+    protected bool $isPullRequestSent = false;
+
     /**
      * @param bool $isSuccessful
      * @param string|null $outputMessage
@@ -285,6 +290,25 @@ public function addBlocker(ValidatorViolationDto $blockerInfo): void
         $this->blockers[$currentReleaseGroupId][] = $blockerInfo;
     }
 
+    /**
+     * @param string $title
+     *
+     * @return void
+     */
+    public function removeBlockersByTitle(string $title): void
+    {
+        $currentReleaseGroupId = $this->currentReleaseGroupId;
+
+        if (!isset($this->blockers[$currentReleaseGroupId])) {
+            return;
+        }
+
+        $this->blockers[$currentReleaseGroupId] = array_filter(
+            $this->blockers[$currentReleaseGroupId],
+            static fn (ValidatorViolationDto $violation): bool => $violation->getTitle() !== $title
+        );
+    }
+
     /**
      * @return array<int, array<\Upgrade\Application\Dto\ValidatorViolationDto>>
      */
@@ -293,6 +317,14 @@ public function getBlockers(): array
         return $this->blockers;
     }
 
+    /**
+     * @return bool
+     */
+    public function hasBlockers(): bool
+    {
+        return count($this->blockers) > 0;
+    }
+
     /**
      * @param int $releaseGroupId
      *
@@ -485,4 +517,38 @@ public function addFilterResponse(ReleaseGroupFilterResponseDto $responseDto): v
     {
         $this->filterResponseList[] = $responseDto;
     }
+
+    /**
+     * @return bool
+     */
+    public function isPullRequestSent(): bool
+    {
+        return $this->isPullRequestSent;
+    }
+
+    /**
+     * @param bool $isPullRequestSent
+     *
+     * @return void
+     */
+    public function setIsPullRequestSent(bool $isPullRequestSent): void
+    {
+        $this->isPullRequestSent = $isPullRequestSent;
+    }
+
+    /**
+     * @return bool
+     */
+    public function hasErrors(): bool
+    {
+        return count($this->getBlockers()) > 0
+            || count($this->getViolations()) > 0
+            || count(
+                array_filter(
+                    $this->getIntegratorResponseCollection(),
+                    static fn (IntegratorResponseDto $response): bool => count($response->getWarnings()) > 0,
+                ),
+            ) > 0
+            || count($this->getProjectViolations()) > 0;
+    }
 }
diff --git a/src/Upgrade/Application/Factory/ComposerViolationDtoFactory.php b/src/Upgrade/Application/Factory/ComposerViolationDtoFactory.php
@@ -0,0 +1,34 @@
+<?php
+
+/**
+ * Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
+ * Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
+ */
+
+declare(strict_types=1);
+
+namespace Upgrade\Application\Factory;
+
+use Upgrade\Application\Dto\PackageManagerResponseDto;
+use Upgrade\Application\Dto\ValidatorViolationDto;
+
+class ComposerViolationDtoFactory
+{
+    /**
+     * @var string
+     */
+    public const VIOLATION_TITLE = 'Composer issues';
+
+    /**
+     * @param \Upgrade\Application\Dto\PackageManagerResponseDto $packageManagerResponseDto
+     *
+     * @return \Upgrade\Application\Dto\ValidatorViolationDto
+     */
+    public function createFromPackageManagerResponse(PackageManagerResponseDto $packageManagerResponseDto): ValidatorViolationDto
+    {
+        $errorMessage = $packageManagerResponseDto->getOutputMessage() ?? 'Module fetcher error';
+        preg_match('/(?<error>Problem 1.*)/s', $errorMessage, $matches);
+
+        return new ValidatorViolationDto(static::VIOLATION_TITLE, $matches['error'] ?? $errorMessage);
+    }
+}
diff --git a/src/Upgrade/Application/Strategy/AbstractStrategy.php b/src/Upgrade/Application/Strategy/AbstractStrategy.php
@@ -20,20 +20,28 @@ abstract class AbstractStrategy implements StrategyInterface
      */
     protected StepExecutorInterface $stepExecutor;
 
+    /**
+     * @var \Upgrade\Application\Executor\StepExecutorInterface
+     */
+    protected StepExecutorInterface $sendEmptyPrStepExecutor;
+
     /**
      * @var \Psr\Log\LoggerInterface
      */
     protected LoggerInterface $logger;
 
     /**
      * @param \Upgrade\Application\Executor\StepExecutorInterface $stepExecutor
+     * @param \Upgrade\Application\Executor\StepExecutorInterface $sendEmptyPrStepExecutor
      * @param \Psr\Log\LoggerInterface $logger
      */
     public function __construct(
         StepExecutorInterface $stepExecutor,
+        StepExecutorInterface $sendEmptyPrStepExecutor,
         LoggerInterface $logger
     ) {
         $this->stepExecutor = $stepExecutor;
+        $this->sendEmptyPrStepExecutor = $sendEmptyPrStepExecutor;
         $this->logger = $logger;
     }
 
@@ -44,8 +52,40 @@ public function upgrade(): StepsResponseDto
     {
         $stepsResponseDto = $this->stepExecutor->execute(new StepsResponseDto(true));
 
+        $this->sendEmptyPrWithErrors($stepsResponseDto);
+
         $this->logger->info('Steps execution is finished', [$stepsResponseDto]);
 
         return $stepsResponseDto;
     }
+
+    /**
+     * @param \Upgrade\Application\Dto\StepsResponseDto $stepsResponseDto
+     *
+     * @return void
+     */
+    protected function sendEmptyPrWithErrors(StepsResponseDto $stepsResponseDto): void
+    {
+        if (!$this->shouldSendErrorsWithPr($stepsResponseDto)) {
+            return;
+        }
+
+        $this->logger->info('Send an empty PR with the errors');
+
+        $isSuccessful = $stepsResponseDto->getIsSuccessful();
+
+        $this->sendEmptyPrStepExecutor->execute($stepsResponseDto);
+
+        $stepsResponseDto->setIsSuccessful($isSuccessful);
+    }
+
+    /**
+     * @param \Upgrade\Application\Dto\StepsResponseDto $stepsResponseDto
+     *
+     * @return bool
+     */
+    protected function shouldSendErrorsWithPr(StepsResponseDto $stepsResponseDto): bool
+    {
+        return !$stepsResponseDto->getIsSuccessful() && $stepsResponseDto->hasErrors() && !$stepsResponseDto->isPullRequestSent();
+    }
 }
diff --git a/src/Upgrade/Application/Strategy/Composer/Fixer/FeatureDevMasterPackageFixerStep.php b/src/Upgrade/Application/Strategy/Composer/Fixer/FeatureDevMasterPackageFixerStep.php
@@ -11,6 +11,7 @@
 
 use Upgrade\Application\Adapter\PackageManagerAdapterInterface;
 use Upgrade\Application\Dto\StepsResponseDto;
+use Upgrade\Application\Factory\ComposerViolationDtoFactory;
 use Upgrade\Domain\Entity\Collection\PackageCollection;
 use Upgrade\Domain\Entity\Package;
 
@@ -87,6 +88,10 @@ public function run(StepsResponseDto $stepsExecutionDto): StepsResponseDto
         $stepsExecutionDto->setOutputMessages($messages);
         $stepsExecutionDto->addOutputMessage(sprintf('Versions were changed to %s for %s feature package(s)', static::MASK_ALIAS_DEV_MASTER, count($matches[static::KEY_FEATURES])));
 
+        if ($stepsExecutionDto->getIsSuccessful()) {
+            $stepsExecutionDto->removeBlockersByTitle(ComposerViolationDtoFactory::VIOLATION_TITLE);
+        }
+
         return $stepsExecutionDto;
     }
 }
diff --git a/src/Upgrade/Application/Strategy/Composer/Fixer/FeaturePackageFixerStep.php b/src/Upgrade/Application/Strategy/Composer/Fixer/FeaturePackageFixerStep.php
@@ -11,6 +11,7 @@
 
 use Upgrade\Application\Adapter\PackageManagerAdapterInterface;
 use Upgrade\Application\Dto\StepsResponseDto;
+use Upgrade\Application\Factory\ComposerViolationDtoFactory;
 use Upgrade\Domain\Entity\Collection\PackageCollection;
 use Upgrade\Domain\Entity\Package;
 
@@ -92,6 +93,10 @@ public function run(StepsResponseDto $stepsExecutionDto): StepsResponseDto
         $stepsExecutionDto->setOutputMessages($messages);
         $stepsExecutionDto->addOutputMessage(sprintf('Splitted %s feature package(s)', count($matches[static::KEY_FEATURES])));
 
+        if ($stepsExecutionDto->getIsSuccessful()) {
+            $stepsExecutionDto->removeBlockersByTitle(ComposerViolationDtoFactory::VIOLATION_TITLE);
+        }
+
         return $stepsExecutionDto;
     }
 

diff --git a/src/Upgrade/Application/Strategy/ReleaseApp/Processor/SequentialReleaseGroupProcessor.php b/src/Upgrade/Application/Strategy/ReleaseApp/Processor/SequentialReleaseGroupProcessor.php
@@ -13,6 +13,7 @@
 use ReleaseApp\Infrastructure\Shared\Dto\Collection\ReleaseGroupDtoCollection;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 use Upgrade\Application\Dto\StepsResponseDto;
+use Upgrade\Application\Factory\ComposerViolationDtoFactory;
 use Upgrade\Application\Strategy\ReleaseApp\Processor\Event\ReleaseGroupProcessorEvent;
 use Upgrade\Application\Strategy\ReleaseApp\Processor\Event\ReleaseGroupProcessorPostRequireEvent;
 use Upgrade\Application\Strategy\ReleaseApp\ReleaseGroupFilter\ReleaseGroupFilterInterface;
@@ -43,28 +44,36 @@ class SequentialReleaseGroupProcessor extends BaseReleaseGroupProcessor
      */
     protected ReleaseGroupFilterInterface $releaseGroupPackageFilter;
 
+    /**
+     * @var \Upgrade\Application\Factory\ComposerViolationDtoFactory
+     */
+    protected ComposerViolationDtoFactory $composerViolationDtoFactory;
+
     /**
      * @param \Upgrade\Application\Strategy\ReleaseApp\Validator\ReleaseGroupSoftValidatorInterface $releaseGroupValidateManager
      * @param \Upgrade\Application\Strategy\ReleaseApp\Validator\ThresholdSoftValidatorInterface $thresholdSoftValidator
      * @param \Upgrade\Application\Strategy\ReleaseApp\Processor\ModuleFetcher $moduleFetcher
      * @param \Upgrade\Application\Strategy\ReleaseApp\ReleaseGroupFilter\ReleaseGroupFilterInterface $releaseGroupFilter
      * @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $eventDispatcher
      * @param \Psr\Log\LoggerInterface $logger
+     * @param \Upgrade\Application\Factory\ComposerViolationDtoFactory $composerViolationDtoFactory
      */
     public function __construct(
         ReleaseGroupSoftValidatorInterface $releaseGroupValidateManager,
         ThresholdSoftValidatorInterface $thresholdSoftValidator,
         ModuleFetcher $moduleFetcher,
         ReleaseGroupFilterInterface $releaseGroupFilter,
         EventDispatcherInterface $eventDispatcher,
-        LoggerInterface $logger
+        LoggerInterface $logger,
+        ComposerViolationDtoFactory $composerViolationDtoFactory
     ) {
         parent::__construct($eventDispatcher, $logger);
 
         $this->releaseGroupValidator = $releaseGroupValidateManager;
         $this->thresholdValidator = $thresholdSoftValidator;
         $this->moduleFetcher = $moduleFetcher;
         $this->releaseGroupPackageFilter = $releaseGroupFilter;
+        $this->composerViolationDtoFactory = $composerViolationDtoFactory;
     }
 
     /**
@@ -151,15 +160,19 @@ public function process(ReleaseGroupDtoCollection $requireRequestCollection, Ste
 
             if (!$response->isSuccessful()) {
                 $stepsExecutionDto->setIsSuccessful(false);
+                $composerViolation = $this->composerViolationDtoFactory->createFromPackageManagerResponse($response);
+
                 $stepsExecutionDto->setError(
-                    Error::createClientCodeError($response->getOutputMessage() ?? 'Module fetcher error'),
+                    Error::createClientCodeError($composerViolation->getMessage()),
                 );
                 $this->logger->debug(sprintf(
                     'Release group `%s` applying failed, message: %s',
                     $releaseGroup->getId(),
                     $response->getOutputMessage(),
                 ));
 
+                $stepsExecutionDto->addBlocker($composerViolation);
+
                 break;
             }
 

diff --git a/src/Upgrade/Infrastructure/VersionControlSystem/Generator/PullRequestDataGenerator.php b/src/Upgrade/Infrastructure/VersionControlSystem/Generator/PullRequestDataGenerator.php
@@ -91,11 +91,13 @@ public function buildBody(
             . $this->buildViolationsWarnings($stepsResponseDto)
             . $this->buildIntegratorWarnings($stepsResponseDto);
 
-        return $this->buildHeaderText($stepsResponseDto, $releaseGroupId)
+        $hasWarnings = trim($warningsSection) !== '';
+
+        return $this->buildHeaderText($stepsResponseDto, $releaseGroupId, $hasWarnings)
             . PHP_EOL
             . $this->buildReleaseGroupsTable($stepsResponseDto, $releaseGroupId)
             . PHP_EOL
-            . (trim($warningsSection) !== '' ? '## Warnings' . PHP_EOL : '')
+            . ($hasWarnings ? '## ' . $this->createErrorTitle($stepsResponseDto) . PHP_EOL : '')
             . $warningsSection
             . PHP_EOL
             . $this->buildReleaseGroupIntegrationGuideTable($stepsResponseDto)
@@ -108,11 +110,16 @@ public function buildBody(
     /**
      * @param \Upgrade\Application\Dto\StepsResponseDto $stepsResponseDto
      * @param int|null $releaseGroupId
+     * @param bool $hasWarnings
      *
      * @return string
      */
-    protected function buildHeaderText(StepsResponseDto $stepsResponseDto, ?int $releaseGroupId): string
+    protected function buildHeaderText(StepsResponseDto $stepsResponseDto, ?int $releaseGroupId, bool $hasWarnings): string
     {
+        if ($hasWarnings && count($stepsResponseDto->getAppliedReleaseGroups()) === 0) {
+            return 'Unfortunately Upgrader was not able to create a pull request with code changes due to errors. Please see the list below and resolve them.';
+        }
+
         $releaseGroupStatDto = $stepsResponseDto->getReleaseGroupStatDto();
         $composerDiffDto = $stepsResponseDto->getComposerLockDiff();
         $countOfPackages = 0;
@@ -214,6 +221,16 @@ protected function buildReleaseGroupsTable(
         return $text;
     }
 
+    /**
+     * @param \Upgrade\Application\Dto\StepsResponseDto $stepsResponseDto
+     *
+     * @return string
+     */
+    protected function createErrorTitle(StepsResponseDto $stepsResponseDto): string
+    {
+        return count($stepsResponseDto->getAppliedReleaseGroups()) > 0 ? 'Warnings' : 'Errors :warning:';
+    }
+
     /**
      * @param \Upgrade\Application\Dto\StepsResponseDto $stepsResponseDto
      *
@@ -392,7 +409,7 @@ protected function buildWarningsTextBlocks(array $warnings): string
         foreach ($warnings as $title => $messages) {
             $text .= sprintf('<details><summary><h4>%s</h4></summary>', $title);
             foreach ($messages as $message) {
-                $text .= sprintf('<p>%s</p>', $message);
+                $text .= sprintf('<p>%s</p>', nl2br($message));
             }
             $text .= '</details>';
         }