Add branch review creation (#276)

123inkt · May 19, 2023 · 1bc1496 · 1bc1496
1 parent cda7b07
commit 1bc1496
Show file tree

Hide file tree

Showing 91 changed files with 1,825 additions and 379 deletions.
diff --git a/assets/app.ts b/assets/app.ts
@@ -14,6 +14,7 @@ import './styles/review-file-diff-side-by-side.scss';
 import './styles/review-timeline.scss';
 import './styles/review.revisions.scss';
 import './styles/review.notifications.scss';
+import './styles/repository.branch-list.scss';
 import './styles/markdown.scss';
 import './styles/comment.scss';
 import './styles/user-settings.scss';

diff --git a/assets/styles/repository.branch-list.scss b/assets/styles/repository.branch-list.scss
@@ -0,0 +1,5 @@
+.repository-branch-list {
+    .col-create-branch {
+        width: 200px;
+    }
+}
diff --git a/assets/styles/review.revisions.scss b/assets/styles/review.revisions.scss
@@ -4,6 +4,12 @@
     grid-template-columns: auto 1fr 50px;
 }
 
+.review-revision-branch--grid {
+    display: grid;
+    grid-column-gap: 5px;
+    grid-template-columns: 1fr 50px;
+}
+
 .review-revision--author-time {
     font-size: 0.9rem;
 }

diff --git a/config/packages/doctrine.php b/config/packages/doctrine.php
@@ -4,6 +4,7 @@
 
 use DR\Review\Doctrine\Type\CodeReviewerStateType;
 use DR\Review\Doctrine\Type\CodeReviewStateType;
+use DR\Review\Doctrine\Type\CodeReviewType;
 use DR\Review\Doctrine\Type\ColorThemeType;
 use DR\Review\Doctrine\Type\CommentStateType;
 use DR\Review\Doctrine\Type\DiffAlgorithmType;
@@ -28,17 +29,18 @@
                     'enum' => 'string'
                 ],
                 'types'          => [
-                    UriType::TYPE                       => UriType::class,
-                    FrequencyType::TYPE                 => FrequencyType::class,
-                    DiffAlgorithmType::TYPE             => DiffAlgorithmType::class,
-                    MailThemeType::TYPE                 => MailThemeType::class,
-                    FilterType::TYPE                    => FilterType::class,
                     CodeReviewStateType::TYPE           => CodeReviewStateType::class,
+                    CodeReviewType::TYPE                => CodeReviewType::class,
                     CodeReviewerStateType::TYPE         => CodeReviewerStateType::class,
+                    ColorThemeType::TYPE                => ColorThemeType::class,
                     CommentStateType::TYPE              => CommentStateType::class,
+                    DiffAlgorithmType::TYPE             => DiffAlgorithmType::class,
+                    FilterType::TYPE                    => FilterType::class,
+                    FrequencyType::TYPE                 => FrequencyType::class,
+                    MailThemeType::TYPE                 => MailThemeType::class,
                     NotificationStatusType::TYPE        => NotificationStatusType::class,
-                    ColorThemeType::TYPE                => ColorThemeType::class,
-                    SpaceSeparatedStringValueType::TYPE => SpaceSeparatedStringValueType::class
+                    SpaceSeparatedStringValueType::TYPE => SpaceSeparatedStringValueType::class,
+                    UriType::TYPE                       => UriType::class,
                 ]
             ]
         ]

diff --git a/config/services.php b/config/services.php
@@ -27,6 +27,7 @@
 use DR\Review\Security\AzureAd\AzureAdUserBadgeFactory;
 use DR\Review\Security\AzureAd\LoginService;
 use DR\Review\Security\UserChecker;
+use DR\Review\Service\CodeReview\CodeReviewFileService;
 use DR\Review\Service\CodeReview\Comment\CommonMarkdownConverter;
 use DR\Review\Service\Git\CacheableGitRepositoryService;
 use DR\Review\Service\Git\GitCommandBuilderFactory;
@@ -53,6 +54,7 @@
 use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\HttpClient\NativeHttpClient;
+use Symfony\Contracts\Cache\CacheInterface;
 use TheNetworg\OAuth2\Client\Provider\Azure;
 use function Symfony\Component\DependencyInjection\Loader\Configurator\inline_service;
 use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
@@ -156,6 +158,7 @@
     $services->set('lock.review.diff.service', LockableReviewDiffService::class)->arg('$diffService', service('review.diff.service'));
     $services->set(ReviewDiffServiceInterface::class, CacheableReviewDiffService::class)->arg('$diffService', service('lock.review.diff.service'));
     $services->set(ReviewRouter::class)->decorate('router')->args([service('.inner')]);
+    $services->set(CodeReviewFileService::class)->arg('$revisionCache', service(CacheInterface::class . ' $revisionCache'));
 
     // Code inspection parsers
     $services->set(CheckStyleIssueParser::class)->tag('code_inspection_issue_parser', ['key' => CheckStyleIssueParser::FORMAT]);

diff --git a/docker/nodejs/run/server.js b/docker/nodejs/run/server.js
@@ -3,11 +3,13 @@ const url  = require('url');
 // load common languages
 const hljs = require('highlight.js/lib/common');
 // supplement with custom languages
-hljs.registerLanguage('ini', require('highlight.js/lib/languages/ini'));
 hljs.registerLanguage('apache', require('highlight.js/lib/languages/apache'));
+hljs.registerLanguage('dockerfile', require('highlight.js/lib/languages/dockerfile'));
+hljs.registerLanguage('ini', require('highlight.js/lib/languages/ini'));
 hljs.registerLanguage('twig', require('highlight.js/lib/languages/twig'));
 hljs.registerLanguage('xml', require('highlight.js/lib/languages/xml'));
 
+
 const hostname = process.env.NODEJS_HOST;
 const port     = process.env.NODEJS_PORT;
 

diff --git a/migrations/Version20230517064259.php b/migrations/Version20230517064259.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Auto-generated Migration: Please modify to your needs!
+ */
+final class Version20230517064259 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return '';
+    }
+
+    public function up(Schema $schema): void
+    {
+        // this up() migration is auto-generated, please modify it to your needs
+        $this->addSql(
+            'ALTER TABLE code_review ADD type ENUM(\'commits\', \'branch\') DEFAULT \'commits\' NOT NULL AFTER `description`'
+        );
+    }
+
+    public function down(Schema $schema): void
+    {
+        // this down() migration is auto-generated, please modify it to your needs
+        $this->addSql('ALTER TABLE code_review DROP type');
+    }
+}
diff --git a/src/Controller/App/Review/CreateBranchReviewController.php b/src/Controller/App/Review/CreateBranchReviewController.php
@@ -0,0 +1,59 @@
+<?php
+declare(strict_types=1);
+
+namespace DR\Review\Controller\App\Review;
+
+use DR\Review\Controller\AbstractController;
+use DR\Review\Doctrine\Type\CodeReviewType;
+use DR\Review\Entity\Repository\Repository;
+use DR\Review\Message\Review\ReviewCreated;
+use DR\Review\Repository\Review\CodeReviewRepository;
+use DR\Review\Security\Role\Roles;
+use DR\Review\Service\CodeReview\CodeReviewCreationService;
+use DR\Review\Service\CodeReview\CodeReviewRevisionService;
+use DR\Review\Utility\Arrays;
+use Symfony\Bridge\Doctrine\Attribute\MapEntity;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+use Symfony\Component\Messenger\MessageBusInterface;
+use Symfony\Component\Routing\Annotation\Route;
+use Symfony\Component\Security\Http\Attribute\IsGranted;
+use Throwable;
+
+class CreateBranchReviewController extends AbstractController
+{
+    public function __construct(
+        private readonly CodeReviewCreationService $reviewCreationService,
+        private readonly CodeReviewRevisionService $revisionService,
+        private readonly CodeReviewRepository $reviewRepository,
+        private readonly MessageBusInterface $messageBus,
+    ) {
+    }
+
+    /**
+     * @throws Throwable
+     */
+    #[Route('app/review/{repositoryId<\d+>}/branch-review', name: self::class, methods: 'POST')]
+    #[IsGranted(Roles::ROLE_USER)]
+    public function __invoke(Request $request, #[MapEntity(expr: 'repository.find(repositoryId)')] Repository $repository): Response
+    {
+        $branchName = (string)$request->request->get('branch', '');
+        if (trim($branchName) === '') {
+            throw new BadRequestHttpException('Branch request property is mandatory');
+        }
+
+        $review = $this->reviewRepository->findOneBy(['repository' => $repository, 'type' => CodeReviewType::BRANCH, 'referenceId' => $branchName]);
+        if ($review !== null) {
+            return $this->redirectToRoute(ReviewController::class, ['review' => $review]);
+        }
+
+        $review   = $this->reviewCreationService->createFromBranch($repository, $branchName);
+        $revision = Arrays::lastOrNull($this->revisionService->getRevisions($review));
+        $this->reviewRepository->save($review, true);
+
+        $this->messageBus->dispatch(new ReviewCreated((int)$review->getId(), (int)$revision?->getId(), $this->getUser()->getId()));
+
+        return $this->redirectToRoute(ReviewController::class, ['review' => $review]);
+    }
+}
diff --git a/src/Controller/App/Review/CreateReviewFromRevisionController.php b/src/Controller/App/Review/CreateReviewFromRevisionController.php
@@ -30,7 +30,7 @@ public function __construct(
     /**
      * @throws Throwable
      */
-    #[Route('app/review/createFrom/{id<\d+>}', name: self::class, methods: 'POST')]
+    #[Route('app/review/create-from/{id<\d+>}', name: self::class, methods: 'POST')]
     #[IsGranted(Roles::ROLE_USER)]
     public function __invoke(#[MapEntity] Revision $revision): Response
     {

diff --git a/src/Controller/App/Review/ReviewFileTreeController.php b/src/Controller/App/Review/ReviewFileTreeController.php
@@ -7,6 +7,7 @@
 use DR\Review\Entity\Review\CodeReview;
 use DR\Review\Security\Role\Roles;
 use DR\Review\Service\CodeReview\CodeReviewFileService;
+use DR\Review\Service\CodeReview\CodeReviewRevisionService;
 use DR\Review\Service\Git\Review\ReviewSessionService;
 use DR\Review\ViewModel\App\Review\FileTreeViewModel;
 use DR\Review\ViewModelProvider\FileTreeViewModelProvider;
@@ -22,7 +23,8 @@ class ReviewFileTreeController extends AbstractController
     public function __construct(
         private readonly FileTreeViewModelProvider $viewModelProvider,
         private readonly CodeReviewFileService $fileService,
-        private readonly ReviewSessionService $sessionService
+        private readonly ReviewSessionService $sessionService,
+        private readonly CodeReviewRevisionService $revisionService,
     ) {
     }
 
@@ -38,7 +40,7 @@ public function __invoke(Request $request, #[MapEntity] CodeReview $review): arr
         // get diff files for review
         [$fileTree, $selectedFile] = $this->fileService->getFiles(
             $review,
-            $review->getRevisions()->toArray(),
+            $this->revisionService->getRevisions($review),
             $request->query->get('filePath'),
             $this->sessionService->getDiffComparePolicyForUser()
         );

diff --git a/src/Controller/App/Review/UpdateFileSeenStatusController.php b/src/Controller/App/Review/UpdateFileSeenStatusController.php
@@ -7,6 +7,7 @@
 use DR\Review\Entity\Review\CodeReview;
 use DR\Review\Request\Review\FileSeenStatusRequest;
 use DR\Review\Security\Role\Roles;
+use DR\Review\Service\CodeReview\CodeReviewRevisionService;
 use DR\Review\Service\CodeReview\FileSeenStatusService;
 use DR\Review\Service\Git\Review\FileDiffOptions;
 use DR\Review\Service\Git\Review\ReviewDiffService\ReviewDiffServiceInterface;
@@ -23,7 +24,8 @@ class UpdateFileSeenStatusController extends AbstractController
     public function __construct(
         private readonly FileSeenStatusService $fileSeenStatusService,
         private readonly ReviewDiffServiceInterface $diffService,
-        private readonly ReviewSessionService $sessionService
+        private readonly ReviewSessionService $sessionService,
+        private readonly CodeReviewRevisionService $revisionService,
     ) {
     }
 
@@ -36,9 +38,9 @@ public function __invoke(FileSeenStatusRequest $request, #[MapEntity] CodeReview
     {
         $filePath   = $request->getFilePath();
         $seenStatus = $request->getSeenStatus();
-        $files      = $this->diffService->getDiffFiles(
+        $files      = $this->diffService->getDiffForRevisions(
             Assert::notNull($review->getRepository()),
-            $review->getRevisions()->toArray(),
+            $this->revisionService->getRevisions($review),
             new FileDiffOptions(FileDiffOptions::DEFAULT_LINE_DIFF, $this->sessionService->getDiffComparePolicyForUser())
         );
 

diff --git a/src/Doctrine/Type/CodeReviewType.php b/src/Doctrine/Type/CodeReviewType.php
@@ -0,0 +1,13 @@
+<?php
+declare(strict_types=1);
+
+namespace DR\Review\Doctrine\Type;
+
+class CodeReviewType extends AbstractEnumType
+{
+    public const COMMITS = 'commits';
+    public const BRANCH  = 'branch';
+
+    public const TYPE   = 'enum_code_review_type';
+    public const VALUES = [self::COMMITS, self::BRANCH];
+}
diff --git a/src/Entity/Review/CodeReview.php b/src/Entity/Review/CodeReview.php
@@ -18,6 +18,7 @@
 use DR\Review\ApiPlatform\StateProcessor\CodeReviewProcessor;
 use DR\Review\Doctrine\Type\CodeReviewerStateType;
 use DR\Review\Doctrine\Type\CodeReviewStateType;
+use DR\Review\Doctrine\Type\CodeReviewType;
 use DR\Review\Entity\PropertyChangeTrait;
 use DR\Review\Entity\Repository\Repository;
 use DR\Review\Entity\Revision\Revision;
@@ -95,6 +96,9 @@ class CodeReview
     #[ORM\Column(length: 255)]
     private ?string $description = null;
 
+    #[ORM\Column(type: CodeReviewType::TYPE, options: ["default" => CodeReviewType::COMMITS])]
+    private string $type = CodeReviewType::COMMITS;
+
     #[ORM\Column(type: CodeReviewStateType::TYPE, options: ["default" => CodeReviewStateType::OPEN])]
     #[Groups(['code_review_write'])]
     private string $state = CodeReviewStateType::OPEN;
@@ -193,6 +197,18 @@ public function setDescription(?string $description): self
         return $this;
     }
 
+    public function getType(): string
+    {
+        return $this->type;
+    }
+
+    public function setType(string $type): self
+    {
+        $this->type = $type;
+
+        return $this;
+    }
+
     public function getState(): ?string
     {
         return $this->state;

diff --git a/src/MessageHandler/DiffFileCacheMessageHandler.php b/src/MessageHandler/DiffFileCacheMessageHandler.php
@@ -3,6 +3,7 @@
 
 namespace DR\Review\MessageHandler;
 
+use DR\Review\Doctrine\Type\CodeReviewType;
 use DR\Review\Entity\Git\Diff\DiffComparePolicy;
 use DR\Review\Message\Review\ReviewCreated;
 use DR\Review\Message\Revision\ReviewRevisionAdded;
@@ -37,6 +38,10 @@ public function handleEvent(ReviewCreated|ReviewRevisionAdded|ReviewRevisionRemo
             return;
         }
 
+        if ($review->getType() === CodeReviewType::BRANCH) {
+            return;
+        }
+
         // @codeCoverageIgnoreStart
         $systemLoad = function_exists('sys_getloadavg') ? (sys_getloadavg()[1] ?? 0) : 0; // system load in the last 5 minutes
         if ($systemLoad >= 1.1) {
@@ -55,7 +60,7 @@ public function handleEvent(ReviewCreated|ReviewRevisionAdded|ReviewRevisionRemo
         }
 
         // fetch diff files for current review and trigger cache refresh
-        $this->diffService->getDiffFiles(
+        $this->diffService->getDiffForRevisions(
             Assert::notNull($review->getRepository()),
             $revisions->toArray(),
             new FileDiffOptions(FileDiffOptions::DEFAULT_LINE_DIFF, DiffComparePolicy::ALL)

diff --git a/src/Repository/Review/CodeReviewRepository.php b/src/Repository/Review/CodeReviewRepository.php
@@ -85,15 +85,17 @@ public function getPaginatorForSearchQuery(
     /**
      * @throws NonUniqueResultException
      */
-    public function findOneByReferenceId(int $repositoryId, string $referenceId): ?CodeReview
+    public function findOneByReferenceId(int $repositoryId, string $referenceId, string $reviewType): ?CodeReview
     {
         /** @var CodeReview|null $review */
         $review = $this->createQueryBuilder('c')
             ->where('c.referenceId = :referenceId')
             ->andWhere('c.repository = :repositoryId')
+            ->andWhere('c.type = :type')
             ->orderBy('c.id', 'DESC')
             ->setParameter('referenceId', $referenceId)
             ->setParameter('repositoryId', $repositoryId)
+            ->setParameter('type', $reviewType)
             ->setMaxResults(1)
             ->getQuery()
             ->getOneOrNullResult(AbstractQuery::HYDRATE_OBJECT);

diff --git a/src/Service/CodeHighlight/HighlightedFileService.php b/src/Service/CodeHighlight/HighlightedFileService.php
@@ -8,6 +8,8 @@
 use Exception;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Throwable;
 
@@ -26,7 +28,7 @@ public function __construct(
     }
 
     /**
-     * @throws Exception
+     * @throws Exception|TransportExceptionInterface
      */
     public function fromDiffFile(DiffFile $diffFile): ?HighlightedFile
     {
@@ -45,6 +47,12 @@ public function fromDiffFile(DiffFile $diffFile): ?HighlightedFile
             return null;
         }
 
+        if ($response->getStatusCode() !== Response::HTTP_OK) {
+            $this->logger?->info('Failed to get code highlighting: ' . $response->getContent(false));
+
+            return null;
+        }
+
         return new HighlightedFile(
             $diffFile->getPathname(),
             fn() => $this->splitter->split($response->getContent(false))