Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ZMS-3460): create endpoint for free appointments grouped by office #797

Merged
merged 17 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions zmscitizenapi/routing.php
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,48 @@ function createLanguageRoutes($app, $path, $controller, $name, $method = 'get'):
"get"
);

/**
* @swagger
* /available-appointments-by-office/:
* get:
* summary: Get available appointments for a specific day grouped by office
* tags:
* - appointments
* parameters:
* - name: date
* description: Date in format YYYY-MM-DD
* in: query
* required: true
* type: string
* - name: officeId
* description: Comma separated Office IDs
* in: query
* required: true
* type: string
* - name: serviceIds
* description: Comma separated Service IDs
* in: query
* required: true
* type: string
* responses:
* 200:
* description: List of available appointments grouped by office id
* schema:
* type: object
* properties:
* meta:
* $ref: "schema/metaresult.json"
* data:
* $ref: "schema/citizenapi/availableAppointments.json"
*/
createLanguageRoutes(
\App::$slim,
'/available-appointments-by-office/',
'\BO\Zmscitizenapi\Controllers\Availability\AvailableAppointmentsListController',
"AvailableAppointmentsListController",
"get"
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Verify the controller path in the route definition.

The route is using AvailableAppointmentsListController but based on the AI summary, there should be a new AvailableAppointmentsListByOfficeController. This might cause runtime errors.

-    '\BO\Zmscitizenapi\Controllers\Availability\AvailableAppointmentsListController',
+    '\BO\Zmscitizenapi\Controllers\Availability\AvailableAppointmentsListByOfficeController',

Also, consider updating the Swagger documentation to include:

  • Response format for error cases (400, 404)
  • Example values for the parameters
  • More detailed description of the response schema

Committable suggestion skipped: line range outside the PR's diff.


/**
* @swagger
* /appointment/:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
declare(strict_types=1);

namespace BO\Zmscitizenapi\Controllers\Availability;

use BO\Zmscitizenapi\BaseController;
use BO\Zmscitizenapi\Localization\ErrorMessages;
use BO\Zmscitizenapi\Services\Availability\AvailableAppointmentsListService;
use BO\Zmscitizenapi\Services\Core\ValidationService;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

class AvailableAppointmentsListByOfficeController extends BaseController
{
private AvailableAppointmentsListService $service;

public function __construct()
{
$this->service = new AvailableAppointmentsListService();
}

public function readResponse(RequestInterface $request, ResponseInterface $response, array $args): ResponseInterface
{
$requestErrors = ValidationService::validateServerGetRequest($request);
if (!empty($requestErrors['errors'])) {
return $this->createJsonResponse(
$response,
$requestErrors,
ErrorMessages::get('invalidRequest', $this->language)['statusCode']
);
}

$result = $this->service->getAvailableAppointmentsListByOffice($request->getQueryParams());

return is_array($result) && isset($result['errors'])
? $this->createJsonResponse(
$response,
$result,
ErrorMessages::getHighestStatusCode($result['errors'])
)
: $this->createJsonResponse($response, $result->toArray(), 200);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php
declare(strict_types=1);

namespace BO\Zmscitizenapi\Models;

use BO\Zmsentities\Schema\Entity;
use InvalidArgumentException;
use JsonSerializable;

class ProcessFreeSlotsGroupByOffice extends Entity implements JsonSerializable
{

public static $schema = 'citizenapi/processFreeSlots.json';

/** @var array|null */
public array|null $appointmentTimestamps = [];

/**
* @param array $appointmentTimestamps
*/
public function __construct(array $appointmentTimestamps = [])
{

$this->appointmentTimestamps = array_map('intval', $appointmentTimestamps);

$this->ensureValid();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Validate the structure of $appointmentTimestamps

Currently, the constructor maps $appointmentTimestamps using array_map('intval', $appointmentTimestamps); but since the data is intended to be grouped by office, which suggests a nested array structure, the mapping may not function as expected.

Adjust the logic to handle nested arrays appropriately. For example:

-            $this->appointmentTimestamps = array_map('intval', $appointmentTimestamps);
+            foreach ($appointmentTimestamps as $officeId => $timestamps) {
+                $this->appointmentTimestamps[$officeId] = array_map('intval', $timestamps);
+            }

This ensures that timestamps for each office are properly converted to integers.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$this->appointmentTimestamps = array_map('intval', $appointmentTimestamps);
$this->ensureValid();
foreach ($appointmentTimestamps as $officeId => $timestamps) {
$this->appointmentTimestamps[$officeId] = array_map('intval', $timestamps);
}
$this->ensureValid();

}

private function ensureValid(): void
{
if (!$this->testValid()) {
throw new InvalidArgumentException('The provided data is invalid according to the schema.');
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the schema path in the $schema property

The $schema property currently points to 'citizenapi/processFreeSlots.json', which may not correspond to the correct schema for the ProcessFreeSlotsGroupByOffice class.

Update the schema path to reflect the appropriate JSON schema file:

-    public static $schema = 'citizenapi/processFreeSlots.json';
+    public static $schema = 'citizenapi/processFreeSlotsGroupByOffice.json';

This ensures that the ensureValid() method validates against the correct schema.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static $schema = 'citizenapi/processFreeSlots.json';
/** @var array|null */
public array|null $appointmentTimestamps = [];
/**
* @param array $appointmentTimestamps
*/
public function __construct(array $appointmentTimestamps = [])
{
$this->appointmentTimestamps = array_map('intval', $appointmentTimestamps);
$this->ensureValid();
}
private function ensureValid(): void
{
if (!$this->testValid()) {
throw new InvalidArgumentException('The provided data is invalid according to the schema.');
}
}
public static $schema = 'citizenapi/processFreeSlotsGroupByOffice.json';
/** @var array|null */
public array|null $appointmentTimestamps = [];
/**
* @param array $appointmentTimestamps
*/
public function __construct(array $appointmentTimestamps = [])
{
$this->appointmentTimestamps = array_map('intval', $appointmentTimestamps);
$this->ensureValid();
}
private function ensureValid(): void
{
if (!$this->testValid()) {
throw new InvalidArgumentException('The provided data is invalid according to the schema.');
}
}


/**
* Converts the model data back into an array for serialization.
*
* @return array
*/
public function toArray(): array
{
return [
'appointmentTimestamps' => $this->appointmentTimestamps,
];
}

/**
* Implementation of JsonSerializable.
*/
public function jsonSerialize(): mixed
{
return $this->toArray();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ private function extractClientData(array $queryParams): object
{
return (object) [
'date' => isset($queryParams['date']) ? (string) $queryParams['date'] : null,
'officeId' => isset($queryParams['officeId']) ? (int) $queryParams['officeId'] : null,
'officeIds' => isset($queryParams['officeId'])
? array_map('trim', explode(',', (string) $queryParams['officeId']))
: [],
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve array mapping for office IDs.

The current implementation splits and trims the office IDs but doesn't validate the values. Consider adding validation for numeric values and removing empty entries.

             'officeIds' => isset($queryParams['officeId'])
-                ? array_map('trim', explode(',', (string) $queryParams['officeId']))
+                ? array_filter(
+                    array_map(function($id) {
+                        $id = trim($id);
+                        return is_numeric($id) ? (int)$id : null;
+                    }, explode(',', (string) $queryParams['officeId'])),
+                    function($id) { return $id !== null; }
+                )
                 : [],
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
'officeIds' => isset($queryParams['officeId'])
? array_map('trim', explode(',', (string) $queryParams['officeId']))
: [],
'officeIds' => isset($queryParams['officeId'])
? array_filter(
array_map(function($id) {
$id = trim($id);
return is_numeric($id) ? (int)$id : null;
}, explode(',', (string) $queryParams['officeId'])),
function($id) { return $id !== null; }
)
: [],

'serviceIds' => isset($queryParams['serviceId'])
? array_map('trim', explode(',', $queryParams['serviceId']))
? array_map('trim', explode(',', (string) $queryParams['serviceId']))
: [],
'serviceCounts' => isset($queryParams['serviceCount'])
? array_map('trim', explode(',', $queryParams['serviceCount']))
? array_map('trim', explode(',', (string) $queryParams['serviceCount']))
: []
];
}
Expand All @@ -39,19 +41,32 @@ private function validateClientData(object $data): array
{
return ValidationService::validateGetAvailableAppointments(
$data->date,
$data->officeId,
$data->officeIds,
$data->serviceIds,
$data->serviceCounts
);
}

private function getAvailableAppointments(object $data): array|AvailableAppointments
private function getAvailableAppointments(object $data, ?bool $groupByOffice = false): array|AvailableAppointments
{
return ZmsApiFacadeService::getAvailableAppointments(
$data->date,
$data->officeId,
$data->officeIds,
$data->serviceIds,
$data->serviceCounts
$data->serviceCounts,
$groupByOffice
);
}

public function getAvailableAppointmentsListByOffice($queryParams)
{
$clientData = $this->extractClientData($queryParams);

$errors = $this->validateClientData($clientData);
if (!empty($errors['errors'])) {
return $errors;
}

return $this->getAvailableAppointments($clientData, true);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add type hints and documentation for the new method.

The new method should include proper type hints and documentation for better code quality and maintainability.

+/**
+ * Get available appointments grouped by office
+ *
+ * @param array $queryParams The query parameters containing date, officeId, serviceId, and serviceCount
+ * @return array|AvailableAppointments Appointments grouped by office or validation errors
+ */
-public function getAvailableAppointmentsListByOffice($queryParams)
+public function getAvailableAppointmentsListByOffice(array $queryParams): array|AvailableAppointments
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function getAvailableAppointmentsListByOffice($queryParams)
{
$clientData = $this->extractClientData($queryParams);
$errors = $this->validateClientData($clientData);
if (!empty($errors['errors'])) {
return $errors;
}
return $this->getAvailableAppointments($clientData, true);
}
/**
* Get available appointments grouped by office
*
* @param array $queryParams The query parameters containing date, officeId, serviceId, and serviceCount
* @return array|AvailableAppointments Appointments grouped by office or validation errors
*/
public function getAvailableAppointmentsListByOffice(array $queryParams): array|AvailableAppointments
{
$clientData = $this->extractClientData($queryParams);
$errors = $this->validateClientData($clientData);
if (!empty($errors['errors'])) {
return $errors;
}
return $this->getAvailableAppointments($clientData, true);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,12 @@ private function extractClientData(array $queryParams): object
{
$serviceCount = $queryParams['serviceCount'] ?? '';
$serviceCounts = !empty($serviceCount)
? array_map('trim', explode(',', $serviceCount))
? array_map('trim', explode(',', (string) $serviceCount))
: [];

return (object) [
'officeId' => isset($queryParams['officeId']) && is_numeric($queryParams['officeId'])
? (int)$queryParams['officeId']
: null,
'serviceId' => isset($queryParams['serviceId']) && is_numeric($queryParams['serviceId'])
? (int)$queryParams['serviceId']
: null,
'officeIds' => array_map('trim', explode(',', (string) $queryParams['officeId'] ?? '')),
'serviceIds' => array_map('trim', explode(',', (string) $queryParams['serviceId'] ?? '')),
'serviceCounts' => $serviceCounts,
'startDate' => $queryParams['startDate'] ?? null,
'endDate' => $queryParams['endDate'] ?? null
Expand All @@ -44,19 +40,19 @@ private function extractClientData(array $queryParams): object
private function validateClientData(object $data): array
{
return ValidationService::validateGetBookableFreeDays(
$data->officeId,
$data->serviceId,
$data->officeIds,
$data->serviceIds,
$data->startDate,
$data->endDate,
$data->serviceCounts
);
}

private function getAvailableDays(object $data): array|AvailableDays
private function getAvailableDays(object $data): AvailableDays
{
return ZmsApiFacadeService::getBookableFreeDays(
$data->officeId,
$data->serviceId,
$data->officeIds,
$data->serviceIds,
$data->serviceCounts,
$data->startDate,
$data->endDate
Expand Down
31 changes: 18 additions & 13 deletions zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,19 @@ public static function validateServiceLocationCombination(int $officeId, array $
}

public static function validateGetBookableFreeDays(
?int $officeId,
?int $serviceId,
?array $officeIds,
?array $serviceIds,
?string $startDate,
?string $endDate,
?array $serviceCounts
): array {
$errors = [];

if (!self::isValidOfficeId($officeId)) {
if (!self::isValidOfficeIds($officeIds)) {
$errors[] = self::getError('invalidOfficeId');
}

if (!self::isValidServiceId($serviceId)) {
if (!self::isValidServiceIds($serviceIds)) {
$errors[] = self::getError('invalidServiceId');
}

Expand Down Expand Up @@ -143,7 +143,7 @@ public static function validateGetProcessById(?int $processId, ?string $authKey)

public static function validateGetAvailableAppointments(
?string $date,
?int $officeId,
?array $officeIds,
?array $serviceIds,
?array $serviceCounts
): array {
Expand All @@ -153,7 +153,7 @@ public static function validateGetAvailableAppointments(
$errors[] = self::getError('invalidDate');
}

if (!self::isValidOfficeId($officeId)) {
if (!self::isValidOfficeIds($officeIds)) {
$errors[] = self::getError('invalidOfficeId');
}

Expand Down Expand Up @@ -357,14 +357,9 @@ private static function isValidNumericArray(array $array): bool
return !empty($array) && array_filter($array, 'is_numeric') === $array;
}

private static function isValidOfficeId(?int $officeId): bool
{
return !empty($officeId) && $officeId > 0;
}

private static function isValidServiceId(?int $serviceId): bool
private static function isValidOfficeIds(?array $officeIds): bool
{
return !empty($serviceId) && $serviceId > 0;
return !empty($officeIds) && self::isValidNumericArray($officeIds);
}

private static function isValidScopeId(?int $scopeId): bool
Expand Down Expand Up @@ -426,4 +421,14 @@ private static function isValidCustomTextfield(?string $customTextfield): bool
{
return $customTextfield === null || (is_string($customTextfield) && strlen(trim($customTextfield)) > 0);
}

private static function isValidOfficeId(?int $officeId): bool
{
return !empty($officeId) && $officeId > 0;
}

private static function isValidServiceId(?int $serviceId): bool
{
return !empty($serviceId) && $serviceId > 0;
}
}
Loading
Loading