Notify board members about moved cards according to their Deck setting
Build package / unit-tests (push) Failing after 20s
Build package / xml-lint (push) Successful in 46s
Build package / php-lint (8.4) (push) Successful in 1m30s
Build package / php-lint (8.5) (push) Successful in 1m36s
Build package / package (push) Skipped

Replaces the yes/no notify_email flag with notify_mode:
- off:   no mail
- owner: only the board owner, as before (migrated from notify_email=1)
- board: everyone with access to the board, filtered by their own Deck
         "Notifications" setting for it (board:<id>:notify-due: off /
         assigned / all), mirroring Deck's NotificationHelper rules

Recipients are resolved via PermissionService::findUsers() (owner, ACL,
groups and circles expanded), AssignmentMapper::isUserAssigned() and
IConfig, none of which reads Deck's frozen $userId. If they cannot be
resolved, no mail is sent rather than one to the wrong people.

Moved cards are collected over the whole run and each recipient gets a
single digest mail afterwards, in their own language.

Bumps to 35.2.0 so the migrations run.
This commit is contained in:
Patrick Niebeling committed 2026-09-30 10:31:45 +02:00
1 parent 27717205cd
commit 578b235f08
14 files changed
+579 -100

No files matched your search

+82 -3
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace OCA\WorkflowDeckAutomation\Service;
use OCA\Deck\Db\AssignmentMapper;
use OCA\Deck\Db\BoardMapper;
use OCA\Deck\Db\Card;
use OCA\Deck\Db\CardMapper;
@@ -14,6 +15,7 @@ use OCA\Deck\Service\PermissionService;
use OCA\Deck\Service\StackService;
use OCP\App\IAppManager;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IConfig;
use OCP\IUser;
use OCP\Server;
use Psr\Log\LoggerInterface;
@@ -66,6 +68,7 @@ class DeckIntegrationService {
public function __construct(
private IAppManager $appManager,
private IConfig $config,
private LoggerInterface $logger,
) {
}
@@ -302,6 +305,81 @@ class DeckIntegrationService {
return $owner;
}
/**
* Who should hear about each of the given (moved) cards, according to
* every board member's own Deck notification setting for this board —
* the "Notifications: none / assigned cards / all cards" menu, stored as
* the user preference `deck` / `board:<id>:notify-due`. The decision
* itself is NotificationPolicy::shouldNotify(), a copy of Deck's rules.
*
* Like findStackIds(), assembled only from pieces that take the users
* explicitly and never consult Deck's frozen `$userId`:
* PermissionService::findUsers() reads the owner and ACL through mappers
* and expands groups and circles itself; AssignmentMapper is a plain
* mapper that also resolves group/circle assignments; the preference is
* read straight from IConfig.
*
* Throws on any failure, so the caller can log and skip the mail instead
* of mailing a half-resolved or wrong set of people.
*
* @param int[] $cardIds
* @return array<int, list<string>> card id => uids to notify
* @throws DeckUnavailableException
*/
public function findNotificationRecipients(int $boardId, array $cardIds): array {
try {
$board = $this->resolve(BoardMapper::class)->find($boardId, false, true);
$owner = $this->unwrapUid($board->getOwner());
// Deck notifies the owner of a board nobody else can see about
// every card: "assigned" means nothing there.
$unshared = count($board->getAcl() ?? []) === 0;
// `true` bypasses PermissionService's per-process cache, which
// would otherwise outlive ACL changes for the whole cron pass.
$participants = $this->resolve(PermissionService::class)->findUsers($boardId, true);
$assignments = $this->resolve(AssignmentMapper::class);
$settings = [];
foreach ($participants as $uid => $participant) {
// Users of other instances (FederatedUser) have no local
// account and no preference to read.
if (!is_a($participant, 'OCA\Deck\Db\User')) {
continue;
}
// Numeric uids come back as integer array keys.
$uid = (string)$uid;
$settings[$uid] = (string)$this->config->getUserValue(
$uid,
self::DECK_APP_ID,
'board:' . $boardId . ':notify-due',
NotificationPolicy::DECK_DEFAULT,
);
}
$recipients = [];
foreach ($cardIds as $cardId) {
$recipients[$cardId] = [];
foreach ($settings as $uid => $setting) {
$uid = (string)$uid;
$notify = NotificationPolicy::shouldNotify(
$setting,
$unshared && $uid === $owner,
static fn (): bool => (bool)$assignments->isUserAssigned($cardId, $uid),
);
if ($notify) {
$recipients[$cardId][] = $uid;
}
}
}
} catch (DeckUnavailableException $e) {
throw $e;
} catch (Throwable $e) {
throw new DeckUnavailableException('Could not resolve notification recipients for board ' . $boardId . ': ' . $e->getMessage(), 0, $e);
}
return $recipients;
}
/**
* Deck's own exception classes are referenced by name: `is_a()` with a
* string simply returns false when the class does not exist, so a missing
@@ -368,9 +446,10 @@ class DeckIntegrationService {
public function getActiveCardsInStack(int $stackId): array {
return $this->call(function () use ($stackId) {
// Deliberately not enriched: CardService::enrichCards() only adds
// labels, assignees and comment counts — none of which is used —
// and runs BoardService::find(), a permission check against
// Deck's frozen uid (see beginUserContext()).
// labels, assignees and comment counts, and runs
// BoardService::find(), a permission check against Deck's frozen
// uid (see beginUserContext()). Assignments, where needed for
// mails, come from findNotificationRecipients() instead.
$cards = $this->resolve(CardMapper::class)->findAllByStack($stackId);
return array_values(array_filter($cards, static function (Card $card) {
+82 -51
View File
@@ -9,6 +9,7 @@ use OCA\WorkflowDeckAutomation\Db\Workflow;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
use OCP\Mail\IEMailTemplate;
use OCP\Mail\IMailer;
use OCP\Util;
@@ -16,6 +17,8 @@ use Psr\Log\LoggerInterface;
use Throwable;
class NotificationMailer {
private const APP_ID = 'workflow_deck_automation';
/**
* Descriptions are unbounded in Deck; keep the mail readable and well
* clear of any MTA size limits.
@@ -25,70 +28,88 @@ class NotificationMailer {
public function __construct(
private IMailer $mailer,
private IURLGenerator $urlGenerator,
private IL10N $l10n,
private IFactory $l10nFactory,
private LoggerInterface $logger,
) {
}
/**
* @param array{board: string, sourceStack: string, targetStack: string} $targets
* titles resolved by DeckIntegrationService::describeTargets()
* One mail listing every card moved for $recipient in this run — they
* may come from several workflows and boards.
*
* @param non-empty-list<array{workflow: Workflow, card: Card, targets: array{board: string, sourceStack: string, targetStack: string}}> $entries
* targets are the titles resolved by DeckIntegrationService::describeTargets()
*/
public function sendCardMovedNotification(IUser $user, Workflow $workflow, Card $card, array $targets): void {
$email = $user->getEMailAddress();
public function sendCardsMovedNotification(IUser $recipient, array $entries): void {
$email = $recipient->getEMailAddress();
if ($email === null || $email === '') {
$this->logger->info('Skipping notification for workflow ' . $workflow->getId() . ': user ' . $user->getUID() . ' has no email address', [
'app' => 'workflow_deck_automation',
$this->logger->info('Skipping moved-cards mail: user ' . $recipient->getUID() . ' has no email address', [
'app' => self::APP_ID,
]);
return;
}
try {
$cardLink = $this->urlGenerator->getAbsoluteURL(
'/apps/deck/board/' . $workflow->getBoardId() . '/card/' . $card->getId(),
);
$count = count($entries);
$l10n = $this->l10nFor($recipient);
$template = $this->mailer->createEMailTemplate('workflow_deck_automation.CardMoved', [
'cardTitle' => $card->getTitle(),
'workflowTitle' => $workflow->getTitle(),
try {
$template = $this->mailer->createEMailTemplate(self::APP_ID . '.CardsMoved', [
'count' => $count,
'cardTitles' => array_map(static fn (array $entry) => $entry['card']->getTitle(), $entries),
]);
$template->setSubject($this->l10n->t('Deck card moved: %s', [$card->getTitle()]));
$template->setSubject($count === 1
? $l10n->t('Deck card moved: %s', [$entries[0]['card']->getTitle()])
: $l10n->n('%n Deck card moved', '%n Deck cards moved', $count));
$template->addHeader();
$template->addHeading($this->l10n->t('A card was moved automatically'), false);
$template->addBodyText($this->l10n->t(
'The card "%1$s" was moved because it is overdue (workflow "%2$s").',
[$card->getTitle(), $workflow->getTitle()],
));
// Single-argument addBodyText() runs htmlspecialchars() itself, so
// these board- and stack-titles (user input) are safe as-is —
// unlike the description below, which passes both parts.
$template->addBodyText($this->l10n->t(
'Board "%1$s": moved from "%2$s" to "%3$s".',
[$targets['board'], $targets['sourceStack'], $targets['targetStack']],
));
$this->addCardDescription($template, $card);
$template->addBodyButton($this->l10n->t('Open card'), $cardLink);
$template->addHeading($l10n->n('A card was moved automatically', '%n cards were moved automatically', $count), false);
foreach ($entries as $entry) {
$this->addCard($template, $l10n, $entry['workflow'], $entry['card'], $entry['targets']);
}
$template->addFooter();
$message = $this->mailer->createMessage();
$message->setTo([$email => $user->getDisplayName()]);
$message->setFrom([Util::getDefaultEmailAddress('noreply') => $this->l10n->t('Deck Workflow Automation')]);
$message->setTo([$email => $recipient->getDisplayName()]);
$message->setFrom([Util::getDefaultEmailAddress('noreply') => $l10n->t('Deck Workflow Automation')]);
$message->useTemplate($template);
$failedRecipients = $this->mailer->send($message);
if (!empty($failedRecipients)) {
$this->logger->error('Notification mail for card ' . $card->getId() . ' failed for: ' . implode(', ', $failedRecipients), [
'app' => 'workflow_deck_automation',
$this->logger->error('Moved-cards mail failed for: ' . implode(', ', $failedRecipients), [
'app' => self::APP_ID,
]);
}
} catch (Throwable $e) {
$this->logger->error('Could not send notification mail for card ' . $card->getId() . ': ' . $e->getMessage(), [
'app' => 'workflow_deck_automation',
$this->logger->error('Could not send moved-cards mail to ' . $recipient->getUID() . ': ' . $e->getMessage(), [
'app' => self::APP_ID,
'exception' => $e,
]);
}
}
/**
* @param array{board: string, sourceStack: string, targetStack: string} $targets
*/
private function addCard(IEMailTemplate $template, IL10N $l10n, Workflow $workflow, Card $card, array $targets): void {
$template->addBodyText($l10n->t(
'The card "%1$s" was moved because it is overdue (workflow "%2$s").',
[$card->getTitle(), $workflow->getTitle()],
));
// Single-argument addBodyText() runs htmlspecialchars() itself, so
// these board- and stack-titles (user input) are safe as-is —
// unlike the description below, which passes both parts.
$template->addBodyText($l10n->t(
'Board "%1$s": moved from "%2$s" to "%3$s".',
[$targets['board'], $targets['sourceStack'], $targets['targetStack']],
));
$this->addCardDescription($template, $l10n, $card);
$template->addBodyButton(
$l10n->t('Open card'),
$this->urlGenerator->getAbsoluteURL('/apps/deck/board/' . $workflow->getBoardId() . '/card/' . $card->getId()),
);
}
/**
* One-off notice that a workflow was switched off because the board or
* stack it points at is gone.
@@ -99,55 +120,65 @@ class NotificationMailer {
$email = $user->getEMailAddress();
if ($email === null || $email === '') {
$this->logger->info('Cannot notify about disabled workflow ' . $workflow->getId() . ': user ' . $user->getUID() . ' has no email address', [
'app' => 'workflow_deck_automation',
'app' => self::APP_ID,
]);
return;
}
$l10n = $this->l10nFor($user);
$reason = match ($brokenTarget) {
WorkflowRunner::TARGET_SOURCE_STACK => $this->l10n->t('its source stack no longer exists'),
WorkflowRunner::TARGET_TARGET_STACK => $this->l10n->t('its target stack no longer exists'),
default => $this->l10n->t('its board no longer exists or is no longer available to you'),
WorkflowRunner::TARGET_SOURCE_STACK => $l10n->t('its source stack no longer exists'),
WorkflowRunner::TARGET_TARGET_STACK => $l10n->t('its target stack no longer exists'),
default => $l10n->t('its board no longer exists or is no longer available to you'),
};
try {
$settingsLink = $this->urlGenerator->getAbsoluteURL('/settings/user/workflow_deck_automation');
$settingsLink = $this->urlGenerator->getAbsoluteURL('/settings/user/' . self::APP_ID);
$template = $this->mailer->createEMailTemplate('workflow_deck_automation.WorkflowDisabled', [
$template = $this->mailer->createEMailTemplate(self::APP_ID . '.WorkflowDisabled', [
'workflowTitle' => $workflow->getTitle(),
]);
$template->setSubject($this->l10n->t('Deck workflow deactivated: %s', [$workflow->getTitle()]));
$template->setSubject($l10n->t('Deck workflow deactivated: %s', [$workflow->getTitle()]));
$template->addHeader();
$template->addHeading($this->l10n->t('A workflow was deactivated'), false);
$template->addBodyText($this->l10n->t(
$template->addHeading($l10n->t('A workflow was deactivated'), false);
$template->addBodyText($l10n->t(
'The workflow "%1$s" was switched off automatically because %2$s. No cards are being moved by it any more.',
[$workflow->getTitle(), $reason],
));
$template->addBodyText($this->l10n->t(
$template->addBodyText($l10n->t(
'Delete the workflow or point it at an existing board and stack to reactivate it. You will not be reminded about this workflow again.',
));
$template->addBodyButton($this->l10n->t('Open settings'), $settingsLink);
$template->addBodyButton($l10n->t('Open settings'), $settingsLink);
$template->addFooter();
$message = $this->mailer->createMessage();
$message->setTo([$email => $user->getDisplayName()]);
$message->setFrom([Util::getDefaultEmailAddress('noreply') => $this->l10n->t('Deck Workflow Automation')]);
$message->setFrom([Util::getDefaultEmailAddress('noreply') => $l10n->t('Deck Workflow Automation')]);
$message->useTemplate($template);
$failedRecipients = $this->mailer->send($message);
if (!empty($failedRecipients)) {
$this->logger->error('Deactivation mail for workflow ' . $workflow->getId() . ' failed for: ' . implode(', ', $failedRecipients), [
'app' => 'workflow_deck_automation',
'app' => self::APP_ID,
]);
}
} catch (Throwable $e) {
$this->logger->error('Could not send deactivation mail for workflow ' . $workflow->getId() . ': ' . $e->getMessage(), [
'app' => 'workflow_deck_automation',
'app' => self::APP_ID,
'exception' => $e,
]);
}
}
/**
* The recipient's own language. Cron has no request to take one from, so
* an injected IL10N would always speak the instance default — wrong as
* soon as a mail goes to someone other than whoever set that default.
*/
private function l10nFor(IUser $user): IL10N {
return $this->l10nFactory->get(self::APP_ID, $this->l10nFactory->getUserLanguage($user));
}
/**
* Appends the card's description, if it has one.
*
@@ -155,7 +186,7 @@ class NotificationMailer {
* rendered: pulling a Markdown parser in just for the mail would be a
* lot of machinery, and unrendered Markdown still reads fine.
*/
private function addCardDescription(IEMailTemplate $template, Card $card): void {
private function addCardDescription(IEMailTemplate $template, IL10N $l10n, Card $card): void {
$description = trim((string)$card->getDescription());
if ($description === '') {
return;
@@ -165,7 +196,7 @@ class NotificationMailer {
$description = mb_substr($description, 0, self::MAX_DESCRIPTION_LENGTH) . ' […]';
}
$template->addBodyText($this->l10n->t('Card content:'));
$template->addBodyText($l10n->t('Card content:'));
// addBodyText() only runs htmlspecialchars() when it has to build the
// plain-text part itself. Passing both parts means escaping is on us
+70
View File
@@ -0,0 +1,70 @@
<?php
declare(strict_types=1);
namespace OCA\WorkflowDeckAutomation\Service;
/**
* Who gets a mail about a moved card — pure logic, no Nextcloud or Deck
* dependencies, so it is unit-testable like WorkflowRunner::isOverdue().
*/
class NotificationPolicy {
/** No mail at all. */
public const MODE_OFF = 'off';
/** Only the workflow's owner (the board owner), regardless of Deck settings. */
public const MODE_OWNER = 'owner';
/** Everyone with access to the board, filtered by their own Deck setting. */
public const MODE_BOARD = 'board';
public const MODES = [self::MODE_OFF, self::MODE_OWNER, self::MODE_BOARD];
/**
* Values of Deck's per-user, per-board preference
* `deck` / `board:<boardId>:notify-due` (Deck's
* ConfigService::SETTING_BOARD_NOTIFICATION_DUE_*), inlined so a renamed
* constant cannot fatal this app. It is the board's "Notifications" menu
* in Deck — the due-date reminder setting, which is exactly what fits a
* card being moved because it fell due.
*/
public const DECK_OFF = 'off';
public const DECK_ASSIGNED = 'assigned';
public const DECK_ALL = 'all';
/** What Deck assumes for a user who never touched the setting. */
public const DECK_DEFAULT = self::DECK_ASSIGNED;
public static function isValidMode(string $mode): bool {
return in_array($mode, self::MODES, true);
}
/**
* Mirrors Deck's own decision in NotificationHelper::sendCardDuedate():
*
* - `off` never notifies — not even the owner of an unshared board;
* - `all` always notifies;
* - the owner of an unshared board is notified for every card, since on a
* board nobody else can see "assigned" would otherwise mean nothing;
* - `assigned` notifies users assigned to the card, directly or through a
* group or circle.
*
* An unknown value is treated as Deck's default. Deck rejects anything
* else when storing the setting, so that only happens if the preference
* was edited by hand.
*
* @param bool|callable(): bool $isAssigned a callable is only invoked when
* the answer actually depends on it — it costs a query per user
*/
public static function shouldNotify(string $deckSetting, bool $isOwnerOfUnsharedBoard, bool|callable $isAssigned): bool {
if (!in_array($deckSetting, [self::DECK_OFF, self::DECK_ASSIGNED, self::DECK_ALL], true)) {
$deckSetting = self::DECK_DEFAULT;
}
if ($deckSetting === self::DECK_OFF) {
return false;
}
if ($deckSetting === self::DECK_ALL || $isOwnerOfUnsharedBoard) {
return true;
}
return is_callable($isAssigned) ? (bool)$isAssigned() : $isAssigned;
}
}
+97 -16
View File
@@ -38,6 +38,13 @@ class WorkflowRunner {
private ?IUser $impersonationRestore = null;
/**
* Moved cards per recipient uid, collected over the whole run.
*
* @var array<string, list<array{workflow: Workflow, card: Card, targets: array{board: string, sourceStack: string, targetStack: string}}>>
*/
private array $pendingMails = [];
public function __construct(
private WorkflowMapper $workflowMapper,
private DeckIntegrationService $deckService,
@@ -56,6 +63,7 @@ class WorkflowRunner {
return;
}
$this->pendingMails = [];
$byUser = [];
$owners = [];
foreach ($workflows as $workflow) {
@@ -80,6 +88,8 @@ class WorkflowRunner {
]);
}
}
$this->sendPendingMails();
}
/**
@@ -211,10 +221,7 @@ class WorkflowRunner {
return;
}
// Resolved at most once per run, and only if a card actually moves
// and the workflow wants a mail — this is decoration, not a check.
$targets = null;
$moved = [];
$now = $this->timeFactory->getDateTime();
foreach ($cards as $card) {
@@ -230,22 +237,96 @@ class WorkflowRunner {
'target' => $workflow->getTargetStackId(),
]);
if (!$this->moveCard($workflow, $card)) {
continue;
}
if ($workflow->getNotifyEmail()) {
$targets ??= $this->deckService->describeTargets(
$workflow->getBoardId(),
$workflow->getSourceStackId(),
$workflow->getTargetStackId(),
);
$this->mailer->sendCardMovedNotification($user, $workflow, $card, $targets);
if ($this->moveCard($workflow, $card)) {
$moved[] = $card;
}
}
$workflow->setLastRun($this->timeFactory->getDateTime());
$this->workflowMapper->update($workflow);
if ($moved !== []) {
$this->queueMails($user, $workflow, $moved);
}
}
/**
* Works out who hears about the moved cards and parks them in
* $pendingMails; sendPendingMails() delivers them once per recipient at
* the end of the run.
*
* Mails are decoration, not part of the move: nothing in here may fail
* the workflow, and if Deck cannot say who wants a mail, nobody gets one
* rather than people who asked Deck for silence.
*
* @param Card[] $cards
*/
private function queueMails(IUser $owner, Workflow $workflow, array $cards): void {
$cardIds = array_map(static fn (Card $card): int => (int)$card->getId(), $cards);
switch ($workflow->getNotifyMode()) {
case NotificationPolicy::MODE_OWNER:
$recipients = array_fill_keys($cardIds, [$owner->getUID()]);
break;
case NotificationPolicy::MODE_BOARD:
try {
$recipients = $this->deckService->findNotificationRecipients($workflow->getBoardId(), $cardIds);
} catch (DeckUnavailableException $e) {
$this->logger->warning('No mails for workflow {id}: ' . $e->getMessage(), [
'app' => 'workflow_deck_automation',
'id' => $workflow->getId(),
]);
return;
}
break;
default:
return;
}
$targets = $this->deckService->describeTargets(
$workflow->getBoardId(),
$workflow->getSourceStackId(),
$workflow->getTargetStackId(),
);
foreach ($cards as $card) {
foreach ($recipients[(int)$card->getId()] ?? [] as $uid) {
$this->pendingMails[$uid][] = [
'workflow' => $workflow,
'card' => $card,
'targets' => $targets,
];
}
}
}
/**
* One mail per recipient and run, however many cards and workflows it
* covers — a board with twenty overdue cards and fifteen members would
* otherwise send three hundred mails at once.
*
* Runs after the per-user loop, so no impersonation is active any more.
*/
private function sendPendingMails(): void {
$pending = $this->pendingMails;
$this->pendingMails = [];
foreach ($pending as $uid => $entries) {
// Same reasoning as in run(): nothing may escape.
try {
$recipient = $this->userManager->get((string)$uid);
if ($recipient === null || !$recipient->isEnabled()) {
continue;
}
$this->mailer->sendCardsMovedNotification($recipient, $entries);
} catch (Throwable $e) {
$this->logger->error('Could not send the moved-cards mail to {user}: ' . $e->getMessage(), [
'app' => 'workflow_deck_automation',
'user' => $uid,
'exception' => $e,
]);
}
}
}
/**
@@ -289,7 +370,7 @@ class WorkflowRunner {
'target' => $brokenTarget,
]);
// Sent regardless of the workflow's notifyEmail setting: that flag is
// Sent to the owner regardless of the workflow's notifyMode: that is
// about moved cards, this is a one-off notice that the automation the
// user configured has been switched off. It stays one-off because a
// disabled workflow is not picked up again.