From 30f4cd33cca1744e1f35a24767dce9b2f630c25f Mon Sep 17 00:00:00 2001 From: Michael Philipp Date: Wed, 30 Sep 2026 14:20:22 +0200 Subject: [PATCH 1/4] Enable access to roles (Endpoint roles) $client->role() If you create a user you can specify role_ids but there's no way to get those ids. --- src/Core/Repository/RepositoryRegistry.php | 3 ++ src/Core/Traits/RepositoryAccessors.php | 6 ++++ src/Endpoints/Roles/RoleDTO.php | 39 ++++++++++++++++++++++ src/Endpoints/Roles/RoleRepository.php | 21 ++++++++++++ 4 files changed, 69 insertions(+) create mode 100644 src/Endpoints/Roles/RoleDTO.php create mode 100644 src/Endpoints/Roles/RoleRepository.php diff --git a/src/Core/Repository/RepositoryRegistry.php b/src/Core/Repository/RepositoryRegistry.php index e841221..5e357c6 100644 --- a/src/Core/Repository/RepositoryRegistry.php +++ b/src/Core/Repository/RepositoryRegistry.php @@ -10,6 +10,8 @@ use ZammadAPIClient\Endpoints\Groups\GroupRepository; use ZammadAPIClient\Endpoints\Links\LinkDTO; use ZammadAPIClient\Endpoints\Links\LinkRepository; +use ZammadAPIClient\Endpoints\Roles\RoleDTO; +use ZammadAPIClient\Endpoints\Roles\RoleRepository; use ZammadAPIClient\Endpoints\Tags\TagDTO; use ZammadAPIClient\Endpoints\Tags\TagRepository; use ZammadAPIClient\Endpoints\TextModules\TextModuleDTO; @@ -45,6 +47,7 @@ final class RepositoryRegistry OrganizationRepository::class => ['path' => 'organizations', 'dto' => OrganizationDTO::class], GroupRepository::class => ['path' => 'groups', 'dto' => GroupDTO::class], LinkRepository::class => ['path' => 'links', 'dto' => LinkDTO::class], + RoleRepository::class => ['path' => 'roles', 'dto' => RoleDTO::class], TicketArticleRepository::class => ['path' => 'ticket_articles', 'dto' => TicketArticleDTO::class], TicketStateRepository::class => ['path' => 'ticket_states', 'dto' => TicketStateDTO::class], TicketPriorityRepository::class => ['path' => 'ticket_priorities', 'dto' => TicketPriorityDTO::class], diff --git a/src/Core/Traits/RepositoryAccessors.php b/src/Core/Traits/RepositoryAccessors.php index e6fe3c1..3137dc3 100644 --- a/src/Core/Traits/RepositoryAccessors.php +++ b/src/Core/Traits/RepositoryAccessors.php @@ -8,6 +8,7 @@ use ZammadAPIClient\Endpoints\Groups\GroupRepository; use ZammadAPIClient\Endpoints\Links\LinkRepository; use ZammadAPIClient\Endpoints\Organizations\OrganizationRepository; +use ZammadAPIClient\Endpoints\Roles\RoleRepository; use ZammadAPIClient\Endpoints\Tags\TagRepository; use ZammadAPIClient\Endpoints\TextModules\TextModuleRepository; use ZammadAPIClient\Endpoints\TicketArticles\TicketArticleRepository; @@ -43,6 +44,11 @@ public function group(): GroupRepository return $this->repo(GroupRepository::class); } + public function role(): RoleRepository + { + return $this->repo(RoleRepository::class); + } + public function ticketArticle(): TicketArticleRepository { return $this->repo(TicketArticleRepository::class); diff --git a/src/Endpoints/Roles/RoleDTO.php b/src/Endpoints/Roles/RoleDTO.php new file mode 100644 index 0000000..cbca30f --- /dev/null +++ b/src/Endpoints/Roles/RoleDTO.php @@ -0,0 +1,39 @@ + + */ +final class RoleRepository extends AbstractRepository +{ +} From 4fd4280f61dea762fa10455cfcf40bc8ecb2dc54 Mon Sep 17 00:00:00 2001 From: Michael Philipp Date: Wed, 30 Sep 2026 14:31:26 +0200 Subject: [PATCH 2/4] reduced comments | was copy & paste from TicketPriorities --- src/Endpoints/Roles/RoleDTO.php | 13 +------------ src/Endpoints/Roles/RoleRepository.php | 9 +-------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/Endpoints/Roles/RoleDTO.php b/src/Endpoints/Roles/RoleDTO.php index cbca30f..c1415af 100644 --- a/src/Endpoints/Roles/RoleDTO.php +++ b/src/Endpoints/Roles/RoleDTO.php @@ -10,18 +10,7 @@ use ZammadAPIClient\Core\Traits\SerializesToArray; /** - * Represents a Zammad ticket priority resource (`/api/v1/ticket_priorities`). - * - * Ticket priorities classify the urgency level of a ticket (e.g. "1 low", - * "2 normal", "3 high"). The default Zammad installation ships with three - * priorities; administrators can create additional ones via the API or UI. - * - * The `name` field is the display label; Zammad uses the numeric `id` when - * assigning a priority to a ticket via the `priority_id` field on - * {@see \ZammadAPIClient\Endpoints\Tickets\TicketDTO}. - * - * Timestamp fields (`created_at`, `updated_at`) are provided by - * {@see \ZammadAPIClient\Core\Traits\HasTimestamps}. + * Represents a Zammad role resource (`/api/v1/roles`). */ final class RoleDTO implements DTOInterface { diff --git a/src/Endpoints/Roles/RoleRepository.php b/src/Endpoints/Roles/RoleRepository.php index 2ec104b..4aa2377 100644 --- a/src/Endpoints/Roles/RoleRepository.php +++ b/src/Endpoints/Roles/RoleRepository.php @@ -7,14 +7,7 @@ use ZammadAPIClient\Core\Repository\AbstractRepository; /** - * Repository for the `/api/v1/ticket_priorities` endpoint. - * - * Ticket priorities classify the urgency of a ticket (e.g. "low", "normal", "high"). - * Like states, priorities are system-configured in Zammad. This repository - * provides full CRUD access for retrieving available priorities (to populate - * dropdowns) and for managing custom priority levels. - * - * @extends AbstractRepository + * Repository for the `/api/v1/roles` endpoint. */ final class RoleRepository extends AbstractRepository { From 8bc283d227ad6e9987640b81d6abccdf3fff5579 Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Fri, 2 Oct 2026 16:19:18 +0200 Subject: [PATCH 3/4] Add tests and documentation for the roles endpoint Covers the roles support added in PR #167 with unit and integration tests and documents the endpoint: - RoleRepositoryTest: all(), find(), create(), patch(), search() and the BadMethodCallException from the unsupported delete() - RoleDTO added to the shared DTO provider, role accessor to the repository accessor provider - RoleIntegrationTest: listing, default roles, find(), pagination and the motivating use case (resolve a role name to the id used in role_ids) - Restore the house-style docblocks on RoleDTO/RoleRepository, including the @extends annotation phpstan needs - README: role accessor, Roles section, RoleDTO reference, delete table entry, admin permission note for integration tests; CHANGELOG and migration examples Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 3 +- README.md | 42 +++++- docs/migration-v3-examples.md | 1 + src/Endpoints/Roles/RoleDTO.php | 10 ++ src/Endpoints/Roles/RoleRepository.php | 28 ++++ test/Integration/RoleIntegrationTest.php | 138 ++++++++++++++++++ test/Unit/DTOs/DTOTest.php | 5 + test/Unit/Repositories/RoleRepositoryTest.php | 125 ++++++++++++++++ test/Unit/RepositoryAccessorsTest.php | 3 + 9 files changed, 352 insertions(+), 3 deletions(-) create mode 100644 test/Integration/RoleIntegrationTest.php create mode 100644 test/Unit/Repositories/RoleRepositoryTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e4ca0c..6458262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,9 @@ ## [3.0.0] — unreleased ### Added +- `RoleRepository` / `RoleDTO` for `/api/v1/roles`, accessible via `$client->role()` — resolve role names to the IDs needed for `UserDTO::$role_ids` - PSR-18 / PSR-17 compliant HTTP layer (`RequestHandler`, `RetryAfterMiddleware`) -- Typed DTOs for all 10 resources (`Ticket`, `User`, `Organization`, `Group`, `TicketArticle`, `TicketState`, `TicketPriority`, `Tag`, `TextModule`, `Link`) +- Typed DTOs for all 11 resources (`Ticket`, `User`, `Organization`, `Group`, `Role`, `TicketArticle`, `TicketState`, `TicketPriority`, `Tag`, `TextModule`, `Link`) - Repository pattern with generator-based pagination (`AbstractRepository`) - `patch()` method for partial updates via `array` or `TicketUpdateDTO` - Proper exception hierarchy: `AuthenticationException`, `NotFoundException`, `ValidationException`, `RateLimitException`, `ServerErrorException`, `NetworkException` diff --git a/README.md b/README.md index ad6cf1b..36df944 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ This library offers three interaction styles. Choose based on your use case: | **Stateful Resource** | `$client->ticket()->resource($id)->save()` / `destroy()` | Interactive editing — mutate properties step by step, only changes are sent. | | **Raw HTTP** | `$client->getHandler()->get()`, `delete()`, etc. | Calling endpoints that have no dedicated repository. Escape hatch. | -Repositories are accessed via typed accessors: `$client->ticket()`, `$client->user()`, `$client->organization()`, `$client->group()`, `$client->ticketArticle()`, `$client->ticketState()`, `$client->ticketPriority()`, `$client->tag()`, `$client->textModule()`, `$client->link()`. The underlying `repo()` method is internal. +Repositories are accessed via typed accessors: `$client->ticket()`, `$client->user()`, `$client->organization()`, `$client->group()`, `$client->role()`, `$client->ticketArticle()`, `$client->ticketState()`, `$client->ticketPriority()`, `$client->tag()`, `$client->textModule()`, `$client->link()`. The underlying `repo()` method is internal. ### Connecting @@ -215,6 +215,7 @@ All repositories expose a `delete()` method. Repositories implementing `Deletabl | `TicketArticleRepository` | exception | Zammad API does not allow article deletion | | `TicketStateRepository` | exception | System resource, read-only | | `TicketPriorityRepository` | exception | System resource, read-only | +| `RoleRepository` | exception | The API does not allow deleting roles; patch `active` to `false` | ```php $client->ticket()->delete(1); @@ -294,6 +295,29 @@ foreach ($repo->all(['object' => 'Ticket', 'o_id' => $ticketId]) as $tag) { $results = $repo->tagSearch('urg'); // Autocomplete ``` +### Roles + +Roles bundle permissions. A user carries them via `role_ids`, so the usual task is +resolving a role name to its numeric ID before creating or updating a user: + +```php +$roles = []; +foreach ($client->role()->all() as $role) { + $roles[$role->name] = $role->id; // 'Admin' => 1, 'Agent' => 2, 'Customer' => 3 +} + +$client->user()->create(new UserDTO( + email: 'agent@example.com', + firstname: 'New', + lastname: 'Agent', + role_ids: [$roles['Agent']], +)); +``` + +Reading roles requires a token with admin permissions — `/api/v1/roles` is admin-only +and otherwise answers with `ForbiddenException`. Roles cannot be deleted through the +API; deactivate one with `$client->role()->patch($id, ['active' => false])` instead. + ### CSV import ```php @@ -530,7 +554,7 @@ $client->ticket()->patch(42, new TicketUpdateDTO( | `phone` | `?string` | — | | | `organization_id` | `?int` | — | Primary organization | | `organization_ids` | `?array` | — | Array of secondary organization IDs | -| `role_ids` | `?array` | — | Array of role IDs (e.g. `[2]` for Agent) | +| `role_ids` | `?array` | — | Array of role IDs (e.g. `[2]` for Agent); resolve by name via `$client->role()` | | `active` | `?bool` | — | Whether the user account is active | | `id` | `?int` | — | Server-assigned | | `created_at` | `?DateTimeImmutable` | — | Read-only | @@ -561,6 +585,17 @@ $client->ticket()->patch(42, new TicketUpdateDTO( | `updated_at` | `?DateTimeImmutable` | — | Read-only | | `customFields` | `array` | — | | +### RoleDTO + +| Field | Type | Required | Notes | +|-------|------|:--------:|-------| +| `name` | `string` | yes | Display label (e.g. `'Agent'`, `'Customer'`) | +| `note` | `?string` | — | | +| `active` | `?bool` | — | | +| `id` | `?int` | — | Server-assigned; used in `UserDTO::$role_ids` | +| `created_at` | `?DateTimeImmutable` | — | Read-only | +| `updated_at` | `?DateTimeImmutable` | — | Read-only | + ### TicketArticleDTO | Field | Type | Notes | @@ -689,6 +724,9 @@ These require a running Zammad instance and authentication credentials: \* Either `ZAMMAD_PHP_API_CLIENT_UNIT_TESTS_TOKEN` or `USERNAME`+`PASSWORD` must be set. +The credentials need admin permissions: some suites touch admin-only endpoints +(e.g. `RoleIntegrationTest` reads `/api/v1/roles`). + ## Migration from v2 See [docs/migration-v3-examples.md](docs/migration-v3-examples.md) for side-by-side code examples. diff --git a/docs/migration-v3-examples.md b/docs/migration-v3-examples.md index f814a6a..02d55ef 100644 --- a/docs/migration-v3-examples.md +++ b/docs/migration-v3-examples.md @@ -365,6 +365,7 @@ $client->ticket()->find(1); $client->user()->find(1); $client->organization()->find(1); $client->group()->find(1); +$client->role()->all(); $client->ticketArticle()->getForTicket(1); $client->ticketState()->all(); $client->ticketPriority()->all(); diff --git a/src/Endpoints/Roles/RoleDTO.php b/src/Endpoints/Roles/RoleDTO.php index c1415af..35ff1b6 100644 --- a/src/Endpoints/Roles/RoleDTO.php +++ b/src/Endpoints/Roles/RoleDTO.php @@ -11,6 +11,16 @@ /** * Represents a Zammad role resource (`/api/v1/roles`). + * + * A role is a named bundle of permissions. The default Zammad installation + * ships with "Admin", "Agent" and "Customer"; administrators can add more. + * + * The `name` field is the display label; Zammad uses the numeric `id` when + * assigning roles to a user via the `role_ids` field on + * {@see \ZammadAPIClient\Endpoints\Users\UserDTO}. + * + * Timestamp fields (`created_at`, `updated_at`) are provided by + * {@see \ZammadAPIClient\Core\Traits\HasTimestamps}. */ final class RoleDTO implements DTOInterface { diff --git a/src/Endpoints/Roles/RoleRepository.php b/src/Endpoints/Roles/RoleRepository.php index 4aa2377..61fc6f8 100644 --- a/src/Endpoints/Roles/RoleRepository.php +++ b/src/Endpoints/Roles/RoleRepository.php @@ -8,6 +8,34 @@ /** * Repository for the `/api/v1/roles` endpoint. + * + * Roles bundle permissions in Zammad. Every user carries one or more roles via + * the `role_ids` field on {@see \ZammadAPIClient\Endpoints\Users\UserDTO}; the + * default installation ships with "Admin", "Agent" and "Customer". + * + * The typical use case is resolving a role name to its numeric ID before + * creating or updating a user: + * + * $roles = []; + * foreach ($client->role()->all() as $role) { + * $roles[$role->name] = $role->id; + * } + * + * $client->user()->create(new UserDTO( + * email: 'agent@example.com', + * role_ids: [$roles['Agent']], + * )); + * + * Listing and reading roles requires admin permissions (`admin.role` or + * `admin.user`); a token limited to agent permissions receives a 403 and the + * client raises {@see \ZammadAPIClient\Exceptions\ForbiddenException}. + * + * Roles cannot be removed through the API, so this repository does not + * implement {@see \ZammadAPIClient\Core\Contracts\DeletableInterface}: + * `delete()` throws a `BadMethodCallException`. Deactivate a role by patching + * `active` to `false` instead. + * + * @extends AbstractRepository */ final class RoleRepository extends AbstractRepository { diff --git a/test/Integration/RoleIntegrationTest.php b/test/Integration/RoleIntegrationTest.php new file mode 100644 index 0000000..00e0bdc --- /dev/null +++ b/test/Integration/RoleIntegrationTest.php @@ -0,0 +1,138 @@ +role()->all() as $role) { + self::assertInstanceOf(RoleDTO::class, $role); + self::assertGreaterThan(0, $role->id); + self::assertNotSame('', $role->name); + $count++; + } + + self::assertGreaterThan(0, $count, 'Should find at least one role'); + } + + /** + * A default Zammad installation ships with Admin, Agent and Customer. + */ + public function testDefaultRolesArePresent(): void + { + $names = []; + foreach (self::$client->role()->all() as $role) { + $names[] = $role->name; + } + + self::assertContains('Admin', $names); + self::assertContains('Agent', $names); + self::assertContains('Customer', $names); + } + + /** + * find() returns the same role that all() yielded. + */ + public function testFindRole(): void + { + $first = null; + foreach (self::$client->role()->all() as $role) { + $first = $role; + break; + } + + self::assertInstanceOf(RoleDTO::class, $first, 'Should find at least one role'); + + $found = self::$client->role()->find($first->id); + + self::assertSame($first->id, $found->id); + self::assertSame($first->name, $found->name); + } + + /** + * The motivating use case of the roles endpoint: resolve a role name to its + * ID and use it as `role_ids` when creating a user. + */ + public function testCreateUserWithResolvedRoleId(): void + { + $customerRoleId = null; + foreach (self::$client->role()->all() as $role) { + if ($role->name === 'Customer') { + $customerRoleId = $role->id; + break; + } + } + + self::assertNotNull($customerRoleId, 'Customer role should exist'); + + $email = 'role-it-' . uniqid('', true) . '@example.com'; + $user = self::$client->repo(UserRepository::class)->create(new UserDTO( + email: $email, + firstname: 'Role', + lastname: 'Test', + role_ids: [$customerRoleId], + )); + + try { + self::assertGreaterThan(0, $user->id); + self::assertIsArray($user->role_ids); + self::assertContains($customerRoleId, $user->role_ids); + } finally { + self::$client->repo(UserRepository::class)->delete($user->id); + } + } + + /** + * Roles are returned under the "roles" list key and paginate like any other + * resource — a page size of 1 must still yield every role exactly once. + */ + public function testPaginationWithSmallPageSize(): void + { + $repo = new RoleRepository( + self::$client->getHandler(), + 'roles', + RoleDTO::class, + 1, + ); + + $ids = []; + foreach ($repo->all() as $role) { + $ids[] = $role->id; + } + + self::assertGreaterThan(0, count($ids)); + self::assertSame(array_unique($ids), $ids, 'Pagination must not repeat roles'); + } +} diff --git a/test/Unit/DTOs/DTOTest.php b/test/Unit/DTOs/DTOTest.php index 506c34d..8718d8e 100644 --- a/test/Unit/DTOs/DTOTest.php +++ b/test/Unit/DTOs/DTOTest.php @@ -10,6 +10,7 @@ use ZammadAPIClient\Endpoints\Groups\GroupDTO; use ZammadAPIClient\Endpoints\Links\LinkDTO; use ZammadAPIClient\Endpoints\Organizations\OrganizationDTO; +use ZammadAPIClient\Endpoints\Roles\RoleDTO; use ZammadAPIClient\Endpoints\Tags\TagDTO; use ZammadAPIClient\Endpoints\TextModules\TextModuleDTO; use ZammadAPIClient\Endpoints\TicketArticles\TicketArticleDTO; @@ -37,6 +38,10 @@ public static function dtoProvider(): array OrganizationDTO::class, ['id' => 1, 'name' => 'Zammad GmbH', 'active' => true, 'note' => 'Our company'], ], + 'RoleDTO' => [ + RoleDTO::class, + ['id' => 2, 'name' => 'Agent', 'note' => 'Access to tickets', 'active' => true], + ], 'TagDTO' => [ TagDTO::class, ['id' => 1, 'object' => 'Ticket', 'o_id' => 42, 'value' => 'bug'], diff --git a/test/Unit/Repositories/RoleRepositoryTest.php b/test/Unit/Repositories/RoleRepositoryTest.php new file mode 100644 index 0000000..6a3a72f --- /dev/null +++ b/test/Unit/Repositories/RoleRepositoryTest.php @@ -0,0 +1,125 @@ +shouldReceive('get') + ->once() + ->with('roles', ['page' => '1', 'per_page' => '2']) + ->andReturn(['roles' => [ + ['id' => 1, 'name' => 'Admin'], + ['id' => 2, 'name' => 'Agent'], + ]]); + $handler->shouldReceive('get') + ->once() + ->with('roles', ['page' => '2', 'per_page' => '2']) + ->andReturn([]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class, 2); + $roles = iterator_to_array($repo->all()); + + self::assertCount(2, $roles); + self::assertContainsOnlyInstancesOf(RoleDTO::class, $roles); + self::assertSame('Admin', $roles[0]->name); + self::assertSame('Agent', $roles[1]->name); + } + + public function testFindReturnsRoleDto(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('get') + ->once() + ->with('roles/3', ['expand' => 'true']) + ->andReturn(['id' => 3, 'name' => 'Customer', 'note' => 'Regular user', 'active' => true]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + $role = $repo->find(3); + + self::assertSame(3, $role->id); + self::assertSame('Customer', $role->name); + self::assertSame('Regular user', $role->note); + self::assertTrue($role->active); + } + + public function testCreatePostsAndReturnsDto(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('post') + ->once() + ->with('roles', ['name' => 'Supervisor', 'note' => 'Team lead', 'active' => true]) + ->andReturn(['id' => 42, 'name' => 'Supervisor', 'note' => 'Team lead', 'active' => true]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + $role = $repo->create(new RoleDTO(name: 'Supervisor', note: 'Team lead', active: true)); + + self::assertSame(42, $role->id); + self::assertSame('Supervisor', $role->name); + } + + public function testPatchSendsOnlyChangedFields(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('put') + ->once() + ->with('roles/42', ['active' => false]) + ->andReturn(['id' => 42, 'name' => 'Supervisor', 'active' => false]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + $role = $repo->patch(42, ['active' => false]); + + self::assertSame(42, $role->id); + self::assertFalse($role->active); + } + + /** + * Roles cannot be removed through the Zammad API, so RoleRepository does not + * implement DeletableInterface — the inherited delete() must throw instead + * of silently issuing a request. + */ + public function testDeleteThrowsBecauseRolesAreNotDeletable(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldNotReceive('delete'); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + + $this->expectException(BadMethodCallException::class); + $this->expectExceptionMessage('does not support delete()'); + + $repo->delete(1); + } + + public function testSearchUsesSearchEndpoint(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('get') + ->once() + ->with('roles/search', ['query' => 'Agent', 'page' => '1', 'per_page' => '1']) + ->andReturn(['roles' => [['id' => 2, 'name' => 'Agent']]]); + $handler->shouldReceive('get') + ->once() + ->with('roles/search', ['query' => 'Agent', 'page' => '2', 'per_page' => '1']) + ->andReturn(['roles' => []]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class, 1); + $roles = iterator_to_array($repo->search('Agent')); + + self::assertCount(1, $roles); + self::assertSame('Agent', $roles[0]->name); + } +} diff --git a/test/Unit/RepositoryAccessorsTest.php b/test/Unit/RepositoryAccessorsTest.php index d5dcfb5..a33d94f 100644 --- a/test/Unit/RepositoryAccessorsTest.php +++ b/test/Unit/RepositoryAccessorsTest.php @@ -15,6 +15,8 @@ use ZammadAPIClient\Endpoints\Links\LinkRepository; use ZammadAPIClient\Endpoints\Organizations\OrganizationDTO; use ZammadAPIClient\Endpoints\Organizations\OrganizationRepository; +use ZammadAPIClient\Endpoints\Roles\RoleDTO; +use ZammadAPIClient\Endpoints\Roles\RoleRepository; use ZammadAPIClient\Endpoints\Tags\TagDTO; use ZammadAPIClient\Endpoints\Tags\TagRepository; use ZammadAPIClient\Endpoints\TextModules\TextModuleDTO; @@ -49,6 +51,7 @@ public static function accessorProvider(): array 'user' => ['user', UserRepository::class, 'users', UserDTO::class], 'organization' => ['organization', OrganizationRepository::class, 'organizations', OrganizationDTO::class], 'group' => ['group', GroupRepository::class, 'groups', GroupDTO::class], + 'role' => ['role', RoleRepository::class, 'roles', RoleDTO::class], 'ticketArticle' => [ 'ticketArticle', TicketArticleRepository::class, 'ticket_articles', TicketArticleDTO::class, ], From 2daad26f489fdd33298347736452cf01c531786e Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Fri, 2 Oct 2026 16:26:37 +0200 Subject: [PATCH 4/4] Roles: add permission_ids, group_ids, default_at_signup and fix API docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified against the Zammad server source instead of assuming: - No DELETE route exists for roles (config/routes/role.rb maps index, search, show, create and update only), so the non-deletable documentation holds. - /api/v1/roles/search does exist and supports with_total_count, so totalCount() works — covered by an integration test. - index and show are permitted to ticket.agent, admin.role and ticket.customer, not admin-only as documented before. A customer sees a reduced role whose name Zammad masks as "Role_" (Role::Assets#filter_unauthorized_attributes), so name resolution needs an agent or admin token; search, create and patch need admin.role. - permission_ids and group_ids are applied on create and update via associations_from_param, and group_ids is a group-ID-to-access map rather than a flat list. RoleDTO therefore gains default_at_signup, permission_ids and group_ids, with unit tests for hydration of the map shape, the reduced customer view and payload construction, plus the corrected README and docblocks. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- README.md | 30 ++++- src/Endpoints/Roles/RoleDTO.php | 19 +++ src/Endpoints/Roles/RoleRepository.php | 11 +- test/Integration/RoleIntegrationTest.php | 41 ++++++- test/Unit/DTOs/DTOTest.php | 3 +- test/Unit/Repositories/RoleRepositoryTest.php | 111 +++++++++++++++++- 7 files changed, 196 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6458262..a22ca2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## [3.0.0] — unreleased ### Added -- `RoleRepository` / `RoleDTO` for `/api/v1/roles`, accessible via `$client->role()` — resolve role names to the IDs needed for `UserDTO::$role_ids` +- `RoleRepository` / `RoleDTO` for `/api/v1/roles`, accessible via `$client->role()` — resolve role names to the IDs needed for `UserDTO::$role_ids`; `permission_ids` and `group_ids` are writable on create and update - PSR-18 / PSR-17 compliant HTTP layer (`RequestHandler`, `RetryAfterMiddleware`) - Typed DTOs for all 11 resources (`Ticket`, `User`, `Organization`, `Group`, `Role`, `TicketArticle`, `TicketState`, `TicketPriority`, `Tag`, `TextModule`, `Link`) - Repository pattern with generator-based pagination (`AbstractRepository`) diff --git a/README.md b/README.md index 36df944..595a6b3 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ All repositories expose a `delete()` method. Repositories implementing `Deletabl | `TicketArticleRepository` | exception | Zammad API does not allow article deletion | | `TicketStateRepository` | exception | System resource, read-only | | `TicketPriorityRepository` | exception | System resource, read-only | -| `RoleRepository` | exception | The API does not allow deleting roles; patch `active` to `false` | +| `RoleRepository` | exception | Zammad exposes no DELETE route for roles; patch `active` to `false` | ```php $client->ticket()->delete(1); @@ -314,9 +314,24 @@ $client->user()->create(new UserDTO( )); ``` -Reading roles requires a token with admin permissions — `/api/v1/roles` is admin-only -and otherwise answers with `ForbiddenException`. Roles cannot be deleted through the -API; deactivate one with `$client->role()->patch($id, ['active' => false])` instead. +Permissions and group access come along as IDs, and are writable on create and update: + +```php +$client->role()->create(new RoleDTO( + name: 'Supervisor', + permission_ids: [10, 11], + group_ids: [1 => 'full'], // map of group ID to access level, agent roles only +)); +``` + +`all()` and `find()` work with agent, admin and customer tokens. A customer however +only sees `id`, `active`, `permission_ids` and `group_ids` — Zammad replaces `name` +with the placeholder `Role_` — so resolving a role by name needs an agent or admin +token. `search()`, `totalCount()`, `create()` and `patch()` require `admin.role` and +otherwise raise a `ForbiddenException`. + +Zammad exposes no DELETE route for roles; deactivate one with +`$client->role()->patch($id, ['active' => false])` instead. ### CSV import @@ -589,9 +604,12 @@ $client->ticket()->patch(42, new TicketUpdateDTO( | Field | Type | Required | Notes | |-------|------|:--------:|-------| -| `name` | `string` | yes | Display label (e.g. `'Agent'`, `'Customer'`) | +| `name` | `string` | yes | Display label (e.g. `'Agent'`, `'Customer'`); masked as `Role_` for customer tokens | | `note` | `?string` | — | | | `active` | `?bool` | — | | +| `default_at_signup` | `?bool` | — | Role assigned to users who sign up themselves | +| `permission_ids` | `?array` | — | IDs of the granted permissions; writable | +| `group_ids` | `?array` | — | Map of group ID to access level (`[1 => 'full']`); agent roles only, writable | | `id` | `?int` | — | Server-assigned; used in `UserDTO::$role_ids` | | `created_at` | `?DateTimeImmutable` | — | Read-only | | `updated_at` | `?DateTimeImmutable` | — | Read-only | @@ -725,7 +743,7 @@ These require a running Zammad instance and authentication credentials: \* Either `ZAMMAD_PHP_API_CLIENT_UNIT_TESTS_TOKEN` or `USERNAME`+`PASSWORD` must be set. The credentials need admin permissions: some suites touch admin-only endpoints -(e.g. `RoleIntegrationTest` reads `/api/v1/roles`). +(e.g. `RoleIntegrationTest` calls `totalCount()`, which goes through `/api/v1/roles/search`). ## Migration from v2 diff --git a/src/Endpoints/Roles/RoleDTO.php b/src/Endpoints/Roles/RoleDTO.php index 35ff1b6..2c9d3d5 100644 --- a/src/Endpoints/Roles/RoleDTO.php +++ b/src/Endpoints/Roles/RoleDTO.php @@ -19,6 +19,15 @@ * assigning roles to a user via the `role_ids` field on * {@see \ZammadAPIClient\Endpoints\Users\UserDTO}. * + * `permission_ids` and `group_ids` are writable on create and update: Zammad + * applies them as associations, so a role can be created with its permissions + * in a single request. + * + * Note that a customer token sees a reduced view of a role — Zammad replaces + * `name` with the placeholder `"Role_"` and omits `note` and + * `default_at_signup`. Resolving a role by name therefore requires an agent or + * admin token. + * * Timestamp fields (`created_at`, `updated_at`) are provided by * {@see \ZammadAPIClient\Core\Traits\HasTimestamps}. */ @@ -28,10 +37,20 @@ final class RoleDTO implements DTOInterface use HydratesFromArray; use SerializesToArray; + /** + * @param array|null $permission_ids IDs of the permissions this role grants. + * @param array>|null $group_ids Map of group ID to + * access level (e.g. `[1 => 'full', 42 => ['read', 'change']]`). Only + * meaningful for roles that carry the `ticket.agent` permission — + * Zammad clears it for all others. + */ public function __construct( public readonly string $name, public readonly ?string $note = null, public readonly ?bool $active = null, + public readonly ?bool $default_at_signup = null, + public readonly ?array $permission_ids = null, + public readonly ?array $group_ids = null, public readonly ?int $id = null, ) { } diff --git a/src/Endpoints/Roles/RoleRepository.php b/src/Endpoints/Roles/RoleRepository.php index 61fc6f8..883428d 100644 --- a/src/Endpoints/Roles/RoleRepository.php +++ b/src/Endpoints/Roles/RoleRepository.php @@ -26,11 +26,14 @@ * role_ids: [$roles['Agent']], * )); * - * Listing and reading roles requires admin permissions (`admin.role` or - * `admin.user`); a token limited to agent permissions receives a 403 and the - * client raises {@see \ZammadAPIClient\Exceptions\ForbiddenException}. + * Permissions differ per method. `all()` and `find()` are open to agent, + * admin and customer tokens; a customer however only sees `id`, `active`, + * `permission_ids` and `group_ids`, with `name` replaced by the placeholder + * `"Role_"`. `search()`, `searchList()`, `totalCount()`, `create()` and + * `patch()` require `admin.role` and otherwise raise + * {@see \ZammadAPIClient\Exceptions\ForbiddenException}. * - * Roles cannot be removed through the API, so this repository does not + * Zammad exposes no DELETE route for roles, so this repository does not * implement {@see \ZammadAPIClient\Core\Contracts\DeletableInterface}: * `delete()` throws a `BadMethodCallException`. Deactivate a role by patching * `active` to `false` instead. diff --git a/test/Integration/RoleIntegrationTest.php b/test/Integration/RoleIntegrationTest.php index 00e0bdc..bc3a563 100644 --- a/test/Integration/RoleIntegrationTest.php +++ b/test/Integration/RoleIntegrationTest.php @@ -14,10 +14,11 @@ /** * Roles are reference data: these tests read the roles a Zammad installation - * ships with instead of creating new ones, because the API offers no way to - * delete a role again. + * ships with instead of creating new ones, because Zammad exposes no DELETE + * route for roles — a created role could not be cleaned up again. * - * Requires a token with admin permissions — `/api/v1/roles` is admin-only. + * Requires a token with `admin.role`: listing is open to agents too, but + * totalCount() goes through `/roles/search`, which is admin-only. */ #[Group('integration')] final class RoleIntegrationTest extends TestCase @@ -114,6 +115,40 @@ public function testCreateUserWithResolvedRoleId(): void } } + /** + * Roles carry their permissions as IDs; group access is a map of group ID + * to access level and is only populated for agent roles. + */ + public function testRolesCarryPermissionIds(): void + { + $agentRole = null; + foreach (self::$client->role()->all() as $role) { + if ($role->name === 'Agent') { + $agentRole = $role; + break; + } + } + + self::assertNotNull($agentRole, 'Agent role should exist'); + self::assertIsArray($agentRole->permission_ids); + self::assertNotEmpty($agentRole->permission_ids, 'Agent role should grant permissions'); + self::assertIsArray($agentRole->group_ids); + } + + /** + * totalCount() goes through /api/v1/roles/search with with_total_count — + * that route exists for roles, but needs an admin token. + */ + public function testTotalCountMatchesListing(): void + { + $listed = 0; + foreach (self::$client->role()->all() as $role) { + $listed++; + } + + self::assertSame($listed, self::$client->role()->totalCount()); + } + /** * Roles are returned under the "roles" list key and paginate like any other * resource — a page size of 1 must still yield every role exactly once. diff --git a/test/Unit/DTOs/DTOTest.php b/test/Unit/DTOs/DTOTest.php index 8718d8e..711590e 100644 --- a/test/Unit/DTOs/DTOTest.php +++ b/test/Unit/DTOs/DTOTest.php @@ -40,7 +40,8 @@ public static function dtoProvider(): array ], 'RoleDTO' => [ RoleDTO::class, - ['id' => 2, 'name' => 'Agent', 'note' => 'Access to tickets', 'active' => true], + ['id' => 2, 'name' => 'Agent', 'note' => 'Access to tickets', 'active' => true, + 'default_at_signup' => false, 'permission_ids' => [10, 11]], ], 'TagDTO' => [ TagDTO::class, diff --git a/test/Unit/Repositories/RoleRepositoryTest.php b/test/Unit/Repositories/RoleRepositoryTest.php index 6a3a72f..5b4b06a 100644 --- a/test/Unit/Repositories/RoleRepositoryTest.php +++ b/test/Unit/Repositories/RoleRepositoryTest.php @@ -45,7 +45,15 @@ public function testFindReturnsRoleDto(): void $handler->shouldReceive('get') ->once() ->with('roles/3', ['expand' => 'true']) - ->andReturn(['id' => 3, 'name' => 'Customer', 'note' => 'Regular user', 'active' => true]); + ->andReturn([ + 'id' => 3, + 'name' => 'Customer', + 'note' => 'Regular user', + 'active' => true, + 'default_at_signup' => true, + 'permission_ids' => [10, 11], + 'permissions' => ['ticket.customer'], + ]); $repo = new RoleRepository($handler, 'roles', RoleDTO::class); $role = $repo->find(3); @@ -54,6 +62,56 @@ public function testFindReturnsRoleDto(): void self::assertSame('Customer', $role->name); self::assertSame('Regular user', $role->note); self::assertTrue($role->active); + self::assertTrue($role->default_at_signup); + self::assertSame([10, 11], $role->permission_ids); + } + + /** + * Zammad returns group access on a role as a map of group ID to access + * level, not as a flat list of IDs. + */ + public function testFindHydratesGroupAccessMap(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('get') + ->once() + ->with('roles/2', ['expand' => 'true']) + ->andReturn([ + 'id' => 2, + 'name' => 'Agent', + 'group_ids' => ['1' => 'full', '42' => ['read', 'change']], + ]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + $role = $repo->find(2); + + self::assertSame(['1' => 'full', '42' => ['read', 'change']], $role->group_ids); + } + + /** + * A customer token gets a reduced role: Zammad masks the name and omits + * note and default_at_signup. Hydration must still succeed. + */ + public function testFindHydratesReducedCustomerView(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('get') + ->once() + ->with('roles/3', ['expand' => 'true']) + ->andReturn([ + 'id' => 3, + 'name' => 'Role_3', + 'active' => true, + 'group_ids' => [], + 'permission_ids' => [10], + ]); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + $role = $repo->find(3); + + self::assertSame('Role_3', $role->name); + self::assertNull($role->note); + self::assertNull($role->default_at_signup); } public function testCreatePostsAndReturnsDto(): void @@ -61,14 +119,51 @@ public function testCreatePostsAndReturnsDto(): void $handler = Mockery::mock(RequestHandlerInterface::class); $handler->shouldReceive('post') ->once() - ->with('roles', ['name' => 'Supervisor', 'note' => 'Team lead', 'active' => true]) - ->andReturn(['id' => 42, 'name' => 'Supervisor', 'note' => 'Team lead', 'active' => true]); + ->with('roles', [ + 'name' => 'Supervisor', + 'note' => 'Team lead', + 'active' => true, + 'permission_ids' => [10, 11], + ]) + ->andReturn([ + 'id' => 42, + 'name' => 'Supervisor', + 'note' => 'Team lead', + 'active' => true, + 'permission_ids' => [10, 11], + ]); $repo = new RoleRepository($handler, 'roles', RoleDTO::class); - $role = $repo->create(new RoleDTO(name: 'Supervisor', note: 'Team lead', active: true)); + $role = $repo->create(new RoleDTO( + name: 'Supervisor', + note: 'Team lead', + active: true, + permission_ids: [10, 11], + )); self::assertSame(42, $role->id); self::assertSame('Supervisor', $role->name); + self::assertSame([10, 11], $role->permission_ids); + } + + /** + * Unset fields must stay out of the payload so create() does not clear + * permissions or group access that the caller never touched. + */ + public function testCreateOmitsUnsetFields(): void + { + $handler = Mockery::mock(RequestHandlerInterface::class); + $handler->shouldReceive('post') + ->once() + ->with('roles', ['name' => 'Minimal']) + ->andReturn(['id' => 43, 'name' => 'Minimal']); + + $repo = new RoleRepository($handler, 'roles', RoleDTO::class); + $role = $repo->create(new RoleDTO(name: 'Minimal')); + + self::assertSame(43, $role->id); + self::assertNull($role->permission_ids); + self::assertNull($role->group_ids); } public function testPatchSendsOnlyChangedFields(): void @@ -104,17 +199,21 @@ public function testDeleteThrowsBecauseRolesAreNotDeletable(): void $repo->delete(1); } + /** + * `/roles/search` answers with a bare JSON array rather than a `roles` + * envelope, so the generator path has to fall back to the whole body. + */ public function testSearchUsesSearchEndpoint(): void { $handler = Mockery::mock(RequestHandlerInterface::class); $handler->shouldReceive('get') ->once() ->with('roles/search', ['query' => 'Agent', 'page' => '1', 'per_page' => '1']) - ->andReturn(['roles' => [['id' => 2, 'name' => 'Agent']]]); + ->andReturn([['id' => 2, 'name' => 'Agent']]); $handler->shouldReceive('get') ->once() ->with('roles/search', ['query' => 'Agent', 'page' => '2', 'per_page' => '1']) - ->andReturn(['roles' => []]); + ->andReturn([]); $repo = new RoleRepository($handler, 'roles', RoleDTO::class, 1); $roles = iterator_to_array($repo->search('Agent'));