Merge pull request #20 from ITK-Leantime/feature/8000-modified-column

feat: own the sync watermark in an itk_data_api_modified column

Author: tuj

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## [Unreleased]
 
+* [PR-20](https://github.com/ITK-Leantime/data-api/pull/20)
+  * Added a plugin owned `itk_data_api_modified` column, maintained by database triggers, on projects, tickets, timesheets and users, so no write path can leave the sync watermark behind.
+  * Changed `modifiedAfter` to filter on that column, so edits to existing tickets and milestones are no longer missed and time logged from the weekly grid is picked up.
+  * Added `modified` to the users endpoint.
+  * Changed the deletion triggers to stamp `dateDeleted` in UTC, so `deleted` filters against the same clock the responses are read in.
+  * Moved the schema handling into a SchemaRepository, executing one statement at a time so installation reports failures instead of swallowing them, and made installing idempotent.
+  * Dropped the `dateDeleted` default on the deletion tables, so a row inserted without a trigger in place is left null rather than stamped with the server's local time.
 * [PR-19](https://github.com/ITK-Leantime/data-api/pull/19)
   * Validated request parameters, so malformed input answers 400 with a reason instead of failing with a 500.
   * Rejected a limit below 1, which previously dropped the LIMIT clause and returned every row, and capped limit at 1000.
@@ -10,7 +17,6 @@
   * Fixed an empty projectIds list dropping the filter, which answered with every row instead of none.
   * Trimmed whitespace around ids, projectIds and types elements sent in array form.
   * Renamed InvalidRequestException to BadRequestException, matching the 400 it turns into.
-
 * [PR-18](https://github.com/ITK-Leantime/data-api/pull/18)
   * Allowed null values in API models, so entries referencing deleted users or deleted tickets no longer fail the whole request.
   * Added userId to timesheets, so hours logged by a deleted user stay attributable.

Model/WorkerData.php

@@ -2,11 +2,16 @@
 
 namespace Leantime\Plugins\APIData\Model;
 
+use Carbon\CarbonInterface;
+
 readonly class WorkerData
 {
     public function __construct(
         public int $id,
         public ?string $email,
+        // getWorkers() maps an all-blank name to null rather than to a string
+        // of whitespace, so a user with no name at all has none here.
         public ?string $name,
+        public ?CarbonInterface $modified,
     ) {}
 }

README.md

@@ -4,15 +4,37 @@ An API plugin for exposing data to external applications.
 
 Copy the plugin to the folder app/Plugins/APIData, install and enable.
 
-During installation the following tables will be created to tracked deleted entities:
+## What installation changes in the database
+
+The following tables are created to track deleted entities:
 
 * itk_projects_deleted
 * itk_tickets_deleted
 * itk_timesheets_deleted
 
-3 triggers will also be installed that populate the tables when entities are deleted.
+3 triggers populate those tables when entities are deleted.
+
+An `itk_data_api_modified` column, with an index, is added to `zp_projects`, `zp_tickets`,
+`zp_timesheets` and `zp_user`, and 8 more triggers (insert and update, one pair per table) keep it
+current. This column exists because Leantime does not maintain its own `modified` column on every write
+path — time logged from the weekly grid, for instance, leaves it untouched. Since the triggers sit in the
+database, no write path can bypass them.
+
+The column is written as UTC, and `modifiedAfter` filters on it.
+
+The Leantime database user needs `ALTER` on `zp_projects`, `zp_tickets`, `zp_timesheets` and
+`zp_user`, on top of the `CREATE` and `TRIGGER` the plugin already needed. Installation fails, and
+says so, if the grant is missing.
+
+NB! Install and update the plugin with the site down. The triggers are absent while the plugin is
+being replaced, and an edit made in that window is not recoverable — installing only stamps rows that
+have no timestamp at all, which covers new rows and nothing else.
+
+NB! Installing stamps every existing row with the install time, so **the first sync after installing
+returns everything once**.
 
-NB! The triggers are removed on uninstall, but the tables are left alone to avoid data loss through install/uninstalls.
+NB! All 11 triggers are removed on uninstall, but the tables, the column and its data are left alone to
+avoid data loss through install/uninstalls.
 
 ## Endpoints
 
@@ -25,14 +47,15 @@ The API consists of the following endpoints:
 
 GET/POST: `https://{{YOUR_DOMAIN}}/apidata/api/{{TYPE}}`
 
-TYPE: projects, milestones, tickets, timesheets
+TYPE: projects, milestones, tickets, timesheets, users
 
 Attach query/body parameters to the request:
 
 * start: Starting id of the results.
 * limit: Maximum number of results to get from start id in ascending order. Must be at least 1,
   and is capped at 1000. The limit that was actually applied is echoed in `parameters`.
 * modifiedAfter: Only retrieve entries that have a modified later than modifiedAfter (unix timestamp).
+  All five types, users included, carry a `modified` timestamp in the response.
 * ids: Array of ids to retrieve. A comma separated string is also accepted, e.g. `?ids=1,2,3`.
 * projectIds: Array of projectIds. Limits the entities to those attached to projects in projectIds.
   Only applies for types: milestone, tickets, timesheets.

Repositories/ApiDataRepository.php

@@ -17,10 +17,10 @@ private function query(): Builder
     public function getProjects(int $startId, int $limit, ?int $modifiedAfter = null, ?array $ids = null): array
     {
         return $this->query()
-            ->select(["id", "name", "modified"])
+            ->select(["project.id", "project.name", $this->modifiedSelect("project")])
             ->from("zp_projects", "project")
             ->where("project.id", ">=", $startId)
-            ->when($modifiedAfter !== null, fn ($query) => $query->where("project.modified", ">=", CarbonImmutable::createFromTimestamp($modifiedAfter)->format(APIData::DATE_FORMAT)))
+            ->when($modifiedAfter !== null, fn ($query) => $query->where($this->modified("project"), ">=", $this->cutoff($modifiedAfter)))
             ->when($ids !== null, fn ($query) => $query->whereIn("project.id", $ids))
             ->orderBy("id", "ASC")
             ->limit($limit)
@@ -31,11 +31,11 @@ public function getProjects(int $startId, int $limit, ?int $modifiedAfter = null
     public function getMilestones(int $startId, int $limit, ?int $modifiedAfter = null, ?array $ids = null, ?array $projectIds = null): array
     {
         return $this->query()
-            ->select(["id", "headline", "projectId", "modified"])
+            ->select(["ticket.id", "ticket.headline", "ticket.projectId", $this->modifiedSelect("ticket")])
             ->from("zp_tickets", "ticket")
             ->where("ticket.id", ">=", $startId)
             ->where("ticket.type", "=", "milestone")
-            ->when($modifiedAfter !== null, fn ($query) => $query->where("ticket.date", ">=", CarbonImmutable::createFromTimestamp($modifiedAfter)->format(APIData::DATE_FORMAT)))
+            ->when($modifiedAfter !== null, fn ($query) => $query->where($this->modified("ticket"), ">=", $this->cutoff($modifiedAfter)))
             ->when($ids !== null, fn ($query) => $query->whereIn("ticket.id", $ids))
             ->when($projectIds !== null, fn ($query) => $query->whereIn("ticket.projectId", $projectIds))
             ->orderBy("id", "ASC")
@@ -47,12 +47,12 @@ public function getMilestones(int $startId, int $limit, ?int $modifiedAfter = nu
     public function getTickets(int $startId, int $limit, ?int $modifiedAfter = null, ?array $ids = null, ?array $projectIds = null): array
     {
         return $this->query()
-            ->select(["ticket.id", "ticket.headline", "ticket.projectId", "ticket.status", "ticket.planHours", "ticket.hourRemaining", "ticket.tags", "ticket.dateToFinish", "ticket.editTo", "ticket.milestoneid", "ticket.modified", "user.username"])
+            ->select(["ticket.id", "ticket.headline", "ticket.projectId", "ticket.status", "ticket.planHours", "ticket.hourRemaining", "ticket.tags", "ticket.dateToFinish", "ticket.editTo", "ticket.milestoneid", $this->modifiedSelect("ticket"), "user.username"])
             ->from("zp_tickets", "ticket")
             ->where("ticket.id", ">=", $startId)
             ->where("ticket.type", "<>", "milestone")
             ->leftJoin('zp_user as user', "user.id", "=", "ticket.editorId")
-            ->when($modifiedAfter !== null, fn ($query) => $query->where("ticket.date", ">=", CarbonImmutable::createFromTimestamp($modifiedAfter)->format(APIData::DATE_FORMAT)))
+            ->when($modifiedAfter !== null, fn ($query) => $query->where($this->modified("ticket"), ">=", $this->cutoff($modifiedAfter)))
             ->when($ids !== null, fn ($query) => $query->whereIn("ticket.id", $ids))
             ->when($projectIds !== null, fn ($query) => $query->whereIn("ticket.projectId", $projectIds))
             ->orderBy("id", "ASC")
@@ -65,12 +65,12 @@ public function getTimesheets(int $startId, int $limit, ?int $modifiedAfter = nu
     {
         return $this->query()
             ->from("zp_timesheets", "timesheet")
-            ->select(["timesheet.id", "timesheet.description", "timesheet.hours", "timesheet.workDate", "timesheet.modified", "timesheet.ticketId", "timesheet.userId", "timesheet.kind", "user.username", "ticket.projectId"])
+            ->select(["timesheet.id", "timesheet.description", "timesheet.hours", "timesheet.workDate", $this->modifiedSelect("timesheet"), "timesheet.ticketId", "timesheet.userId", "timesheet.kind", "user.username", "ticket.projectId"])
             ->where("timesheet.id", ">=", $startId)
             ->whereNotNull("timesheet.hours")
             ->leftJoin('zp_user as user', "user.id", "=", "timesheet.userId")
             ->leftJoin('zp_tickets as ticket', "ticket.id", "=", "timesheet.ticketId")
-            ->when($modifiedAfter !== null, fn ($query) => $query->where("timesheet.modified", ">=", CarbonImmutable::createFromTimestamp($modifiedAfter)->format(APIData::DATE_FORMAT)))
+            ->when($modifiedAfter !== null, fn ($query) => $query->where($this->modified("timesheet"), ">=", $this->cutoff($modifiedAfter)))
             ->when($ids !== null, fn ($query) => $query->whereIn("timesheet.id", $ids))
             ->when($projectIds !== null, fn ($query) => $query->whereIn("ticket.projectId", $projectIds))
             ->orderBy("timesheet.id", "ASC")
@@ -86,10 +86,10 @@ public function getWorkers(int $startId, int $limit, ?int $modifiedAfter = null,
             // CONCAT_WS skips a missing name part, so a worker with only a
             // firstname keeps a usable name. NULLIF turns an all-blank name into
             // null rather than a string of whitespace.
-            ->select(["worker.id", "worker.username", DB::raw("NULLIF(TRIM(CONCAT_WS(' ', worker.firstname, worker.lastname)), '') as name")])
+            ->select(["worker.id", "worker.username", DB::raw("NULLIF(TRIM(CONCAT_WS(' ', worker.firstname, worker.lastname)), '') as name"), $this->modifiedSelect("worker")])
             ->where("worker.id", ">=", $startId)
             ->where("worker.source", "<>", "api")
-            ->when($modifiedAfter !== null, fn ($query) => $query->where("worker.modified", ">=", CarbonImmutable::createFromTimestamp($modifiedAfter)->format(APIData::DATE_FORMAT)))
+            ->when($modifiedAfter !== null, fn ($query) => $query->where($this->modified("worker"), ">=", $this->cutoff($modifiedAfter)))
             ->when($ids !== null, fn ($query) => $query->whereIn("worker.id", $ids))
             ->orderBy("worker.id", "ASC")
             ->limit($limit)
@@ -111,8 +111,34 @@ public function getDeleted(string $type, ?int $deletedAfter = null): array
             ->select(["entryId", "dateDeleted"])
             ->when($type === APIData::TYPE_MILESTONES, fn ($query) => $query->where('type', '=', 'milestone'))
             ->when($type === APIData::TYPE_TICKETS, fn ($query) => $query->where('type', '<>', 'milestone'))
-            ->when($deletedAfter !== null, fn ($query) => $query->where("entry.dateDeleted", ">=", CarbonImmutable::createFromTimestamp($deletedAfter)->format(APIData::DATE_FORMAT)))
+            ->when($deletedAfter !== null, fn ($query) => $query->where("entry.dateDeleted", ">=", $this->cutoff($deletedAfter)))
             ->get()
             ->toArray();
     }
+
+    /**
+     * The plugin-owned timestamp column, qualified by the query's table alias.
+     * Core's own `modified` is not maintained on every write path, so it cannot
+     * carry the modifiedAfter contract — see SchemaRepository.
+     */
+    private function modified(string $alias): string
+    {
+        return sprintf('%s.%s', $alias, SchemaRepository::COLUMN);
+    }
+
+    /**
+     * Exposed to consumers as plain `modified`, so the column swap is invisible
+     * to them and to the mapping in APIData.
+     */
+    private function modifiedSelect(string $alias): string
+    {
+        return sprintf('%s as modified', $this->modified($alias));
+    }
+
+    private function cutoff(int $timestamp): string
+    {
+        // Explicit UTC: Carbon 3 defaults to it, but Carbon comes from the host
+        // Leantime install, and the triggers write UTC_TIMESTAMP().
+        return CarbonImmutable::createFromTimestamp($timestamp, 'UTC')->format(APIData::DATE_FORMAT);
+    }
 }