Skip to content

Commit 2b5c034

Browse files
frankdekkerfrankdekker
authored
Make gitlab sync mandatory (#1208)
1 parent a6fe33a commit 2b5c034

File tree

11 files changed

+314
-4
lines changed

11 files changed

+314
-4
lines changed

‎.env‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ GITLAB_ACCESS_TOKEN=
111111
# Set callback url to: https://<your-123view-domain/app/user/gitlab-auth-finalize
112112
GITLAB_COMMENT_SYNC=false
113113
GITLAB_REVIEWER_SYNC=false
114+
GITLAB_SYNC_MANDATORY=false
114115

115116
##
116117
# The regex pattern to match against branches to sync reviewers approval status to the PR/MR

‎config/services.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
->bind('$allowCustomRecipients', '%env(bool:ALLOW_CUSTOM_RECIPIENTS_PER_RULE)%')
103103
->bind('$gitlabCommentSyncEnabled', '%env(bool:GITLAB_COMMENT_SYNC)%')
104104
->bind('$gitlabReviewerSyncEnabled', '%env(bool:GITLAB_REVIEWER_SYNC)%')
105+
->bind('$gitlabSyncMandatory', '%env(bool:GITLAB_SYNC_MANDATORY)%')
105106
->bind('$gitlabApiUrl', '%env(GITLAB_API_URL)%')
106107
->bind('$applicationName', '%env(APP_NAME)%')
107108
->bind('$appAbsoluteUrl', '%env(APP_ABSOLUTE_URL)%')

‎phpmd.baseline.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
<violation rule="PHPMD\Rule\Design\TooManyFields" file="src/Entity/Repository/Repository.php"/>
99
<violation rule="PHPMD\Rule\Naming\LongVariable" file="src/Entity/Repository/Repository.php"/>
1010
<violation rule="PHPMD\Rule\Design\TooManyFields" file="src/Entity/Review/CodeReview.php"/>
11+
<violation rule="PHPMD\Rule\UnusedFormalParameter" file="src/EventSubscriber/MandatoryGitlabSyncSubscriber.php"/>
1112
<violation rule="PHPMD\Rule\UnusedFormalParameter" file="src/MessageHandler/Gitlab/ReviewerStateChangeMessageHandler.php"/>
1213
<violation rule="PHPMD\Rule\Design\LongParameterList" file="src/Model/Review/CodeReviewDto.php" method="__construct"/>
1314
<violation rule="PHPMD\Rule\UnusedFormalParameter" file="src/Service/Api/Gitlab/GitlabApiProvider.php"/>

‎src/Controller/App/User/UserGitSyncController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
class UserGitSyncController extends AbstractController
1616
{
17-
public function __construct(private readonly bool $gitlabCommentSyncEnabled)
17+
public function __construct(private readonly bool $gitlabCommentSyncEnabled, private readonly bool $gitlabReviewerSyncEnabled)
1818
{
1919
}
2020

@@ -30,6 +30,6 @@ public function __invoke(): array
3030
$user = $this->getUser();
3131
$token = $user->getGitAccessTokens()->findFirst(static fn($key, $token) => $token->getGitType() === RepositoryGitType::GITLAB);
3232

33-
return ['gitSyncModel' => new UserGitSyncViewModel($this->gitlabCommentSyncEnabled, $token !== null)];
33+
return ['gitSyncModel' => new UserGitSyncViewModel($this->gitlabCommentSyncEnabled || $this->gitlabReviewerSyncEnabled, $token !== null)];
3434
}
3535
}

‎src/Controller/App/User/UserMandatoryGitlabSyncController.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace DR\Review\Controller\App\User;
5+
6+
use DR\Review\Controller\AbstractController;
7+
use DR\Review\Security\Role\Roles;
8+
use Symfony\Bridge\Twig\Attribute\Template;
9+
use Symfony\Component\Routing\Annotation\Route;
10+
use Symfony\Component\Security\Http\Attribute\IsGranted;
11+
12+
class UserMandatoryGitlabSyncController extends AbstractController
13+
{
14+
#[Route('/app/user-gitlab-sync-mandatory', self::class, methods: ['GET'])]
15+
#[Template('app/user/user.gitlab.sync.mandatory.html.twig')]
16+
#[IsGranted(Roles::ROLE_USER)]
17+
public function __invoke(): void
18+
{
19+
// no special logic required, template only
20+
}
21+
}

‎src/EventSubscriber/MandatoryGitlabSyncSubscriber.php‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace DR\Review\EventSubscriber;
5+
6+
use DR\Review\Controller\App\User\Gitlab\UserGitlabOAuth2FinishController;
7+
use DR\Review\Controller\App\User\Gitlab\UserGitlabOAuth2StartController;
8+
use DR\Review\Controller\App\User\UserMandatoryGitlabSyncController;
9+
use DR\Review\Controller\Auth\LogoutController;
10+
use DR\Review\Doctrine\Type\RepositoryGitType;
11+
use DR\Review\Entity\User\User;
12+
use Symfony\Bundle\SecurityBundle\Security;
13+
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
14+
use Symfony\Component\HttpFoundation\RedirectResponse;
15+
use Symfony\Component\HttpKernel\Event\RequestEvent;
16+
use Symfony\Component\HttpKernel\KernelEvents;
17+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
18+
19+
#[AsEventListener(KernelEvents::REQUEST)]
20+
readonly class MandatoryGitlabSyncSubscriber
21+
{
22+
private const array ALLOWED_CONTROLLERS = [
23+
UserGitlabOAuth2StartController::class,
24+
UserGitlabOAuth2FinishController::class,
25+
UserMandatoryGitlabSyncController::class,
26+
LogoutController::class
27+
];
28+
29+
public function __construct(
30+
private bool $gitlabCommentSyncEnabled,
31+
private bool $gitlabReviewerSyncEnabled,
32+
private bool $gitlabSyncMandatory,
33+
private Security $security,
34+
private UrlGeneratorInterface $urlGenerator
35+
) {
36+
}
37+
38+
public function __invoke(RequestEvent $event): void
39+
{
40+
$user = $this->security->getUser();
41+
// skip if not logged in
42+
if ($user instanceof User === false) {
43+
return;
44+
}
45+
46+
// skip if the controller is whitelisted
47+
$attributes = $event->getRequest()->attributes->all();
48+
if (in_array($attributes['_controller'] ?? '', self::ALLOWED_CONTROLLERS, true)) {
49+
return;
50+
}
51+
52+
// skip api urls
53+
if (str_starts_with($event->getRequest()->getRequestUri(), '/api')) {
54+
return;
55+
}
56+
57+
// skip if sync is not configured
58+
if ($this->gitlabCommentSyncEnabled === false && $this->gitlabReviewerSyncEnabled === false) {
59+
return;
60+
}
61+
62+
// skip if sync is not mandatory
63+
if ($this->gitlabSyncMandatory === false) {
64+
return;
65+
}
66+
67+
// skip if user already has a gitlab token
68+
$token = $user->getGitAccessTokens()->findFirst(static fn($key, $token) => $token->getGitType() === RepositoryGitType::GITLAB);
69+
if ($token !== null) {
70+
return;
71+
}
72+
73+
// redirect to mandatory sync page
74+
$url = $this->urlGenerator->generate(UserMandatoryGitlabSyncController::class);
75+
$event->setResponse(new RedirectResponse($url));
76+
}
77+
}

‎templates/app/user/user.gitlab.sync.mandatory.html.twig‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{% extends 'app/app.base.html.twig' %}
2+
3+
{% block page_content %}
4+
<div class="mt-5 text-center">
5+
<div class="alert alert-danger">
6+
{{ 'user.gitlab.sync.mandatory.text'|trans({app_name: app_name}) }}
7+
</div>
8+
9+
<div class="mt-3">
10+
{{ 'git.sync.explanation'|trans }}
11+
</div>
12+
13+
<div class="mt-3">
14+
<a class="btn btn-primary" href="{{ path('DR\\Review\\Controller\\App\\User\\Gitlab\\UserGitlabOAuth2StartController') }}">
15+
{{ 'git.sync.enable'|trans }}
16+
</a>
17+
</div>
18+
</div>
19+
{% endblock %}

‎tests/Unit/Controller/App/User/UserGitSyncControllerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ public function testInvokeWithoutToken(): void
4343

4444
public function getController(): AbstractController
4545
{
46-
return new UserGitSyncController(true);
46+
return new UserGitSyncController(true, true);
4747
}
4848
}

‎tests/Unit/Controller/App/User/UserMandatoryGitlabSyncControllerTest.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace DR\Review\Tests\Unit\Controller\App\User;
5+
6+
use DR\Review\Controller\App\User\UserMandatoryGitlabSyncController;
7+
use DR\Review\Tests\AbstractTestCase;
8+
use PHPUnit\Framework\Attributes\CoversClass;
9+
10+
#[CoversClass(UserMandatoryGitlabSyncController::class)]
11+
class UserMandatoryGitlabSyncControllerTest extends AbstractTestCase
12+
{
13+
public function testInvoke(): void
14+
{
15+
$this->expectNotToPerformAssertions();
16+
$controller = new UserMandatoryGitlabSyncController();
17+
($controller)();
18+
}
19+
}

‎tests/Unit/EventSubscriber/MandatoryGitlabSyncSubscriberTest.php‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace DR\Review\Tests\Unit\EventSubscriber;
5+
6+
use DR\Review\Controller\App\User\Gitlab\UserGitlabOAuth2FinishController;
7+
use DR\Review\Controller\App\User\Gitlab\UserGitlabOAuth2StartController;
8+
use DR\Review\Controller\App\User\UserMandatoryGitlabSyncController;
9+
use DR\Review\Controller\Auth\LogoutController;
10+
use DR\Review\Doctrine\Type\RepositoryGitType;
11+
use DR\Review\Entity\User\GitAccessToken;
12+
use DR\Review\Entity\User\User;
13+
use DR\Review\EventSubscriber\MandatoryGitlabSyncSubscriber;
14+
use DR\Review\Tests\AbstractTestCase;
15+
use PHPUnit\Framework\Attributes\CoversClass;
16+
use PHPUnit\Framework\Attributes\TestWith;
17+
use PHPUnit\Framework\MockObject\MockObject;
18+
use Symfony\Bundle\SecurityBundle\Security;
19+
use Symfony\Component\HttpFoundation\ParameterBag;
20+
use Symfony\Component\HttpFoundation\RedirectResponse;
21+
use Symfony\Component\HttpFoundation\Request;
22+
use Symfony\Component\HttpKernel\Event\RequestEvent;
23+
use Symfony\Component\HttpKernel\HttpKernelInterface;
24+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
25+
26+
#[CoversClass(MandatoryGitlabSyncSubscriber::class)]
27+
class MandatoryGitlabSyncSubscriberTest extends AbstractTestCase
28+
{
29+
private Security&MockObject $security;
30+
private UrlGeneratorInterface&MockObject $urlGenerator;
31+
32+
protected function setUp(): void
33+
{
34+
parent::setUp();
35+
$this->security = $this->createMock(Security::class);
36+
$this->urlGenerator = $this->createMock(UrlGeneratorInterface::class);
37+
}
38+
39+
public function testInvokeSkipsWhenUserNotLoggedIn(): void
40+
{
41+
$subscriber = $this->createSubscriber();
42+
$event = $this->createRequestEvent();
43+
44+
$this->security->expects($this->once())->method('getUser')->willReturn(null);
45+
$this->urlGenerator->expects($this->never())->method('generate');
46+
47+
($subscriber)($event);
48+
49+
static::assertNull($event->getResponse());
50+
}
51+
52+
#[TestWith([UserGitlabOAuth2StartController::class])]
53+
#[TestWith([UserGitlabOAuth2FinishController::class])]
54+
#[TestWith([UserMandatoryGitlabSyncController::class])]
55+
#[TestWith([LogoutController::class])]
56+
public function testInvokeSkipsForAllowedControllers(string $controllerClass): void
57+
{
58+
$subscriber = $this->createSubscriber();
59+
$event = $this->createRequestEvent($controllerClass);
60+
$user = new User();
61+
62+
$this->security->expects($this->once())->method('getUser')->willReturn($user);
63+
$this->urlGenerator->expects($this->never())->method('generate');
64+
65+
($subscriber)($event);
66+
67+
static::assertNull($event->getResponse());
68+
}
69+
70+
public function testInvokeSkipsWhenSyncNotConfigured(): void
71+
{
72+
$subscriber = $this->createSubscriber(false, false, false);
73+
$event = $this->createRequestEvent();
74+
$user = new User();
75+
76+
$this->security->expects($this->once())->method('getUser')->willReturn($user);
77+
$this->urlGenerator->expects($this->never())->method('generate');
78+
79+
($subscriber)($event);
80+
81+
static::assertNull($event->getResponse());
82+
}
83+
84+
public function testInvokeSkipsSyncIsNotMandatory(): void
85+
{
86+
$subscriber = $this->createSubscriber(true, true, false);
87+
$event = $this->createRequestEvent();
88+
$user = new User();
89+
90+
$this->security->expects($this->once())->method('getUser')->willReturn($user);
91+
$this->urlGenerator->expects($this->never())->method('generate');
92+
93+
($subscriber)($event);
94+
95+
static::assertNull($event->getResponse());
96+
}
97+
98+
public function testInvokeSkipsWhenUserHasGitlabToken(): void
99+
{
100+
$subscriber = $this->createSubscriber();
101+
$event = $this->createRequestEvent();
102+
$gitlabToken = (new GitAccessToken())->setGitType(RepositoryGitType::GITLAB);
103+
104+
$user = new User();
105+
$user->getGitAccessTokens()->add($gitlabToken);
106+
107+
$this->security->expects($this->once())->method('getUser')->willReturn($user);
108+
$this->urlGenerator->expects($this->never())->method('generate');
109+
110+
($subscriber)($event);
111+
112+
static::assertNull($event->getResponse());
113+
}
114+
115+
public function testInvokeSkipsWhenInvokingApiUrl(): void
116+
{
117+
$subscriber = $this->createSubscriber();
118+
$event = $this->createRequestEvent(requestUri: '/api/some-endpoint');
119+
$user = new User();
120+
121+
$this->security->expects($this->once())->method('getUser')->willReturn($user);
122+
$this->urlGenerator->expects($this->never())->method('generate');
123+
124+
($subscriber)($event);
125+
126+
static::assertNull($event->getResponse());
127+
}
128+
129+
public function testInvokeRedirectsToMandatorySyncPage(): void
130+
{
131+
$subscriber = $this->createSubscriber();
132+
$event = $this->createRequestEvent();
133+
$user = new User();
134+
$expectedUrl = '/mandatory-gitlab-sync';
135+
136+
$this->security->expects($this->once())->method('getUser')->willReturn($user);
137+
$this->urlGenerator->expects($this->once())
138+
->method('generate')
139+
->with(UserMandatoryGitlabSyncController::class)
140+
->willReturn($expectedUrl);
141+
142+
($subscriber)($event);
143+
144+
$response = $event->getResponse();
145+
static::assertInstanceOf(RedirectResponse::class, $response);
146+
static::assertSame($expectedUrl, $response->getTargetUrl());
147+
}
148+
149+
private function createSubscriber(
150+
bool $gitlabCommentSyncEnabled = true,
151+
bool $gitlabReviewerSyncEnabled = true,
152+
bool $gitlabSyncMandatory = true
153+
): MandatoryGitlabSyncSubscriber {
154+
return new MandatoryGitlabSyncSubscriber(
155+
$gitlabCommentSyncEnabled,
156+
$gitlabReviewerSyncEnabled,
157+
$gitlabSyncMandatory,
158+
$this->security,
159+
$this->urlGenerator
160+
);
161+
}
162+
163+
private function createRequestEvent(string $controller = 'some.controller', string $requestUri = '/app/test'): RequestEvent
164+
{
165+
$request = new Request(server: ['REQUEST_URI' => $requestUri]);
166+
$request->attributes = new ParameterBag(['_controller' => $controller]);
167+
168+
return new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
169+
}
170+
}

0 commit comments

Comments
 (0)