Test into main

[ThomasAFink]

Pull Request Checklist (Feature Branch to next):

• Ich habe die neuesten Änderungen aus dem next Branch in meinen Feature-Branch gemergt.
• Das Code-Review wurde abgeschlossen.
• Fachliche Tests wurden durchgeführt und sind abgeschlossen.

Summary by CodeRabbit

• New Features

  • Launched a streamlined emergency management interface in the admin area with improved call controls.
  • Introduced an enhanced appointment booking capability, enabling users to retrieve available appointments by office.

• Refactor

  • Simplified user-interaction events for a smoother experience on workstation pages.

• Chores

  • Upgraded office detail displays by adding more contextual information.
  • Implemented performance optimizations including improved sorting and database enhancements.

[coderabbitai]

Walkthrough

This pull request introduces a new JavaScript class for handling emergency cancellation in the admin interface while removing redundant event handlers across pages. It also updates several PHP backend components by enhancing validation, supporting arrays for office/service IDs, and modifying API routes and models for available appointments. Additional changes include JSON schema updates, SQL index creations, and test modifications, along with a minor update to slot cancellation timing.

Changes

File(s)	Change Summary
zmsadmin/js/block/scope/emergencyend.js, zmsadmin/js/index.js	Added the new EndEmergencyView class with a button event listener to trigger emergency cancellation and integrated its initialization in the jQuery context.
zmsadmin/js/page/counter/index.js, zmsadmin/js/page/workstation/index.js	Removed window.onblur, mouseenter, and mouseleave event handlers to simplify reload timer management.
zmsadmin/src/Zmsadmin/QueueTable.php	Introduced sorting of $queueList based on arrivalTime via uasort.
zmsadmin/templates/block/emergency/emergency.twig	Wrapped the emergency call button in a conditional block that checks if workstation.name is defined.
zmsadmin/templates/block/process/info.twig	Removed the conditional rendering for the process redirect button; now always renders as an anchor tag.
zmsadmin/templates/block/scope/form.twig	Enhanced the emergency form by adding data attributes and modifying the checkbox’s label and class.
zmsapi/src/Zmsapi/ProcessRedirect.php	Modified redirection logic to use process queue number if available, falling back to process id.
zmscitizenapi/bootstrap.php	Commented out the additions of SecurityHeadersMiddleware and CorsMiddleware.
zmscitizenapi/routing.php	Added a new GET route for available appointments by office with proper Swagger documentation.
zmscitizenapi/src/Zmscitizenapi/Controllers/Availability/AvailableAppointmentsListByOfficeController.php	Added a new controller to handle requests for available appointments grouped by office.
zmscitizenapi/src/Zmscitizenapi/Models/AvailableAppointmentsByOffice.php	Created a new model for appointments grouped by office with validation and JSON serialization.
zmscitizenapi/src/Zmscitizenapi/Models/Office.php	Updated the Office model to include new properties: displayNameAlternatives, showAlternativeLocations, organization, organizationUnit, and slotTimeInMinutes.
zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableAppointmentsListService.php	Modified service methods to accept multiple office IDs and added a new method for appointments grouped by office.
zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableDaysListService.php	Refactored the service to extract and validate officeIds and serviceIds as arrays.
zmscitizenapi/src/Zmscitizenapi/Services/Core/MapperService.php	Extended office mapping to include the new Office properties from provider data.
zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php	Updated validation methods to handle arrays for office and service IDs, replacing single value checks.
zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php	Modified facade methods to work with array input and support groupByOffice in appointment retrieval; simplified free slots processing.
zmscitizenapi/tests/**	Updated tests to reflect new array-based parameters and extended Office properties; commented out middleware tests and removed an outdated test for writeCanceledByTime.
zmsdb/migrations/91737550001-add-indexes.sql	Added several SQL indexes to optimize query performance on slot and slot_process tables.
zmsdb/src/Zmsdb/Helper/CalculateSlots.php	Changed the default cancellation time adjustment from "+10 minutes" to "+5 minutes" in writeCanceledSlots method.
zmsdb/tests/Zmsdb/SlotTest.php	Removed the test case for writeCanceledByTime, eliminating checks on the cancelled slot status.
zmsentities/schema/citizenapi/{availableAppointmentsByOffice.json, collections/officeList.json, collections/officeServiceAndRelationList.json, office.json}	Updated and added JSON schemas to include new Office properties (showAlternativeLocations, displayNameAlternatives, organization, organizationUnit, slotTimeInMinutes) and defined a new schema for AvailableAppointmentsByOffice.
zmsentities/src/Zmsentities/Schema/Entity.php	Improved withCleanedUpFormData method by checking for the existence of the 'save' key before unsetting it.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant UI as Admin UI
    participant EV as EndEmergencyView
    participant AJAX as AJAX Request

    U->>UI: Clicks emergency end button (.emergency__button-end)
    UI->>EV: Event listener triggers endEmergency()
    EV->>EV: Updates internal state (emergency ended)
    EV->>AJAX: Calls sendEmergencyCancel() (GET request with constructed URL)
    AJAX-->>EV: Returns success (or error logged)
    EV->>UI: (Optionally) Update UI based on response

Loading

sequenceDiagram
    participant C as Client
    participant R as API Router
    participant CT as AvailableAppointmentsListByOfficeController
    participant S as AvailableAppointmentsListService
    participant DB as Data/Model Layer

    C->>R: GET /available-appointments-by-office/?date=&officeId=&serviceIds=
    R->>CT: Route request to controller
    CT->>CT: Validate request parameters
    CT->>S: Call getAvailableAppointmentsListByOffice(queryParams)
    S->>DB: Retrieve and group appointment data by office
    DB-->>S: Return appointment data
    S-->>CT: Return appointments grouped by office
    CT-->>C: Send JSON response with appointment data

Loading

Possibly related PRs

• feat(ZMS-3503): added logic to end emergency with checkbox #805: Implements similar handling in the JavaScript EndEmergencyView class for emergency cancellation.
• feat(ZMS-3212) Notfall für Tresen deaktiveren #791: Involves conditional rendering of the emergency button based on workstation.name, aligning with the changes here.

Suggested reviewers

• Fabinatix97

Poem

I'm a little rabbit, hopping with delight,
Code changes dancing in the moonlit night.
Emergencies end with a swift button click,
Appointments now group and flow real slick.
Through tweaks and tests our code sings anew,
Carrots and commits—celebration for our crew!
🐰💻 Happy coding, under skies so blue!

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (24)

zmscitizenapi/tests/Zmscitizenapi/Services/Core/ValidationServiceTest.php (1)

91-96: LGTM! Consider adding edge cases for array validation.

The changes correctly update the test cases to use arrays for office and service IDs. The test coverage remains comprehensive for basic validation scenarios.

Consider adding test cases for these edge cases:

• Empty arrays
• Arrays with multiple valid values
• Arrays with mixed valid/invalid values
• Arrays with duplicate values

Example:

 public function testValidateGetAvailableAppointments(): void
 {
+    // Test empty array
+    $result = ValidationService::validateGetAvailableAppointments(
+        '2025-01-01',
+        [],
+        [1],
+        [1]
+    );
+    $this->assertContains(
+        ErrorMessages::get('invalidOfficeId'),
+        $result['errors']
+    );
+
+    // Test multiple valid values
+    $result = ValidationService::validateGetAvailableAppointments(
+        '2025-01-01',
+        [1, 2, 3],
+        [1],
+        [1]
+    );
+    $this->assertEmpty($result['errors']);
+
+    // Test mixed valid/invalid values
+    $result = ValidationService::validateGetAvailableAppointments(
+        '2025-01-01',
+        [1, 'invalid', 3],
+        [1],
+        [1]
+    );
+    $this->assertContains(
+        ErrorMessages::get('invalidOfficeId'),
+        $result['errors']
+    );

Also applies to: 100-105, 113-117, 124-129, 136-141

zmscitizenapi/tests/Zmscitizenapi/Services/Availability/AvailableDaysListServiceTest.php (1)

26-154: Improve test organization and coverage.

Consider the following improvements to the test suite:

1. Extract repeated test data into class constants or data providers
2. Add test cases for the new array functionality
3. Group related test cases using test suites or annotations

Example refactor for test data:

private const VALID_SINGLE_OFFICE = '123';
private const VALID_SINGLE_SERVICE = '456';
private const VALID_MULTIPLE_OFFICES = '123,456,789';
private const VALID_MULTIPLE_SERVICES = '111,222,333';
private const VALID_DATE_RANGE = [
    'startDate' => '2025-01-01',
    'endDate' => '2025-01-31'
];

private function getTestQueryParams(array $overrides = []): array
{
    return array_merge([
        'officeId' => self::VALID_SINGLE_OFFICE,
        'serviceId' => self::VALID_SINGLE_SERVICE,
        'serviceCount' => '1',
        'startDate' => self::VALID_DATE_RANGE['startDate'],
        'endDate' => self::VALID_DATE_RANGE['endDate']
    ], $overrides);
}

zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableDaysListService.php (1)

26-27: Add input validation and data cleaning for office/service IDs.

While the type conversion and array splitting logic works, consider adding validation and cleaning steps:

1. Filter out empty elements after splitting
2. Remove duplicate IDs
3. Validate the format of individual IDs

Consider applying this improvement:

-            'officeIds' => array_map('trim', explode(',', $queryParams['officeId'])),
-            'serviceIds' => array_map('trim', explode(',', $queryParams['serviceId'])),
+            'officeIds' => array_unique(array_filter(array_map('trim', explode(',', $queryParams['officeId'])))),
+            'serviceIds' => array_unique(array_filter(array_map('trim', explode(',', $queryParams['serviceId'])))),

Also applies to: 30-30, 34-35

zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php (1)

560-560: Changed signature for processFreeSlots

Changing the return type from ProcessFreeSlots|array to just array simplifies consumption of the timestamps. Be sure to update any references still expecting a ProcessFreeSlots object.

zmsadmin/js/block/scope/emergencyend.js (2)

20-22: update method merges state

Object.assign merges data shallowly. If nested objects are expected, consider a deep merge strategy.

---

24-37: sendEmergencyCancel using jQuery.ajax

Returning a new Promise is functional. Consider these possible improvements:

1. Using a POST request for an action that modifies server state, if the API supports it.
2. Embracing async/await for more modern promise handling.

zmscitizenapi/src/Zmscitizenapi/Controllers/Availability/AvailableAppointmentsListByOfficeController.php (1)

15-20: Consider using dependency injection instead of direct instantiation.

The service is directly instantiated in the constructor. Consider using dependency injection to improve testability and flexibility.

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

zmscitizenapi/src/Zmscitizenapi/Models/Office.php (1)

45-69: Consider adding parameter validation for new fields.

While the constructor is well-structured, consider adding specific validation rules for the new fields in ensureValid().

     private function ensureValid()
     {
+        if ($this->slotTimeInMinutes !== null && $this->slotTimeInMinutes <= 0) {
+            throw new InvalidArgumentException("Slot time must be positive.");
+        }
         if (!$this->testValid()) {
             throw new InvalidArgumentException("The provided data is invalid according to the schema.");
         }
     }

zmsadmin/templates/block/emergency/emergency.twig (1)

10-14: Good defensive check added!

The conditional check for workstation.name prevents rendering the emergency button when the workstation is not properly configured. Consider adding a warning message or placeholder when workstation.name is not defined to help administrators identify misconfigured workstations.

 {% if workstation.name %}
   <button class="button button--emergency emergency__button-trigger" >
       <i class="fas fa-bell" aria-hidden="true"></i> NOTRUF
   </button>
+{% else %}
+   <div class="alert alert--warning">
+       <i class="fas fa-exclamation-triangle" aria-hidden="true"></i>
+       Workstation nicht konfiguriert - Notruf nicht verfügbar
+   </div>
 {% endif %}

zmsadmin/src/Zmsadmin/QueueTable.php (1)

53-55: Optimize sorting implementation.

The sorting logic is correct but can be simplified using PHP's spaceship operator for more concise and readable code.

-$queueList->uasort(function ($queueA, $queueB) {
-    return $queueA->arrivalTime - $queueB->arrivalTime;
-});
+$queueList->uasort(fn($queueA, $queueB) => $queueA->arrivalTime <=> $queueB->arrivalTime);

zmscitizenapi/tests/Zmscitizenapi/Services/Availability/AvailableAppointmentsListServiceTest.php (1)

158-159: Add test cases for multiple office IDs.

The mock services now support arrays of office IDs, but the test cases only cover single office scenarios. Consider adding test cases that verify the behavior with multiple office IDs.

Example test case:

public function testGetAvailableAppointmentsListWithMultipleOffices(): void
{
    // Arrange
    $queryParams = [
        'date' => '2025-01-15',
        'officeId' => '123,456',  // Multiple office IDs
        'serviceId' => '789',
        'serviceCount' => '1'
    ];
    
    $expectedAppointments = new AvailableAppointments([1705317600, 1705321200]);
    
    $this->createMockValidationService([]);
    $this->createMockFacade($expectedAppointments);

    // Act
    $result = $this->service->getAvailableAppointmentsList($queryParams);

    // Assert
    $this->assertInstanceOf(AvailableAppointments::class, $result);
    $this->assertEquals($expectedAppointments, $result);
}

Also applies to: 175-176

zmsentities/src/Zmsentities/Schema/Entity.php (1)

311-313: Good defensive programming! Consider applying the same check for 'removeImage'.

The addition of the existence check before unsetting the 'save' key is a good practice to prevent potential warnings.

Consider applying the same existence check for the 'removeImage' key:

-        if (isset($entity['save'])) { 
-            unset($entity['save']);
-        } 
-        if (isset($entity['removeImage'])) {
-            unset($entity['removeImage']);
-        }
+        $keysToUnset = ['save', 'removeImage'];
+        foreach ($keysToUnset as $key) {
+            if (isset($entity[$key])) {
+                unset($entity[$key]);
+            }
+        }

This refactoring makes the code more maintainable and consistent in handling form data cleanup.

zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php (3)

87-127: LGTM! Consider adding PHPDoc for the updated parameters.

The changes to support array inputs for office and service IDs are well-implemented. The validation logic correctly handles the new array parameters.

Add PHPDoc to document the new parameter types:

+ /**
+  * Validates parameters for getting bookable free days
+  * @param array|null $officeIds Array of office IDs
+  * @param array|null $serviceIds Array of service IDs
+  * @param string|null $startDate Start date in Y-m-d format
+  * @param string|null $endDate End date in Y-m-d format
+  * @param array|null $serviceCounts Array of service counts
+  * @return array Array containing validation errors, if any
+  */

---

144-169: LGTM! Consider adding PHPDoc for the updated parameters.

The changes to support array input for office IDs are well-implemented and consistent with the previous method changes.

Add PHPDoc to document the parameter types:

+ /**
+  * Validates parameters for getting available appointments
+  * @param string|null $date Date in Y-m-d format
+  * @param array|null $officeIds Array of office IDs
+  * @param array|null $serviceIds Array of service IDs
+  * @param array|null $serviceCounts Array of service counts
+  * @return array Array containing validation errors, if any
+  */

---

360-433: LGTM! Consider adding PHPDoc for the validation methods.

The implementation of array validation methods while retaining single-value validation maintains good backward compatibility.

Add PHPDoc to document the validation methods:

+ /**
+  * Validates an array of office IDs
+  * @param array|null $officeIds Array of office IDs to validate
+  * @return bool True if all IDs are valid, false otherwise
+  */
private static function isValidOfficeIds(?array $officeIds): bool

+ /**
+  * Validates a single office ID
+  * @param int|null $officeId Office ID to validate
+  * @return bool True if the ID is valid, false otherwise
+  */
private static function isValidOfficeId(?int $officeId): bool

+ /**
+  * Validates an array of service IDs
+  * @param array|null $serviceIds Array of service IDs to validate
+  * @return bool True if all IDs are valid, false otherwise
+  */
private static function isValidServiceIds(?array $serviceIds): bool

+ /**
+  * Validates a single service ID
+  * @param int|null $serviceId Service ID to validate
+  * @return bool True if the ID is valid, false otherwise
+  */
private static function isValidServiceId(?int $serviceId): bool

zmscitizenapi/src/Zmscitizenapi/Services/Core/MapperService.php (1)

62-84: LGTM! Consider adding type hints for the new properties.

The addition of new office properties is well-implemented with proper default value handling.

Add type hints in the Office constructor's PHPDoc to document the new properties:

+ /**
+  * @param bool|null $showAlternativeLocations Whether to show alternative locations
+  * @param array $displayNameAlternatives Array of alternative display names
+  * @param string|null $organization Organization name
+  * @param string|null $organizationUnit Organization unit name
+  * @param int|null $slotTimeInMinutes Duration of each slot in minutes
+  */

zmsentities/schema/citizenapi/office.json (1)

1-119: Office JSON Schema Enhancements: New Properties Added
The schema enhancements—including the addition of "showAlternativeLocations", "displayNameAlternatives", "organization", "organizationUnit", and "slotTimeInMinutes"—are implemented consistently. One minor suggestion: if the "scope" property is expected to follow a specific structure later, consider defining its schema rather than leaving it as an empty object.

zmsentities/schema/citizenapi/collections/officeList.json (1)

1-134: Enhanced OfficeList Schema: Consistency with Office Model
The modifications to include new office attributes are consistent with the updates made in the Office schema. Ensure that backend validations and model mappings are also updated to handle these new properties. Optionally, additional validation rules might be added based on business requirements.

zmscitizenapi/tests/Zmscitizenapi/fixtures/GET_SourceGet_dldb.json (1)

27-28: Test Fixture Update: Provider "showAlternativeLocations" Field
The new property "showAlternativeLocations" is correctly added with boolean values (false for provider with id "9999998" and true for provider with id "9999999"). Just ensure that the consuming tests validate the proper handling of this field in the application logic.

Also applies to: 50-51

zmsentities/schema/citizenapi/collections/officeServiceAndRelationList.json (5)

29-35: Include Default Value for showAlternativeLocations
The new property "showAlternativeLocations" is defined with types "boolean" and "null". Consider adding a default (e.g., false) to ensure clients have a predictable behavior when the value is omitted.

---

73-79: Define Array Items for displayNameAlternatives
The "displayNameAlternatives" property is declared as an array but does not specify an "items" schema. Defining the expected type (likely "string") for the array elements would enhance validation and clarity.

---

80-86: Align Description for organization
The "organization" field is correctly defined as "string" or "null". To maintain consistency with similar properties, consider adjusting the description to use a consistent tone (e.g., "The name of the organization").

---

87-93: Clarify organizationUnit Description
The "organizationUnit" property currently uses the description "The name of the organization", which may not clearly convey its purpose. Consider updating the description to something like "The organizational unit name" or "The name of the organizational unit" for clarity.

---

94-100: Add Validation Constraint for slotTimeInMinutes
While "slotTimeInMinutes" is defined correctly as an integer (or null), you might want to add a constraint (e.g., "minimum": 0) if negative values are not valid for slot time. This will help enforce data integrity.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e748ef8 and 144412d.

⛔ Files ignored due to path filters (3)

• zmsadmin/package-lock.json is excluded by !**/package-lock.json
• zmscalldisplay/package-lock.json is excluded by !**/package-lock.json
• zmsstatistic/composer.lock is excluded by !**/*.lock

📒 Files selected for processing (37)

• zmsadmin/js/block/scope/emergencyend.js (1 hunks)
• zmsadmin/js/index.js (2 hunks)
• zmsadmin/js/page/counter/index.js (0 hunks)
• zmsadmin/js/page/workstation/index.js (0 hunks)
• zmsadmin/src/Zmsadmin/QueueTable.php (1 hunks)
• zmsadmin/templates/block/emergency/emergency.twig (1 hunks)
• zmsadmin/templates/block/process/info.twig (1 hunks)
• zmsadmin/templates/block/scope/form.twig (1 hunks)
• zmsapi/src/Zmsapi/ProcessRedirect.php (1 hunks)
• zmscitizenapi/bootstrap.php (1 hunks)
• zmscitizenapi/routing.php (1 hunks)
• zmscitizenapi/src/Zmscitizenapi/Controllers/Availability/AvailableAppointmentsListByOfficeController.php (1 hunks)
• zmscitizenapi/src/Zmscitizenapi/Models/AvailableAppointmentsByOffice.php (1 hunks)
• zmscitizenapi/src/Zmscitizenapi/Models/Office.php (2 hunks)
• zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableAppointmentsListService.php (3 hunks)
• zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableDaysListService.php (2 hunks)
• zmscitizenapi/src/Zmscitizenapi/Services/Core/MapperService.php (1 hunks)
• zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php (5 hunks)
• zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php (10 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficeListByServiceControllerTest.php (3 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficesListControllerTest.php (2 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficesServicesRelationsControllerTest.php (2 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Middleware/CorsMiddlewareTest.php (5 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Middleware/SecurityHeadersMiddlewareTest.php (3 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Services/Availability/AvailableAppointmentsListServiceTest.php (2 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Services/Availability/AvailableDaysListServiceTest.php (2 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Services/Core/ValidationServiceTest.php (5 hunks)
• zmscitizenapi/tests/Zmscitizenapi/Services/Core/ZmsApiFacadeServiceTest.php (1 hunks)
• zmscitizenapi/tests/Zmscitizenapi/fixtures/GET_SourceGet_dldb.json (2 hunks)
• zmsdb/migrations/91737550001-add-indexes.sql (1 hunks)
• zmsdb/src/Zmsdb/Helper/CalculateSlots.php (1 hunks)
• zmsdb/tests/Zmsdb/SlotTest.php (0 hunks)
• zmsentities/schema/citizenapi/availableAppointmentsByOffice.json (1 hunks)
• zmsentities/schema/citizenapi/collections/officeList.json (3 hunks)
• zmsentities/schema/citizenapi/collections/officeServiceAndRelationList.json (2 hunks)
• zmsentities/schema/citizenapi/office.json (3 hunks)
• zmsentities/src/Zmsentities/Schema/Entity.php (1 hunks)

💤 Files with no reviewable changes (3)

• zmsadmin/js/page/workstation/index.js
• zmsdb/tests/Zmsdb/SlotTest.php
• zmsadmin/js/page/counter/index.js

✅ Files skipped from review due to trivial changes (2)

• zmscitizenapi/tests/Zmscitizenapi/Middleware/SecurityHeadersMiddlewareTest.php
• zmscitizenapi/tests/Zmscitizenapi/Middleware/CorsMiddlewareTest.php

🔇 Additional comments (45)

zmscitizenapi/tests/Zmscitizenapi/Services/Core/ValidationServiceTest.php (2)

151-156: LGTM! Test cases properly updated for array parameters.

The test cases for validatePostAppointmentReserve have been correctly updated to use arrays for office and service IDs while maintaining comprehensive validation coverage.

Also applies to: 160-165, 172-177, 184-189, 196-201

---

1-501: Great test organization and coverage!

The test class is well-structured with comprehensive coverage:

• Each validation method has dedicated test cases
• Both positive and negative scenarios are covered
• Test methods follow consistent naming and organization patterns
• Error messages are properly validated

zmscitizenapi/tests/Zmscitizenapi/Services/Availability/AvailableDaysListServiceTest.php (2)

156-172: Add test cases for multiple office/service IDs.

The mock ValidationService now accepts arrays for officeIds and serviceIds, but all test cases only test with single values. Consider adding test cases that verify the behavior with multiple office and service IDs to ensure complete test coverage.

Example test data to consider:

• Multiple office IDs: 'officeId' => '123,456,789'
• Multiple service IDs: 'serviceId' => '111,222,333'

---

174-190: Enhance test coverage for ZmsApiFacadeService.

The mock ZmsApiFacadeService has two areas that need additional test coverage:

1. The method accepts arrays for officeIds and serviceIds, but all test cases only test with single values
2. The return type AvailableDays|array suggests the method can return an array, but there are no test cases verifying this scenario

Consider adding test cases that:

• Test with multiple office and service IDs
• Verify the behavior when the method returns an array instead of an AvailableDays object

zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableDaysListService.php (2)

44-50: LGTM!

The parameter updates correctly align with the changes in the ValidationService interface.

---

53-61: Consider maintaining consistent return type order.

The return type order has been changed from array|AvailableDays to AvailableDays|array. While this doesn't affect functionality, consider maintaining consistency with other similar methods in the codebase.

Let's check the return type order in other methods:

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Search for similar return type declarations
rg -t php "array\|AvailableDays|AvailableDays\|array" -A 2

Length of output: 1673

---

Return type order is consistent across the codebase.

• Both the service method and the facade method use the AvailableDays|array return type, matching the test expectations.
• No further changes are needed regarding the return type order.

zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php (10)

8-13: New imports for available appointments

The added imports for "AvailableAppointmentsByOffice" and "ProcessFreeSlotsGroupByOffice" correctly align with the updated return types and grouping logic. Please ensure that any references to the removed or replaced classes (if any) are also updated accordingly.

---

66-70: Introducing additional Office fields

Adding “showAlternativeLocations”, “displayNameAlternatives”, “organization”, “organizationUnit”, and “slotTimeInMinutes” extends the Office details. Confirm that all these fields are correctly populated and consumed downstream; if any are optional or uninitialized in provider->data, consider adding type checks or fallback values.

---

437-443: Multiple offices and services for getBookableFreeDays

Transitioning to arrays for $officeIds and $serviceIds increases flexibility. Ensure that all upstream invocations have switched to providing arrays, and confirm that no leftover single-int references remain. Consider adding input validation if $officeIds or $serviceIds might be empty.

---

446-448: Separate collections for services and providers

Declaring $services = [] and $providers = [] before usage clarifies your intent to store multiple items. This helps keep the code organized.

---

449-457: Index-based mapping of serviceCounts

Incrementing $serviceNumber for each service is fine if $serviceIds and $serviceCounts are guaranteed to have matching lengths. Otherwise, this may cause an out-of-range exception.

Do you want to add a quick length check to avoid run-time errors?

---

459-464: Populating providers array

Storing each $officeId in the $providers array is consistent with how the code processes multiple offices. Just confirm that all references to the single $officeId usage have been removed or replaced.

---

516-522: Expanded return type for getAvailableAppointments

Returning AvailableAppointments, AvailableAppointmentsByOffice, or array based on $groupByOffice is a versatile approach. Confirm that the calling code branches correctly depending on the different return types (since the shape of data differs).

---

534-540: Reinserting provider references for free timeslots

Appending multiple providers in $providers matches the updated method signature. This keeps the approach uniform with getBookableFreeDays.

---

554-556: Conditional return of grouped vs. ungrouped appointments

When $groupByOffice is true, returning AvailableAppointmentsByOffice is correct. Otherwise, only returning data from the first provider (via array_values($timestamps)[0]) might omit other providers if multiple are passed. Confirm this is intended.

Also applies to: 558-558

---

573-579: Iterating provider slots & validating timestamps

1. Casting $slot->scope->provider->id to (int) is straightforward, but ensure that scope and provider are always present.
2. Timestamps are stored in an associative array keyed by provider ID. Only returning them if they exceed the current time is sensible.
3. The final sort ensures that timestamps are ascending.
4. If you rely on old code that used a dedicated ProcessFreeSlots object, it may break.

Everything else looks correct for processing.

Also applies to: 589-592, 599-599

zmsadmin/js/block/scope/emergencyend.js (4)

1-2: Module import

Importing jQuery is essential for this file’s DOM manipulations. This is consistent with the rest of the codebase.

---

3-14: Class definition and constructor

The constructor correctly captures includeUrl, scope, and workstationName. Ensure that these fields exist in options during instantiation to avoid undefined behavior.

---

15-18: endEmergency method

This method updates the local state and triggers cancellation. Straightforward and clean.

---

38-40: Class export

Exporting EndEmergencyView by default ensures easy import elsewhere.

zmscitizenapi/src/Zmscitizenapi/Models/AvailableAppointmentsByOffice.php (8)

1-3: File header and strict typing

Using strict_types=1 is consistent with modern PHP standards.

---

4-9: Namespace and imports

Organizing classes under BO\Zmscitizenapi\Models with JsonSerializable is proper. The schema-based approach signals a robust design.

---

10-14: Class introduction with schema property

The static $schema property clarifies which JSON schema is used to validate.

---

15-17: officeAppointments property

Marking it as array|null = [] is fine, but ensure that null usage cannot conflict with the default empty array.

---

18-27: Constructor and immediate validation

The constructor calls ensureValid() to confirm data against the schema. Throwing InvalidArgumentException early helps maintain data integrity.

---

29-34: ensureValid method

testValid() presumably checks schema compliance. This centralizes data integrity.

---

36-51: toArray method with array_map

Generating a structured “offices” array is clean. The callback approach for mapping each set of appointments to an office placeholder is elegant.

---

53-60: jsonSerialize implementation

Simply returning toArray() is standard and ensures consistent JSON output.

zmscitizenapi/src/Zmscitizenapi/Controllers/Availability/AvailableAppointmentsListByOfficeController.php (1)

22-42: LGTM! Well-structured error handling and response creation.

The method follows good practices:

• Validates request first
• Returns early on validation errors
• Uses consistent error response format
• Properly handles service response

zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableAppointmentsListService.php (3)

51-62: LGTM! Well-structured method with clear type hints.

The method:

• Has clear parameter and return type hints
• Uses union types appropriately
• Follows single responsibility principle

---

64-74: LGTM! Consistent implementation with base method.

The new method follows the same pattern as getAvailableAppointmentsList:

• Extracts and validates client data
• Returns early on validation errors
• Uses the base method with appropriate parameters

---

29-31: Consider adding input validation for office IDs.

The office IDs are split and trimmed but not validated for format or emptiness.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check if there's any validation for office IDs in the codebase
rg -A 5 "validateOfficeId|validateOffice" .

Length of output: 3628

---

I'll now run a search within the AvailableAppointmentsListService.php file to verify if there's any input validation related to office IDs present beyond the splitting and trimming.

---

🏁 Script executed:

#!/bin/bash
# Searching for any input validation call in AvailableAppointmentsListService.php related to office IDs
rg -n "validate" zmscitizenapi/src/Zmscitizenapi/Services/Availability/AvailableAppointmentsListService.php

Length of output: 364

---

I'll run a search for the implementation of validateGetAvailableAppointments in the repository to see if it includes any validation logic for office IDs.

---

🏁 Script executed:

#!/bin/bash
# Searching for the implementation of validateGetAvailableAppointments to check office ID validation.
rg -n "function validateGetAvailableAppointments" .

Length of output: 364

---

I'll now check the implementation of validateGetAvailableAppointments in the ValidationService to see whether it validates the office IDs.

---

🏁 Script executed:

#!/bin/bash
# Print context for validateGetAvailableAppointments function in ValidationService.php to inspect office ID validation
rg -A 20 -B 20 "public static function validateGetAvailableAppointments" zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php

Length of output: 1232

---

I'll now inspect the implementation of the isValidOfficeIds function to see if it properly validates the office IDs.

---

🏁 Script executed:

#!/bin/bash
# Search for the implementation of isValidOfficeIds in ValidationService.php to check office ID validation details.
rg -n "function isValidOfficeIds" zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php

Length of output: 177

---

Input Validation for Office IDs Already Handled

The office IDs processed in AvailableAppointmentsListService are later passed to ValidationService::validateGetAvailableAppointments, which internally calls isValidOfficeIds to check their validity (format, emptiness, etc.). Given this design, the concern raised in the review comment is already addressed via centralized validation.

zmscitizenapi/src/Zmscitizenapi/Models/Office.php (2)

24-38: LGTM! Well-documented properties with proper type hints.

Properties are:

• Properly documented with PHPDoc
• Use appropriate nullability
• Have clear and descriptive names

---

83-97: LGTM! Consistent serialization of new properties.

The toArray method properly includes all new properties while maintaining the existing structure.

zmsadmin/templates/block/process/info.twig (1)

63-64: Verify unconditional redirect button.

The redirect button is now always visible, regardless of the process state. Please confirm this is intentional and won't lead to invalid states when clicked.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Search for any conditions or checks related to process redirection in the codebase
rg -A 5 "workstationProcessRedirect|button-finish" --type php --type js

Length of output: 1610

---

Unconditional Redirect Button Verified

The search confirms that the route for "workstationProcessRedirect" is defined and used in the backend (see zmsadmin/routing.php and WorkstationProcessRedirect). There is no additional conditional logic in the twig template hiding or controlling the button's display. This indicates that the always-visible redirect button is an intentional design choice, with error handling managed by the redirect controller.

zmsapi/src/Zmsapi/ProcessRedirect.php (1)

57-57: Verify process reference integrity.

The change to use queue['number'] as the primary identifier with id as fallback could affect process tracking. Please ensure:

1. All consumers of redirected processes can handle queue numbers
2. No process references are broken by this change

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Search for process ID references in redirected contexts
ast-grep --pattern 'redirectToScope($_, $_, $process->id, $_)'
rg -A 5 "redirectToScope.*process.*id"

Length of output: 105

---

I'm not seeing any output from the initial search, so I'd like to gather further context. Please run the script below to list all invocations of the redirectToScope function and to show the context where the new process identifier (using the queue number with fallback to id) is used. This will help verify that all consumers of the redirected process handle the queue number appropriately.

Once you review the output, please verify if the new identifier logic is consistently applied and that no consumers rely solely on $process->id for process tracking.

---

🏁 Script executed:

#!/bin/bash
echo "=== Listing all redirectToScope invocations with context ==="
rg -A10 "redirectToScope\("

echo "=== Showing instances of queue['number'] usage ==="
rg -n "\$process->queue\['number'\]" 

echo "=== (Optional) Showing instances of process->id usage in similar contexts ==="
rg -A10 "\$process->id"

Length of output: 1955

---

Process Reference Integrity Verified

• The invocation in ProcessRedirect.php now passes ($process->queue['number'] ?? $process->id) to redirectToScope, which expects an integer parameter.
• The function definition in zmsdb/src/Zmsdb/Process.php confirms the third parameter (waitingNumber) is declared as int.
• No additional invocations or dependencies relying solely on $process->id were found.

Based on the gathered context, the change appears consistent and should not break process tracking.

zmsadmin/js/index.js (1)

29-29: LGTM! The emergency end feature is properly integrated.

The changes follow the established patterns for view initialization in the admin interface.

Also applies to: 112-114

zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficesListControllerTest.php (1)

44-48: LGTM! Test cases properly cover the new office fields.

The test data is comprehensive and includes appropriate default values for the new fields.

Also applies to: 85-89

zmsdb/src/Zmsdb/Helper/CalculateSlots.php (1)

169-169: Verify the impact of reducing the slot cancellation window.

The time window for slot cancellation has been reduced from 10 minutes to 5 minutes. This change could affect:

1. System performance and load
2. User experience during emergency scenarios
3. Slot availability calculations

Please ensure this change has been:

• Tested with high load scenarios
• Validated with stakeholders
• Documented in release notes

Run the following script to check for any related configurations or dependencies:

❌ Verification inconclusive

🏁 Script executed:

#!/bin/bash
# Description: Search for related slot timing configurations

# Search for slot timing configurations
rg -A 5 "slot.*time|time.*slot" --type php

# Search for emergency-related configurations
rg -A 5 "emergency.*cancel|cancel.*emergency" --type php

Length of output: 22333

---

Verify the impact of the reduced cancellation window.

The new default value of "+5 minutes" in writeCanceledSlots (down from "+10 minutes") isn’t contradicted by any other slot timing or emergency cancellation configurations based on our search. However, since this change directly affects how quickly slots are marked canceled, please ensure the following:

• Confirm that load and performance tests (e.g., those in zmsdb/tests/Zmsdb/SlotTest.php) cover scenarios where the cancellation cutoff is critical.
• Validate the behavior in emergency scenarios to ensure that the reduced window doesn’t inadvertently cause unexpected slot unavailability.
• Verify with stakeholders that the 5‐minute window aligns with current user experience and operational requirements.
• Update any relevant documentation or release notes to reflect this significant change.

zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficesServicesRelationsControllerTest.php (1)

44-48: LGTM! Test data structure enhanced with new office fields.

The test data has been properly updated to include the new office-related fields (showAlternativeLocations, displayNameAlternatives, organization, organizationUnit, slotTimeInMinutes). The values are consistent across test cases.

Also applies to: 85-89

zmscitizenapi/tests/Zmscitizenapi/Services/Core/ZmsApiFacadeServiceTest.php (1)

67-68: LGTM! Provider test data updated with showAlternativeLocations field.

The test data has been properly updated to include the new showAlternativeLocations field in the provider data.

zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficeListByServiceControllerTest.php (1)

45-49: LGTM! Test data structure enhanced with new office fields.

The test data has been properly updated to include the new office-related fields (showAlternativeLocations, displayNameAlternatives, organization, organizationUnit, slotTimeInMinutes). The values are consistent across test cases.

Also applies to: 106-110, 144-148

zmscitizenapi/routing.php (1)

312-352: Verify the response schema for the new endpoint.

The route is well-documented and follows the existing pattern. However, the response schema reference points to the same schema as /available-appointments/.

Please verify if the response structure for grouped appointments by office matches the referenced schema:

❌ Verification inconclusive

🌐 Web query:

What is the schema structure for "schema/citizenapi/availableAppointments.json"? Does it support grouping appointments by office?

💡 Result:

The provided search results don't explicitly mention a "schema/citizenapi/availableAppointments.json" file or its structure. However, based on general schema patterns from Microsoft Graph Bookings APIs and JSON schema principles in Cedar Policy Language:

### Schema Structure Patterns
1. **Core Appointment Attributes** would likely include:
   - `appointmentId`
   - `startDateTime`
   - `endDateTime`
   - `officeId` (for grouping)
   - `serviceType`
   - `availableSlots`

2. **Grouping by Office** could be implemented using:
   ```json
   {
     "type": "object",
     "properties": {
       "offices": {
         "type": "array",
         "items": {
           "$ref": "#/definitions/OfficeGroup"
         }
       }
     },
     "definitions": {
       "OfficeGroup": {
         "type": "object",
         "properties": {
           "officeId": {"type": "string"},
           "appointments": {
             "type": "array",
             "items": {"$ref": "#/definitions/Appointment"}
           }
         }
       }
     }
   }
   ```

### Evidence for Grouping Support
- Microsoft Graph Bookings APIs show explicit support for **group appointments** through attributes like:
  - `maximumAttendeesCount` (inherited from service definitions)
  - `filledAttendeesCount` [3]
- ServiceNow's appointment system demonstrates nested resource organization through API endpoints that could support office-grouped appointments [7]

While no direct example exists in the search results, these patterns suggest JSON Schema could easily accommodate office-grouped appointments through array structures containing office-specific appointment collections[1][3][7].

Citations:

- 1: https://docs.cedarpolicy.com/schema/json-schema.html
- 2: https://community.dynamics.com/forums/thread/details/?threadid=6c5142bb-6584-4435-8f1c-f2ac9a6b890e
- 3: https://devblogs.microsoft.com/microsoft365dev/new-features-in-microsoft-graph-bookings-apis-provide-enhanced-booking-infrastructure/
- 4: https://www.jotform.com/blog/government-scheduling-software/
- 5: https://www.servicenow.com/community/developer-blog/task-appointment-scheduling/ba-p/2279379
- 6: https://wiki.citizensnpcs.co/API
- 7: https://www.servicenow.com/community/field-service-management-forum/appointment-booking-api-documentation-and-usage/m-p/315156
- 8: https://downloads.apiabroad.com/enroll/ld/API%20Spain%20Student%20VIsa%20FAQ.pdf
- 9: https://wiki.citizensnpcs.co/Frequently_Asked_Questions
- 10: https://ofac.treasury.gov/faqs/all-faqs

---

Schema Verification Needed: Confirm Grouping Structure Support

Based on our findings, the referenced JSON schema (“schema/citizenapi/availableAppointments.json”) isn’t explicitly documented to support grouping appointments by office. In typical grouped schemas you’d expect an additional level of nesting—such as an "offices" array where each element holds an office identifier and its associated appointments—whereas the schema for the non‐grouped endpoint likely just lists appointments.

Please verify whether the backend implementation for this grouped endpoint actually formats the results accordingly. If the response is grouped by office as intended, the JSON schema should be updated (or replaced with a dedicated grouped schema) to reflect that structure.

zmsadmin/templates/block/scope/form.twig (1)

630-648: LGTM! Clear and informative UI changes.

The emergency functionality updates improve user understanding and JavaScript interactivity:

• Data attributes provide necessary context to JavaScript
• Updated label clearly explains the impact of disabling the function
• New class enables proper event handling

zmsdb/migrations/91737550001-add-indexes.sql (1)

1-4: SQL Indexes Creation: Confirming Optimal Indexes
The new index creation statements are syntactically correct and likely to improve query performance on the indicated columns. Ensure that these indexes do not conflict with any previously existing ones in production if similar indexes were already defined.

[coderabbitai]

🛠️ Refactor suggestion

Narrowed return type for getOffices

Changing from “OfficeList|array” to “OfficeList” is a good step toward stricter typing. Verify that no callers rely on the old array return type, as this might introduce a breaking change.

[coderabbitai]

🛠️ Refactor suggestion

⚠️ Potential issue

JSON Schema: Misplaced "required" Fields
There is an inconsistency with the "required" declarations. The root-level "required": ["appointmentTimestamps"] does not match any direct property (the only declared property is "offices"). Similarly, within the "items" definition, "required": ["offices"] is likely incorrect because the expected mandatory properties in each office item are "id" and "appointmentTimestamps".
Please consider the following suggested changes:

-  "required": ["appointmentTimestamps"],
+  "required": ["offices"],

-        "required": ["offices"]
+        "required": ["id", "appointmentTimestamps"]

These adjustments ensure that the required fields accurately reflect the schema structure and the intended business rules.

📝 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

	{
	"title": "AvailableAppointmentsByOffice",
	"type": ["array", "object", "null"],
	"properties": {
	"offices": {
	"type": ["array", "object", "null"],
	"description": "Offices which have free time slots",
	"items": {
	"type": ["array", "object", "null"],
	"properties": {
	"id": {
	"type": "integer",
	"description": "Office ID"
	},
	"appointmentTimestamps": {
	"type": ["array", "null"],
	"description": "Numeric timestamps (seconds) for each available slot.",
	"items": {
	"type": "integer"
	}
	}
	},
	"required": ["offices"]
	}
	}
	},
	"required": ["appointmentTimestamps"],
	"description": "Schema defining the available appointments for a specific date, office, and service"
	}
	{
	"title": "AvailableAppointmentsByOffice",
	"type": ["array", "object", "null"],
	"properties": {
	"offices": {
	"type": ["array", "object", "null"],
	"description": "Offices which have free time slots",
	"items": {
	"type": ["array", "object", "null"],
	"properties": {
	"id": {
	"type": "integer",
	"description": "Office ID"
	},
	"appointmentTimestamps": {
	"type": ["array", "null"],
	"description": "Numeric timestamps (seconds) for each available slot.",
	"items": {
	"type": "integer"
	}
	}
	},
	"required": ["id", "appointmentTimestamps"]
	}
	}
	},
	"required": ["offices"],
	"description": "Schema defining the available appointments for a specific date, office, and service"
	}