Surface dead filters, and name board and stacks in the moved-card mail
A filter whose entries have all been deleted from the board was the one failure mode of this app that was completely invisible: the workflow kept running, kept updating last_run and matched nothing, forever. It happens with deleted labels and just as easily when a filtered user loses board access, because Deck's BoardService::deleteAcl() calls assignedUsersMapper->deleteByParticipantOnBoard() and wipes that user's card assignments. Two low-stakes signals, no new column and no migration: - WorkflowRunner::warnAboutDeadFilters() logs a warning on every run. - workflowIssues in PersonalSettings.vue marks the row red. Deliberately no mail and no `enabled = false`. One deleted label out of three is harmless, and even a fully dead filter can be one board edit away from being live again. Both obey the rule the disable path already follows: only positive knowledge. DeckIntegrationService::findFilterOptions() therefore throws instead of degrading to [], unlike listLabels()/listParticipants(), where an empty list only means an empty dropdown. That also fixes an existing false positive of the same family: loadStacksFor() stored [] when the request failed, so a single failed fetch reported "Quell-Stapel und Ziel-Stapel nicht mehr vorhanden" for an untouched board. Failed requests now store null and are read as "unknown". The comparison itself lives in WorkflowRunner::findDeadFilters(), static and side-effect free like cardMatchesFilters(), with unit tests -- notably that an empty filter is never dead, which would otherwise flag every unfiltered workflow. Separately, the moved-card mail now names the board and both stacks. DeckIntegrationService::describeTargets() resolves the titles at most once per workflow run, and only when a card actually moved and the workflow wants a mail; unreadable titles degrade to #<id> rather than costing the user their notification.
This commit is contained in:
Patrick Niebeling
@@ -27,7 +27,7 @@ In cron that is *never* the user being impersonated: `null` if any Deck-owned ba
|
||||
|
||||
Consequence for `getUserBoards()`: `listBoardsForCurrentUser()` stays **request-only** anyway — Deck may already have cached board lists behind that field, and the job has no use for a board *list*; it asks `findStackIds()` about one known board id. Before using any other `OCA\Deck\*` service from the runner, check whether it carries a frozen `$userId` — the failure mode is silent and user-crossing, not an error.
|
||||
|
||||
The filter-matching logic (`WorkflowRunner::cardMatchesFilters()`, `::isOverdue()`) is deliberately `static` and side-effect-free so it's unit-testable without a real Deck installation — see `tests/Unit/Service/WorkflowRunnerFilterTest.php`.
|
||||
The filter logic (`WorkflowRunner::cardMatchesFilters()`, `::isOverdue()`, `::findDeadFilters()`) is deliberately `static` and side-effect-free so it's unit-testable without a real Deck installation — see `tests/Unit/Service/WorkflowRunnerFilterTest.php`. Keep new pure logic in that shape; it is the only part of this app that has tests at all.
|
||||
|
||||
## Workflows are auto-disabled when their Deck targets vanish
|
||||
|
||||
@@ -40,6 +40,21 @@ The filter-matching logic (`WorkflowRunner::cardMatchesFilters()`, `::isOverdue(
|
||||
|
||||
Archived boards deliberately do *not* trigger this (they stay readable, so `findStackIds()` succeeds), even though they're filtered out of the settings dropdowns.
|
||||
|
||||
## Dead filters are surfaced, never disabled
|
||||
|
||||
`findBrokenTarget()` covers the board and the two stacks — **not** `filter_user_ids` / `filter_label_ids`. Those are plain id lists compared with `array_intersect()` in `cardMatchesFilters()`, so an entry that no longer exists simply stops matching. With three labels of which one was deleted that is exactly right, and disabling would be wrong.
|
||||
|
||||
The pathological case is a filter whose entries are *all* gone: the workflow keeps running, updates `last_run`, and matches nothing — forever, with no mail, no log line and a healthy-looking row. It happens on deleted labels and just as easily on a user losing board access, because Deck's `BoardService::deleteAcl()` calls `assignedUsersMapper->deleteByParticipantOnBoard()` and wipes that user's card assignments.
|
||||
|
||||
Two low-stakes signals, no state and no migration:
|
||||
|
||||
- `WorkflowRunner::warnAboutDeadFilters()` logs a warning on every run.
|
||||
- `workflowIssues` in `src/PersonalSettings.vue` marks the row red.
|
||||
|
||||
Both obey the same rule as the disable path: **only positive knowledge**. `DeckIntegrationService::findFilterOptions()` therefore *throws* instead of degrading to `[]` — unlike its siblings `listLabels()`/`listParticipants()`, which are wrapped in `call(…, [])` for the dropdowns, where an empty list is harmless. On the JS side the per-board caches store `null` for a failed request, and `isDeadFilter()` returns false for `null`; storing `[]` there would report every stack and filter of the board as deleted the moment one request fails. Don't collapse those two states back together.
|
||||
|
||||
Deliberately no mail and no `enabled = false`: one board edit can revive the filter, and there is no "already warned" column to keep a mail one-off with (unlike the disable case, which is one-off for free because a disabled workflow is never picked up again).
|
||||
|
||||
## The job has no interval — one run per cron pass
|
||||
|
||||
`RunWorkflowsJob` extends `OCP\BackgroundJob\Job`, **not** `TimedJob`, so it runs every time cron picks it up. There is no interval and no knob: the cadence *is* the instance's cron cadence. Don't reintroduce one — an earlier version read `workflow_deck_automation.interval` from `config.php` (default 300, 60s floor), and that config key is gone as of v0.2.0. `TimedJob` with `setInterval(0)` would behave nearly identically but keeps an interval concept nothing uses.
|
||||
|
||||
@@ -48,6 +48,19 @@ Ein Workflow verweist per ID auf ein Board und zwei Stapel. Wird eines davon in
|
||||
|
||||
Ist Deck vorübergehend nicht erreichbar, wird **nicht** deaktiviert — der Job überspringt den Workflow und versucht es beim nächsten Lauf erneut. Archivierte Boards lösen die Deaktivierung ebenfalls nicht aus, sie sind nur in den Auswahlfeldern ausgeblendet.
|
||||
|
||||
### Gelöschte Labels und entfernte Benutzer
|
||||
|
||||
Filter sind ein anderer Fall: Sie werden **nicht** deaktiviert. Wird eines von drei ausgewählten Labels gelöscht, greifen die anderen beiden weiter — das abzuschalten wäre falsch.
|
||||
|
||||
Problematisch ist nur der Grenzfall, dass **alle** Einträge eines Filters verschwunden sind. Der Workflow läuft dann weiter, kann aber keine einzige Karte mehr treffen. Das passiert bei gelöschten Labels und ebenso, wenn ein gefilterter Benutzer die Board-Freigabe verliert — Deck entfernt dabei alle seine Kartenzuweisungen auf diesem Board.
|
||||
|
||||
Damit dieser Zustand nicht unsichtbar bleibt:
|
||||
|
||||
- **In der Übersicht** wird der Workflow rot markiert („Label-Filter trifft keine Karte mehr", „Benutzer-Filter trifft keine Karte mehr").
|
||||
- **Im Hintergrundjob** landet bei jedem Lauf eine Warnung im `nextcloud.log`.
|
||||
|
||||
Keine E-Mail und keine Deaktivierung: Ein einziger Board-Klick kann den Filter wieder lebendig machen. Und wie bei Board und Stapel gilt auch hier — lässt sich das Board gerade nicht abfragen, wird nichts gemeldet statt etwas Falsches.
|
||||
|
||||
## Ausführungstakt
|
||||
|
||||
Der Hintergrundjob hat **kein eigenes Intervall** und ist auch nicht konfigurierbar: Er läuft bei **jedem** Cron-Durchlauf von Nextcloud. Der Takt ist damit exakt der des System-Crons — üblicherweise alle 5 Minuten, bei einem häufiger eingerichteten Cron entsprechend öfter.
|
||||
|
||||
@@ -315,17 +315,80 @@ class DeckIntegrationService {
|
||||
* @return array<int, array{id: int, title: string, color: string|null}>
|
||||
*/
|
||||
public function listLabels(int $boardId): array {
|
||||
$labels = $this->call(function () use ($boardId) {
|
||||
$board = $this->resolve(BoardService::class)->find($boardId, true);
|
||||
return $board->getLabels() ?? [];
|
||||
}, []);
|
||||
$labels = $this->call(fn () => $this->readLabels($boardId), []);
|
||||
|
||||
return array_map(
|
||||
static fn ($label) => ['id' => $label->getId(), 'title' => $label->getTitle(), 'color' => $label->getColor()],
|
||||
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.
|
||||
*
|
||||
* Best effort by design: this only decorates a mail that is being sent
|
||||
* because a card *was* already moved, so anything unreadable degrades to
|
||||
* `#<id>` rather than costing the user their notification.
|
||||
*
|
||||
* @return array{board: string, sourceStack: string, targetStack: string}
|
||||
*/
|
||||
public function describeTargets(int $boardId, int $sourceStackId, int $targetStackId): array {
|
||||
$titles = $this->call(function () use ($boardId) {
|
||||
$stackTitles = [];
|
||||
foreach ($this->resolve(StackMapper::class)->findAll($boardId) as $stack) {
|
||||
$stackTitles[(int)$stack->getId()] = (string)$stack->getTitle();
|
||||
}
|
||||
|
||||
return [
|
||||
'board' => (string)$this->resolve(BoardMapper::class)->find($boardId)->getTitle(),
|
||||
'stacks' => $stackTitles,
|
||||
];
|
||||
}, ['board' => '', 'stacks' => []]);
|
||||
|
||||
return [
|
||||
'board' => self::titleOr($titles['board'], $boardId),
|
||||
'sourceStack' => self::titleOr($titles['stacks'][$sourceStackId] ?? '', $sourceStackId),
|
||||
'targetStack' => self::titleOr($titles['stacks'][$targetStackId] ?? '', $targetStackId),
|
||||
];
|
||||
}
|
||||
|
||||
private static function titleOr(string $title, int $id): string {
|
||||
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
|
||||
@@ -334,32 +397,7 @@ class DeckIntegrationService {
|
||||
* @return array<int, array{uid: string, displayName: string}>
|
||||
*/
|
||||
public function listParticipants(int $boardId): array {
|
||||
$uids = $this->call(function () use ($boardId) {
|
||||
$board = $this->resolve(BoardService::class)->find($boardId, true);
|
||||
|
||||
$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;
|
||||
}, []);
|
||||
$uids = $this->call(fn () => $this->collectParticipantUids($this->readBoard($boardId)), []);
|
||||
|
||||
$participants = [];
|
||||
foreach ($uids as $uid) {
|
||||
@@ -375,6 +413,47 @@ class DeckIntegrationService {
|
||||
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[]
|
||||
*/
|
||||
|
||||
@@ -30,7 +30,11 @@ class NotificationMailer {
|
||||
) {
|
||||
}
|
||||
|
||||
public function sendCardMovedNotification(IUser $user, Workflow $workflow, Card $card): void {
|
||||
/**
|
||||
* @param array{board: string, sourceStack: string, targetStack: string} $targets
|
||||
* titles resolved by DeckIntegrationService::describeTargets()
|
||||
*/
|
||||
public function sendCardMovedNotification(IUser $user, Workflow $workflow, Card $card, array $targets): void {
|
||||
$email = $user->getEMailAddress();
|
||||
if ($email === null || $email === '') {
|
||||
$this->logger->info('Skipping notification for workflow ' . $workflow->getId() . ': user ' . $user->getUID() . ' has no email address', [
|
||||
@@ -55,6 +59,13 @@ class NotificationMailer {
|
||||
'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->addFooter();
|
||||
|
||||
@@ -159,6 +159,11 @@ class WorkflowRunner {
|
||||
|
||||
$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;
|
||||
|
||||
foreach ($cards as $card) {
|
||||
if (!self::isOverdue($card->getDaysUntilDue())) {
|
||||
@@ -171,7 +176,18 @@ class WorkflowRunner {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->moveAndNotify($user, $workflow, $card);
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
$workflow->setLastRun($this->timeFactory->getDateTime());
|
||||
@@ -226,7 +242,7 @@ class WorkflowRunner {
|
||||
$this->mailer->sendWorkflowDisabledNotification($user, $workflow, $brokenTarget);
|
||||
}
|
||||
|
||||
private function moveAndNotify(IUser $user, Workflow $workflow, Card $card): void {
|
||||
private function moveCard(Workflow $workflow, Card $card): bool {
|
||||
try {
|
||||
$this->deckService->moveCard($card->getId(), $workflow->getTargetStackId());
|
||||
} catch (Throwable $e) {
|
||||
@@ -235,11 +251,55 @@ class WorkflowRunner {
|
||||
'card' => $card->getId(),
|
||||
'id' => $workflow->getId(),
|
||||
]);
|
||||
return false;
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
if ($workflow->getNotifyEmail()) {
|
||||
$this->mailer->sendCardMovedNotification($user, $workflow, $card);
|
||||
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(),
|
||||
],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -247,6 +307,38 @@ class WorkflowRunner {
|
||||
return $daysUntilDue !== null && $daysUntilDue < 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
|
||||
+83
-23
@@ -159,7 +159,10 @@ const loading = ref(true)
|
||||
const loadError = ref('')
|
||||
const workflows = ref([])
|
||||
const boards = ref([])
|
||||
// Per board, and `null` whenever the request failed — see loadStacksFor().
|
||||
const stacksByBoard = reactive({})
|
||||
const labelsByBoard = reactive({})
|
||||
const participantsByBoard = reactive({})
|
||||
|
||||
const showForm = ref(false)
|
||||
const saving = ref(false)
|
||||
@@ -181,9 +184,15 @@ const sourceStackOptions = computed(() => stackOptions.value.filter((option) =>
|
||||
const targetStackOptions = computed(() => stackOptions.value.filter((option) => option.value !== form.sourceStackId))
|
||||
|
||||
/**
|
||||
* Workflows whose board or stacks were deleted in Deck, keyed by id. The
|
||||
* background job switches these off by itself, but a row that just stopped
|
||||
* working needs to say so instead of looking healthy.
|
||||
* Workflows that stopped working, keyed by id. Two different kinds:
|
||||
*
|
||||
* - Board or stacks deleted in Deck. The background job switches these off
|
||||
* by itself, but a row that just stopped working needs to say so instead
|
||||
* of looking healthy.
|
||||
* - Every entry of a filter deleted from the board. Nothing switches those
|
||||
* off — deliberately, since one deleted label out of three is harmless —
|
||||
* yet a filter that cannot match a single card any more is otherwise
|
||||
* completely invisible: the workflow keeps running and moves nothing.
|
||||
*/
|
||||
const workflowIssues = computed(() => {
|
||||
const issues = {}
|
||||
@@ -195,25 +204,49 @@ const workflowIssues = computed(() => {
|
||||
}
|
||||
|
||||
const stacks = stacksByBoard[workflow.boardId]
|
||||
if (!stacks) {
|
||||
continue
|
||||
if (stacks) {
|
||||
const missing = []
|
||||
if (!stacks.some((stack) => stack.id === workflow.sourceStackId)) {
|
||||
missing.push('Quell-Stapel')
|
||||
}
|
||||
if (!stacks.some((stack) => stack.id === workflow.targetStackId)) {
|
||||
missing.push('Ziel-Stapel')
|
||||
}
|
||||
if (missing.length) {
|
||||
issues[workflow.id] = `${missing.join(' und ')} nicht mehr vorhanden`
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
const missing = []
|
||||
if (!stacks.some((stack) => stack.id === workflow.sourceStackId)) {
|
||||
missing.push('Quell-Stapel')
|
||||
const dead = []
|
||||
if (isDeadFilter(workflow.filterLabelIds, labelsByBoard[workflow.boardId], 'id')) {
|
||||
dead.push('Label-Filter')
|
||||
}
|
||||
if (!stacks.some((stack) => stack.id === workflow.targetStackId)) {
|
||||
missing.push('Ziel-Stapel')
|
||||
if (isDeadFilter(workflow.filterUserIds, participantsByBoard[workflow.boardId], 'uid')) {
|
||||
dead.push('Benutzer-Filter')
|
||||
}
|
||||
if (missing.length) {
|
||||
issues[workflow.id] = `${missing.join(' und ')} nicht mehr vorhanden`
|
||||
if (dead.length > 1) {
|
||||
issues[workflow.id] = `${dead.join(' und ')} treffen keine Karte mehr`
|
||||
} else if (dead.length) {
|
||||
issues[workflow.id] = `${dead[0]} trifft keine Karte mehr`
|
||||
}
|
||||
}
|
||||
|
||||
return issues
|
||||
})
|
||||
|
||||
/**
|
||||
* True only on positive knowledge: the filter has entries, the board's list
|
||||
* was actually loaded, and not one entry is still on it. A failed request
|
||||
* (null) says nothing and must not turn a healthy row red.
|
||||
*/
|
||||
function isDeadFilter(selected, available, key) {
|
||||
if (!selected.length || !available) {
|
||||
return false
|
||||
}
|
||||
return !selected.some((value) => available.some((entry) => entry[key] === value))
|
||||
}
|
||||
|
||||
const form = reactive(emptyForm())
|
||||
|
||||
function emptyForm() {
|
||||
@@ -242,11 +275,27 @@ async function loadStacksFor(boardId) {
|
||||
if (!boardId || stacksByBoard[boardId]) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
stacksByBoard[boardId] = await fetchStacks(boardId)
|
||||
} catch (e) {
|
||||
stacksByBoard[boardId] = []
|
||||
// `null`, not `[]` — "could not ask Deck" has to stay distinguishable
|
||||
// from "this board has none", or a single failed request reports every
|
||||
// stack of the board as deleted.
|
||||
stacksByBoard[boardId] = await fetchStacks(boardId).catch(() => null)
|
||||
}
|
||||
|
||||
/**
|
||||
* Labels and participants of a board — needed only to judge whether a stored
|
||||
* filter can still match anything, so this is fetched just for the boards of
|
||||
* workflows that actually use filters.
|
||||
*/
|
||||
async function loadFiltersFor(boardId) {
|
||||
if (!boardId || labelsByBoard[boardId]) {
|
||||
return
|
||||
}
|
||||
const [labels, participants] = await Promise.all([
|
||||
fetchLabels(boardId).catch(() => null),
|
||||
fetchParticipants(boardId).catch(() => null),
|
||||
])
|
||||
labelsByBoard[boardId] = labels
|
||||
participantsByBoard[boardId] = participants
|
||||
}
|
||||
|
||||
async function loadAll() {
|
||||
@@ -256,7 +305,13 @@ async function loadAll() {
|
||||
const [loadedWorkflows, loadedBoards] = await Promise.all([fetchWorkflows(), fetchBoards()])
|
||||
workflows.value = loadedWorkflows
|
||||
boards.value = loadedBoards
|
||||
await Promise.all(loadedWorkflows.map((workflow) => loadStacksFor(workflow.boardId)))
|
||||
await Promise.all(loadedWorkflows.flatMap((workflow) => {
|
||||
const pending = [loadStacksFor(workflow.boardId)]
|
||||
if (workflow.filterLabelIds.length || workflow.filterUserIds.length) {
|
||||
pending.push(loadFiltersFor(workflow.boardId))
|
||||
}
|
||||
return pending
|
||||
}))
|
||||
} catch (e) {
|
||||
loadError.value = 'Konnte Workflows oder Deck-Boards nicht laden. Ist die Deck-App aktiviert?'
|
||||
} finally {
|
||||
@@ -281,15 +336,20 @@ async function loadBoardOptions(boardId) {
|
||||
|
||||
// One failing endpoint must not take the other two dropdowns down with it.
|
||||
const [stacks, labels, participants] = await Promise.all([
|
||||
fetchStacks(boardId).catch(() => []),
|
||||
fetchLabels(boardId).catch(() => []),
|
||||
fetchParticipants(boardId).catch(() => []),
|
||||
fetchStacks(boardId).catch(() => null),
|
||||
fetchLabels(boardId).catch(() => null),
|
||||
fetchParticipants(boardId).catch(() => null),
|
||||
])
|
||||
|
||||
// Feed the overview's caches too, so its red markers agree with what the
|
||||
// form just loaded. `null` propagates on purpose: it means "unknown".
|
||||
stacksByBoard[boardId] = stacks
|
||||
stackOptions.value = stacks.map((stack) => ({ value: stack.id, label: stack.title }))
|
||||
labelOptions.value = labels.map((label) => ({ value: label.id, label: label.title }))
|
||||
participantOptions.value = participants.map((participant) => ({ value: participant.uid, label: participant.displayName }))
|
||||
labelsByBoard[boardId] = labels
|
||||
participantsByBoard[boardId] = participants
|
||||
|
||||
stackOptions.value = (stacks ?? []).map((stack) => ({ value: stack.id, label: stack.title }))
|
||||
labelOptions.value = (labels ?? []).map((label) => ({ value: label.id, label: label.title }))
|
||||
participantOptions.value = (participants ?? []).map((participant) => ({ value: participant.uid, label: participant.displayName }))
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -43,4 +43,28 @@ class WorkflowRunnerFilterTest extends TestCase {
|
||||
// both match -> true
|
||||
$this->assertTrue(WorkflowRunner::cardMatchesFilters(['alice'], [1], ['alice'], [1]));
|
||||
}
|
||||
|
||||
public function testEmptyFiltersAreNeverDead(): void {
|
||||
// "no restriction" must not be reported as a broken filter, however
|
||||
// little the board has left to offer.
|
||||
$this->assertSame([], WorkflowRunner::findDeadFilters([], [], [], []));
|
||||
$this->assertSame([], WorkflowRunner::findDeadFilters([], [], ['alice'], [1]));
|
||||
}
|
||||
|
||||
public function testFilterSurvivesAsLongAsOneEntryRemains(): void {
|
||||
$this->assertSame([], WorkflowRunner::findDeadFilters(['alice', 'bob'], [1, 2], ['bob'], [2]));
|
||||
}
|
||||
|
||||
public function testFilterIsDeadWhenEveryEntryIsGone(): void {
|
||||
$this->assertSame(['label'], WorkflowRunner::findDeadFilters([], [1, 2], [], [3]));
|
||||
$this->assertSame(['user'], WorkflowRunner::findDeadFilters(['alice'], [], ['bob'], []));
|
||||
$this->assertSame(
|
||||
['label', 'user'],
|
||||
WorkflowRunner::findDeadFilters(['alice'], [1], ['bob'], [2]),
|
||||
);
|
||||
}
|
||||
|
||||
public function testAnEmptyBoardKillsOnlyTheFiltersThatAreSet(): void {
|
||||
$this->assertSame(['label'], WorkflowRunner::findDeadFilters([], [1], [], []));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user