Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add open in ide button to reviews #476

Merged
merged 7 commits into from
Nov 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,13 @@ CODE_REVIEW_EXCLUDE_AUTHORS=
# The delay in milliseconds before a notification mail will be send to recipients
#
MAILER_NOTIFICATION_DELAY=600000

##
# Enable "open in editor" button for review files and code issues.
# IDE_URL_ENABLED=true
# IDE_URL_PATTERN=http://localhost:63342/api/file/?file={file}&line={line}
# IDE_URL_TITLE='Open file in PHPStorm'
#
IDE_URL_ENABLED=false
IDE_URL_PATTERN=
IDE_URL_TITLE=
13 changes: 13 additions & 0 deletions assets/ts/controllers/ide_button_controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {Controller} from '@hotwired/stimulus';

export default class extends Controller {
public onClick(event: Event): void {
event.preventDefault();

const iframe = document.createElement('iframe');
document.body.appendChild(iframe);
iframe.onload = () => iframe.remove();
iframe.onerror = () => iframe.remove();
iframe.src = (this.element as HTMLAnchorElement).href;
}
}
7 changes: 6 additions & 1 deletion config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
use DR\Review\Service\Report\Coverage\Parser\CloverParser;
use DR\Review\Service\Revision\RevisionPatternMatcher;
use DR\Review\Service\Webhook\WebhookExecutionService;
use DR\Review\Twig\IdeButtonExtension;
use DR\Review\Twig\InlineCss\CssToInlineStyles;
use Highlight\Highlighter;
use League\CommonMark\MarkdownConverter;
Expand Down Expand Up @@ -113,7 +114,10 @@
$services->set(LoginService::class);
$services->set(UserChecker::class);
$services->set(User::class)->public()->factory([service(Security::class), 'getUser']);
$services->set(ContentSecurityPolicyResponseSubscriber::class)->arg('$hostname', '%env(APP_HOSTNAME)%');
$services->set(ContentSecurityPolicyResponseSubscriber::class)
->arg('$hostname', '%env(APP_HOSTNAME)%')
->arg('$ideUrlEnabled', '%env(bool:IDE_URL_ENABLED)%')
->arg('$ideUrlPattern', '%env(IDE_URL_PATTERN)%');
$services->set(ProblemJsonResponseFactory::class)->arg('$debug', '%env(APP_DEBUG)%');
$services->set('monolog.formatter.line', LineFormatter::class)
->arg('$format', "[%%datetime%%] %%channel%%.%%level_name%%: %%message%% %%extra%%\n")
Expand Down Expand Up @@ -147,6 +151,7 @@
$services->set(DiffFileParser::class);
$services->set(JBDiff::class);
$services->set(CssToInlineStyles::class);
$services->set(IdeButtonExtension::class)->args(['%env(bool:IDE_URL_ENABLED)%', '%env(IDE_URL_PATTERN)%', '%env(IDE_URL_TITLE)%']);
$services->set(Highlighter::class);
$services->set(MarkdownConverter::class, CommonMarkdownConverter::class);
$services->set(GitCommandBuilderFactory::class)->arg('$git', '%env(GIT_BINARY)%');
Expand Down
16 changes: 13 additions & 3 deletions src/EventSubscriber/ContentSecurityPolicyResponseSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

class ContentSecurityPolicyResponseSubscriber implements EventSubscriberInterface
{
public function __construct(private readonly string $hostname)
public function __construct(private readonly string $hostname, private readonly bool $ideUrlEnabled, private readonly string $ideUrlPattern)
{
}

Expand All @@ -23,9 +23,19 @@ public function onResponse(ResponseEvent $event): void
// only allow content from own host.
// allow image svg+xml
// allow websocket to connect to any port.
$policy = sprintf("default-src 'self'; img-src 'self' data:; object-src: 'none'; connect-src 'self' %s:*", $this->hostname);
$policy = [
"default-src 'self'",
"img-src 'self' data:",
"object-src: 'none'",
sprintf("connect-src 'self' %s:*", $this->hostname),
];

$response->headers->set("Content-Security-Policy", $policy);
// if IDE url is allowed, allow iframe host from http or https url
if ($this->ideUrlEnabled && preg_match('#^(https?://[^:/]+)#', $this->ideUrlPattern, $matches) === 1) {
$policy[] = sprintf("frame-src %s:*", $matches[1]);
}

$response->headers->set("Content-Security-Policy", implode('; ', $policy));
}

/**
Expand Down
48 changes: 48 additions & 0 deletions src/Twig/IdeButtonExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php
declare(strict_types=1);

namespace DR\Review\Twig;

use Twig\Environment;
use Twig\Error\LoaderError;
use Twig\Error\RuntimeError;
use Twig\Error\SyntaxError;
use Twig\Extension\AbstractExtension;
use Twig\TwigFunction;

class IdeButtonExtension extends AbstractExtension
{
public function __construct(
private readonly bool $enabled,
private readonly string $url,
private readonly string $title,
private readonly Environment $twig,
) {
}

/**
* @return TwigFunction[]
*/
public function getFunctions(): array
{
return [new TwigFunction('ide_button', [$this, 'createLink'], ['is_safe' => ['all']])];
}

/**
* @throws SyntaxError|RuntimeError|LoaderError
*/
public function createLink(string $file, int $line = 1): string
{
if ($this->enabled === false) {
return '';
}

return $this->twig->render(
'/extension/ide-button.widget.html.twig',
[
'url' => str_ireplace(['{file}', '{line}'], [urlencode($file), $line], $this->url),
'title' => $this->title
]
);
}
}
3 changes: 3 additions & 0 deletions templates/app/review/commit/commit.header.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
{%- endif -%}
{%- endif -%}

{# open in editor #}
{{ ide_button(file.filePathAfter ?? file.filePathBefore) }}

<div class="float-end pe-2">
{% set commentVisibility = fileDiffViewModel.commentsViewModel.commentVisibility.value %}
{% set comparisonPolicy = fileDiffViewModel.commentsViewModel.comparisonPolicy.value %}
Expand Down
21 changes: 11 additions & 10 deletions templates/app/review/review.summary.code-issues.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@
{%- for file,issues in inspectionIssueViewModel.getGroupByFile() -%}
<ul>
<li>
<a href="{{ path('DR\\Review\\Controller\\App\\Review\\ReviewController', {review: review, filePath: file}) }}"
class="d-block">{{ file }}</a>

<div>
<a href="{{ path('DR\\Review\\Controller\\App\\Review\\ReviewController', {review: review, filePath: file}) }}">{{ file }}</a>
{{ ide_button(file, issues[0].lineNumber) }}
</div>
<ul>
{% for issue in issues %}
<li>
<span class="fw-bold">{{ issue.report.inspectionId }}</span>
<code>{{ 'line'|trans|lower }}:{{ issue.lineNumber }}</code>
<span>{{ issue.message }}</span>
</li>
{% endfor %}
{% for issue in issues %}
<li>
<span class="fw-bold">{{ issue.report.inspectionId }}</span>
<code>{{ 'line'|trans|lower }}:{{ issue.lineNumber }}</code>
<span>{{ issue.message }}</span>
</li>
{% endfor %}
</ul>
</li>
</ul>
Expand Down
8 changes: 8 additions & 0 deletions templates/extension/ide-button.widget.html.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<a href="{{ url }}"
title="{{ title }}"
class="btn btn-outline-primary btn-sm border-0 ms-1"
{{ stimulus_controller('ide-button') }}
{{ stimulus_action('ide-button', 'onClick', 'click') }}
>
<i class="bi bi-arrow-up-right-square-fill"></i>
</a>
Original file line number Diff line number Diff line change
Expand Up @@ -5,49 +5,51 @@

use DR\Review\EventSubscriber\ContentSecurityPolicyResponseSubscriber;
use DR\Review\Tests\AbstractTestCase;
use PHPUnit\Framework\Attributes\CoversClass;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;

/**
* @coversDefaultClass \DR\Review\EventSubscriber\ContentSecurityPolicyResponseSubscriber
* @covers ::__construct
*/
#[CoversClass(ContentSecurityPolicyResponseSubscriber::class)]
class ContentSecurityPolicyResponseSubscriberTest extends AbstractTestCase
{
/**
* @covers ::getSubscribedEvents
*/
public function testGetSubscribedEvents(): void
{
static::assertSame([KernelEvents::RESPONSE => 'onResponse'], ContentSecurityPolicyResponseSubscriber::getSubscribedEvents());
}

/**
* @covers ::onResponse
*/
public function testOnResponseShouldNotOverrideExisting(): void
{
$response = new Response();
$response->headers->set('Content-Security-Policy', '');
$event = new ResponseEvent($this->createMock(HttpKernelInterface::class), new Request(), 1, $response);
$subscriber = new ContentSecurityPolicyResponseSubscriber('host');
$subscriber = new ContentSecurityPolicyResponseSubscriber('host', true, 'url');

$subscriber->onResponse($event);

static::assertSame("", $response->headers->get("Content-Security-Policy"));
}

/**
* @covers ::onResponse
*/
public function testOnResponse(): void
public function testOnResponseWithIdeUrl(): void
{
$response = new Response();
$event = new ResponseEvent($this->createMock(HttpKernelInterface::class), new Request(), 1, $response);
$subscriber = new ContentSecurityPolicyResponseSubscriber('host');
$subscriber = new ContentSecurityPolicyResponseSubscriber('host', true, 'http://localhost:8080/file');
$subscriber->onResponse($event);

static::assertSame(
"default-src 'self'; img-src 'self' data:; object-src: 'none'; connect-src 'self' host:*; frame-src http://localhost:*",
$response->headers->get("Content-Security-Policy")
);
}

public function testOnResponseWithoutIdeUrl(): void
{
$response = new Response();
$event = new ResponseEvent($this->createMock(HttpKernelInterface::class), new Request(), 1, $response);
$subscriber = new ContentSecurityPolicyResponseSubscriber('host', false, 'http://localhost:8080/file');
$subscriber->onResponse($event);

static::assertSame(
Expand Down
62 changes: 62 additions & 0 deletions tests/Unit/Twig/IdeButtonExtensionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php
declare(strict_types=1);

namespace DR\Review\Tests\Unit\Twig;

use DR\Review\Tests\AbstractTestCase;
use DR\Review\Twig\IdeButtonExtension;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\MockObject\MockObject;
use Twig\Environment;
use Twig\Error\LoaderError;
use Twig\Error\RuntimeError;
use Twig\Error\SyntaxError;
use Twig\TwigFunction;

#[CoversClass(IdeButtonExtension::class)]
class IdeButtonExtensionTest extends AbstractTestCase
{
private Environment&MockObject $twig;
private IdeButtonExtension $extension;

protected function setUp(): void
{
parent::setUp();
$this->twig = $this->createMock(Environment::class);
$this->extension = new IdeButtonExtension(true, 'url {file} {line}', 'title', $this->twig);
}

public function testGetFunctions(): void
{
$expected = [new TwigFunction('ide_button', [$this->extension, 'createLink'], ['is_safe' => ['all']])];
static::assertEquals($expected, $this->extension->getFunctions());
}

/**
* @throws SyntaxError|RuntimeError|LoaderError
*/
public function testCreateLink(): void
{
$this->twig->expects(static::once())->method('render')->with(
'/extension/ide-button.widget.html.twig',
[
'url' => 'url file 123',
'title' => 'title'
]
)->willReturn('html');

static::assertSame('html', $this->extension->createLink('file', 123));
}

/**
* @throws SyntaxError|RuntimeError|LoaderError
*/
public function testCreateLinkDisabled(): void
{
$extension = new IdeButtonExtension(false, 'url {file} {line}', 'title', $this->twig);

$this->twig->expects(static::never())->method('render');

static::assertSame('', $extension->createLink('file', 123));
}
}