From 2998f43e6dfe50b2303e5b55cd17d89f1a8b7568 Mon Sep 17 00:00:00 2001
From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com>
Date: Wed, 29 Jul 2026 15:43:03 +0200
Subject: [PATCH 1/6] Entries referencing deleted users or tickets no longer
fail the whole request. Added userId to timesheets and stopped resolving
ticket status against the session project when a ticket has no project.
Added PHPUnit setup and a Taskfile for running tests.
---
.claude/settings.json | 9 ++
.github/workflows/pr.yml | 22 ++++
.gitignore | 2 +
CHANGELOG.md | 6 +
Model/DeletedData.php | 4 +-
Model/MilestoneData.php | 4 +-
Model/ProjectData.php | 2 +-
Model/TicketData.php | 4 +-
Model/TimesheetData.php | 9 +-
Repositories/ApiDataRepository.php | 2 +-
Services/APIData.php | 29 +++--
Taskfile.yml | 62 +++++++++
bin/release-exclude.txt | 5 +
compose.yml | 1 +
composer.json | 13 ++
phpunit.xml.dist | 21 +++
tests/Model/DeletedDataTest.php | 25 ++++
tests/Model/MilestoneDataTest.php | 25 ++++
tests/Model/ProjectDataTest.php | 22 ++++
tests/Model/TicketDataTest.php | 45 +++++++
tests/Model/TimesheetDataTest.php | 97 ++++++++++++++
tests/Service/APIDataTest.php | 197 +++++++++++++++++++++++++++++
tests/Stub/LeantimeTickets.php | 40 ++++++
23 files changed, 624 insertions(+), 22 deletions(-)
create mode 100644 .claude/settings.json
create mode 100644 Taskfile.yml
create mode 100644 phpunit.xml.dist
create mode 100644 tests/Model/DeletedDataTest.php
create mode 100644 tests/Model/MilestoneDataTest.php
create mode 100644 tests/Model/ProjectDataTest.php
create mode 100644 tests/Model/TicketDataTest.php
create mode 100644 tests/Model/TimesheetDataTest.php
create mode 100644 tests/Service/APIDataTest.php
create mode 100644 tests/Stub/LeantimeTickets.php
diff --git a/.claude/settings.json b/.claude/settings.json
new file mode 100644
index 0000000..56fc68a
--- /dev/null
+++ b/.claude/settings.json
@@ -0,0 +1,9 @@
+{
+ "permissions": {
+ "allow": [
+ "Bash(task test:*)",
+ "Bash(task composer:*)",
+ "Bash(task lint:*)"
+ ]
+ }
+}
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
index 4ed1f3c..a32163b 100644
--- a/.github/workflows/pr.yml
+++ b/.github/workflows/pr.yml
@@ -15,3 +15,25 @@ jobs:
- name: Check that changelog has been updated.
run: git diff --exit-code origin/${{ github.base_ref }} -- CHANGELOG.md && exit 1 || exit 0
+
+ test:
+ runs-on: ubuntu-latest
+ name: Unit tests
+ strategy:
+ fail-fast: false
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+
+ # Matches the PHP version in Dockerfile (itkdev/php8.3-fpm).
+ - name: Setup PHP
+ uses: shivammathur/setup-php@v2
+ with:
+ php-version: '8.3'
+ coverage: none
+
+ - name: Install dependencies
+ run: composer install --no-interaction --prefer-dist --no-progress
+
+ - name: Run tests
+ run: vendor/bin/phpunit
diff --git a/.gitignore b/.gitignore
index e0d067c..a3fd9f3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,3 +7,5 @@ composer.lock
release/
checksum.txt
*.tar.gz
+
+economics
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0bfa34e..e6c8054 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
## [Unreleased]
+* [PR-14](https://github.com/ITK-Leantime/data-api/pull/14)
+ * 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.
+ * Stopped resolving ticket status against the session's project when a ticket has no project.
+ * Added PHPUnit test setup and a Taskfile for running it.
+
## [0.1.2] - 2026-03-06
* [PR-12](https://github.com/ITK-Leantime/data-api/pull/12)
diff --git a/Model/DeletedData.php b/Model/DeletedData.php
index 74e379c..9433a0c 100644
--- a/Model/DeletedData.php
+++ b/Model/DeletedData.php
@@ -7,7 +7,7 @@
class DeletedData
{
public function __construct(
- public int $id,
- public CarbonInterface $deletedDate,
+ public ?int $id,
+ public ?CarbonInterface $deletedDate,
) {}
}
diff --git a/Model/MilestoneData.php b/Model/MilestoneData.php
index 0a06bd2..f222b7c 100644
--- a/Model/MilestoneData.php
+++ b/Model/MilestoneData.php
@@ -8,8 +8,8 @@
{
public function __construct(
public int $id,
- public int $projectId,
- public string $name,
+ public ?int $projectId,
+ public ?string $name,
public ?CarbonInterface $modified,
) {}
}
diff --git a/Model/ProjectData.php b/Model/ProjectData.php
index 0c07e16..388ada8 100644
--- a/Model/ProjectData.php
+++ b/Model/ProjectData.php
@@ -8,7 +8,7 @@
{
public function __construct(
public int $id,
- public string $name,
+ public ?string $name,
public ?CarbonInterface $modified,
) {}
}
diff --git a/Model/TicketData.php b/Model/TicketData.php
index 8826a6a..6ff561f 100644
--- a/Model/TicketData.php
+++ b/Model/TicketData.php
@@ -8,8 +8,8 @@
{
public function __construct(
public int $id,
- public int $projectId,
- public string $name,
+ public ?int $projectId,
+ public ?string $name,
public ?string $status,
public ?int $milestoneId,
public array $tags,
diff --git a/Model/TimesheetData.php b/Model/TimesheetData.php
index b14c145..cd726b5 100644
--- a/Model/TimesheetData.php
+++ b/Model/TimesheetData.php
@@ -8,13 +8,14 @@
{
public function __construct(
public int $id,
- public int $ticketId,
- public int $projectId,
+ public ?int $ticketId,
+ public ?int $projectId,
public ?string $description,
public float $hours,
- public string $username,
+ public ?int $userId,
+ public ?string $username,
+ public ?string $kind,
public ?CarbonInterface $workDate = null,
public ?CarbonInterface $modified = null,
- public string $kind,
) {}
}
diff --git a/Repositories/ApiDataRepository.php b/Repositories/ApiDataRepository.php
index f5cc80a..e6460db 100644
--- a/Repositories/ApiDataRepository.php
+++ b/Repositories/ApiDataRepository.php
@@ -65,7 +65,7 @@ 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.kind", "user.username", "ticket.projectId"])
+ ->select(["timesheet.id", "timesheet.description", "timesheet.hours", "timesheet.workDate", "timesheet.modified", "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")
diff --git a/Services/APIData.php b/Services/APIData.php
index 8929f87..af28091 100644
--- a/Services/APIData.php
+++ b/Services/APIData.php
@@ -144,7 +144,12 @@ public function getTickets(int $startId, int $limit, int $modifiedAfter = null,
$values = $this->apiDataRepository->getTickets($startId, $limit, $modifiedAfter, $ids, $projectIds);
return array_map(function ($value) {
- $projectStatuses = $this->ticketRepository->getStateLabels($value->projectId);
+ // Asked for labels without a project id, Leantime falls back to
+ // session('currentProject'), which would resolve the status against
+ // an unrelated project.
+ $projectStatuses = $value->projectId !== null
+ ? $this->ticketRepository->getStateLabels($value->projectId)
+ : [];
return new TicketData(
$value->id,
@@ -168,16 +173,20 @@ public function getTimesheets(int $startId, int $limit, ?int $modifiedAfter = nu
$values = $this->apiDataRepository->getTimesheets($startId, $limit, $modifiedAfter, $ids, $projectIds);
return array_map(function ($value) {
+ // Named arguments: CarbonImmutable has a __toString(), so a
+ // mis-ordered date would be coerced into one of the string
+ // parameters instead of raising a TypeError.
return new TimesheetData(
- $value->id,
- $value->ticketId,
- $value->projectId,
- $value->description,
- $value->hours,
- $value->username,
- $this->getCarbonFromDatabaseValue($value->workDate),
- $this->getCarbonFromDatabaseValue($value->modified),
- $value->kind,
+ id: $value->id,
+ ticketId: $value->ticketId,
+ projectId: $value->projectId,
+ description: $value->description,
+ hours: $value->hours,
+ userId: $value->userId,
+ username: $value->username,
+ kind: $value->kind,
+ workDate: $this->getCarbonFromDatabaseValue($value->workDate),
+ modified: $this->getCarbonFromDatabaseValue($value->modified),
);
}, $values);
}
diff --git a/Taskfile.yml b/Taskfile.yml
new file mode 100644
index 0000000..4601cfe
--- /dev/null
+++ b/Taskfile.yml
@@ -0,0 +1,62 @@
+# https://taskfile.dev — install with `brew install go-task` (or see docs).
+# Run `task` (or `task --list-all`) to see all available commands.
+#
+# This plugin has no long-running stack, so everything runs in a one-off
+# container built from the local Dockerfile (itkdev/php8.3-fpm).
+
+version: "3"
+
+vars:
+ # https://taskfile.dev/reference/templating/
+ DOCKER_COMPOSE: '{{ .TASK_DOCKER_COMPOSE | default "docker compose" }}'
+
+tasks:
+ default:
+ desc: List all tasks
+ cmds:
+ - task --list-all
+ silent: true
+
+ # -------------------------------------------------------------- Wrappers ---
+
+ compose:
+ desc: "Run a docker compose command. Example: task compose -- build php."
+ cmds:
+ - "{{ .DOCKER_COMPOSE }} {{ .CLI_ARGS }}"
+
+ php:
+ desc: "Run a command in a one-off php container. Example: task php -- php --version."
+ cmds:
+ - task compose -- run --rm --no-deps php {{ .CLI_ARGS }}
+ silent: true
+
+ composer:
+ desc: "Run a composer command. Example: task composer -- install."
+ cmds:
+ - task php -- composer {{ .CLI_ARGS }}
+ silent: true
+
+ # ------------------------------------------------------------ Lifecycle ---
+
+ setup:
+ desc: Build the php image and install dev dependencies.
+ cmds:
+ - task compose -- build php
+ - task composer -- install
+
+ # ------------------------------------------------------------ PHP tests ---
+
+ test:
+ desc: "Run the PHP test suite. Example: task test -- --filter TimesheetData."
+ cmds:
+ - task php -- vendor/bin/phpunit {{ .CLI_ARGS }}
+ silent: true
+
+ lint:
+ desc: Syntax-check all PHP files.
+ cmds:
+ - >-
+ task php -- sh -c
+ 'find Controllers Model Repositories Services -name "*.php" -print0
+ | xargs -0 -n1 -- php -l'
+ silent: true
diff --git a/bin/release-exclude.txt b/bin/release-exclude.txt
index 3331c0c..0e9974d 100755
--- a/bin/release-exclude.txt
+++ b/bin/release-exclude.txt
@@ -1,6 +1,7 @@
*.tar.gz
.git*
.php-cs-fixer.dist.php
+.phpunit.cache
.twig-cs-fixer.dist.php
bin
checksum.txt
@@ -8,3 +9,7 @@ compose.yaml
composer.lock
vendor
Dockerfile
+phpunit.xml
+phpunit.xml.dist
+Taskfile.yml
+tests
diff --git a/compose.yml b/compose.yml
index b40cbb9..908a2c5 100644
--- a/compose.yml
+++ b/compose.yml
@@ -1,5 +1,6 @@
services:
php:
build: .
+ working_dir: /app
volumes:
- .:/app
diff --git a/composer.json b/composer.json
index bb27426..f1e60a9 100644
--- a/composer.json
+++ b/composer.json
@@ -12,9 +12,22 @@
],
"homepage": "https://github.com/ITK-Leantime/leantime-data-api",
"require-dev": {
+ "illuminate/database": "^11.0",
+ "nesbot/carbon": "^2.72.2 || ^3.0",
+ "phpunit/phpunit": "^11.0"
},
"config": {
},
+ "autoload-dev": {
+ "psr-4": {
+ "Leantime\\Plugins\\APIData\\": "",
+ "Leantime\\Plugins\\APIData\\Tests\\": "tests/"
+ },
+ "classmap": [
+ "tests/Stub/"
+ ]
+ },
"scripts": {
+ "test": "phpunit"
}
}
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
new file mode 100644
index 0000000..2a306e4
--- /dev/null
+++ b/phpunit.xml.dist
@@ -0,0 +1,21 @@
+
+
+
+
+ tests
+ tests/Stub
+
+
+
+
+ Model
+ Services
+
+
+
diff --git a/tests/Model/DeletedDataTest.php b/tests/Model/DeletedDataTest.php
new file mode 100644
index 0000000..26e42d2
--- /dev/null
+++ b/tests/Model/DeletedDataTest.php
@@ -0,0 +1,25 @@
+assertNull($deleted->id);
+ $this->assertNull($deleted->deletedDate);
+ }
+}
diff --git a/tests/Model/MilestoneDataTest.php b/tests/Model/MilestoneDataTest.php
new file mode 100644
index 0000000..3ebb4ac
--- /dev/null
+++ b/tests/Model/MilestoneDataTest.php
@@ -0,0 +1,25 @@
+assertSame(5, $milestone->id);
+ $this->assertNull($milestone->projectId);
+ $this->assertNull($milestone->name);
+ }
+}
diff --git a/tests/Model/ProjectDataTest.php b/tests/Model/ProjectDataTest.php
new file mode 100644
index 0000000..859a12d
--- /dev/null
+++ b/tests/Model/ProjectDataTest.php
@@ -0,0 +1,22 @@
+assertSame(92, $project->id);
+ $this->assertNull($project->name);
+ }
+}
diff --git a/tests/Model/TicketDataTest.php b/tests/Model/TicketDataTest.php
new file mode 100644
index 0000000..069fd5b
--- /dev/null
+++ b/tests/Model/TicketDataTest.php
@@ -0,0 +1,45 @@
+makeTicket(['projectId' => null, 'name' => null]);
+
+ $this->assertNull($ticket->projectId);
+ $this->assertNull($ticket->name);
+ }
+
+ /**
+ * @param array $overrides
+ */
+ private function makeTicket(array $overrides = []): TicketData
+ {
+ return new TicketData(...[
+ 'id' => 4711,
+ 'projectId' => 92,
+ 'name' => 'Fix the thing',
+ 'status' => 'NEW',
+ 'milestoneId' => null,
+ 'tags' => [],
+ 'worker' => 'anne@aarhus.dk',
+ 'plannedHours' => null,
+ 'remainingHours' => null,
+ 'dueDate' => null,
+ 'resolutionDate' => null,
+ 'modified' => null,
+ ...$overrides,
+ ]);
+ }
+}
diff --git a/tests/Model/TimesheetDataTest.php b/tests/Model/TimesheetDataTest.php
new file mode 100644
index 0000000..12ac0c9
--- /dev/null
+++ b/tests/Model/TimesheetDataTest.php
@@ -0,0 +1,97 @@
+makeTimesheet(['username' => null]);
+
+ $this->assertNull($timesheet->username);
+ $this->assertSame(7.5, $timesheet->hours);
+ }
+
+ /**
+ * `ticket.projectId` comes from the other left join, so a timesheet whose
+ * ticket was deleted has neither a ticket id nor a project id.
+ */
+ public function testAcceptsNullTicketIdAndProjectIdWhenTheTicketWasDeleted(): void
+ {
+ $timesheet = $this->makeTimesheet(['ticketId' => null, 'projectId' => null]);
+
+ $this->assertNull($timesheet->ticketId);
+ $this->assertNull($timesheet->projectId);
+ }
+
+ public function testAcceptsNullKind(): void
+ {
+ $timesheet = $this->makeTimesheet(['kind' => null]);
+
+ $this->assertNull($timesheet->kind);
+ }
+
+ /**
+ * Hours logged by a deleted user stay attributable through the user id, so
+ * a consumer can group them per departed worker instead of collapsing them
+ * into one anonymous pile.
+ */
+ public function testExposesUserIdSoOrphanedHoursStayAttributable(): void
+ {
+ $timesheet = $this->makeTimesheet(['userId' => 57, 'username' => null]);
+
+ $this->assertSame(57, $timesheet->userId);
+ $this->assertNull($timesheet->username);
+ }
+
+ /**
+ * `$kind` has to move ahead of the optional date parameters, since a
+ * required parameter after an optional one is deprecated. CarbonImmutable
+ * implements __toString() and this codebase does not use strict_types, so a
+ * date landing in a string parameter would be coerced silently instead of
+ * raising a TypeError. Pin the declarations so a swap cannot go unnoticed.
+ */
+ public function testKindHoldsAStatusStringAndDatesHoldCarbonInstances(): void
+ {
+ $timesheet = $this->makeTimesheet([
+ 'kind' => 'GENERAL_BILLABLE',
+ 'workDate' => CarbonImmutable::parse('2026-03-02 09:00:00'),
+ ]);
+
+ $this->assertSame('GENERAL_BILLABLE', $timesheet->kind);
+ $this->assertInstanceOf(CarbonInterface::class, $timesheet->workDate);
+ $this->assertSame('2026-03-02 09:00:00', $timesheet->workDate->format('Y-m-d H:i:s'));
+ }
+
+ /**
+ * @param array $overrides
+ */
+ private function makeTimesheet(array $overrides = []): TimesheetData
+ {
+ return new TimesheetData(...[
+ 'id' => 1,
+ 'ticketId' => 2,
+ 'projectId' => 3,
+ 'description' => 'Worked on stuff',
+ 'hours' => 7.5,
+ 'userId' => 57,
+ 'username' => 'anne@aarhus.dk',
+ 'kind' => 'GENERAL_BILLABLE',
+ 'workDate' => null,
+ 'modified' => null,
+ ...$overrides,
+ ]);
+ }
+}
diff --git a/tests/Service/APIDataTest.php b/tests/Service/APIDataTest.php
new file mode 100644
index 0000000..6d1435d
--- /dev/null
+++ b/tests/Service/APIDataTest.php
@@ -0,0 +1,197 @@
+makeServiceReturningTimesheets([
+ $this->timesheetRow(['username' => null, 'userId' => 57]),
+ ]);
+
+ $timesheets = $service->getTimesheets(0, 100);
+
+ $this->assertCount(1, $timesheets);
+ $this->assertNull($timesheets[0]->username);
+ $this->assertSame(57, $timesheets[0]->userId);
+ $this->assertSame(7.5, $timesheets[0]->hours);
+ }
+
+ /**
+ * Guards the constructor argument order. `$kind` moved ahead of the two
+ * optional date parameters, and CarbonImmutable implements __toString(), so
+ * a mis-ordered date would land in `$kind` as a coerced string instead of
+ * raising a TypeError. Only asserting each field individually catches that.
+ */
+ public function testGetTimesheetsMapsEveryFieldToItsOwnProperty(): void
+ {
+ $service = $this->makeServiceReturningTimesheets([$this->timesheetRow()]);
+
+ $timesheet = $service->getTimesheets(0, 100)[0];
+
+ $this->assertSame(4711, $timesheet->id);
+ $this->assertSame(12, $timesheet->ticketId);
+ $this->assertSame(92, $timesheet->projectId);
+ $this->assertSame('Worked on stuff', $timesheet->description);
+ $this->assertSame(7.5, $timesheet->hours);
+ $this->assertSame(57, $timesheet->userId);
+ $this->assertSame('anne@aarhus.dk', $timesheet->username);
+ $this->assertSame('GENERAL_BILLABLE', $timesheet->kind);
+
+ $this->assertInstanceOf(CarbonInterface::class, $timesheet->workDate);
+ $this->assertSame('2026-03-02 09:00:00', $timesheet->workDate->format('Y-m-d H:i:s'));
+ $this->assertSame('UTC', $timesheet->workDate->timezoneName);
+
+ $this->assertInstanceOf(CarbonInterface::class, $timesheet->modified);
+ $this->assertSame('2026-03-03 11:30:00', $timesheet->modified->format('Y-m-d H:i:s'));
+ }
+
+ /**
+ * Leantime stores "no date" as the zero date rather than null. Carbon throws
+ * on a format mismatch, so this guard has to stay in place.
+ */
+ public function testGetTimesheetsTreatsTheZeroDateSentinelAsNull(): void
+ {
+ $service = $this->makeServiceReturningTimesheets([
+ $this->timesheetRow(['workDate' => '0000-00-00 00:00:00', 'modified' => null]),
+ ]);
+
+ $timesheet = $service->getTimesheets(0, 100)[0];
+
+ $this->assertNull($timesheet->workDate);
+ $this->assertNull($timesheet->modified);
+ }
+
+ /**
+ * Leantime's `getStateLabels()` falls back to `session('currentProject')`
+ * when it is handed a null project id, so asking it for labels would return
+ * some unrelated project's status list and put a wrong status on the ticket.
+ * A ticket without a project has no status to resolve.
+ */
+ public function testGetTicketsDoesNotLookUpStatusLabelsForATicketWithoutAProject(): void
+ {
+ $ticketRepository = $this->createMock(TicketRepository::class);
+ $ticketRepository->expects($this->never())->method('getStateLabels');
+
+ $repository = $this->createMock(ApiDataRepository::class);
+ $repository->method('getTickets')->willReturn([$this->ticketRow(['projectId' => null])]);
+
+ $tickets = (new APIData($ticketRepository, $repository))->getTickets(0, 100);
+
+ $this->assertNull($tickets[0]->projectId);
+ $this->assertNull($tickets[0]->status);
+ }
+
+ /**
+ * The normal path still has to resolve the status against the ticket's own
+ * project.
+ */
+ public function testGetTicketsResolvesStatusFromTheProjectsOwnLabels(): void
+ {
+ $ticketRepository = $this->createMock(TicketRepository::class);
+ $ticketRepository->expects($this->once())
+ ->method('getStateLabels')
+ ->with(92)
+ ->willReturn($this->stateLabels());
+
+ $repository = $this->createMock(ApiDataRepository::class);
+ $repository->method('getTickets')->willReturn([
+ $this->ticketRow(['projectId' => 92, 'status' => 3]),
+ ]);
+
+ $tickets = (new APIData($ticketRepository, $repository))->getTickets(0, 100);
+
+ $this->assertSame(92, $tickets[0]->projectId);
+ $this->assertSame('NEW', $tickets[0]->status);
+ }
+
+ /**
+ * @param list