From 28b4edc81119a3ea981e29d37a54218134228028 Mon Sep 17 00:00:00 2001 From: Patrick Niebeling <1+gnilebein@noreply.example.org> Date: Wed, 26 Aug 2026 07:10:55 +0200 Subject: [PATCH] revert 0e66319a06b0ad1e0dadc195b3373f26e86314c4 revert Let each workflow watch the due date or the start date Deck 1.18 added a start date to cards (Card::$startdate), so a workflow no longer has to mean "overdue". Each rule now carries a date_field ('due' | 'start'), chosen in the settings form next to board and stacks. Existing rules keep the due date: the new column's default supplies it for every stored row, so there is no backfill step, and Workflow::getDateFieldOrDue() covers entities that were never near the database as well as hand-edited values. The controller is the one place that rejects an unknown value instead of normalising it - a client asking for a rule it would not get should hear about it. The probe for the field is property_exists(), not method_exists(): Deck's entities declare no getter as real code - getStartdate(), getDuedate(), even getId() all go through Entity::__call(), which method_exists() ignores by definition. A method_exists() guard would have been false on every Deck version and would have turned every start-date rule into a silent no-op. assertDeckAvailable() now also refuses Deck older than 1.18.0. That floor cannot live in appinfo/info.xml - neither the server's nor the app store's schema has an app-to-app dependency element - but already implies it, since 1.18.x is the only Deck release line published for Nextcloud 34. isOverdue() becomes hasPassed(): one function for both dates, since the comparison and the argument against Deck's day-truncating getDaysUntilDue() are identical for either. Also carries a pending NcSelect icon-alignment fix that was already in the working tree. Bump to 34.1.0. --- CLAUDE.md | 37 +------- README.md | 7 +- appinfo/info.xml | 20 ++-- lib/Controller/WorkflowController.php | 20 +--- lib/Db/Workflow.php | 30 ------ .../Version1001Date20260825120000.php | 47 ---------- lib/Service/DeckIntegrationService.php | 91 +------------------ lib/Service/NotificationMailer.php | 17 +--- lib/Service/WorkflowRunner.php | 22 ++--- package.json | 2 +- src/PersonalSettings.vue | 73 +-------------- tests/Unit/Db/WorkflowTest.php | 43 --------- .../Unit/Service/WorkflowRunnerFilterTest.php | 22 ++--- 13 files changed, 41 insertions(+), 390 deletions(-) delete mode 100644 lib/Migration/Version1001Date20260825120000.php delete mode 100644 tests/Unit/Db/WorkflowTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 56822b4..670a8c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## What this is -A Nextcloud app (`workflow_deck_automation`, namespace `WorkflowDeckAutomation`) targeting **Nextcloud Hub 26 Spring (server 34.x)**. It lets each user configure, from their personal settings, automation "workflows" that move [Deck](https://github.com/nextcloud/deck) cards from one stack to another once their due date or start date has passed, with optional assigned-user/label filters and an email notification. A background `Job` evaluates all enabled workflows on every cron pass. +A Nextcloud app (`workflow_deck_automation`, namespace `WorkflowDeckAutomation`) targeting **Nextcloud Hub 26 Spring (server 34.x)**. It lets each user configure, from their personal settings, automation "workflows" that move overdue [Deck](https://github.com/nextcloud/deck) cards from one stack to another, with optional assigned-user/label filters and an email notification. A background `Job` evaluates all enabled workflows on every cron pass. ## Non-negotiable architectural constraint @@ -12,27 +12,6 @@ A Nextcloud app (`workflow_deck_automation`, namespace `WorkflowDeckAutomation`) All Deck access is funneled through `lib/Service/DeckIntegrationService.php` — it is the *only* class that references `OCA\Deck\*`. If you need new Deck data, extend that class rather than reaching into Deck internals from elsewhere. Every call in there is wrapped (`class_exists()` checks, `IAppManager::isEnabledForUser('deck', …)`, try/catch → `DeckUnavailableException`) because `OCA\Deck\*` is not a documented/stable public API — it has no `@since` markers and can change between Deck releases without notice. -## `method_exists()` cannot see Deck's getters — use `property_exists()` - -`Card`, `Board`, `Label`, `Acl` and `Assignment` declare **no getters as real code**. `getStartdate()`, `getDuedate()`, `getDeletedAt()`, `getArchived()`, even `getId()` are `@method` docblocks resolved by `OCP\AppFramework\Db\Entity::__call()` (`Entity` itself declares neither `getId()` nor `setId()`). `method_exists()` ignores `__call()` by definition, so **every `method_exists($deckEntity, 'getSomething')` probe in this app is permanently false**, on every Deck version, and the guarded branch is dead code that fails open or closed without a sound. - -`DeckIntegrationService::getCardDate()` therefore probes `property_exists($card, 'startdate')` — the protected property declaration is visible, and it is exactly what `Entity::getter()` checks before throwing `BadFunctionCallException`. Use that shape for any new optional Deck field. - -**Known dead probes still in the tree** (each one a separate fix, none of them yet made — verify against a live instance before touching, since the symptoms are silent): - -- `getCardLabelIds()` / `extractParticipantUid()` — always return `[]`/`null`, so **label and assigned-user filters cannot match any card**, and ACL participants never reach the settings dropdown. Only the board owner survives, because `RelationalObject::getPrimaryKey()` *is* real code — which is why the bug is invisible on a private board (empty ACL, owner = the user themselves) and looks fixed. -- `getActiveCardsInStack()`'s `getDeletedAt` check — trashed cards are moved. -- `findStackIds()`'s `getDeletedAt` check — a board in the trash is not recognised as gone. -- `isBoardHidden()` — dead, but masked by `getUserBoards(null, false)` doing the filtering server-side. - -## The Deck version floor lives in code, because info.xml cannot express it - -`DeckIntegrationService::MIN_DECK_VERSION` (`1.18.0`) plus the `version_compare()` in `assertDeckAvailable()` is the *only* enforcement point for "this app needs a recent Deck". Neither the server's `resources/app-info.xsd` nor the app store's `info.xsd` has an app-to-app dependency element — `` takes only `php`, `database`, `command`, `lib`, `owncloud`, `nextcloud`, `architecture`, `backend`. Don't go looking for a ``; it does not exist in either schema. - -`` already does most of the work and is the strictest pin available: **Deck 1.18.x is the only Deck release line declaring `>=34.0.0,<35.0.0`**, and 1.18.0 is exactly the release that added the card start date (`Card::$startdate`, PR #7749). So on any Nextcloud this app installs on, Deck already has that field, and 1.17.x cannot be enabled at all (its own `max-version` is 33, so a server upgrade to 34 disables it). What the version check still covers is a Deck dropped in by hand or checked out from git. - -`version_compare()` sorts `1.18.0-beta.x` *below* `1.18.0`. That is intended — pre-releases of that line may predate the field. Since `assertDeckAvailable()` is called from both the controller (→ `OCSPreconditionFailedException`, visible as an error on the settings page) and the runner (→ skip + info log), a too-old Deck degrades in both places instead of failing halfway through a card move. - ## Per-user impersonation in the background job `RunWorkflowsJob` has no logged-in user by default and must evaluate workflows belonging to many different users within one PHP process. `lib/Service/WorkflowRunner.php` therefore groups workflows by owner and impersonates each owner in turn. **Impersonation here is two steps, and `IUserSession::setUser()` is the one that barely matters:** @@ -48,19 +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 logic (`WorkflowRunner::cardMatchesFilters()`, `::hasPassed()`, `::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. - -## Due date or start date, per workflow - -Each workflow carries a `date_field` (`'due'` | `'start'`, `Workflow::DATE_FIELD_*`) deciding which Deck card date it watches. `WorkflowRunner::hasPassed()` (formerly `isOverdue()`) is one function for both, because the comparison is identical — the raw timestamp against `$now` — and the whole argument against Deck's day-truncating `getDaysUntilDue()` applies unchanged to the start date. What differs is only the wording, in the UI and in `NotificationMailer`. - -Three places keep an unknown value from becoming a surprise trigger, and all three resolve **towards the due date**, because that is what every workflow meant before the choice existed: - -- the column default `'due'`, which is also the entire migration for existing rows (`Version1001Date20260825120000`, a separate step because `Version1000…` is wrapped in `hasTable()` and would never reach an existing install), -- `Workflow::getDateFieldOrDue()`, for entities that were never near the database and for hand-edited rows, -- `DeckIntegrationService::CARD_DATE_PROPERTIES`'s `?? DEFAULT_CARD_DATE`. - -The controller is the exception: it *rejects* an unknown `dateField` instead of normalising it, since a client asking for a rule it would not get should hear about it. +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 diff --git a/README.md b/README.md index db44bc7..a64d319 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,6 @@ # Deck Workflow-Automatisierung -Nextcloud-App für **Nextcloud Hub 26 Spring (Server 34.x)**, die Karten der [Deck](https://github.com/nextcloud/deck)-App automatisch von einem Stapel in einen anderen verschiebt, sobald ihr Fälligkeits- oder Startdatum erreicht ist, und optional eine E-Mail-Benachrichtigung verschickt. - -Setzt **Deck ab 1.18.0** voraus (die Version, die das Startdatum eingeführt hat) — auf Nextcloud 34 ist das automatisch erfüllt, da 1.18.x die einzige dort freigegebene Deck-Reihe ist. +Nextcloud-App für **Nextcloud Hub 26 Spring (Server 34.x)**, die überfällige Karten der [Deck](https://github.com/nextcloud/deck)-App automatisch von einem Stapel in einen anderen verschiebt und optional eine E-Mail-Benachrichtigung verschickt. Ersetzt das früher genutzte eigenständige PHP-Cron-Skript durch eine vollwertige App mit Oberfläche in den **persönlichen Einstellungen** — jeder Nutzer kann dort seine eigenen Regeln ("Workflows") anlegen, ohne Admin-Rechte oder Server-Zugriff zu benötigen. @@ -11,8 +9,7 @@ Ersetzt das früher genutzte eigenständige PHP-Cron-Skript durch eine vollwerti Pro Workflow lässt sich konfigurieren: - Quell-Board und Quell-Stapel -- Ziel-Stapel, in den die Karten verschoben werden -- **auslösendes Datum: Fälligkeitsdatum oder Startdatum der Karte** (Karten ohne das gewählte Datum werden nie verschoben; Regeln aus älteren Versionen laufen unverändert auf dem Fälligkeitsdatum) +- Ziel-Stapel, in den überfällige Karten verschoben werden - optionaler Filter auf zugewiesene Benutzer (Karte muss **mindestens einem** der gewählten Benutzer zugewiesen sein) - optionaler Filter auf Labels/Tags (Karte muss **mindestens eines** der gewählten Labels haben) - Checkbox: E-Mail-Benachrichtigung an den Workflow-Besitzer, sobald eine Karte verschoben wurde diff --git a/appinfo/info.xml b/appinfo/info.xml index 5427e90..959f224 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -4,37 +4,31 @@ workflow_deck_automation Deck Workflow-Automatisierung Deck Workflow Automation - Verschiebt Deck-Karten automatisch anhand konfigurierbarer Regeln - Automatically moves Deck cards based on configurable rules + Verschiebt überfällige Deck-Karten automatisch anhand konfigurierbarer Regeln + Automatically moves overdue Deck cards based on configurable rules - 34.1.0 + 34.0.2 agpl Patrick Niebeling WorkflowDeckAutomation diff --git a/lib/Controller/WorkflowController.php b/lib/Controller/WorkflowController.php index b567030..87349fa 100644 --- a/lib/Controller/WorkflowController.php +++ b/lib/Controller/WorkflowController.php @@ -56,15 +56,14 @@ class WorkflowController extends OCSController { int $targetStackId, array $filterUserIds = [], array $filterLabelIds = [], - string $dateField = Workflow::DATE_FIELD_DUE, bool $notifyEmail = false, bool $enabled = true, ): DataResponse { - $this->validate($title, $boardId, $sourceStackId, $targetStackId, $dateField); + $this->validate($title, $boardId, $sourceStackId, $targetStackId); $workflow = new Workflow(); $workflow->setUserId($this->currentUserId()); - $this->applyFields($workflow, $title, $boardId, $sourceStackId, $targetStackId, $filterUserIds, $filterLabelIds, $dateField, $notifyEmail, $enabled); + $this->applyFields($workflow, $title, $boardId, $sourceStackId, $targetStackId, $filterUserIds, $filterLabelIds, $notifyEmail, $enabled); $workflow = $this->workflowMapper->insert($workflow); return new DataResponse($workflow->jsonSerialize(), Http::STATUS_CREATED); @@ -83,11 +82,10 @@ class WorkflowController extends OCSController { int $targetStackId, array $filterUserIds = [], array $filterLabelIds = [], - string $dateField = Workflow::DATE_FIELD_DUE, bool $notifyEmail = false, bool $enabled = true, ): DataResponse { - $this->validate($title, $boardId, $sourceStackId, $targetStackId, $dateField); + $this->validate($title, $boardId, $sourceStackId, $targetStackId); try { $workflow = $this->workflowMapper->findForUser($id, $this->currentUserId()); @@ -95,7 +93,7 @@ class WorkflowController extends OCSController { throw new OCSNotFoundException('Workflow not found'); } - $this->applyFields($workflow, $title, $boardId, $sourceStackId, $targetStackId, $filterUserIds, $filterLabelIds, $dateField, $notifyEmail, $enabled); + $this->applyFields($workflow, $title, $boardId, $sourceStackId, $targetStackId, $filterUserIds, $filterLabelIds, $notifyEmail, $enabled); $workflow = $this->workflowMapper->update($workflow); return new DataResponse($workflow->jsonSerialize()); @@ -149,19 +147,13 @@ class WorkflowController extends OCSController { } } - private function validate(string $title, int $boardId, int $sourceStackId, int $targetStackId, string $dateField): void { + private function validate(string $title, int $boardId, int $sourceStackId, int $targetStackId): void { if (trim($title) === '') { throw new OCSBadRequestException('Title must not be empty'); } if ($sourceStackId === $targetStackId) { throw new OCSBadRequestException('Source and target stack must differ'); } - // Rejected rather than silently normalised to 'due': a client sending - // something else is asking for a rule it would not get, and the runner - // falls back to the due date without a word. - if (!in_array($dateField, Workflow::DATE_FIELDS, true)) { - throw new OCSBadRequestException('Unknown date field: ' . $dateField); - } // Reject references to boards or stacks that are gone (or were never // the user's) instead of storing a workflow that can only fail later. @@ -198,7 +190,6 @@ class WorkflowController extends OCSController { int $targetStackId, array $filterUserIds, array $filterLabelIds, - string $dateField, bool $notifyEmail, bool $enabled, ): void { @@ -208,7 +199,6 @@ class WorkflowController extends OCSController { $workflow->setTargetStackId($targetStackId); $workflow->setFilterUserIds($filterUserIds === [] ? null : json_encode(array_values($filterUserIds))); $workflow->setFilterLabelIds($filterLabelIds === [] ? null : json_encode(array_values($filterLabelIds))); - $workflow->setDateField($dateField); $workflow->setNotifyEmail($notifyEmail); $workflow->setEnabled($enabled); } diff --git a/lib/Db/Workflow.php b/lib/Db/Workflow.php index 37ae61f..11d71fa 100644 --- a/lib/Db/Workflow.php +++ b/lib/Db/Workflow.php @@ -22,8 +22,6 @@ use OCP\DB\Types; * @method void setFilterUserIds(?string $filterUserIds) * @method string|null getFilterLabelIds() * @method void setFilterLabelIds(?string $filterLabelIds) - * @method string|null getDateField() - * @method void setDateField(string $dateField) * @method bool getNotifyEmail() * @method void setNotifyEmail(bool $notifyEmail) * @method bool getEnabled() @@ -32,14 +30,6 @@ use OCP\DB\Types; * @method void setLastRun(?\DateTime $lastRun) */ class Workflow extends Entity implements \JsonSerializable { - /** Deck's `Card::$duedate` — the card is picked up once it is overdue. */ - public const DATE_FIELD_DUE = 'due'; - - /** Deck's `Card::$startdate` — the card is picked up once its start date has been reached. */ - public const DATE_FIELD_START = 'start'; - - public const DATE_FIELDS = [self::DATE_FIELD_DUE, self::DATE_FIELD_START]; - protected $userId; protected $title; protected $boardId; @@ -47,7 +37,6 @@ class Workflow extends Entity implements \JsonSerializable { protected $targetStackId; protected $filterUserIds; protected $filterLabelIds; - protected $dateField; protected $notifyEmail; protected $enabled; protected $lastRun; @@ -61,29 +50,11 @@ class Workflow extends Entity implements \JsonSerializable { $this->addType('targetStackId', Types::INTEGER); $this->addType('filterUserIds', Types::STRING); $this->addType('filterLabelIds', Types::STRING); - $this->addType('dateField', Types::STRING); $this->addType('notifyEmail', Types::BOOLEAN); $this->addType('enabled', Types::BOOLEAN); $this->addType('lastRun', Types::DATETIME); } - /** - * The date field to evaluate, never anything else. - * - * Rows written before the column existed carry 'due' from the schema - * default, but a freshly constructed entity holds `null` and a hand-edited - * row could hold anything. Falling back to the due date keeps the old - * behaviour for every value that is not explicitly the start date — the - * safe direction, since 'due' is what every workflow meant before this - * choice existed. - * - * @return self::DATE_FIELD_* - */ - public function getDateFieldOrDue(): string { - $field = $this->getDateField(); - return in_array($field, self::DATE_FIELDS, true) ? $field : self::DATE_FIELD_DUE; - } - /** * @return string[] */ @@ -119,7 +90,6 @@ class Workflow extends Entity implements \JsonSerializable { 'targetStackId' => $this->getTargetStackId(), 'filterUserIds' => $this->getFilterUserIdsArray(), 'filterLabelIds' => $this->getFilterLabelIdsArray(), - 'dateField' => $this->getDateFieldOrDue(), 'notifyEmail' => (bool)$this->getNotifyEmail(), 'enabled' => (bool)$this->getEnabled(), 'lastRun' => $this->getLastRun()?->format(\DateTimeInterface::ATOM), diff --git a/lib/Migration/Version1001Date20260825120000.php b/lib/Migration/Version1001Date20260825120000.php deleted file mode 100644 index 9d54cc2..0000000 --- a/lib/Migration/Version1001Date20260825120000.php +++ /dev/null @@ -1,47 +0,0 @@ -hasTable(...))`, so changing it would only ever - * reach fresh installs. - * - * The `default` is what migrates the existing rows — every workflow written - * before this release was evaluated against the due date, and the column - * default gives exactly that without a backfill step. - */ -class Version1001Date20260825120000 extends SimpleMigrationStep { - public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { - /** @var ISchemaWrapper $schema */ - $schema = $schemaClosure(); - - if (!$schema->hasTable('wfda_workflows')) { - return null; - } - - $table = $schema->getTable('wfda_workflows'); - if ($table->hasColumn('date_field')) { - return null; - } - - $table->addColumn('date_field', Types::STRING, [ - 'notnull' => true, - 'length' => 16, - 'default' => Workflow::DATE_FIELD_DUE, - ]); - - return $schema; - } -} diff --git a/lib/Service/DeckIntegrationService.php b/lib/Service/DeckIntegrationService.php index 26d742f..0e9e740 100644 --- a/lib/Service/DeckIntegrationService.php +++ b/lib/Service/DeckIntegrationService.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace OCA\WorkflowDeckAutomation\Service; -use DateTimeInterface; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; @@ -36,25 +35,6 @@ use Throwable; class DeckIntegrationService { private const DECK_APP_ID = 'deck'; - /** - * Oldest Deck this app will talk to. - * - * Deck 1.18.0 is the release that added the card start date - * (`Card::$startdate`, PR #7749), which the start-date workflows read. - * The floor cannot live in appinfo/info.xml: neither the server's - * app-info.xsd nor the app store's info.xsd has an app-to-app dependency - * element, only . - * - * That pin does most of the work anyway — Deck 1.18.x is the only release - * line declaring `>=34.0.0,<35.0.0`, so on the Nextcloud 34 this app - * requires, every installable Deck already carries the field. What is left - * for this check is a Deck put in place by hand or from git. - * - * version_compare() sorts `1.18.0-beta.x` *below* `1.18.0`, and that is - * intended: the betas of that line may predate the field. - */ - private const MIN_DECK_VERSION = '1.18.0'; - /** * Deck's Acl::PERMISSION_TYPE_* values, inlined so this class keeps * working (degrading to "treat it as a user") if Deck ever moves or @@ -88,31 +68,11 @@ class DeckIntegrationService { 'OCA\Deck\Activity\ActivityManager', ]; - /** - * `Workflow::DATE_FIELD_*` mapped to the property on Deck's Card entity. - * Kept here rather than on the Workflow so this class stays the only one - * that has to know what Deck calls its columns. - */ - private const CARD_DATE_PROPERTIES = [ - 'due' => 'duedate', - 'start' => 'startdate', - ]; - - private const DEFAULT_CARD_DATE = 'due'; - /** * @var list */ private array $identityRestore = []; - /** - * Card properties already reported as missing, so the warning in - * getCardDate() is one line per process instead of one per card. - * - * @var array - */ - private array $reportedMissingCardProperties = []; - public function __construct( private IAppManager $appManager, private IUserManager $userManager, @@ -206,16 +166,6 @@ class DeckIntegrationService { if (!$this->appManager->isEnabledForUser(self::DECK_APP_ID, $user)) { throw new DeckUnavailableException('Deck is not enabled for user ' . $user->getUID()); } - - $version = $this->appManager->getAppVersion(self::DECK_APP_ID); - if ($version === '' || version_compare($version, self::MIN_DECK_VERSION, '<')) { - throw new DeckUnavailableException(sprintf( - 'Deck %s is installed, but %s or newer is required', - $version !== '' ? $version : '(unknown version)', - self::MIN_DECK_VERSION, - )); - } - if (!class_exists(CardService::class) || !class_exists(BoardService::class) || !class_exists(StackService::class)) { throw new DeckUnavailableException('Deck internal classes are not available'); } @@ -537,7 +487,7 @@ class DeckIntegrationService { // *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 hasPassed() was always false and no workflow ever matched a card. + // 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); @@ -550,45 +500,6 @@ class DeckIntegrationService { }, []); } - /** - * The card date a workflow is evaluated against: Deck's due date or its - * start date, per `Workflow::DATE_FIELD_*`. Anything unrecognised reads the - * due date, which is what every workflow meant before the choice existed. - * - * The guard is `property_exists()`, deliberately, and this is the one thing - * to get right here: **`method_exists()` cannot see Deck's getters.** - * `Card`, `Board`, `Label`, `Acl` and `Assignment` declare none of them as - * real code — `getStartdate()`, `getDuedate()`, even `getId()` all go - * through `OCP\AppFramework\Db\Entity::__call()`, and `method_exists()` - * ignores `__call()` by definition. A `method_exists($card, 'getStartdate')` - * probe would therefore be false on *every* Deck version and silently turn - * every start-date workflow into a no-op. `property_exists()` sees the - * protected `$startdate` declaration and is exactly what `Entity::getter()` - * checks internally before throwing. - * - * In practice the property is always there — `assertDeckAvailable()` - * enforces Deck >= MIN_DECK_VERSION, which is the release that added it — - * so this branch is a backstop for a hand-installed Deck, loud enough to be - * found in the log rather than silently matching nothing. - */ - public function getCardDate(Card $card, string $dateField): ?DateTimeInterface { - $property = self::CARD_DATE_PROPERTIES[$dateField] ?? self::CARD_DATE_PROPERTIES[self::DEFAULT_CARD_DATE]; - - if (!property_exists($card, $property)) { - if (!isset($this->reportedMissingCardProperties[$property])) { - $this->reportedMissingCardProperties[$property] = true; - $this->logger->warning( - 'This Deck version has no Card::$' . $property . '; workflows using it cannot match any card', - ['app' => 'workflow_deck_automation'], - ); - } - return null; - } - - $value = $property === 'startdate' ? $card->getStartdate() : $card->getDuedate(); - return $value instanceof DateTimeInterface ? $value : null; - } - /** * @return string[] */ diff --git a/lib/Service/NotificationMailer.php b/lib/Service/NotificationMailer.php index 2199448..9144c2f 100644 --- a/lib/Service/NotificationMailer.php +++ b/lib/Service/NotificationMailer.php @@ -55,19 +55,10 @@ class NotificationMailer { $template->setSubject($this->l10n->t('Deck card moved: %s', [$card->getTitle()])); $template->addHeader(); $template->addHeading($this->l10n->t('A card was moved automatically'), false); - // Two whole strings rather than one sentence with an interpolated - // reason: the trigger is the point of the mail, and a translator - // needs to see it in the sentence to get the grammar right. - $reason = $workflow->getDateFieldOrDue() === Workflow::DATE_FIELD_START - ? $this->l10n->t( - 'The card "%1$s" was moved because its start date has been reached (workflow "%2$s").', - [$card->getTitle(), $workflow->getTitle()], - ) - : $this->l10n->t( - 'The card "%1$s" was moved because it is overdue (workflow "%2$s").', - [$card->getTitle(), $workflow->getTitle()], - ); - $template->addBodyText($reason); + $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. diff --git a/lib/Service/WorkflowRunner.php b/lib/Service/WorkflowRunner.php index 1af6302..2808ada 100644 --- a/lib/Service/WorkflowRunner.php +++ b/lib/Service/WorkflowRunner.php @@ -17,8 +17,7 @@ use Psr\Log\LoggerInterface; use Throwable; /** - * Evaluates all enabled workflows and moves the cards whose watched date - - * Deck's due date or its start date, per workflow - has passed. + * Evaluates all enabled workflows and moves overdue cards. * * This runner has to evaluate rules for many different users within a * single background-job process, so workflows are grouped by owner and @@ -168,10 +167,9 @@ class WorkflowRunner { $targets = null; $now = $this->timeFactory->getDateTime(); - $dateField = $workflow->getDateFieldOrDue(); foreach ($cards as $card) { - if (!self::hasPassed($this->deckService->getCardDate($card, $dateField), $now)) { + if (!self::isOverdue($card->getDuedate(), $now)) { continue; } @@ -181,12 +179,11 @@ class WorkflowRunner { continue; } - $this->logger->info('Card {card} ("{title}") matches workflow {id} on its {dateField} date; moving to stack {target}.', [ + $this->logger->info('Card {card} ("{title}") matches workflow {id}; moving to stack {target}.', [ 'app' => 'workflow_deck_automation', 'card' => $card->getId(), 'title' => $card->getTitle(), 'id' => $workflow->getId(), - 'dateField' => $dateField, 'target' => $workflow->getTargetStackId(), ]); @@ -326,21 +323,16 @@ class WorkflowRunner { } /** - * Whether the card date a workflow watches - its due date or its start date, - * see Workflow::DATE_FIELD_* - lies in the past. A card without that date set - * is never picked up. - * - * Compares the card's actual timestamp against $now instead of Deck's own + * Compares the card's actual due timestamp against $now instead of Deck's own * Card::getDaysUntilDue(), which truncates both sides to midnight before diffing * (DateInterval's %a is a whole-day count) and therefore reports 0 - not overdue - * for any card whose due time has already passed today. That silently held back * every card for up to 24h after it actually fell due; comparing the raw datetime * instead matches what Deck's own UI shows (e.g. "vor 2 Stunden") and has no - * day-rounding step to get wrong. The same reasoning applies unchanged to the - * start date, which is why this is one function and not two. + * day-rounding step to get wrong. */ - public static function hasPassed(?DateTimeInterface $cardDate, DateTimeInterface $now): bool { - return $cardDate !== null && $cardDate < $now; + public static function isOverdue(?DateTimeInterface $duedate, DateTimeInterface $now): bool { + return $duedate !== null && $duedate < $now; } /** diff --git a/package.json b/package.json index e7812a5..de0f383 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "workflow_deck_automation", - "version": "34.1.0", + "version": "34.0.2", "private": true, "type": "module", "description": "Automates moving overdue Deck cards between stacks, configurable per user.", diff --git a/src/PersonalSettings.vue b/src/PersonalSettings.vue index b4d397e..cacf277 100644 --- a/src/PersonalSettings.vue +++ b/src/PersonalSettings.vue @@ -1,7 +1,7 @@