gnilebein/nextcloud-workflow-deck-automation
1
0 Fork 0
Code Issues Pull Requests Actions Projects Releases 10 Activity

Remove the assigned-user and label filters
Build package / php-lint (8.4) (push) Successful in 1m33s
Details
Build package / php-lint (8.5) (push) Successful in 49s
Details
Build package / xml-lint (push) Successful in 13s
Details
Build package / unit-tests (push) Successful in 38s
Details
Build package / package (push) Successful in 1m44s
Details

Browse Source 
Workflows now move every overdue card of their source stack. A new
migration drops filter_user_ids and filter_label_ids; workflows that
had a filter keep running unfiltered. The version goes to 35.1.0 so
the migration runs on update.

Removed along with the filters: the labels and participants endpoints
and their Deck lookups, the dead-filter warnings and markers, and
CardService::enrichCards() in the card read. enrichCards() only added
labels, assignees and comment counts, and ran a permission check
against Deck's frozen uid.
This commit is contained in:
Patrick Niebeling committed 2026-09-30 09:55:54 +02:00
1 parent e84123654e
commit 0904292c73
14 files changed
+122 -651
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

lib/Service/DeckIntegrationService.php
+9 -201
View File
@@ -14,9 +14,7 @@ use OCA\Deck\Service\PermissionService;
use OCA\Deck\Service\StackService;
use OCP\App\IAppManager;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Server;
use Psr\Log\LoggerInterface;
use ReflectionProperty;
@@ -36,16 +34,9 @@ class DeckIntegrationService {
private const DECK_APP_ID = 'deck';
/**
* Deck's Acl::PERMISSION_TYPE_* values, inlined so this class keeps
* working (degrading to "treat it as a user") if Deck ever moves or
* renames the constants.
*/
private const ACL_TYPE_USER = 0;
private const ACL_TYPE_GROUP = 1;
/**
* Deck's Acl::PERMISSION_READ, inlined for the same reason. It is the key
* under which PermissionService::getPermissions() reports read access.
* Deck's Acl::PERMISSION_READ, inlined so a moved or renamed constant
* cannot fatal this class. It is the key under which
* PermissionService::getPermissions() reports read access.
*/
private const ACL_PERMISSION_READ = 0;
@@ -75,8 +66,6 @@ class DeckIntegrationService {
public function __construct(
private IAppManager $appManager,
private IUserManager $userManager,
private IGroupManager $groupManager,
private LoggerInterface $logger,
) {
}
@@ -331,51 +320,6 @@ class DeckIntegrationService {
return is_a($e, 'OCA\Deck\NotFoundException');
}
/**
* @return array<int, array{id: int, title: string, color: string|null}>
*/
public function listLabels(int $boardId): array {
$labels = $this->call(fn () => $this->readLabels($boardId), []);
return array_map(
static fn ($label) => ['id' => (int)$label->getId(), 'title' => $label->getTitle(), 'color' => $label->getColor()],
$labels,
);
}
/**
* What a workflow's filters *could* legitimately refer to on this board.
*
* Unlike listLabels()/listParticipants() this throws instead of degrading
* to an empty list, because its only caller draws a conclusion from
* emptiness ("this filter cannot match anything any more"). Same rule as
* findStackIds(): a Deck hiccup must never be read as positive knowledge.
*
* @return array{labelIds: int[], participantUids: string[]}
* @throws DeckUnavailableException
*/
public function findFilterOptions(int $boardId): array {
try {
$board = $this->readBoard($boardId);
return [
'labelIds' => array_map(
static fn ($label) => (int)$label->getId(),
$board->getLabels() ?? [],
),
'participantUids' => array_values(array_unique($this->collectParticipantUids($board))),
];
} catch (DeckUnavailableException $e) {
throw $e;
} catch (Throwable $e) {
$this->logger->error('Could not read filter options of board ' . $boardId . ': ' . $e->getMessage(), [
'app' => 'workflow_deck_automation',
'exception' => $e,
]);
throw new DeckUnavailableException('Deck call failed: ' . $e->getMessage(), 0, $e);
}
}
/**
* Titles of a workflow's board and stacks, for the notification mail.
*
@@ -409,85 +353,9 @@ class DeckIntegrationService {
return $title !== '' ? $title : ('#' . $id);
}
/**
* Everyone who can hold a card on this board: the board owner (who is
* *not* part of the ACL — a private board has an empty ACL) plus every
* ACL entry, with group shares expanded to their members.
*
* @return array<int, array{uid: string, displayName: string}>
*/
public function listParticipants(int $boardId): array {
$uids = $this->call(fn () => $this->collectParticipantUids($this->readBoard($boardId)), []);
$participants = [];
foreach ($uids as $uid) {
if (isset($participants[$uid])) {
continue;
}
$participants[$uid] = [
'uid' => $uid,
'displayName' => $this->userManager->get($uid)?->getDisplayName() ?? $uid,
];
}
return array_values($participants);
}
/**
* The enriched board — `$fullDetails = true` is what populates labels and
* the ACL. Shared by every caller that needs one so a single Deck read
* serves them all.
*/
private function readBoard(int $boardId): mixed {
return $this->resolve(BoardService::class)->find($boardId, true);
}
private function readLabels(int $boardId): array {
return $this->readBoard($boardId)->getLabels() ?? [];
}
/**
* @return string[] may contain duplicates
*/
private function collectParticipantUids(mixed $board): array {
$uids = [];
$owner = $this->unwrapUid($board->getOwner());
if ($owner !== null) {
$uids[] = $owner;
}
foreach ($board->getAcl() ?? [] as $entry) {
$principal = $this->extractParticipantUid($entry);
if ($principal === null) {
continue;
}
$type = method_exists($entry, 'getType') ? (int)$entry->getType() : self::ACL_TYPE_USER;
if ($type === self::ACL_TYPE_GROUP) {
array_push($uids, ...$this->groupMemberUids($principal));
} elseif ($type === self::ACL_TYPE_USER) {
$uids[] = $principal;
}
// Circles and federated shares are skipped: their members
// cannot be resolved to plain uids here.
}
return $uids;
}
/**
* @return string[]
*/
private function groupMemberUids(string $groupId): array {
$group = $this->groupManager->get($groupId);
if ($group === null) {
return [];
}
return array_map(static fn (IUser $user) => $user->getUID(), $group->getUsers());
}
/**
* Cards in the given stack that are neither archived, deleted nor marked
* done, enriched so getAssignedUsers()/getLabels() are populated.
* done.
*
* The `deletedAt` check is not redundant: Deck's
* `CardMapper::findAllByStack()` filters on `stack_id` and
@@ -499,17 +367,11 @@ class DeckIntegrationService {
*/
public function getActiveCardsInStack(int $stackId): array {
return $this->call(function () use ($stackId) {
$cardMapper = $this->resolve(CardMapper::class);
$cardService = $this->resolve(CardService::class);
$cards = $cardMapper->findAllByStack($stackId);
// enrichCards() mutates each $cards entry in place (labels, assigned users, ...) but
// *returns* CardDetails wrappers whose own fields (duedate, stackId, ...) were never
// copied from the wrapped card - only its overridden jsonSerialize() reads through to
// them. Capturing that return value here made every card's getDuedate()/getDaysUntilDue()
// resolve to null, so isOverdue() was always false and no workflow ever matched a card.
// Keep using the original, now-enriched $cards instead.
$cardService->enrichCards($cards);
// 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()).
$cards = $this->resolve(CardMapper::class)->findAllByStack($stackId);
return array_values(array_filter($cards, static function (Card $card) {
if (method_exists($card, 'getDeletedAt') && (int)$card->getDeletedAt() > 0) {
@@ -520,35 +382,6 @@ class DeckIntegrationService {
}, []);
}
/**
* @return string[]
*/
public function getCardAssignedUserIds(Card $card): array {
$assigned = $card->getAssignedUsers() ?? [];
$uids = [];
foreach ($assigned as $entry) {
$uid = $this->extractParticipantUid($entry);
if ($uid !== null) {
$uids[] = $uid;
}
}
return $uids;
}
/**
* @return int[]
*/
public function getCardLabelIds(Card $card): array {
$labels = $card->getLabels() ?? [];
$ids = [];
foreach ($labels as $label) {
if (method_exists($label, 'getId')) {
$ids[] = (int)$label->getId();
}
}
return $ids;
}
/**
* Moves a card to another stack using Deck's own reorder logic
* (the exact same code path behind Deck's "move card" action).
@@ -595,31 +428,6 @@ class DeckIntegrationService {
return Server::get($class);
}
/**
* Deck's assignment/ACL entries are internal, undocumented value
* objects whose exact accessor shape has changed across versions, so
* we probe the common accessor names defensively instead of relying
* on one fixed method signature.
*/
private function extractParticipantUid(mixed $entry): ?string {
$uid = $this->unwrapUid($entry);
if ($uid !== null) {
return $uid;
}
if (!is_object($entry)) {
return null;
}
foreach (['getParticipant', 'getUid', 'getParticipantUid', 'getUserId'] as $method) {
if (method_exists($entry, $method)) {
$uid = $this->unwrapUid($entry->$method());
if ($uid !== null) {
return $uid;
}
}
}
return null;
}
/**
* Turns whatever Deck hands out for a "participant"/"owner" field into
* a plain uid.
lib/Service/WorkflowRunner.php
-114
View File
@@ -209,10 +209,6 @@ class WorkflowRunner {
return;
}
$filterUserIds = $workflow->getFilterUserIdsArray();
$filterLabelIds = $workflow->getFilterLabelIdsArray();
$this->warnAboutDeadFilters($workflow, $filterUserIds, $filterLabelIds);
// 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;
@@ -224,12 +220,6 @@ class WorkflowRunner {
continue;
}
$assignedUserIds = $this->deckService->getCardAssignedUserIds($card);
$labelIds = $this->deckService->getCardLabelIds($card);
if (!self::cardMatchesFilters($assignedUserIds, $labelIds, $filterUserIds, $filterLabelIds)) {
continue;
}
$this->logger->info('Card {card} ("{title}") matches workflow {id}; moving to stack {target}.', [
'app' => 'workflow_deck_automation',
'card' => $card->getId(),
@@ -327,52 +317,6 @@ class WorkflowRunner {
return true;
}
/**
* A filter whose every entry has been deleted from the board is not an
* error — the workflow still runs, it just cannot match a single card any
* more, forever, without anything in the UI or the log saying so. That is
* the one failure mode of this app that is completely invisible, so it
* gets a log line on every run.
*
* Explicitly *not* a reason to disable the workflow or mail the user: a
* filter with three labels of which one was deleted still works as
* intended, and even a fully dead filter may be one board edit away from
* being live again. The settings page shows the same condition in red.
*
* @param string[] $filterUserIds
* @param int[] $filterLabelIds
*/
private function warnAboutDeadFilters(Workflow $workflow, array $filterUserIds, array $filterLabelIds): void {
if ($filterUserIds === [] && $filterLabelIds === []) {
return;
}
try {
$options = $this->deckService->findFilterOptions($workflow->getBoardId());
} catch (DeckUnavailableException $e) {
// Same rule as everywhere else: no answer is not a bad answer.
$this->logger->debug('Could not check the filters of workflow {id}: ' . $e->getMessage(), [
'app' => 'workflow_deck_automation',
'id' => $workflow->getId(),
]);
return;
}
$dead = self::findDeadFilters($filterUserIds, $filterLabelIds, $options['participantUids'], $options['labelIds']);
if ($dead !== []) {
$this->logger->warning(
'Workflow {id} can no longer match any card: none of its ' . implode('/', $dead)
. ' filter entries exists on board {board} any more',
[
'app' => 'workflow_deck_automation',
'id' => $workflow->getId(),
'board' => $workflow->getBoardId(),
],
);
}
}
/**
* Compares the card's actual due timestamp against $now instead of Deck's own
* Card::getDaysUntilDue(), which truncates both sides to midnight before diffing
@@ -386,64 +330,6 @@ class WorkflowRunner {
return $duedate !== null && $duedate < $now;
}
/**
* Which filters cannot match anything any more, because not one of their
* entries is still offered by the board. An empty filter is not dead, it
* is "no restriction on this dimension" — the same reading as in
* cardMatchesFilters(), and getting it backwards would report every
* unfiltered workflow as broken.
*
* Static and side-effect free for the same reason as the matching logic:
* testable without a Deck installation.
*
* @param string[] $filterUserIds
* @param int[] $filterLabelIds
* @param string[] $boardUserIds everyone who can hold a card on the board
* @param int[] $boardLabelIds the board's remaining labels
* @return string[] any of 'label', 'user', in that order
*/
public static function findDeadFilters(
array $filterUserIds,
array $filterLabelIds,
array $boardUserIds,
array $boardLabelIds,
): array {
$dead = [];
if ($filterLabelIds !== [] && array_intersect($filterLabelIds, $boardLabelIds) === []) {
$dead[] = 'label';
}
if ($filterUserIds !== [] && array_intersect($filterUserIds, $boardUserIds) === []) {
$dead[] = 'user';
}
return $dead;
}
/**
* Pure filter-matching logic (kept static/side-effect free so it can
* be unit tested without real Deck objects). Both filters are OR
* inside themselves and AND between each other: an empty filter list
* means "no restriction on this dimension".
*
* @param string[] $cardAssignedUserIds
* @param int[] $cardLabelIds
* @param string[] $filterUserIds
* @param int[] $filterLabelIds
*/
public static function cardMatchesFilters(
array $cardAssignedUserIds,
array $cardLabelIds,
array $filterUserIds,
array $filterLabelIds,
): bool {
if ($filterUserIds !== [] && array_intersect($filterUserIds, $cardAssignedUserIds) === []) {
return false;
}
if ($filterLabelIds !== [] && array_intersect($filterLabelIds, $cardLabelIds) === []) {
return false;
}
return true;
}
/**
* @throws DeckUnavailableException if Deck cannot be pinned to $user
*/