Feature zms 3499 auto refresh queue #816
Conversation
…eature-zms-3499-auto-refresh-queue
…ei Mouse-Hovering
WalkthroughThis pull request updates several parts of the application’s configuration, frontend event handling, view templates, service logic, data models, tests, and JSON schemas. Key changes include updating an environment variable to point to a local API endpoint, removing and simplifying UI event handlers for reload timers, adjusting conditional rendering in templates, modifying redirection logic to use an optional queue number, commenting out middleware tests, and expanding the Office entity with additional fields. The changes are spread across JavaScript, PHP, Twig, and JSON schema files. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant PR as ProcessRedirect
participant RTS as redirectToScope Service
C->>PR: Send API request
PR->>PR: Determine identifier (queue number or process id)
PR->>RTS: Call redirectToScope(identifier)
RTS-->>C: Return redirection
sequenceDiagram
participant Client as Client
participant API as ZmsApiFacadeService
participant Mapper as MapperService
participant Office as Office Model
Client->>API: Request offices
API->>Mapper: Map provider data to offices
Mapper->>Office: Instantiate Office (with new fields)
Office-->>Mapper: Office object
Mapper-->>API: OfficeList
API-->>Client: Return OfficeList
Possibly related PRs
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
zmsentities/schema/citizenapi/office.json (1)
50-53: Improve the description of organizationUnit.The description "The name of the organization" is too generic and similar to the organization field's description. Consider making it more specific to clarify the difference between organization and organizationUnit.
Apply this diff to improve the description:
- "description": "The name of the organization" + "description": "The name of the specific unit or department within the organization"zmsentities/schema/citizenapi/collections/officeList.json (1)
69-72: Improve the description of organizationUnit.The description "The name of the organization" is too generic and similar to the organization field's description. Consider making it more specific to clarify the difference between organization and organizationUnit.
Apply this diff to improve the description:
- "description": "The name of the organization" + "description": "The name of the specific unit or department within the organization"zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php (1)
64-67: Consider extracting the Office creation logic to reduce duplication.The Office instantiation logic with the new properties is duplicated in both
getOfficesandgetOfficesThatProvideServicemethods. Consider extracting this into a private helper method.Here's a suggested implementation:
private static function createOfficeFromProvider($provider, ?ThinnedScope $scope = null): Office { return new Office( id: (int) $provider->id, name: $provider->displayName ?? $provider->name, address: $provider->data['address'] ?? null, displayNameAlternatives: $provider->data['displayNameAlternatives'] ?? [], organization: $provider->data['organization'] ?? null, organizationUnit: $provider->data['organizationUnit'] ?? null, slotTimeInMinutes: $provider->data['slotTimeInMinutes'] ?? null, geo: $provider->data['geo'] ?? null, scope: $scope ); }Then update both methods to use this helper:
// In getOffices method -$offices[] = new Office( - id: (int) $provider->id, - name: $provider->displayName ?? $provider->name, - address: $provider->data['address'] ?? null, - displayNameAlternatives: $provider->data['displayNameAlternatives'] ?? [], - organization: $provider->data['organization'] ?? null, - organizationUnit: $provider->data['organizationUnit'] ?? null, - slotTimeInMinutes: $provider->data['slotTimeInMinutes'] ?? null, - geo: $provider->data['geo'] ?? null, - scope: $matchingScope ? new ThinnedScope(...) : null -); +$offices[] = self::createOfficeFromProvider( + $provider, + $matchingScope ? new ThinnedScope(...) : null +); // In getOfficesThatProvideService method -$offices[] = new Office( - id: (int) $provider->id, - name: $provider->displayName ?? $provider->name, - address: $provider->data['address'] ?? null, - displayNameAlternatives: $provider->data['displayNameAlternatives'] ?? [], - organization: $provider->data['organization'] ?? null, - organizationUnit: $provider->data['organizationUnit'] ?? null, - slotTimeInMinutes: $provider->data['slotTimeInMinutes'] ?? null, - geo: $provider->data['geo'] ?? null, - scope: $scope instanceof ThinnedScope ? $scope : null -); +$offices[] = self::createOfficeFromProvider( + $provider, + $scope instanceof ThinnedScope ? $scope : null +);Also applies to: 374-377
zmsentities/schema/citizenapi/collections/officeServiceAndRelationList.json (3)
66-69: Review of 'displayNameAlternatives' property: Specify array item type if applicable.
The new property is correctly added, but it lacks an"items"definition to clarify the expected type of values. If this is meant to be an array of strings, consider adding an"items": { "type": "string" }schema.
74-77: Nitpick on 'organizationUnit' property: Clarify description.
The description fororganizationUnit—"The name of the organization"—is similar to the one fororganization, which may lead to ambiguity. It might be beneficial to differentiate the two (for example, by specifying that this field refers to a subdivision or unit within the organization).
78-81: Review of 'slotTimeInMinutes' property: Consider minimum value constraint.
While the property is correctly defined, it could be enhanced by adding a"minimum": 0constraint to prevent negative slot times if such values are not valid in your domain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
zmsadmin/package-lock.jsonis excluded by!**/package-lock.jsonzmscalldisplay/package-lock.jsonis excluded by!**/package-lock.jsonzmsstatistic/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.ddev/.env.template(1 hunks)zmsadmin/js/page/counter/index.js(0 hunks)zmsadmin/js/page/workstation/index.js(0 hunks)zmsadmin/templates/block/emergency/emergency.twig(1 hunks)zmsadmin/templates/block/process/info.twig(1 hunks)zmsapi/src/Zmsapi/ProcessRedirect.php(1 hunks)zmscitizenapi/bootstrap.php(1 hunks)zmscitizenapi/src/Zmscitizenapi/Models/Office.php(2 hunks)zmscitizenapi/src/Zmscitizenapi/Services/Core/MapperService.php(1 hunks)zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php(3 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)zmsdb/src/Zmsdb/Helper/CalculateSlots.php(1 hunks)zmsentities/schema/citizenapi/collections/officeList.json(1 hunks)zmsentities/schema/citizenapi/collections/officeServiceAndRelationList.json(1 hunks)zmsentities/schema/citizenapi/office.json(1 hunks)
💤 Files with no reviewable changes (2)
- zmsadmin/js/page/workstation/index.js
- zmsadmin/js/page/counter/index.js
✅ Files skipped from review due to trivial changes (3)
- .ddev/.env.template
- zmscitizenapi/tests/Zmscitizenapi/Middleware/SecurityHeadersMiddlewareTest.php
- zmscitizenapi/tests/Zmscitizenapi/Middleware/CorsMiddlewareTest.php
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
zmscitizenapi/src/Zmscitizenapi/Models/Office.php (3)
24-34: LGTM! Well-structured property declarations.The new properties are properly typed and documented with PHPDoc comments.
42-64: LGTM! Constructor maintains backward compatibility.The constructor is well-designed with:
- Type declarations for parameters
- Nullable parameters with default values
- Proper property initialization
78-91: LGTM! Consistent serialization.The toArray method properly includes all new properties in the serialized output.
zmsadmin/templates/block/emergency/emergency.twig (1)
10-14: LGTM! Improved button rendering logic.The conditional check prevents the emergency button from rendering when workstation name is undefined, avoiding potential UI issues.
zmsadmin/templates/block/process/info.twig (1)
63-64: Verify the impact of always showing the redirect button.The "Weiterleiten" button is now always visible, regardless of appointment status. Please confirm if this change is intentional and doesn't affect the business logic.
zmsapi/src/Zmsapi/ProcessRedirect.php (1)
54-59: LGTM! Enhanced process redirection logic.The change improves process tracking by:
- Using queue number as the primary identifier when available
- Safely falling back to process ID using the null coalescing operator
zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficesListControllerTest.php (1)
44-47: LGTM! Test cases updated with new Office model fields.The test cases have been properly updated to include the new Office model fields with appropriate default values.
Also applies to: 84-87
zmsdb/src/Zmsdb/Helper/CalculateSlots.php (1)
169-169: Verify the impact of reduced slot cancellation window.The default time window for canceling slots has been reduced from 10 minutes to 5 minutes. This change could affect:
- User experience - less time to manage appointments
- System behavior - more frequent slot availability updates
Please confirm:
- Is this change intentional?
- Has this been communicated to users?
- Are there any dependent systems that need to be updated?
zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficesServicesRelationsControllerTest.php (1)
44-47: LGTM! Test cases updated with new Office model fields.The test cases have been properly updated to include the new Office model fields with appropriate default values.
Also applies to: 84-87
zmscitizenapi/src/Zmscitizenapi/Services/Core/MapperService.php (1)
66-69: LGTM! The implementation handles the new Office properties correctly.The code safely extracts the new properties from the provider data using null coalescing operators, ensuring graceful handling of missing data.
zmscitizenapi/src/Zmscitizenapi/Services/Core/ZmsApiFacadeService.php (1)
44-44: LGTM! Return type is now more specific.The return type change from
OfficeList|arraytoOfficeListimproves type safety.zmsentities/schema/citizenapi/collections/officeServiceAndRelationList.json (1)
70-73: Review of 'organization' property: Schema looks good.
The property is correctly defined as a string (or null) with a clear description. No issues found here.
zmscitizenapi/tests/Zmscitizenapi/Controllers/Office/OfficeListByServiceControllerTest.php
Outdated
Show resolved
Hide resolved
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Summary by CodeRabbit
New Features
UI Improvements
Enhancements