Skip to content

Commit 86837a4

Browse files
fmontesfmontes
committed
feat(navigation): Refactor NavigationItem to implement JsonSerializable and update NavigationRequest tests
- Changed NavigationItem to implement JsonSerializable instead of extending AbstractModel for better structure. - Updated constructor properties in NavigationItem to public readonly. - Enhanced NavigationRequest validation and path building logic. - Added comprehensive tests for NavigationRequest and NavigationItem, ensuring proper functionality and structure.
1 parent f9a1d58 commit 86837a4

7 files changed

Lines changed: 710 additions & 25 deletions

File tree

‎src/Model/NavigationItem.php‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88
* Class NavigationItem
99
*
1010
* Represents a navigation item from the dotCMS Navigation API.
11+
*
12+
* Note: This class doesn't extend AbstractModel because we have a well-defined
13+
* structure with public properties, making array access unnecessary. We only
14+
* implement JsonSerializable for JSON conversion.
1115
*
1216
* @package Dotcms\PhpSdk\Model
1317
*/
14-
class NavigationItem extends AbstractModel
18+
class NavigationItem implements \JsonSerializable
1519
{
1620
/**
1721
* @var NavigationItem[]|null Array of child navigation items
@@ -32,16 +36,16 @@ class NavigationItem extends AbstractModel
3236
* @param array|null $children Array of child navigation items data
3337
*/
3438
public function __construct(
35-
private readonly ?string $code,
36-
private readonly ?string $folder,
37-
private readonly string $host,
38-
private readonly int $languageId,
39-
private readonly string $href,
40-
private readonly string $title,
41-
private readonly string $type,
42-
private readonly int $hash,
43-
private readonly string $target,
44-
private readonly int $order,
39+
public readonly ?string $code,
40+
public readonly ?string $folder,
41+
public readonly string $host,
42+
public readonly int $languageId,
43+
public readonly string $href,
44+
public readonly string $title,
45+
public readonly string $type,
46+
public readonly int $hash,
47+
public readonly string $target,
48+
public readonly int $order,
4549
private readonly ?array $children = null
4650
) {
4751
// Map children to NavigationItem objects if they exist
@@ -126,4 +130,4 @@ public function jsonSerialize(): array
126130
'children' => $this->childrenItems
127131
];
128132
}
129-
}
133+
}

‎src/Request/NavigationRequest.php‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ public function __construct(
4343
*/
4444
private function validate(): void
4545
{
46-
if (!v::stringType()->notEmpty()->validate($this->path)) {
46+
if (! v::stringType()->notEmpty()->validate($this->path)) {
4747
throw new InvalidArgumentException("Path must be a non-empty string");
4848
}
49-
50-
if (!v::intVal()->min(1)->validate($this->depth)) {
49+
50+
if (! v::intVal()->min(1)->validate($this->depth)) {
5151
throw new InvalidArgumentException("Depth must be a positive integer");
5252
}
53-
54-
if (!v::intVal()->min(1)->validate($this->languageId)) {
53+
54+
if (! v::intVal()->min(1)->validate($this->languageId)) {
5555
throw new InvalidArgumentException("Language ID must be a positive integer");
5656
}
5757
}
@@ -93,8 +93,9 @@ public function getLanguageId(): int
9393
*/
9494
public function buildPath(): string
9595
{
96-
return sprintf('/api/v1/nav%s',
97-
$this->path === '/' ? '/' : '/' . ltrim($this->path, '/')
96+
return sprintf(
97+
'/api/v1/nav%s',
98+
$this->path === '/' ? '/' : '/' . trim($this->path, '/')
9899
);
99100
}
100101

@@ -107,7 +108,7 @@ public function buildQueryParams(): array
107108
{
108109
return [
109110
'depth' => $this->depth,
110-
'languageId' => $this->languageId
111+
'languageId' => $this->languageId,
111112
];
112113
}
113-
}
114+
}

‎src/Service/NavigationService.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public function getNavigationAsync(NavigationRequest $request): PromiseInterface
7272
function (Response $response) {
7373
$responseData = $response->toArray();
7474
$this->validateResponse($responseData);
75+
7576
return $this->mapResponseToNavigationItem($responseData);
7677
},
7778
function (\Exception $e) {
@@ -96,7 +97,7 @@ function (\Exception $e) {
9697
*/
9798
private function validateResponse(array $response): void
9899
{
99-
if (!isset($response['entity'])) {
100+
if (! isset($response['entity'])) {
100101
throw new ResponseException('Invalid response: entity missing');
101102
}
102103

@@ -105,14 +106,15 @@ private function validateResponse(array $response): void
105106
// Check for required fields
106107
$requiredFields = ['host', 'languageId', 'href', 'title', 'type', 'hash', 'target', 'order'];
107108
foreach ($requiredFields as $field) {
108-
if (!isset($entity[$field])) {
109+
if (! isset($entity[$field])) {
109110
throw new ResponseException("Invalid response: {$field} missing");
110111
}
111112
}
112113

113114
// Check for errors in the response
114-
if (isset($response['errors']) && !empty($response['errors'])) {
115+
if (isset($response['errors']) && ! empty($response['errors'])) {
115116
$errorMessages = implode(', ', $response['errors']);
117+
116118
throw new ResponseException("API returned errors: {$errorMessages}");
117119
}
118120
}
@@ -141,4 +143,4 @@ private function mapResponseToNavigationItem(array $response): NavigationItem
141143
$entity['children'] ?? null
142144
);
143145
}
144-
}
146+
}

‎tests/DotCMSClientTest.php‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@
77
use Dotcms\PhpSdk\Config\Config;
88
use Dotcms\PhpSdk\DotCMSClient;
99
use Dotcms\PhpSdk\Http\HttpClient;
10+
use Dotcms\PhpSdk\Model\NavigationItem;
1011
use Dotcms\PhpSdk\Model\PageAsset;
12+
use Dotcms\PhpSdk\Request\NavigationRequest;
1113
use Dotcms\PhpSdk\Request\PageRequest;
14+
use Dotcms\PhpSdk\Service\NavigationService;
1215
use Dotcms\PhpSdk\Service\PageService;
1316
use GuzzleHttp\Promise\FulfilledPromise;
1417
use PHPUnit\Framework\TestCase;
@@ -41,6 +44,26 @@ public function testCreatePageRequest(): void
4144
$this->assertEquals($format, $request->getFormat());
4245
}
4346

47+
public function testCreateNavigationRequest(): void
48+
{
49+
// Test with default values
50+
$request = $this->client->createNavigationRequest();
51+
$this->assertInstanceOf(NavigationRequest::class, $request);
52+
$this->assertEquals('/', $request->getPath());
53+
$this->assertEquals(1, $request->getDepth());
54+
$this->assertEquals(1, $request->getLanguageId());
55+
56+
// Test with custom values
57+
$path = '/about-us';
58+
$depth = 2;
59+
$languageId = 3;
60+
$request = $this->client->createNavigationRequest($path, $depth, $languageId);
61+
$this->assertInstanceOf(NavigationRequest::class, $request);
62+
$this->assertEquals($path, $request->getPath());
63+
$this->assertEquals($depth, $request->getDepth());
64+
$this->assertEquals($languageId, $request->getLanguageId());
65+
}
66+
4467
public function testGetPage(): void
4568
{
4669
/*
@@ -131,4 +154,79 @@ public function testGetPageAsync(): void
131154
*/
132155
$this->markTestSkipped('This test would make an HTTP request. See comments for implementation options.');
133156
}
157+
158+
public function testGetNavigation(): void
159+
{
160+
/*
161+
* This test is currently skipped because it would make a real HTTP request.
162+
*
163+
* To properly test this method without making HTTP requests, consider one of these approaches:
164+
*
165+
* 1. Modify DotCMSClient to accept a NavigationService in the constructor for testing:
166+
* - Change constructor to: __construct(Config $config, ?PageService $pageService = null, ?NavigationService $navigationService = null)
167+
* - Initialize navigationService as: $this->navigationService = $navigationService ?? new NavigationService($this->httpClient);
168+
* - Then in tests, create a mock NavigationService and inject it
169+
*
170+
* Example implementation with approach #1:
171+
*
172+
* // Create mocks
173+
* $navigationItemMock = $this->createMock(NavigationItem::class);
174+
* $navigationRequestMock = $this->createMock(NavigationRequest::class);
175+
* $navigationServiceMock = $this->createMock(NavigationService::class);
176+
*
177+
* // Configure the mock
178+
* $navigationServiceMock->expects($this->once())
179+
* ->method('getNavigation')
180+
* ->with($navigationRequestMock)
181+
* ->willReturn($navigationItemMock);
182+
*
183+
* // Create client with mock service
184+
* $client = new DotCMSClient($this->config, null, $navigationServiceMock);
185+
*
186+
* // Call the method
187+
* $result = $client->getNavigation($navigationRequestMock);
188+
*
189+
* // Assert the result
190+
* $this->assertSame($navigationItemMock, $result);
191+
*/
192+
$this->markTestSkipped('This test would make an HTTP request. See comments for implementation options.');
193+
}
194+
195+
public function testGetNavigationAsync(): void
196+
{
197+
/*
198+
* This test is currently skipped because it would make a real HTTP request.
199+
*
200+
* To properly test this method without making HTTP requests, consider one of these approaches:
201+
*
202+
* 1. Modify DotCMSClient to accept a NavigationService in the constructor for testing:
203+
* - Change constructor to: __construct(Config $config, ?PageService $pageService = null, ?NavigationService $navigationService = null)
204+
* - Initialize navigationService as: $this->navigationService = $navigationService ?? new NavigationService($this->httpClient);
205+
* - Then in tests, create a mock NavigationService and inject it
206+
*
207+
* Example implementation with approach #1:
208+
*
209+
* // Create mocks
210+
* $navigationItemMock = $this->createMock(NavigationItem::class);
211+
* $navigationRequestMock = $this->createMock(NavigationRequest::class);
212+
* $promiseMock = new FulfilledPromise($navigationItemMock);
213+
* $navigationServiceMock = $this->createMock(NavigationService::class);
214+
*
215+
* // Configure the mock
216+
* $navigationServiceMock->expects($this->once())
217+
* ->method('getNavigationAsync')
218+
* ->with($navigationRequestMock)
219+
* ->willReturn($promiseMock);
220+
*
221+
* // Create client with mock service
222+
* $client = new DotCMSClient($this->config, null, $navigationServiceMock);
223+
*
224+
* // Call the method
225+
* $result = $client->getNavigationAsync($navigationRequestMock);
226+
*
227+
* // Assert the result
228+
* $this->assertSame($promiseMock, $result);
229+
*/
230+
$this->markTestSkipped('This test would make an HTTP request. See comments for implementation options.');
231+
}
134232
}

0 commit comments

Comments
 (0)