main into next

[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

• Documentation

  • Updated the changelog with a new release section detailing addressed issues, tasks, and enhancements.

• UI Improvements

  • Enhanced emergency features with conditionally displayed buttons and clarified labels.
  • Modified the interface so the "Weiterleiten" link now appears unconditionally, streamlining user interaction.

• API Enhancements

  • Introduced a new endpoint to retrieve available appointments grouped by office with additional office details.

• Performance & Behavior

  • Improved queue ordering and refresh behavior.
  • Adjusted slot cancellation timing for a quicker transition.

[coderabbitai]

Walkthrough

This pull request introduces a new release section to the changelog and implements various code enhancements across multiple modules. In the ZMS admin interface, a new JavaScript class is added to manage emergency cancellation via AJAX and event handlers for reload timers have been removed. On the backend, a sorting mechanism is introduced in the QueueTable. The citizen API sees a new route and controller for retrieving available appointments by office, along with updated validation, mapping, and model properties. Additionally, database indexes, test adjustments, and JSON schema extensions have been applied.

Changes

File(s)	Change Summary
CHANGELOG.md	Added new "Release Muc-20" section with subsections (Bug, Aufgabe, Story) listing issue references.
zmsadmin/js/block/scope/emergencyend.js, zmsadmin/js/index.js	Introduced EndEmergencyView class and integrated its instantiation for handling emergency end cancellation via AJAX.
zmsadmin/js/page/counter/index.js, zmsadmin/js/page/workstation/index.js	Removed event handlers (window.onblur, mouseenter/leave) that managed the reload timer.
zmsadmin/src/Zmsadmin/QueueTable.php	Added a new sorting mechanism using uasort on the $queueList based on arrivalTime.
zmsadmin/templates/block/emergency/emergency.twig, process/info.twig, scope/form.twig	Modified Twig templates to conditionally render emergency elements, adjust labels, and unconditionally display the "Weiterleiten" link.
zmsapi/src/Zmsapi/ProcessRedirect.php	Updated redirection logic to use the process’ queue number if available.
Multiple files in zmscitizenapi/	Added a new route /available-appointments-by-office/, new controller (AvailableAppointmentsListByOfficeController), model (AvailableAppointmentsByOffice), and updated services, validations, and mapping logic to handle arrays of office IDs and extra office properties. Also, middleware tests were commented out and test expectations extended.
zmsdb/migrations/91737550001-add-indexes.sql, zmsdb/src/Zmsdb/Helper/CalculateSlots.php, zmsdb/tests/SlotTest.php	Added new SQL indexes to improve query performance, reduced cancellation time modifier from “+10 minutes” to “+5 minutes”, and removed tests related to slot cancellation.
zmsentities schema files	Extended JSON schemas to include new office properties (showAlternativeLocations, displayNameAlternatives, organization, organizationUnit, slotTimeInMinutes).
zmsentities/src/Zmsentities/Schema/Entity.php	Updated withCleanedUpFormData to check if keys exist before unsetting them, improving robustness.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant E as EndEmergencyView
    participant S as Server
    U->>E: Click ".emergency__button-end"
    E->>E: Call update() to modify state
    E->>S: AJAX GET request (using includeurl & scope.id)
    S-->>E: Return acknowledgement
    E->>U: Update UI state accordingly

Loading

sequenceDiagram
    participant C as Client
    participant R as API Router
    participant AC as AvailableAppointmentsController
    participant Srv as Availability Service
    participant DB as Database
    C->>R: GET /available-appointments-by-office?params
    R->>AC: Route request to controller
    AC->>Srv: Call getAvailableAppointmentsListByOffice(query)
    Srv->>DB: Query available appointment data
    DB-->>Srv: Return grouped appointments data
    Srv-->>AC: Respond with results
    AC-->>C: JSON response with appointment groupings

Loading

Possibly related PRs

• Zms 3519 clusteransicht sortierung uhrzeit #815 – Adjustments in the QueueTable class sorting logic align with this PR’s changes on ordering issues based on arrivalTime.
• feat(ZMS-3459): return slotTimeInMinutes #779 – Introduction of the slotTimeInMinutes property in the Office class is closely related to similar updates in this PR.

Suggested reviewers

• MoDaae

Poem

I'm just a rabbit, hopping with delight,
Celebrating changes that shine so bright.
New codes bloom like carrots in spring,
Bugs scurry off with each tweak we bring.
With fresh features and indexes in sight,
I hop and cheer – our code feels just right! 🐇

✨ 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: 4

🧹 Nitpick comments (26)

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

156-172: Replace eval-based mocking with PHPUnit's mocking framework.

Using eval for mocking is not recommended as it:

• Makes the tests harder to maintain
• Can hide type-related issues
• Poses potential security risks

Consider using PHPUnit's built-in mocking:

$validationService = $this->getMockBuilder(ValidationService::class)
    ->disableOriginalConstructor()
    ->addMethods(['validateGetBookableFreeDays'])
    ->getMock();

$validationService->method('validateGetBookableFreeDays')
    ->willReturn($returnValue);

---

174-190: Improve mocking and test coverage for ZmsApiFacadeService.

1. Replace eval-based mocking with PHPUnit's mocking framework
2. Add test cases for error scenarios where the service returns an array instead of AvailableDays

Consider using PHPUnit's built-in mocking:

$facadeService = $this->getMockBuilder(ZmsApiFacadeService::class)
    ->disableOriginalConstructor()
    ->addMethods(['getBookableFreeDays'])
    ->getMock();

$facadeService->method('getBookableFreeDays')
    ->willReturn($returnValue);

Add test cases to verify error handling:

public function testGetAvailableDaysListHandlesServiceErrors(): void
{
    // Arrange
    $queryParams = [
        'officeId' => '123,456',
        'serviceId' => '456,789',
        'serviceCount' => '1,2',
        'startDate' => '2025-01-01',
        'endDate' => '2025-01-31'
    ];
    
    $expectedError = ['errors' => ['Service unavailable']];
    
    $this->createMockFacade($expectedError);
    
    // Act
    $result = $this->service->getAvailableDaysList($queryParams);
    
    // Assert
    $this->assertIsArray($result);
    $this->assertEquals($expectedError, $result);
}

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

111-121: Add test cases for additional edge cases with office ID arrays.

While the invalid office ID test case is good, consider adding tests for these scenarios:

• Empty array of office IDs
• Array with multiple office IDs
• Array with mixed valid and invalid office IDs

Here's a suggested implementation:

 // Test invalid office ID
 $result = ValidationService::validateGetAvailableAppointments(
     '2025-01-01',
     [''],
     [1],
     [1]
 );
 $this->assertContains(
     ErrorMessages::get('invalidOfficeId'),
     $result['errors']
 );
+
+// Test empty office ID array
+$result = ValidationService::validateGetAvailableAppointments(
+    '2025-01-01',
+    [],
+    [1],
+    [1]
+);
+$this->assertContains(
+    ErrorMessages::get('invalidOfficeId'),
+    $result['errors']
+);
+
+// Test multiple office IDs
+$result = ValidationService::validateGetAvailableAppointments(
+    '2025-01-01',
+    [1, 2, 3],
+    [1],
+    [1]
+);
+$this->assertEmpty($result['errors']);
+
+// Test mixed valid and invalid office IDs
+$result = ValidationService::validateGetAvailableAppointments(
+    '2025-01-01',
+    [1, '', 3],
+    [1],
+    [1]
+);
+$this->assertContains(
+    ErrorMessages::get('invalidOfficeId'),
+    $result['errors']
+);

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

446-457: Service array construction.

Iterating over service IDs and pairing each with its slot count is a logical approach. Consider adding checks if mismatched array lengths might occur, or if $serviceCounts is missing an index.

---

459-464: Provider array construction.

Building parallel provider structures for multiple offices. This is consistent but consider potential future expansions like additional provider metadata.

---

561-600: Refactored processFreeSlots with grouping logic.

This function now aggregates timestamps by provider ID. The approach is coherent, but watch out for edge cases such as empty appointment arrays. Consider robust logging for debugging.

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

1-2: Importing jQuery.

Using jQuery makes sense if the rest of the codebase depends on it. Consider migrating to native fetch or modern frameworks if future features demand it.

---

3-13: Class constructor logic.

Storing options and binding the click event here is straightforward. Consider input validation for the options object in case properties are missing or undefined.

---

20-22: update method merges new data.

Merging new data into this.data is concise. Consider immutability patterns or deeper merges if nested objects are introduced in the future.

---

24-37: sendEmergencyCancel with jQuery AJAX.

Returning a Promise for this asynchronous operation is helpful. For production readiness, consider robust error-handling or user notifications upon failure.

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

16-16: Consider revising the property type declaration.

The property is declared as array|null but initialized with an empty array. This creates ambiguity about when the property could be null. Consider either:

1. Removing the null type if it's never expected to be null
2. Not initializing with an empty array if null is a valid state

-    public array|null $officeAppointments = [];
+    public array $officeAppointments = [];

---

29-34: Enhance error message with validation details.

The current error message is generic. Consider including specific validation errors to help identify the exact issue.

     private function ensureValid(): void
     {
-        if (!$this->testValid()) {
-            throw new InvalidArgumentException('The provided data is invalid according to the schema.');
+        $validationResult = $this->testValid();
+        if (!$validationResult) {
+            $errors = $this->getErrors();
+            throw new InvalidArgumentException(
+                'The provided data is invalid according to the schema: ' . implode(', ', $errors)
+            );
         }
     }

---

44-50: Consider extracting the array mapping logic to a named method.

The array mapping logic in toArray() could be more readable if extracted to a separate method with a descriptive name.

     public function toArray(): array
     {
         return [
-            'offices' => array_map(function($appointments, $officeId) {
-                return [
-                    'officeId' => $officeId,
-                    'appointments' => $appointments
-                ];
-            }, $this->officeAppointments, array_keys($this->officeAppointments))
+            'offices' => $this->mapOfficeAppointments()
         ];
     }
+
+    private function mapOfficeAppointments(): array
+    {
+        return array_map(
+            function($appointments, $officeId) {
+                return [
+                    'officeId' => $officeId,
+                    'appointments' => $appointments
+                ];
+            },
+            $this->officeAppointments,
+            array_keys($this->officeAppointments)
+        );
+    }

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

15-15: Consider making the service injectable for better testability.

The service is currently instantiated in the constructor. Consider injecting it for better testability and dependency management.

-    private AvailableAppointmentsListService $service;
+    private readonly AvailableAppointmentsListService $service;

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

---

35-41: Consider improving readability of the response creation logic.

The nested ternary operator makes the code harder to read. Consider using an if-else structure or extracting the logic to a separate method.

-        return is_array($result) && isset($result['errors'])
-            ? $this->createJsonResponse(
-                $response,
-                $result,
-                ErrorMessages::getHighestStatusCode($result['errors'])
-            )
-            : $this->createJsonResponse($response, $result->toArray(), 200);
+        if (is_array($result) && isset($result['errors'])) {
+            return $this->createJsonResponse(
+                $response,
+                $result,
+                ErrorMessages::getHighestStatusCode($result['errors'])
+            );
+        }
+
+        return $this->createJsonResponse($response, $result->toArray(), 200);

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

26-31: Consider using null coalescing operator for cleaner type casting.

The current type casting logic can be simplified using the null coalescing operator.

-        $queryParams['officeId'] = isset($queryParams['officeId']) ? (string) $queryParams['officeId'] : '';
-        $queryParams['serviceId'] = isset($queryParams['serviceId']) ? (string) $queryParams['serviceId'] : '';
+        $queryParams['officeId'] = (string) ($queryParams['officeId'] ?? '');
+        $queryParams['serviceId'] = (string) ($queryParams['serviceId'] ?? '');

---

53-53: Consider maintaining consistent return type order.

The return type order has been changed from array|AvailableDays to AvailableDays|array. Consider maintaining consistency with other similar methods in the codebase.

-    private function getAvailableDays(object $data): AvailableDays|array
+    private function getAvailableDays(object $data): array|AvailableDays

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

29-37: Consider extracting array mapping logic to a helper method.

The array mapping logic is repeated for multiple parameters. Consider extracting it to a reusable helper method.

+    private function extractArrayFromParam(array $queryParams, string $key): array
+    {
+        return isset($queryParams[$key])
+            ? array_map('trim', explode(',', (string) $queryParams[$key]))
+            : [];
+    }
+
     private function extractClientData(array $queryParams): object
     {
         return (object) [
             'date' => isset($queryParams['date']) ? (string) $queryParams['date'] : null,
-            'officeIds' => isset($queryParams['officeId'])
-                ? array_map('trim', explode(',', (string) $queryParams['officeId']))
-                : [],
-            'serviceIds' => isset($queryParams['serviceId'])
-                ? array_map('trim', explode(',', (string) $queryParams['serviceId']))
-                : [],
-            'serviceCounts' => isset($queryParams['serviceCount'])
-                ? array_map('trim', explode(',', (string) $queryParams['serviceCount']))
-                : []
+            'officeIds' => $this->extractArrayFromParam($queryParams, 'officeId'),
+            'serviceIds' => $this->extractArrayFromParam($queryParams, 'serviceId'),
+            'serviceCounts' => $this->extractArrayFromParam($queryParams, 'serviceCount')
         ];
     }

---

64-74: Consider using composition over inheritance for the new method.

The new method getAvailableAppointmentsListByOffice duplicates most of the logic from getAvailableAppointmentsList. Consider using composition to reuse the existing method.

     public function getAvailableAppointmentsListByOffice($queryParams): AvailableAppointmentsByOffice|array
     {
-        $clientData = $this->extractClientData($queryParams);
-
-        $errors = $this->validateClientData($clientData);
-        if (!empty($errors['errors'])) {
-            return $errors;
-        }
-
-        return $this->getAvailableAppointments($clientData, true);
+        $result = $this->getAvailableAppointmentsList($queryParams);
+        if (is_array($result) && isset($result['errors'])) {
+            return $result;
+        }
+        return $this->getAvailableAppointments($this->extractClientData($queryParams), true);
     }

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

67-68: Add assertion for showAlternativeLocations.

While the test data structure has been updated to include showAlternativeLocations, there's no explicit assertion verifying this property in the test assertions block.

Add this assertion after line 103:

 $this->assertEquals(['lat' => 48.137154, 'lon' => 11.576124], $office->geo);
+$this->assertFalse($office->showAlternativeLocations);
 $this->assertInstanceOf(ThinnedScope::class, $office->scope);

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

45-49: Consider adding assertions for the new fields.

While the test cases include the new fields in the expected response, there are no specific assertions to verify their values. Consider adding test cases that explicitly validate different scenarios for these fields:

• showAlternativeLocations: Test both true/false cases
• displayNameAlternatives: Test with non-empty array
• organization and organizationUnit: Test with non-null values
• slotTimeInMinutes: Test with different time slots

Also applies to: 106-110, 144-148

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

311-313: LGTM! Good defensive programming practice.

The added null check before unsetting the 'save' key prevents potential warnings and is consistent with the existing check for 'removeImage'.

Consider extracting the cleanup logic to a separate method for better maintainability:

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

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

66-70: LGTM! Consider adding documentation for the new fields.

The new fields are properly initialized using null coalescing operators, maintaining consistency with the existing mapping pattern.

Consider adding PHPDoc comments to document the purpose and expected values of the new fields:

+/**
+ * Maps provider data to Office model
+ * @param ProviderList $providerList List of providers
+ * @return OfficeList
+ * 
+ * @property bool $showAlternativeLocations Whether to show alternative locations
+ * @property array $displayNameAlternatives List of alternative display names
+ * @property string|null $organization Organization name
+ * @property string|null $organizationUnit Organization unit name
+ * @property int|null $slotTimeInMinutes Duration of appointment slots in minutes
+ */
public static function mapOfficesWithScope(ProviderList $providerList): OfficeList

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

1-4: LGTM! Well-chosen indexes for optimizing query performance.

The added indexes will improve query performance for:

• Date-based slot lookups by scope and status
• Hierarchical slot queries
• Status-based slot lookups
• Slot process lookups

Consider monitoring the impact of these indexes on:

• Query performance improvements
• Write performance overhead
• Index size and maintenance costs

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

86-92: Fix inconsistent description for organizationUnit.

The description "The name of the organization" is identical to the organization property's description. Consider updating it to "The name of the organization unit" to match the property name.

-            "description": "The name of the organization"
+            "description": "The name of the organization unit"

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

87-93: Fix inconsistent description for organizationUnit.

The description needs to be updated to match the property name, similar to the change needed in officeList.json.

-            "description": "The name of the organization"
+            "description": "The name of the organization unit"

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4f399e2 and 548ea6d.

⛔ 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 (38)

• CHANGELOG.md (1 hunks)
• 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)

• zmsdb/tests/Zmsdb/SlotTest.php
• zmsadmin/js/page/workstation/index.js
• 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 context used

🧠 Learnings (2)

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

Learnt from: ThomasAFink
PR: it-at-m/eappointment#0
File: :0-0
Timestamp: 2025-02-11T15:45:58.866Z
Learning: The ZmsApiFacadeService methods getAvailableAppointments() and getBookableFreeDays() expect single office IDs (?int and int respectively) and cannot handle arrays of office IDs.

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

Learnt from: ThomasAFink
PR: it-at-m/eappointment#0
File: :0-0
Timestamp: 2025-02-11T15:45:58.866Z
Learning: The ZmsApiFacadeService methods getAvailableAppointments() and getBookableFreeDays() expect single office IDs (?int and int respectively) and cannot handle arrays of office IDs.

🔇 Additional comments (38)

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

29-35: Add test cases for multiple office and service IDs.

The test cases currently only test single values for officeId and serviceId, while the implementation now supports arrays. Add test cases that verify the handling of multiple office and service IDs.

Consider adding test cases with:

• Multiple comma-separated office IDs: 'officeId' => '123,456'
• Multiple comma-separated service IDs: 'serviceId' => '456,789'

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

91-97: LGTM! Test case properly validates the new array parameter.

The test case has been correctly updated to use an array for the office ID parameter, maintaining proper test coverage for the modified method signature.

---

88-146:

✅ Verification successful

Verify test coverage in the implementation.

The test changes look good, but let's verify that the implementation in ValidationService correctly handles the array parameter.

---

🏁 Script executed:

#!/bin/bash
# Description: Find and analyze the implementation of validateGetAvailableAppointments

# Search for the implementation
ast-grep --pattern 'function validateGetAvailableAppointments($date, $officeIds, $serviceIds, $serviceCounts)' -A 10

Length of output: 118

---

Let's try a broader search using rg to locate the implementation, as the AST-based search didn't return any results. This will help verify whether the function exists in the codebase and correctly handles the array parameter:

---

🏁 Script executed:

#!/bin/bash
# Using ripgrep to locate the definition of validateGetAvailableAppointments
rg -n "function\s+validateGetAvailableAppointments" .

Length of output: 366

---

Let's inspect the implementation snippet of validateGetAvailableAppointments from ValidationService.php to verify that the function correctly handles the array parameters. We'll print a snippet of the file to examine how the arrays are processed:

---

🏁 Script executed:

#!/bin/bash
# Print the implementation snippet for validateGetAvailableAppointments from ValidationService.php
sed -n '140,250p' zmscitizenapi/src/Zmscitizenapi/Services/Core/ValidationService.php

Length of output: 3203

---

Test coverage confirmed: Array parameter validation works as expected

The tests in ValidationServiceTest.php align perfectly with the implementation in ValidationService.php. The function correctly validates the array parameters using isValidOfficeIds, isValidServiceIds, and isValidServiceCounts. No further changes are required.

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

88-89: Ensure consistent handling of multiple IDs in method signature.

Changing parameters from single IDs to arrays is a good improvement for bulk operations. Keep an eye on all downstream code to confirm proper handling of these parameters.

---

96-97: Validate presence of office IDs.

This check suitably prevents empty or invalid office ID arrays. No issues spotted.

---

100-101: Validate presence of service IDs.

Similarly, this ensures the service ID arrays are present and contain numeric values. Looks correct.

---

146-146: Parameter change for validateGetAvailableAppointments.

Adding an array of office IDs is consistent with the multi-ID approach. Ensure all call sites reflect the updated signature.

---

156-156: Validation of multiple office IDs.

This additional check aligns with the multi-office logic. Keep an eye on future expansions that might require more complex validation rules.

---

360-362: Implementation of isValidOfficeIds.

Using isValidNumericArray is concise and effective for validating multiple office IDs.

---

425-428: Reintroduction of isValidOfficeId.

Retaining single office ID validation is beneficial for backward compatibility and single-ID checks in other parts of the code.

---

430-433: Reintroduction of isValidServiceId.

Likewise, this helps maintain single service ID validation. Retaining both single and multi-ID validations is a flexible approach.

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

8-8: New imports for grouped office appointments.

Imports for new models (e.g., AvailableAppointmentsByOffice, ProcessFreeSlotsGroupByOffice) will help manage the new grouping logic. No issues here.

Also applies to: 13-13

---

46-46: Refined return type for getOffices.

Returning a strict OfficeList instead of a union type can simplify usage. Check for any legacy consumers expecting arrays.

---

66-70: Extended Office constructor fields.

Adding fields (e.g., showAlternativeLocations, organization) improves data completeness. Ensure the front end or downstream code uses these appropriately.

---

438-443: Expanded method signature for getBookableFreeDays.

Shifting to arrays for office IDs and service IDs is consistent with the multi-ID handling. Confirm that all usage sites properly pass arrays rather than single IDs.

---

467-468: Refined call to ZmsApiClientService::getFreeDays.

Passing ProviderList and RequestList created from arrays is consistent with the new multi-office multi-service approach.

---

516-524: Extended parameter set for getAvailableAppointments.

Allowing groupByOffice, along with arrays for office and service IDs, significantly improves flexibility. Ensure all calling code is updated accordingly.

---

535-541: Populating providers from office IDs.

Adding each office ID into a new ProviderList is correct. Consider how potential exceptions or invalid IDs are handled upstream or validated in the ValidationService.

---

554-557: Conditional return of AvailableAppointmentsByOffice.

This is a clean design: the groupByOffice flow returns a distinct model. Ensure the front end is prepared to handle both single and grouped sets of results.

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

15-18: endEmergency method.

This method effectively sets the emergency mode to inactive and calls sendEmergencyCancel. Looks good—just ensure that the UI is updated appropriately after resolution.

---

40-40: Exporting EndEmergencyView as default.

Cleanly exposing the class for external usage. Code reveals no concerns here.

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

24-38: Well-structured property declarations with proper type hints!

The new properties are well-documented with PHPDoc and use appropriate type hints with nullability.

---

45-69: LGTM! Constructor properly handles new properties.

The constructor is well-structured with:

• Type-hinted parameters matching property types
• Proper nullability
• Consistent initialization order

---

89-94: LGTM! Consistent serialization of new properties.

The toArray method properly includes all new properties maintaining consistency with the model structure.

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

10-14: Good defensive programming!

The conditional check ensures the emergency button is only shown when there's a valid workstation context, preventing potential issues with emergency triggers.

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

63-63: Verify the removal of appointment condition for redirect link

The "Weiterleiten" link is now always visible, regardless of appointment status. Please verify this change aligns with the intended business logic.

Was the removal of the workstation.process.withAppointment condition intentional? This change makes the redirect functionality available in all cases.

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

53-55: LGTM! Efficient implementation of queue sorting.

The use of uasort with timestamp comparison is an appropriate and efficient way to sort the queue list while preserving array keys.

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

57-57: LGTM! Safe implementation with fallback.

The use of null coalescing operator (??) provides a clean fallback to process->id when queue number is not available.

zmsadmin/js/index.js (1)

29-29: LGTM! Consistent implementation of emergency end functionality.

The import and initialization of EmergencyEnd follows the established pattern used for other components in the codebase.

Also applies to: 112-114

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

44-48: LGTM! Test coverage expanded for new office properties.

The test has been properly updated to include validation for new office properties: showAlternativeLocations, displayNameAlternatives, organization, organizationUnit, and slotTimeInMinutes. The test data is consistent across different office entries.

Also applies to: 85-89

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

169-169:

❓ Verification inconclusive

Verify test coverage for reduced slot cancellation time.

The change from '+10 minutes' to '+5 minutes' reduces the slot cancellation window, allowing slots to be booked closer to their start time. This aligns with task ZMS-3272 but requires careful testing.

Please ensure:

1. Test cases cover the new 5-minute window behavior
2. Integration tests validate the impact on slot availability
3. No regressions in slot cancellation functionality

Run this script to check for related test coverage:

---

🏁 Script executed:

#!/bin/bash
# Search for test cases related to slot cancellation timing
rg -A 5 "writeCanceledSlots|calculateSlots.*modify" --type php

Length of output: 1158

---

Test Coverage Verification: Reduced Slot Cancellation Window

The update from a default modify parameter of '+10 minutes' to '+5 minutes' directly affects the slot cancellation timing, allowing bookings closer to their start time. The grep search confirms the change in CalculateSlots.php but does not reveal explicit test cases verifying the new 5-minute behavior.

Please ensure the following:

• Unit Tests: Verify that there are dedicated tests covering the 5-minute window logic in the slot cancellation process.
• Integration Tests: Confirm that integration tests validate the impact on slot availability and ensure that no regressions occur in the cancellation functionality.
• Overall Test Coverage: Review and, if necessary, update the test suite to fully exercise the new behavior, aligning with the requirements of task ZMS-3272.

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

44-48: LGTM! Consistent test coverage across controllers.

The test updates mirror those in OfficesListControllerTest.php, maintaining consistency in validating the new office properties across different endpoints.

Also applies to: 85-89

zmscitizenapi/routing.php (1)

312-352:

❓ Verification inconclusive

Verify the response schema reference.

The Swagger documentation references schema/citizenapi/availableAppointments.json for the response schema, but since this endpoint returns appointments grouped by office, it might need to reference a different schema that reflects this structure.

---

🌐 Web query:

Is there a specific JSON Schema for representing appointments grouped by office in the OpenAPI/Swagger specification?

💡 Result:

The OpenAPI Specification (formerly Swagger) does not provide a predefined JSON Schema specifically for representing appointments grouped by office. However, it enables you to create custom schemas using standard JSON Schema features and OpenAPI extensions. Here's how to model this structure effectively:

Core Components

1. Appointment Object:

   Appointment:
     type: object
     properties:
       id:
         type: string
         format: uuid
       startTime:
         type: string
         format: date-time
       endTime:
         type: string
         format: date-time
       client:
         type: string
     required: [id, startTime]  # From TMF646 example [5]

2. Office Grouping Structure:

   OfficeAppointments:
     type: object
     additionalProperties:
       type: array
       items:
         $ref: '#/components/schemas/Appointment'
     description: Key-value pairs where keys are office IDs/names and values are appointment lists
     example:  # Adapted from Actionhero.js example [7]
       "main_office":
         - id: "apt_001"
           startTime: "2025-02-15T09:00:00Z"
           client: "Acme Corp"
       "branch_office":
         - id: "apt_002"
           startTime: "2025-02-15T10:30:00Z" 
           client: "Globex Inc"

Implementation Notes

1. Schema Flexibility:

   • Uses additionalProperties pattern for dynamic office keys [1][9]
   • Supports arbitrary office identifiers while maintaining structured appointment data

2. OpenAPI-Specific Features:

   x-office-grouping-metadata:  # Vendor extension example [2][6]
     groupingStrategy: dynamic
     maxOfficesPerRequest: 50

3. Pagination Support (from TMF646 API [5]):

   parameters:
     - name: limit
       in: query  
       schema:
         type: integer
         minimum: 1
         maximum: 100
     - name: offset
       in: query
       schema:
         type: integer 
         minimum: 0

This approach combines standard JSON Schema capabilities (additionalProperties, $ref) with OpenAPI-specific features like vendor extensions (x- prefixed properties) [2][6], while following patterns seen in real-world implementations like the TM Forum Appointment API [5]. The structure remains flexible for different office identifiers while enforcing consistent appointment data formats.

Citations:

• 1: https://github.com/OAI/OpenAPI-Specification/blob/master/schemas/v2.0/schema.json
• 2: https://github.com/oai/openapi-specification/blob/master/versions/2.0.md
• 3: https://blog.stoplight.io/openapi-json-schema
• 4: https://docs.aws.amazon.com/bedrock/latest/userguide/agents-api-schema.html
• 5: https://github.com/tmforum-apis/TMF646_AppointmentManagement/blob/master/TMF646-Appointment-v4.0.0.swagger.json
• 6: https://docs.bump.sh/guides/openapi/specification/v3.1/data-models/json-schema/
• 7: https://openapi.pteverywhere.com/api/v2/swagger.html
• 8: https://json-schema.org
• 9: https://swagger.io/specification/
• 10: Is type a required field for Schema Objects? OAI/OpenAPI-Specification#1657

---

Subject: Verify the Response Schema for Office-Grouped Appointments

The endpoint’s Swagger documentation currently references schema/citizenapi/availableAppointments.json for the response. However, because this endpoint returns appointments grouped by office, please ensure that the referenced schema accurately models this grouping. OpenAPI doesn’t provide a predefined schema for such grouped data, so verify that:

• The existing schema reflects a mapping of office identifiers to arrays of appointment objects.
• If it only describes a flat list of appointments, consider defining a custom schema (e.g., one using the additionalProperties pattern) to represent the grouped structure appropriately.
• All related documentation and implementations are consistent with this design.

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

630-635: LGTM! The emergency section changes enhance functionality.

The changes add necessary data attributes for AJAX functionality and improve the emergency button label clarity. The new label better explains the consequences of disabling the emergency function.

Also applies to: 643-643, 647-647

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

28-34: LGTM! The new properties enhance the office data model.

The schema additions are well-structured with proper type definitions, nullability, and descriptions.

Also applies to: 72-99

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

28-28: LGTM! Test fixture properly covers both cases.

The test data appropriately includes both true and false values for the new showAlternativeLocations property.

Also applies to: 51-51

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

29-35: LGTM! Schema changes are consistent.

The additions maintain consistency with the officeList.json schema.

Also applies to: 73-100

CHANGELOG.md (1)

1-32: LGTM! Changelog follows proper format.

The new release section maintains consistency with the existing structure and properly documents all changes with JIRA ticket references.

[coderabbitai]

⚠️ Potential issue

Security Warning: Disabled Security Headers and CORS Middleware

Commenting out these security middleware components could expose the application to various security risks:

• Missing security headers could make the application vulnerable to attacks like XSS, clickjacking, etc.
• Disabled CORS middleware might allow unauthorized cross-origin requests

Please either:

1. Re-enable these middleware components, or
2. Document why they were disabled and implement alternative security measures

[coderabbitai]

⚠️ Potential issue

Fix parameter type inconsistency with ZmsApiFacadeService.

The test mocks ValidationService and ZmsApiFacadeService with array parameters for office IDs, but according to the retrieved learning, these services expect single office IDs (?int and int respectively). This inconsistency will cause runtime type errors.

Apply this diff to fix the parameter types:

-                    ?array $officeIds,
+                    ?int $officeId,

-                    ?array $officeIds,
+                    ?int $officeId,

Also applies to: 175-176

[coderabbitai]

⚠️ Potential issue

Fix incorrect required field definitions in schema.

The schema has misplaced required field definitions:

1. "required": ["offices"] is placed at the office item level but "offices" is a root property
2. "required": ["appointmentTimestamps"] is placed at root level but "appointmentTimestamps" is an office item property

Apply this diff to fix the schema:

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

Also applies to: 27-27

[coderabbitai]

⚠️ Potential issue

Fix incorrect description for organizationUnit field.

The description "The name of the organization" is incorrect for the organizationUnit field as it's the same as the organization field's description.

Apply this diff to fix the description:

-            "description": "The name of the organization"
+            "description": "The name of the organizational unit within the organization"

📝 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

	"description": "The name of the organization"
	"description": "The name of the organizational unit within the organization"