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 notification read image to notification mail #504

Merged
merged 11 commits into from
Nov 16, 2023
2 changes: 2 additions & 0 deletions config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use DR\Review\Service\Git\Review\Strategy\BasicCherryPickStrategy;
use DR\Review\Service\Git\Review\Strategy\HesitantCherryPickStrategy;
use DR\Review\Service\Git\Review\Strategy\PersistentCherryPickStrategy;
use DR\Review\Service\Notification\RuleNotificationTokenGenerator;
use DR\Review\Service\Parser\DiffFileParser;
use DR\Review\Service\Parser\DiffParser;
use DR\Review\Service\RemoteEvent\Gitlab\PushEventHandler;
Expand Down Expand Up @@ -156,6 +157,7 @@
$services->set(MarkdownConverter::class, CommonMarkdownConverter::class);
$services->set(GitCommandBuilderFactory::class)->arg('$git', '%env(GIT_BINARY)%');
$services->set(ParserHasFailedFormatter::class);
$services->set(RuleNotificationTokenGenerator::class)->arg('$appSecret', '%env(APP_SECRET)%');

// custom register cache dir
$services->set(CacheableGitRepositoryService::class)->arg('$cacheDirectory', "%kernel.project_dir%/var/git/");
Expand Down
4 changes: 2 additions & 2 deletions src/Command/MailCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

// register notification
$this->notificationService->addRuleNotification($rule, $period);
$notification = $this->notificationService->addRuleNotification($rule, $period);

// send mail
$this->mailService->sendCommitsMail($ruleConfig, $commits);
$this->mailService->sendCommitsMail($ruleConfig, $commits, $notification);
} catch (Throwable $exception) {
$this->logger?->error($exception->getMessage(), ['exception' => $exception]);
$exitCode = self::FAILURE;
Expand Down
12 changes: 8 additions & 4 deletions src/Controller/App/Notification/RuleNotificationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use DR\Review\Security\Role\Roles;
use DR\Review\Security\Voter\RuleVoter;
use DR\Review\Service\RuleProcessor;
use DR\Review\ViewModel\Mail\CommitsViewModel;
use DR\Review\ViewModelProvider\Mail\CommitsViewModelProvider;
use DR\Utils\Assert;
use Symfony\Bridge\Doctrine\Attribute\MapEntity;
use Symfony\Component\HttpFoundation\Response;
Expand All @@ -23,8 +23,11 @@

class RuleNotificationController extends AbstractController
{
public function __construct(private readonly RuleProcessor $ruleProcessor, private readonly RuleNotificationRepository $notificationRepository)
{
public function __construct(
private readonly RuleProcessor $ruleProcessor,
private readonly RuleNotificationRepository $notificationRepository,
private readonly CommitsViewModelProvider $viewModelProvider
) {
}

/**
Expand All @@ -45,7 +48,8 @@ public function __invoke(#[MapEntity] RuleNotification $notification): Response
$commits = $this->ruleProcessor->processRule(new RuleConfiguration(Frequency::getPeriod($currentTime, $frequency), $rule));

// render mail
$response = $this->render('mail/mail.commits.html.twig', ['viewModel' => new CommitsViewModel($commits, $options->getTheme() ?? 'upsource')]);
$viewModel = $this->viewModelProvider->getCommitsViewModel($commits, $rule, $notification);
$response = $this->render('mail/mail.commits.html.twig', ['viewModel' => $viewModel]);
$response->headers->set('Content-Security-Policy', "");

// mark notification as read
Expand Down
36 changes: 36 additions & 0 deletions src/Controller/App/Notification/RuleNotificationReadController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
declare(strict_types=1);

namespace DR\Review\Controller\App\Notification;

use DR\Review\Controller\AbstractController;
use DR\Review\Entity\Notification\RuleNotification;
use DR\Review\Repository\Config\RuleNotificationRepository;
use DR\Review\Service\Notification\RuleNotificationTokenGenerator;
use Symfony\Bridge\Doctrine\Attribute\MapEntity;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Routing\Annotation\Route;

class RuleNotificationReadController extends AbstractController
{
public function __construct(
private readonly RuleNotificationTokenGenerator $tokenGenerator,
private readonly RuleNotificationRepository $notificationRepository
) {
}

#[Route('/app/rules/rule/{id<\d+>?}/{token}', self::class, methods: 'GET')]
public function __invoke(#[MapEntity] RuleNotification $notification, string $token): Response
{
$generatedToken = $this->tokenGenerator->generate($notification);
if (hash_equals($generatedToken, $token) === false) {
throw new BadRequestHttpException('Invalid token');
}

$notification->setRead(true);
$this->notificationRepository->save($notification, true);

return new Response('', Response::HTTP_OK, ['Content-Type' => 'image/png']);
}
}
2 changes: 1 addition & 1 deletion src/Entity/Notification/RuleNotification.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class RuleNotification
#[ORM\Column]
private int $id;

#[ORM\ManyToOne(targetEntity: Rule::class, inversedBy: 'notifications')]
#[ORM\ManyToOne(targetEntity: Rule::class, cascade: ['persist'], inversedBy: 'notifications')]
#[ORM\JoinColumn(nullable: false)]
private Rule $rule;

Expand Down
15 changes: 10 additions & 5 deletions src/Service/Mail/CommitMailService.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

use DR\Review\Entity\Git\Commit;
use DR\Review\Entity\Notification\RuleConfiguration;
use DR\Review\ViewModel\Mail\CommitsViewModel;
use DR\Review\Entity\Notification\RuleNotification;
use DR\Review\ViewModelProvider\Mail\CommitsViewModelProvider;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Symfony\Bridge\Twig\Mime\TemplatedEmail;
Expand All @@ -17,16 +18,20 @@ class CommitMailService implements LoggerAwareInterface
{
use LoggerAwareTrait;

public function __construct(private string $applicationName, private MailerInterface $mailer, private MailSubjectFormatter $subjectFormatter)
{
public function __construct(
private readonly string $applicationName,
private readonly MailerInterface $mailer,
private readonly MailSubjectFormatter $subjectFormatter,
private readonly CommitsViewModelProvider $viewModelProvider
) {
}

/**
* @param Commit[] $commits
*
* @throws TransportExceptionInterface
*/
public function sendCommitsMail(RuleConfiguration $config, array $commits): void
public function sendCommitsMail(RuleConfiguration $config, array $commits, RuleNotification $notification): void
{
$rule = $config->rule;
$subject = $rule->getRuleOptions()?->getSubject() ?? sprintf('[%s] New revisions for: {name}', $this->applicationName);
Expand All @@ -36,7 +41,7 @@ public function sendCommitsMail(RuleConfiguration $config, array $commits): void
->subject($this->subjectFormatter->format($subject, $rule, $commits))
->htmlTemplate('mail/mail.commits.html.twig')
->text('')
->context(['viewModel' => new CommitsViewModel($commits, $rule->getRuleOptions()?->getTheme() ?? 'upsource')]);
->context(['viewModel' => $this->viewModelProvider->getCommitsViewModel($commits, $rule, $notification)]);

foreach ($rule->getRecipients() as $recipient) {
$email->addTo(new Address($recipient->getEmail(), $recipient->getName() ?? ''));
Expand Down
4 changes: 3 additions & 1 deletion src/Service/Notification/RuleNotificationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ public function __construct(private readonly RuleNotificationRepository $reposit
{
}

public function addRuleNotification(Rule $rule, DatePeriod $period): void
public function addRuleNotification(Rule $rule, DatePeriod $period): RuleNotification
{
$notification = new RuleNotification();
$notification->setRule($rule);
$notification->setNotifyTimestamp(Assert::notNull($period->end)->getTimestamp());
$notification->setCreateTimestamp(time());
$this->repository->save($notification, true);

return $notification;
}
}
35 changes: 35 additions & 0 deletions src/Service/Notification/RuleNotificationTokenGenerator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php
declare(strict_types=1);

namespace DR\Review\Service\Notification;

use DR\Review\Entity\Notification\RuleNotification;

class RuleNotificationTokenGenerator
{
public function __construct(private readonly string $appSecret)
{
}

public function generate(RuleNotification $notification): string
{
$notificationId = $notification->getId();
$notifyTimestamp = $notification->getNotifyTimestamp();
$createTimestamp = $notification->getCreateTimestamp();
$userId = $notification->getRule()->getUser()->getId();

$string = sprintf(
'%s%s%s%s%s%s%s%s',
$this->appSecret,
$notificationId,
$this->appSecret,
$notifyTimestamp,
$createTimestamp,
$this->appSecret,
$userId,
$this->appSecret
);

return hash('sha512', $string);
}
}
2 changes: 1 addition & 1 deletion src/ViewModel/Mail/CommitsViewModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class CommitsViewModel
/**
* @param Commit[] $commits
*/
public function __construct(public readonly array $commits, public readonly string $theme)
public function __construct(public readonly array $commits, public readonly string $theme, public readonly ?string $notificationReadUrl = null)
{
}

Expand Down
36 changes: 36 additions & 0 deletions src/ViewModelProvider/Mail/CommitsViewModelProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
declare(strict_types=1);

namespace DR\Review\ViewModelProvider\Mail;

use DR\Review\Controller\App\Notification\RuleNotificationReadController;
use DR\Review\Doctrine\Type\MailThemeType;
use DR\Review\Entity\Git\Commit;
use DR\Review\Entity\Notification\Rule;
use DR\Review\Entity\Notification\RuleNotification;
use DR\Review\Service\Notification\RuleNotificationTokenGenerator;
use DR\Review\ViewModel\Mail\CommitsViewModel;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

class CommitsViewModelProvider
{
public function __construct(
private readonly RuleNotificationTokenGenerator $tokenGenerator,
private readonly UrlGeneratorInterface $urlGenerator,
) {
}

/**
* @param Commit[] $commits
*/
public function getCommitsViewModel(array $commits, Rule $rule, RuleNotification $notification): CommitsViewModel
{
$token = $this->tokenGenerator->generate($notification);
$url = $this->urlGenerator->generate(
RuleNotificationReadController::class,
['id' => $notification->getId(), 'token' => $token]
);

return new CommitsViewModel($commits, $rule->getRuleOptions()?->getTheme() ?? MailThemeType::UPSOURCE, $url);
}
}
3 changes: 3 additions & 0 deletions templates/mail/mail.commits.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
</head>
<body class="mailBody">
<div class="revisions">
{% if viewModel.notificationReadUrl %}
<img src="{{ viewModel.notificationReadUrl }}" width="0" height="0" style="width:0;height:0">
{% endif %}
New revision(s) by {{ viewModel.authors|join(', ') }}.<br><br>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
(new Rule())
->setUser(
(new User())
->setId(123)
->setName($name)
->setEmail($email)
->setRoles([Roles::ROLE_USER])
Expand Down
5 changes: 4 additions & 1 deletion tests/Functional/Command/MailCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ protected function setUp(): void
$this->repositoryService = $this->createMock(CacheableGitRepositoryService::class);
$this->messageCollector = new MessageEventCollector();

$notificationRepository = $this->createMock(RuleNotificationRepository::class);
$notificationRepository->method('save')->willReturnCallback(static fn($notification) => $notification->setId(123));

// register mock repository service
self::getContainer()->set(RuleRepository::class, $this->ruleRepository);
self::getContainer()->set(RuleNotificationRepository::class, $this->createMock(RuleNotificationRepository::class));
self::getContainer()->set(RuleNotificationRepository::class, $notificationRepository);
self::getContainer()->set(ExternalLinkRepository::class, $this->linkRepository);
self::getContainer()->set(CacheableGitRepositoryService::class, $this->repositoryService);
self::getContainer()->set(RevisionFetchService::class, $this->createMock(RevisionFetchService::class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use DR\Review\Service\RuleProcessor;
use DR\Review\Tests\AbstractControllerTestCase;
use DR\Review\ViewModel\Mail\CommitsViewModel;
use DR\Review\ViewModelProvider\Mail\CommitsViewModelProvider;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\MockObject\MockObject;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
Expand All @@ -25,11 +26,13 @@ class RuleNotificationControllerTest extends AbstractControllerTestCase
{
private RuleProcessor&MockObject $ruleProcessor;
private RuleNotificationRepository&MockObject $notificationRepository;
private CommitsViewModelProvider&MockObject $viewModelProvider;

protected function setUp(): void
{
$this->ruleProcessor = $this->createMock(RuleProcessor::class);
$this->notificationRepository = $this->createMock(RuleNotificationRepository::class);
$this->viewModelProvider = $this->createMock(CommitsViewModelProvider::class);
parent::setUp();
}

Expand Down Expand Up @@ -57,9 +60,12 @@ public function testInvoke(): void
$notification->setRule($rule);
$commit = $this->createMock(Commit::class);

$viewModel = new CommitsViewModel([$commit], MailThemeType::DARCULA);

$this->expectDenyAccessUnlessGranted(RuleVoter::EDIT, $rule);
$this->ruleProcessor->expects(self::once())->method('processRule')->with()->willReturn([$commit]);
$this->expectRender('mail/mail.commits.html.twig', ['viewModel' => new CommitsViewModel([$commit], MailThemeType::DARCULA)]);
$this->viewModelProvider->expects(self::once())->method('getCommitsViewModel')->with([$commit], $rule, $notification)->willReturn($viewModel);
$this->expectRender('mail/mail.commits.html.twig', ['viewModel' => $viewModel]);
$this->notificationRepository->expects(self::once())->method('save')->with($notification);

$response = ($this->controller)($notification);
Expand All @@ -70,6 +76,6 @@ public function testInvoke(): void

public function getController(): AbstractController
{
return new RuleNotificationController($this->ruleProcessor, $this->notificationRepository);
return new RuleNotificationController($this->ruleProcessor, $this->notificationRepository, $this->viewModelProvider);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php
declare(strict_types=1);

namespace DR\Review\Tests\Unit\Controller\App\Notification;

use DR\Review\Controller\AbstractController;
use DR\Review\Controller\App\Notification\RuleNotificationReadController;
use DR\Review\Entity\Notification\RuleNotification;
use DR\Review\Repository\Config\RuleNotificationRepository;
use DR\Review\Service\Notification\RuleNotificationTokenGenerator;
use DR\Review\Tests\AbstractControllerTestCase;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\MockObject\MockObject;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;

#[CoversClass(RuleNotificationReadController::class)]
class RuleNotificationReadControllerTest extends AbstractControllerTestCase
{
private RuleNotificationTokenGenerator&MockObject $tokenGenerator;
private RuleNotificationRepository&MockObject $notificationRepository;

protected function setUp(): void
{
$this->tokenGenerator = $this->createMock(RuleNotificationTokenGenerator::class);
$this->notificationRepository = $this->createMock(RuleNotificationRepository::class);
parent::setUp();
}

public function testInvoke(): void
{
$notification = new RuleNotification();

$this->tokenGenerator->expects(self::once())->method('generate')->with($notification)->willReturn('token');
$this->notificationRepository->expects(self::once())->method('save')->with($notification, true);

($this->controller)($notification, 'token');
}

public function testInvokeInvalidToken(): void
{
$notification = new RuleNotification();

$this->tokenGenerator->expects(self::once())->method('generate')->with($notification)->willReturn('token');
$this->notificationRepository->expects(self::never())->method('save');

$this->expectException(BadRequestHttpException::class);
$this->expectExceptionMessage('Invalid token');
($this->controller)($notification, 'foobar');
}

public function getController(): AbstractController
{
return new RuleNotificationReadController($this->tokenGenerator, $this->notificationRepository);
}
}
Loading