chore(zms-3235): initial refactoring #792
Conversation
WalkthroughThis pull request comprises extensive refactoring and type consistency improvements across the repository. Changes include dependency updates (removing deprecated packages and adding a new JSON schema library), method signature modifications for schema validation classes, and widespread adjustments to JSON schema files—such as converting string representations of numeric values to proper numeric types and allowing additional properties. Multiple PHP classes have also been updated to expose their constants publicly, and various tests and workflow scripts have been refined accordingly. Changes
Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 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: 3
🧹 Nitpick comments (2)
zmsentities/src/Zmsentities/Schema/Validator.php (1)
60-65: Null coalescing operator may cause issues in older PHP versions.The use of the null coalescing operator
??in$message = ($message) ? $message : $error->message();can be simplified.Apply this diff for clarity:
public function getCustomMessage($error) { $message = null; $property = new \BO\Zmsentities\Helper\Property($error->schema()); $message = $property['x-locale'][$this->locale]->messages[$error->keyword()]->get(); - return ($message) ? $message : $error->message(); + return $message ?? $error->message(); }Ensure that the target PHP version supports the null coalescing operator (
PHP 7.0or higher).zmsentities/composer.json (1)
48-48: Consider adding version constraints for development tools.The PHP_CodeSniffer version is constrained (^3.5.8), but PHPMD is using @stable. Consider using explicit version constraints for all development tools to ensure reproducible builds.
- "phpmd/phpmd": "@stable", + "phpmd/phpmd": "^2.13",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
zmsentities/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
zmsentities/composer.json(2 hunks)zmsentities/src/Zmsentities/Schema/Extensions/CoerceType.php(1 hunks)zmsentities/src/Zmsentities/Schema/Extensions/SameValues.php(1 hunks)zmsentities/src/Zmsentities/Schema/Validator.php(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Combined Workflow
zmsentities/src/Zmsentities/Schema/Validator.php
[error] 18-18: TypeError: Constructor argument #2 ($schemaObject) must be of type Opis\JsonSchema\Schema, but BO\Zmsentities\Schema\Schema was given
🔇 Additional comments (5)
zmsentities/src/Zmsentities/Schema/Validator.php (2)
68-76: Ensure correct extraction of the origin pointer.The
getOriginPointermethod manipulates the schema location to extract the pointer. Verify that this logic correctly identifies the property in all cases, especially with nested schemas.Consider adding checks for edge cases where
'properties'may not be present or the schema location has a different structure.
23-53:⚠️ Potential issueType mismatch causing constructor error.
The pipeline failure indicates that an object of type
BO\Zmsentities\Schema\Schemais being passed to the constructor, butOpis\JsonSchema\Schemais expected. This will result in aTypeError.To resolve this issue, ensure that
$schemaObjectis an instance ofOpis\JsonSchema\Schema. IfBO\Zmsentities\Schema\Schemais a custom schema class, consider wrapping or converting it to the expected type.Apply this diff to adjust the constructor parameter type:
public function __construct($data, Schema $schemaObject, $locale) { $this->schemaData = $data; - $this->schemaObject = $schemaObject; + $this->schemaObject = Schema::fromJsonString(json_encode($schemaObject)); $this->locale = $locale;Alternatively, ensure that when creating the
Validatorinstance, the$schemaObjectprovided is of the correct type.Likely invalid or redundant comment.
zmsentities/src/Zmsentities/Schema/Extensions/SameValues.php (1)
18-21: Verify the correctness ofValidationErrorinstantiation.Ensure that the parameters passed to
ValidationErrormatch the constructor's expected signature inOpis\JsonSchema.Consult the
Opis\JsonSchema\Errors\ValidationErrordocumentation to confirm the correct usage.zmsentities/composer.json (2)
23-24: Verify PHP version compatibility with new dependencies.The package requires PHP >=7.3.0, but we should verify compatibility with the newly added dependencies:
- opis/json-schema ^2.4
- symfony/twig-bridge ^5.4
24-24: Verify impact of JSON schema library migration.The migration from
league/json-guardtoopis/json-schemarepresents a significant architectural change. Since this is a draft PR, please ensure:
- All validation logic has been updated to use Opis interfaces
- All error handling has been adapted to the new library
- All schema definitions are compatible with Opis validator
✅ Verification successful
Migration to Opis JSON Schema validator is complete and properly implemented.
The codebase shows:
- Complete removal of League\JsonGuard
- Proper implementation of Opis validator
- Comprehensive schema definitions
- Appropriate error handling through exceptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for remaining League\JsonGuard references that need migration # Check for any remaining League\JsonGuard usage rg -l "League\\\\JsonGuard" --type php # Check for potential schema files fd -e json -e yaml -e yml . | xargs rg -l "schema"Length of output: 17933
Script:
#!/bin/bash # Search for schema validation and error handling code ast-grep --pattern 'class $_ { $$$ validate($$$) { $$$ } $$$ }' # Find schema-related classes and validation rg -l "Schema|Validator" --type php # Find actual schema definitions (excluding test fixtures) fd -e json . -E "*test*" -E "*fixture*" | xargs rg -l "schema|properties|type"Length of output: 21226
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
zmsentities/src/Zmsentities/Schema/Entity.php (1)
97-101: Remove unnecessary empty lines and add error handling.The code could be more concise and robust with proper error handling for schema conversion.
Apply this diff to improve the code:
if (Property::__keyExists('$schema', $data)) { unset($data['$schema']); } - - $schemaData = $jsonSchema->toJsonObject(true); - $opisSchema = new \Opis\JsonSchema\Schema($schemaData); - - $validator = new Validator($data->toJsonObject(true), $opisSchema, $locale); + try { + $schemaData = $jsonSchema->toJsonObject(true); + $opisSchema = new \Opis\JsonSchema\Schema($schemaData); + $validator = new Validator($data->toJsonObject(true), $opisSchema, $locale); + return $validator; + } catch (\Exception $e) { + throw new \RuntimeException('Failed to create schema validator: ' . $e->getMessage(), 0, $e); + } - return $validator;zmsentities/src/Zmsentities/Schema/Extensions/CoerceType.php (2)
55-60: Fix indentation in toBoolean method.The indentation in the toBoolean method is inconsistent with the rest of the file.
Apply this diff to fix the indentation:
private function toBoolean($value) { if (is_bool($value)) { - return $value; - } elseif (is_string($value)) { - return filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? $value; - } - return (bool)$value; + return $value; + } elseif (is_string($value)) { + return filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? $value; + } + return (bool)$value; }
14-21: Add type validation before coercion.The coercion should validate that the type list is valid before attempting coercion.
Apply this diff to add type validation:
public function validate($value, SchemaKeyword $schemaKeyword, ValidationContext $context) { $typeList = $schemaKeyword->data(); + if (!is_string($typeList)) { + return new ValidationError( + $context, + $context->schema(), + "Type must be a string", + $typeList + ); + } $coercedValue = $this->coerceType($value, $typeList); if ($coercedValue !== $value) { $context->setData($coercedValue); } // Re-run default validation logic return null; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
zmsentities/src/Zmsentities/Exception/SchemaValidation.php(2 hunks)zmsentities/src/Zmsentities/Schema/Entity.php(1 hunks)zmsentities/src/Zmsentities/Schema/Extensions/CoerceType.php(1 hunks)zmsentities/src/Zmsentities/Schema/Extensions/SameValues.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: call-unit-tests / zmsdb-test
- GitHub Check: call-unit-tests / zmsapi-test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
zmsentities/src/Zmsentities/Schema/Extensions/SameValues.php (1)
13-22: Handle array bounds and improve error message.The validation logic needs improvement in array bounds checking and error messaging.
Apply this diff to enhance the validation:
- if (is_array($value) && count($value) >= 2 && $value[0] === $value[1]) { + if (!is_array($value)) { + return new ValidationError( + $context, + $context->schema(), + "Expected an array, got " . gettype($value), + $value + ); + } + + if (count($value) < 2) { + return new ValidationError( + $context, + $context->schema(), + "Array must contain at least 2 elements", + $value + ); + } + + if ($value[0] === $value[1]) { return null; } return new ValidationError( $context, $context->schema(), - "Values do not match", + sprintf("Values at index 0 (%s) and index 1 (%s) do not match", + json_encode($value[0]), + json_encode($value[1])), $value );
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai here's some errors how do I fix them? |
|
Tip For best results, initiate chat on the files or code changes. @ThomasAFink The errors are occurring because Opis\JsonSchema has stricter validation compared to League\JsonGuard. Here's how to fix each issue:
// In Session class or example data
$example->content['basket']['scope'] = (int)$example->content['basket']['scope']; // Convert string to number
// In Useraccount class or example data
$example->id = (int)$example->id; // Convert string ID to number
// In Workstation class or example data
$example->process = [
'$ref' => 'process/123/' // Update $ref format to match Opis\JsonSchema
];
$example->apiclient = [
'clientKey' => 'example_key', // Add required property
'$ref' => 'apiclient/123/' // Update $ref format
];Additionally, you might want to add a type coercion keyword to handle string-to-number conversions automatically: // In your schema setup
$validator = new \Opis\JsonSchema\Validator();
$validator->resolver()->registerKeyword(
'coerceType',
new \BO\Zmsentities\Schema\Extensions\CoerceType()
);Would you like me to help you implement any of these fixes? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 30587 ✏️ Learnings added
|
|
@coderabbitai can we make the typing less strict with Opis\JsonSchema? |
|
Tip For best results, initiate chat on the files or code changes. @ThomasAFink Yes, we can make the typing less strict! Here's how:
public function __construct($data, $schemaObject, $locale = 'de_DE')
{
$validator = new \Opis\JsonSchema\Validator();
$validator->resolver()->registerKeyword('coerceType', new \BO\Zmsentities\Schema\Extensions\CoerceType());
// ... rest of your constructor
}
{
"type": "object",
"properties": {
"id": {
"type": "number",
"coerceType": "number"
},
"scope": {
"type": "number",
"coerceType": "number"
}
}
}This will automatically convert string values to numbers where needed, making the validation less strict while maintaining type safety. 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 1734 ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
zmsentities/src/Zmsentities/Schema/Validator.php (2)
38-39: Remove commented debug statement.Remove the commented var_dump statement.
- // var_dump("Validation Result: ", $this->validationResult); return $this->validationResult->isValid();
67-67: Remove commented code.Remove the commented line as it's no longer needed.
- //$error->message(),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
zmsadmin/composer.lockis excluded by!**/*.lockzmsapi/composer.lockis excluded by!**/*.lockzmscalldisplay/composer.lockis excluded by!**/*.lockzmscitizenapi/composer.lockis excluded by!**/*.lockzmsclient/composer.lockis excluded by!**/*.lockzmsdb/composer.lockis excluded by!**/*.lockzmsdldb/composer.lockis excluded by!**/*.lockzmsentities/composer.lockis excluded by!**/*.lockzmsmessaging/composer.lockis excluded by!**/*.lockzmsslim/composer.lockis excluded by!**/*.lockzmsstatistic/composer.lockis excluded by!**/*.lockzmsticketprinter/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
zmsentities/src/Zmsentities/Schema/Extensions/CoerceType.php(1 hunks)zmsentities/src/Zmsentities/Schema/Validator.php(1 hunks)
🧰 Additional context used
📓 Learnings (1)
zmsentities/src/Zmsentities/Schema/Extensions/CoerceType.php (1)
Learnt from: ThomasAFink
PR: it-at-m/eappointment#0
File: :0-0
Timestamp: 2025-02-03T12:44:31.097Z
Learning: When using Opis\JsonSchema, you can make type validation less strict by using the coerceType keyword in schema files and registering the CoerceType extension with the validator.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: call-unit-tests / zmsdb-test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-unit-tests / zmsapi-test
🔇 Additional comments (9)
zmsentities/src/Zmsentities/Schema/Extensions/CoerceType.php (4)
5-10: LGTM!The imports and class declaration are correctly updated for the migration to Opis\JsonSchema.
12-15: LGTM!The validate method signature and data retrieval are correctly implemented according to the Opis\JsonSchema interface.
45-45: LGTM!The return statement correctly follows the Opis\JsonSchema interface by returning null when validation passes.
18-43: 🛠️ Refactor suggestionEnhance type coercion logic.
The current implementation has several limitations:
- Number coercion only handles string to float conversion
- Integer coercion only handles positive numbers using
ctype_digit- Boolean coercion could handle more cases
Apply this diff to improve type coercion:
switch ($type) { case 'string': if (is_numeric($data) || is_bool($data)) { $context->setCurrentData((string)$data); } break; case 'number': - if (is_string($data) && is_numeric($data)) { + if (!is_float($data) && !is_int($data)) { + if (is_numeric($data)) { $context->setCurrentData((float)$data); + } elseif (is_bool($data)) { + $context->setCurrentData($data ? 1.0 : 0.0); + } } break; case 'integer': - if (is_string($data) && ctype_digit($data)) { + if (!is_int($data)) { + if (is_numeric($data) && (float)(int)$data === (float)$data) { $context->setCurrentData((int)$data); + } elseif (is_bool($data)) { + $context->setCurrentData($data ? 1 : 0); + } } break; case 'boolean': if (is_string($data)) { - if ($data === 'true') { - $context->setCurrentData(true); - } elseif ($data === 'false') { - $context->setCurrentData(false); - } + $result = filter_var($data, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + if ($result !== null) { + $context->setCurrentData($result); + } } break;Likely invalid or redundant comment.
zmsentities/src/Zmsentities/Schema/Validator.php (5)
5-14: LGTM!The imports and class properties are correctly updated for the migration to Opis\JsonSchema.
81-94: LGTM!The custom message retrieval logic is correctly implemented.
96-122: LGTM!The pointer methods are correctly implemented and handle both origin and translated pointers appropriately.
22-23: 🛠️ Refactor suggestionMake schema path configurable.
The schema path is hardcoded. Consider making it configurable through constructor parameters or environment variables.
- $this->validator = new OpisValidator(); - $this->validator->resolver()->registerPrefix('schema://', '/var/www/html/zmsentities/schema/'); + $this->validator = new OpisValidator(); + $schemaPath = getenv('SCHEMA_PATH') ?: '/var/www/html/zmsentities/schema/'; + $this->validator->resolver()->registerPrefix('schema://', $schemaPath);Likely invalid or redundant comment.
32-33:⚠️ Potential issueRemove debug statements.
Remove var_dump statements before merging to production.
- var_dump("Schema:", json_encode($schema, JSON_PRETTY_PRINT)); - var_dump("Data:", json_encode($data, JSON_PRETTY_PRINT));Likely invalid or redundant comment.
ThomasAFink
force-pushed
the
chore-zms-3235-replace-json-guard-dependency
branch
from
845910d to
29e1793
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
zmsentities/src/Zmsentities/Schema/Validator.php (4)
28-47: Externalize schema file list.Consider moving the schema files list to a configuration file to improve maintainability and make it easier to add/remove schemas without modifying the code.
- $commonSchemas = [ - 'process.json', - 'apiclient.json', - // ... other schemas - 'mimepart.json' - ]; + $commonSchemas = $this->loadSchemaList(); + + private function loadSchemaList(): array + { + $configPath = getenv('SCHEMA_CONFIG_PATH') ?: __DIR__ . '/config/schemas.json'; + return json_decode(file_get_contents($configPath), true); + }
64-68: Optimize JSON conversions.The double JSON conversion (encode then decode) might impact performance with large schemas. Consider using a direct object conversion if possible.
- $schemaJson = json_encode($schemaObject->toJsonObject()); - $schemaJson = json_decode($schemaJson); - $data = json_decode(json_encode($data)); + $schemaJson = $schemaObject->toJsonObject(); + if (!is_object($data)) { + $data = (object)$data; + }
103-111: Clean up commented code and improve error creation.Remove the commented out code and consider adding error context for better debugging.
$errors[] = new OpisValidationError( $error->keyword(), $error->schema(), $error->data(), $this->getCustomMessage($error), - //$error->message(), $error->args(), - [] + ['context' => $error->data()->path()] );
171-189: Consider optimizing reference resolution.The recursive reference resolution could be optimized by:
- Caching resolved references
- Using array_walk_recursive for better performance
+ private $resolvedRefs = []; + private function resolveRefs(&$schema) { + if (is_string($schema)) { + return; + } + + $hash = md5(json_encode($schema)); + if (isset($this->resolvedRefs[$hash])) { + $schema = $this->resolvedRefs[$hash]; + return; + } + if (is_object($schema)) { foreach ($schema as $key => &$value) { if ($key === '$ref' && is_string($value)) { $value = 'schema://' . $value; } elseif (is_object($value) || is_array($value)) { $this->resolveRefs($value); } } + $this->resolvedRefs[$hash] = $schema; } elseif (is_array($schema)) { - foreach ($schema as &$value) { - if (is_object($value) || is_array($value)) { - $this->resolveRefs($value); - } - } + array_walk_recursive($schema, function(&$value) { + if (is_object($value) || is_array($value)) { + $this->resolveRefs($value); + } + }); + $this->resolvedRefs[$hash] = $schema; } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zmsentities/schema/scope.json(1 hunks)zmsentities/src/Zmsentities/Schema/Validator.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- zmsentities/schema/scope.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: call-unit-tests / zmsdb-test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-unit-tests / zmsapi-test
🔇 Additional comments (2)
zmsentities/src/Zmsentities/Schema/Validator.php (2)
25-25: Make schema path configurable.The schema path is hardcoded. Consider making it configurable through constructor parameters or environment variables.
79-79: Remove debug statement.Remove the commented var_dump statement before merging to production.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
zmsentities/src/Zmsentities/Schema/Validator.php (4)
79-79: Remove or update commented code.The commented line suggests incomplete refactoring. Either remove it or update it to use the new validation approach.
84-84: Remove debug statement.Remove the commented var_dump statement.
104-125: Add error handling for subErrors processing.The extractErrors method should handle potential null values in subErrors.
private function extractErrors(OpisValidationError $error) { $errors = []; + if (!$error) { + return $errors; + } $errors[] = new OpisValidationError( $error->keyword(), $error->schema(), $error->data(), $this->getCustomMessage($error), //$error->message(), $error->args(), [] ); + $subErrors = $error->subErrors() ?? []; - foreach ($error->subErrors() as $subError) { + foreach ($subErrors as $subError) { if ($subError instanceof OpisValidationError) { $errors = array_merge($errors, $this->extractErrors($subError)); } } return $errors; }
176-194: Add validation for schema references.The resolveRefs method should validate the schema reference format.
private function resolveRefs(&$schema) { if (is_object($schema)) { foreach ($schema as $key => &$value) { if ($key === '$ref' && is_string($value)) { + // Validate reference format + if (!preg_match('/^[a-zA-Z0-9\-_.]+\.json$/', $value)) { + throw new \InvalidArgumentException("Invalid schema reference format: $value"); + } // Convert relative path to schema:// protocol $value = 'schema://' . $value; } elseif (is_object($value) || is_array($value)) { $this->resolveRefs($value); } } } elseif (is_array($schema)) { foreach ($schema as &$value) { if (is_object($value) || is_array($value)) { $this->resolveRefs($value); } } } }zmsentities/tests/Zmsentities/FactoryTest.php (1)
12-12: Remove debug logging statement.Since this is a test file, debug logging statements should be removed before the PR is finalized. These statements can clutter logs and potentially expose sensitive data.
- error_log(json_encode($data));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
zmsentities/schema/calendar.json(1 hunks)zmsentities/schema/dereferenced/calendar.json(1 hunks)zmsentities/schema/process.json(1 hunks)zmsentities/schema/session.json(2 hunks)zmsentities/src/Zmsentities/Schema/Validator.php(1 hunks)zmsentities/tests/Zmsentities/FactoryTest.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- zmsentities/schema/session.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: call-unit-tests / zmsapi-test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-unit-tests / zmsdb-test
🔇 Additional comments (8)
zmsentities/schema/calendar.json (1)
87-87: Validate the Free Process Identifier Update.
The value for"id"in the first object of thefreeProcessesarray has been updated from123to12345. This change appears intended to standardize identifiers across the schema. Verify that all parts of the application referencing this field are updated accordingly.zmsentities/schema/dereferenced/calendar.json (1)
81-81: Confirm Consistency in Dereferenced Schema.
The change to"id": 12345in thefreeProcessesarray precisely mirrors the update in the base schema (zmsentities/schema/calendar.json). Ensure that this consistent update fulfills the requirements for all dereferenced validation and integration logic.zmsentities/schema/process.json (1)
264-264: Be cautious with enabling additionalProperties.
Changing"additionalProperties": falseto"additionalProperties": trueincreases the flexibility of the schema by allowing additional properties beyond those explicitly defined. This may be intended to accommodate evolving data structures; however, it also opens the door for unintended keys to be passed through validation. Please verify that this aligns with the overall design and that your application logic properly handles or sanitizes extra properties if they are not expected.zmsentities/src/Zmsentities/Schema/Validator.php (5)
5-14: LGTM! Clean migration of imports and class properties.The imports and class setup correctly reflect the migration from League\JsonGuard to Opis\JsonSchema.
71-73: Remove debug statements.Debug statements should be removed before merging to production.
64-68: Add type safety checks for JSON conversion.The JSON encoding/decoding operations lack error checking.
25-25: Make schema path configurable.The schema path is hardcoded. Consider making it configurable through constructor parameters or environment variables.
82-102: LGTM! Clean implementation of validation methods.The validation methods have been correctly updated to work with Opis\JsonSchema's validation results.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
zmsentities/src/Zmsentities/Day.php (1)
23-26:⚠️ Potential issueUpdate default values to match the new integer type.
The default values are set as empty strings, but the
setDateTimemethod now uses integers. This type inconsistency could cause validation issues with Opis\JsonSchema.Apply this diff to align the types:
return [ - 'year' => '', - 'month' => '', - 'day' => '', + 'year' => 0, + 'month' => 0, + 'day' => 0, 'status' => self::NOTBOOKABLE,zmsentities/tests/Zmsentities/MailTest.php (1)
133-133: 🛠️ Refactor suggestionEnsure consistent method usage across test cases.
The test file uses
getFirstClient()in multiple places (lines 133, 220) butgetClient()in the basic test. This inconsistency could lead to maintenance issues.Consider updating all occurrences to use the same method consistently:
- process->getFirstClient()->surveyAccepted = 1; + process->getClient()->surveyAccepted = 1; - process->getFirstClient()->surveyAccepted = 0; + process->getClient()->surveyAccepted = 0;Also applies to: 220-220
♻️ Duplicate comments (1)
zmsentities/tests/Zmsentities/FactoryTest.php (1)
38-40:⚠️ Potential issueConvert string numbers to actual numeric types.
Several numeric fields are still represented as strings, which can cause validation errors with Opis\JsonSchema's stricter type checking. This issue was previously identified in past reviews.
Apply this diff to fix the numeric types:
- "scope" => "123", - "process" => "1234567", - "date" => "1456310041", + "scope" => 123, + "process" => 1234567, + "date" => 1456310041,The
tsfield in the human array is correctly typed as a number, which is good.Also applies to: 52-52
🧹 Nitpick comments (10)
zmsentities/src/Zmsentities/Day.php (1)
50-50: Consider padding single-digit values in date string construction.When constructing the date string, single-digit month/day values might need padding to ensure proper date parsing.
Apply this diff to ensure proper date string formatting:
- $date = Helper\DateTime::createFromFormat('Y-m-d', $this['year'] . '-' . $this['month'] . '-' . $this['day']); + $date = Helper\DateTime::createFromFormat('Y-m-d', sprintf('%d-%02d-%02d', $this['year'], $this['month'], $this['day']));zmsentities/tests/Zmsentities/ExchangeTest.php (1)
198-206: LGTM! Consider adding a test case fortruevalue.The test correctly verifies that boolean values cannot be coerced to integers with Opis\JsonSchema's stricter validation rules. This aligns with the library migration objectives.
Consider adding another test case that uses
trueinstead offalseto ensure complete coverage of boolean values:public function testSchemaIntegerAsBool() { $this->expectException('\BO\Zmsentities\Exception\SchemaValidation'); $now = new \DateTimeImmutable('2016-04-01 11:55:00'); $entity = (new $this->entityclass()); $entity->setPeriod($now, $now); $entity['dictionary'][0]['position'] = false; $entity->testValid(); } + +public function testSchemaIntegerAsBoolTrue() +{ + $this->expectException('\BO\Zmsentities\Exception\SchemaValidation'); + $now = new \DateTimeImmutable('2016-04-01 11:55:00'); + $entity = (new $this->entityclass()); + $entity->setPeriod($now, $now); + $entity['dictionary'][0]['position'] = true; + $entity->testValid(); +}zmsentities/tests/Zmsentities/FactoryTest.php (1)
12-12: Remove debug logging statement.Debug logging in test files can pollute log files in CI/CD environments. Consider removing this temporary debugging code or replacing it with proper test assertions if validation of the JSON structure is needed.
- error_log(json_encode($data));zmsentities/src/Zmsentities/Schema/Factory.php (1)
55-58: Add PHPDoc documentation for consistency.The new method should include PHPDoc documentation to maintain consistency with other methods in the class.
Add the following documentation:
+ /** + * Get the fully qualified class name of the factory + * + * @return string + */ public function getFactoryName() { return static::class; }zmsentities/tests/Zmsentities/MailTest.php (1)
4-376: Add test coverage for schema validation.Given the PR's context about transitioning from League\JsonGuard to Opis\JsonSchema, there should be test cases verifying the schema validation behavior.
Consider adding test cases to verify:
- Schema validation with the new Opis\JsonSchema validator
- Error handling for invalid data
- Type coercion behavior mentioned in the PR objectives
Example test case structure:
public function testSchemaValidation() { $entity = (new $this->entityclass())->getExample(); $process = (new \BO\Zmsentities\Process())->getExample(); // Test valid data $this->assertTrue($entity->validate()); // Test invalid data $process->client = null; $this->expectException(\BO\Zmsentities\Exception\SchemaValidation::class); $entity->validate(); }zmsentities/schema/appointment.json (2)
66-66: Consider the implications of allowing additional properties.Setting
additionalPropertiestotruemakes the schema more permissive but could lead to validation issues if unexpected properties are added. Consider keeping itfalsefor stricter validation, or document the decision if flexibility is required.
26-59: Enhance validation for the availability object.The availability object contains critical scheduling information. Consider adding:
- Validation for time format (HH:mm)
- Range validation for startInDays and endInDays
- Schema reference for workstationCount
zmsentities/schema/cluster.json (1)
12-28: Consider extracting repeated provider contact information.The provider contact information is duplicated across multiple scopes. Consider:
- Creating a separate schema for provider contact info
- Using
$refto reference the common schema{ "type": "object", "properties": { + "providerContact": { + "$ref": "provider-contact.json" + } } }Also applies to: 33-49
zmsentities/schema/exchange.json (1)
48-48: Document the rationale for allowing additional properties.The change to
"additionalProperties": truemakes the schema more permissive. Consider:
- Documenting why additional properties are needed
- Adding a comment about validation implications
zmsentities/schema/queue.json (1)
1-1: Consider documenting schema migration strategy.The changes across all schema files show a coordinated effort to:
- Make schemas more permissive with
additionalProperties: true- Standardize provider contact information
- Adjust type handling for Opis\JsonSchema
Consider:
- Documenting the migration strategy
- Creating shared schemas for common structures
- Adding validation examples for consumers
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
zmsentities/schema/appointment.json(1 hunks)zmsentities/schema/cluster.json(1 hunks)zmsentities/schema/department.json(2 hunks)zmsentities/schema/exchange.json(1 hunks)zmsentities/schema/mail.json(2 hunks)zmsentities/schema/notification.json(1 hunks)zmsentities/schema/process.json(2 hunks)zmsentities/schema/queue.json(2 hunks)zmsentities/schema/workstation.json(2 hunks)zmsentities/src/Zmsentities/Day.php(1 hunks)zmsentities/src/Zmsentities/Schema/Factory.php(1 hunks)zmsentities/src/Zmsentities/Scope.php(1 hunks)zmsentities/tests/Zmsentities/AvailabilityTest.php(8 hunks)zmsentities/tests/Zmsentities/ExchangeTest.php(1 hunks)zmsentities/tests/Zmsentities/FactoryTest.php(2 hunks)zmsentities/tests/Zmsentities/MailTest.php(1 hunks)zmsentities/tests/Zmsentities/ValidationTest.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- zmsentities/tests/Zmsentities/ValidationTest.php
🧰 Additional context used
🪛 Gitleaks (8.21.2)
zmsentities/schema/process.json
67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-unit-tests / zmsapi-test
- GitHub Check: call-unit-tests / zmsdb-test
🔇 Additional comments (13)
zmsentities/src/Zmsentities/Day.php (1)
42-44: LGTM! Type casting improves schema validation.The explicit integer casting aligns with Opis\JsonSchema's stricter validation requirements and ensures consistent data types.
zmsentities/tests/Zmsentities/AvailabilityTest.php (6)
463-469: LGTM! Consistent type changes for weekday values.The conversion of weekday values from strings to integers is consistent across all test cases and aligns with the stricter type validation in Opis\JsonSchema.
Also applies to: 713-719, 738-744, 763-769, 788-794, 813-819, 838-844, 918-924
472-473: LGTM! Consistent type changes for repeat configuration.The conversion of repeat configuration values (
afterWeeksandweekOfMonth) from strings to integers is consistent across all test cases.Also applies to: 722-722, 747-747, 772-772, 797-797, 822-822, 847-847, 927-928
476-477: LGTM! Consistent type changes for bookable configuration.The conversion of bookable configuration values (
startInDaysandendInDays) from strings to integers is consistent across all test cases.Also applies to: 931-932
480-482: LGTM! Consistent type changes for workstation counts.The conversion of workstation count values (
public,callcenter,intern) from strings to integers is consistent across all test cases.Also applies to: 725-727, 750-752, 775-777, 800-802, 825-827, 850-852, 935-937
484-486: LGTM! Consistent type changes for slot configuration.The conversion of slot configuration values (
slotTimeInMinutes) from strings to integers is consistent across all test cases.Also applies to: 729-729, 754-754, 779-779, 804-804, 829-829, 854-854, 939-939
489-489: Verify test coverage for new configuration.The addition of
multipleSlotsAllowedand scope configuration suggests new functionality. Ensure that the test coverage is comprehensive for these features.Also applies to: 944-953
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check test coverage for multipleSlotsAllowed and scope configuration # Search for tests related to multipleSlotsAllowed echo "Searching for tests related to multipleSlotsAllowed..." rg -l "multipleSlotsAllowed" --type php # Search for tests related to scope configuration echo "Searching for tests related to scope configuration..." rg -l "scope.*provider.*source" --type phpLength of output: 1100
Test Coverage for New Configuration Confirmed
The output indicates that tests covering both the
multipleSlotsAllowedconfiguration and the scope configuration exist in several locations. Notably:
multipleSlotsAllowedTests:
zmsentities/tests/Zmsentities/AvailabilityTest.phpzmsadmin/tests/Zmsadmin/AvailabilityUpdateTest.phpzmsadmin/tests/Zmsadmin/AvailabilityConflictsTest.php- (and additional occurrences in related modules)
Scope Configuration Tests:
- Located in files such as
zmscitizenapi/tests/Zmscitizenapi/Services/Core/ZmsApiClientServiceTest.phpand others in thezmscitizenapi,zmsdb, andzmsapiareas.These findings support that the new functionality introduced by
multipleSlotsAllowedand the associated scope configuration are being exercised by tests. You might want to review these test files to ensure that all edge cases and expected behaviors are thoroughly validated.zmsentities/src/Zmsentities/Schema/Factory.php (1)
33-38: Verify entity creation with Opis\JsonSchema.Since this is part of the transition to Opis\JsonSchema, ensure that the entity creation process in
getEntity()is compatible with the new validation framework.Let's check for any potential issues:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for Opis\JsonSchema usage and potential integration points # Search for Opis\JsonSchema usage in entity classes rg -l "use.*Opis\\\\JsonSchema" --type php # Check for UnflattedArray usage with Opis\JsonSchema rg "UnflattedArray.*Opis" --type php -A 5Length of output: 316
Entity creation integration with Opis\JsonSchema verified.
The search results confirm that the integration points for Opis\JsonSchema are present elsewhere in the code (e.g., in the Schema\Extensions and Validator classes). There’s no indication that the instantiation done in
getEntity()—usingnew UnflattedArray($this->data)—conflicts with the new validation framework. TheUnflattedArrayusage doesn’t appear to require modifications for compatibility with Opis\JsonSchema, and the factory method correctly builds the entity as expected.zmsentities/schema/queue.json (1)
69-69: Verify the need for integer type in withAppointment.The
withAppointmentproperty now accepts both integer and boolean types. This seems unusual for a boolean flag. Consider:
- Documenting why integer type is needed
- Adding examples of valid integer values
✅ Verification successful
🌐 Web query:
Does Opis\JsonSchema handle boolean-integer type coercion differently from League\JsonGuard?💡 Result:
Opis\JsonSchema and League\JsonGuard take different approaches to boolean-integer type coercion based on their design philosophies:
Opis\JsonSchema
- No implicit type coercion - The library strictly validates data against the declared schema types without automatic conversions[3][6]. For example:
$schema = Schema::fromJsonString('{"type": "integer"}'); $result = $validator->schemaValidation("123", $schema); // Fails - string vs integer- Requires explicit schema definitions for type conversions using keywords like
pattern,format, or custom validation logic[3][7]- Follows JSON Schema spec without extensions for type coercion[3][9]
League\JsonGuard
- Implements limited JavaScript-style coercion for certain comparisons:
'123' == 123 // true via loose equality true == 1 // true via numeric conversion[2][5]- Maintains type strictness during validation but allows comparison operators to use JS-like coercion[5][8]
- No schema-based type conversion mechanisms comparable to Ajv's
coerceTypes[1][6]Key Differences Table
Feature Opis\JsonSchema League\JsonGuard Implicit type casting Never[3] In equality checks[5][8] Schema-driven coercion Requires custom keywords Not supported Boolean ↔ Number Strict separation[3][7] Loose equality allowed[5] Validation philosophy Type-pure validation[9] JS-type flexibility[2][5] For exact boolean-integer handling:
- Opis rejects1when expectingtrueunless using custom validation logic[3]
- JsonGuard would allow1 == truein comparison contexts but not consider them equal during strict validation[5][8]To enable Ajv-style coercion in PHP environments, developers typically implement pre-processing layers rather than relying on schema validators directly[6].
Citations:
- 1: https://ajv.js.org/coercion.html
- 2: https://www.freecodecamp.org/news/js-type-coercion-explained-27ba3d9a2839/
- 3: https://opis.io/json-schema/1.x/php-validator.html
- 4: Type coercion does not work with anyOf Number or Boolean ajv-validator/ajv#2348
- 5: https://dev.to/morninglit/the-full-practical-guide-to-type-coercion-in-javascript-beware-of-these-3-common-mistakes-1f7p
- 6: JSON Schema Type Conversion / Coercion Vincit/objection.js#1446
- 7: https://shapechange.net/targets/json-schema/
- 8: Datatype coercion of native types json-ld/json-ld.org#98
- 9: https://json-schema.org/understanding-json-schema/reference/numeric
- 10: https://github.com/sinclairzx81/typebox
Integer Type in withAppointment: Clarify Rationale & Expected Values
The web query confirms that, unlike League\JsonGuard, Opis\JsonSchema does not implicitly coerce types—integers and booleans remain strictly separate. Allowing both types suggests one of two things:
- It’s an intentional design to support legacy systems or external data sources that might supply numeric flags (e.g., 0/1) instead of booleans.
- Alternatively, it could lead to inconsistencies if the intent is purely a boolean flag.
Recommendations:
- Document the Decision: Clearly explain why the property supports both
integerandboolean. For example, note if this is to maintain compatibility with systems that use numeric flags.- Provide Examples: Indicate valid integer values (e.g., only
0and1, if that’s the case) to avoid ambiguity during usage.zmsentities/schema/workstation.json (1)
64-64: Verify the impact of allowing additional properties.The schema has been made more permissive by allowing additional properties. This could lead to unexpected data being stored or processed.
Please confirm if this change is intentional and aligns with the refactoring goals.
zmsentities/schema/department.json (1)
69-69: Verify the impact of allowing additional properties.The schema has been made more permissive by allowing additional properties. This could lead to unexpected data being stored or processed.
Please confirm if this change is intentional and aligns with the refactoring goals.
zmsentities/schema/mail.json (1)
81-81: Verify the impact of allowing additional properties.The schema has been made more permissive by allowing additional properties. This could lead to unexpected data being stored or processed.
Please confirm if this change is intentional and aligns with the refactoring goals.
zmsentities/schema/process.json (1)
319-319: Verify the impact of allowing additional properties.The schema has been made more permissive by allowing additional properties. This could lead to unexpected data being stored or processed.
Please confirm if this change is intentional and aligns with the refactoring goals.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
zmsentities/tests/Zmsentities/FactoryTest.php (1)
32-55:⚠️ Potential issueConvert string numeric values to actual numbers for Opis\JsonSchema compatibility.
Several numeric fields are currently represented as strings, which will cause validation errors with Opis\JsonSchema's stricter type checking. Additionally, there's inconsistent type usage where some timestamps are numeric while others are strings.
Apply this diff to fix the type mismatches:
$data = [ "status" => "start", "basket" => [ - "requests" => "120703", - "providers" => "122217", - "scope" => "123", - "process" => "1234567", - "date" => "1456310041", + "requests" => 120703, + "providers" => 122217, + "scope" => 123, + "process" => 1234567, + "date" => 1456310041, "firstDay" => "2016-04-01", "lastDay" => "2016-05-31", ], "entry" => [ "source" => "reinit", - "providers" => "122217", - "requests" => "120703", + "providers" => 122217, + "requests" => 120703, ], "human" => [
🧹 Nitpick comments (4)
zmsentities/src/Zmsentities/Schema/Validator.php (2)
98-101: Consider removing commented code.The commented line
$this->validationResult = $this->validator->dataValidation($data, $schemaData);should be removed if it's no longer needed.
125-145: Consider optimizing error extraction.The error extraction process creates new
OpisValidationErrorinstances, which might be unnecessary. Consider returning the original error objects if no transformation is needed.private function extractErrors(OpisValidationError $error) { $errors = []; - $errors[] = new OpisValidationError( - $error->keyword(), - $error->schema(), - $error->data(), - $this->getCustomMessage($error), - $error->args(), - [] - ); + $errors[] = $error; foreach ($error->subErrors() as $subError) { if ($subError instanceof OpisValidationError) { $errors = array_merge($errors, $this->extractErrors($subError)); } } return $errors; }zmsentities/tests/Zmsentities/CalendarTest.php (1)
32-32: Use a constant instead of a hardcoded value.The hardcoded cluster ID
123reduces maintainability. Consider keeping it as a constant or moving it to a test configuration file.- $entity->addCluster(123); + const CLUSTER = 123; + $entity->addCluster(self::CLUSTER);zmsentities/src/Zmsentities/Provider.php (1)
11-18: Consider improving default values initialization.A few suggestions to enhance the default values:
- Consider using constants for default values to maintain consistency across the codebase.
- The default source value 'dldb' appears in multiple entities - this could be centralized.
- Empty strings for required fields like 'name' might not pass schema validation.
Here's a suggested improvement:
+ private const DEFAULT_SOURCE = 'dldb'; + private const DEFAULT_ID = 123; + public function getDefaults() { return [ - 'id' => 123, - 'name' => '', - 'source' => 'dldb' + 'id' => self::DEFAULT_ID, + 'name' => null, + 'source' => self::DEFAULT_SOURCE ]; }Let's verify the schema requirements for these fields:
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Description: Check schema requirements for id, name, and source fields rg -A 5 '"(id|name|source)":\s*{' provider.jsonLength of output: 159
I'll run a corrected command to re-verify the schema definitions for the "id", "name", and "source" fields. This should help us check if the default value for "name" (an empty string) might conflict with the schema requirements.
🏁 Script executed:
#!/bin/bash # Re-run search with a corrected regex pattern to locate schema definitions for "id", "name", and "source" rg '"(id|name|source)"\s*:' provider.jsonLength of output: 97
Subject: Manual Verification Needed for Schema Requirements
The automated check didn't locate a provider.json file (or its equivalent schema definition) to confirm if the defaults for
'id','name', and'source'are compliant. Without visible schema constraints, the suggestion to switch'name'from an empty string ('') tonulland centralize default values using constants remains a refactor suggestion based on best practices rather than a schema violation.Please manually verify the schema validations for these fields to determine if changing the default for
'name'(and similar adjustments) will cause issues.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
zmsentities/src/Zmsentities/Calendar.php(4 hunks)zmsentities/src/Zmsentities/Process.php(1 hunks)zmsentities/src/Zmsentities/Provider.php(1 hunks)zmsentities/src/Zmsentities/Request.php(1 hunks)zmsentities/src/Zmsentities/Schema/Validator.php(1 hunks)zmsentities/src/Zmsentities/Scope.php(2 hunks)zmsentities/tests/Zmsentities/AppointmentTest.php(1 hunks)zmsentities/tests/Zmsentities/CalendarTest.php(1 hunks)zmsentities/tests/Zmsentities/FactoryTest.php(1 hunks)zmsentities/tests/Zmsentities/ProcessTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- zmsentities/src/Zmsentities/Calendar.php
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-unit-tests / zmsapi-test
- GitHub Check: call-unit-tests / zmsdb-test
🔇 Additional comments (9)
zmsentities/src/Zmsentities/Scope.php (2)
20-20: LGTM!The addition of the 'shortName' key with an empty string default value aligns with the schema updates.
71-73: LGTM!The addition of the null check and explicit exception handling for missing provider improves error handling and maintainability.
zmsentities/src/Zmsentities/Process.php (1)
496-500: LGTM!The changes improve data preservation by retaining provider and shortName fields while safely handling missing data using null coalescing operators.
zmsentities/tests/Zmsentities/ProcessTest.php (1)
93-94: Verify the test logic for appointment validation.The change from
hasAppointment(1451649000, 123)tohasAppointment(1451649000, 0)seems inconsistent with the subsequent test on line 94, which still uses ID 123. Please verify:
- Whether the ID parameter (second argument) should consistently be 0 in both test cases
- If this change aligns with the actual implementation of
hasAppointment()method❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for the implementation and usage of hasAppointment method ast-grep --pattern 'hasAppointment($$$)'Length of output: 930
Action: Review potential test inconsistency in appointment ID parameters
The current test asserts that a valid appointment is accessible with a call to
$this->assertTrue($entity->hasAppointment(1451649000, 0), 'appointment is not accessible');while an invalid appointment is checked with
$this->assertFalse($entity->hasAppointment(1447869173, 123), 'appointment date 1447869173 should not exist');We couldn’t locate the concrete implementation of the
hasAppointment()method in the codebase (our search with ast-grep returned only unrelated SQL files). Without direct insight into how this method uses its second parameter, there remains uncertainty over whether the valid appointment should really use an ID of 0 versus 123.Please verify that:
- The refactored logic of
hasAppointment()now expects the valid appointment’s second parameter to be 0.- The negative test case (using 123) appropriately exercises a scenario where no appointment exists.
Confirming these details in the implementation will help ensure the tests precisely reflect the intended behavior.
zmsentities/tests/Zmsentities/AppointmentTest.php (1)
15-15: Verify the string representation change.The assertion has been changed from checking for '123' to '0' in the entity's string representation. Please ensure this change aligns with the expected behavior after the validation library transition.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for other string representation tests in the codebase. # Search for similar assertions in test files rg -A 2 "assertStringContainsString" "tests/"Length of output: 90
I'll re-run the search command in the correct test directory to verify if there are any similar assertions elsewhere that could shed light on this change. This will help ensure the string representation change (from
'123'to'0') is consistent with the updated expectations from the validation library transition.
🏁 Script executed:
#!/bin/bash # Re-run the search command in the correct test folder rg -A 3 "assertStringContainsString" "zmsentities/tests/"Length of output: 27101
Subject: Verified - Appointment string representation change is consistent with updated expectations
The search in our test suite confirms that the AppointmentTest now explicitly asserts that the entity’s string value contains
'0'rather than'123'. This change is consistent with similar tests in related entities (e.g., CalendarTest) that reflect the updated output following the validation library transition.zmsentities/src/Zmsentities/Schema/Validator.php (4)
93-95: Remove debug statements.Debug statements should be removed before merging to production.
25-25: Make schema path configurable.The schema path is hardcoded. Consider making it configurable through constructor parameters or environment variables.
76-83: Add error handling for schema loading.The schema loading process lacks proper error handling for file operations and JSON parsing.
86-90: Add type safety checks for JSON conversion.The JSON encoding/decoding operations lack error checking.
…e-json-guard-dependency
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
zmsapi/tests/Zmsapi/fixtures/GetOrganisation.json (1)
29-29: Verify 'enabled' Field Data Type:
While most numeric identifiers have been converted, the"enabled"field remains a string ("1"). Please verify if this is intentional per the schema or if it should also be converted to a numeric type.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
zmsapi/tests/Zmsapi/fixtures/GetAvailability.json(1 hunks)zmsapi/tests/Zmsapi/fixtures/GetFreeProcessList.json(16 hunks)zmsapi/tests/Zmsapi/fixtures/GetMail.json(2 hunks)zmsapi/tests/Zmsapi/fixtures/GetNotification.json(2 hunks)zmsapi/tests/Zmsapi/fixtures/GetOrganisation.json(2 hunks)zmsapi/tests/Zmsapi/fixtures/GetOwner.json(1 hunks)zmsapi/tests/Zmsapi/fixtures/GetProcess_10029.json(4 hunks)zmsapi/tests/Zmsapi/fixtures/GetProcess_10030.json(4 hunks)
✅ Files skipped from review due to trivial changes (2)
- zmsapi/tests/Zmsapi/fixtures/GetOwner.json
- zmsapi/tests/Zmsapi/fixtures/GetAvailability.json
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: call-owasp-security-checks / security-scan (zmsclient, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsslim, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsentities, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsstatistic, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdldb, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsslim, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsticketprinter, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsmessaging, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsstatistic, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsmessaging, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscitizenapi, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmscitizenapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscalldisplay, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsapi, 8.0)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-owasp-security-checks / security-scan (zmsadmin, 8.0)
🔇 Additional comments (35)
zmsapi/tests/Zmsapi/fixtures/GetOrganisation.json (4)
4-4: Organization ID Type Conversion:
The"id"field is now a numeric value (54), which is consistent with the expected schema.
12-12: Ticket Printer Protection Flag Update:
The"ticketPrinterProtectionEnabled"flag has been converted from a string to a numeric value (0), ensuring proper type alignment.
17-17: Department ID Format:
The department"id"is now a number (55), correctly matching the schema requirements.
31-32: Ticket Printers Numeric Fields Update:
Both the"id"and"lastUpdate"fields in the ticket printers section are now numeric, which improves consistency with the JSON schema.zmsapi/tests/Zmsapi/fixtures/GetProcess_10030.json (6)
8-15: Enhanced Appointment Scope Details:
The appointment’s"scope"object now includes a numeric"id"(141) and an added"provider"object (with"id","name", and"source"). The inclusion of"shortName"and"source"further enriches the data. Ensure these values align with the updated JSON schema requirements.
48-48: Slot Count Conversion:
The"slotCount"field is now represented as a numeric value (1), which adheres to the expected format.
56-57: Clients’ Counter Fields Update:
The"emailSendCount"and"notificationsSendCount"fields in the clients array have been converted to numbers (0), ensuring consistency.
63-64: Process Metadata Conversion:
Both"createTimestamp"and the process"id"are now numeric (1456312139and10030, respectively), which complies with the schema specifications.
75-80: Queue Object Numeric Updates:
The queue fields"callCount","lastCallTime", and"withAppointment"have been updated to numeric values. Note that while these changes are correct, consider reviewing the data type of the"number"field if consistency with numeric values is desired.
84-102: Enhanced Process Scope:
The process’s second"scope"object now features a numeric"id", an enriched"provider"object (including detailed contact information), and new properties"shortName"and"source". This update aligns well with the broader schema standardization efforts.zmsapi/tests/Zmsapi/fixtures/GetMail.json (3)
11-11: Multipart Queue ID Conversion:
The"queueId"in the first multipart entry has been converted to an integer (1234), ensuring proper type matching with the schema.
17-17: Consistent Queue ID in Multipart:
Similarly, the second multipart entry’s"queueId"is now an integer (1234). This maintains consistency across multipart entries.
35-42: Enhanced Process Scope in Mail Fixture:
The"scope"object now includes a numeric"id", an added"provider"object with detailed provider information, and the new properties"shortName"and"source". This enhancement improves the data model and aligns with the updated schema.zmsapi/tests/Zmsapi/fixtures/GetProcess_10029.json (6)
8-15: Appointment Scope Update:
Within the appointment, the"scope"object now has a numeric"id"(167) and an enriched"provider"object along with"shortName"and"source". This change supports improved type consistency and data completeness.
56-58: Client Counters Standardized:
The client fields"emailSendCount","notificationsSendCount", and"surveyAccepted"have been converted to numeric values, which meets the intended schema requirements.
63-64: Process Metadata Numeric Conversion:
The"createTimestamp"and process"id"fields are now numeric, ensuring alignment with the schema’s data type expectations.
75-80: Queue Data Type Alignment:
The"queue"object fields—specifically"callCount","number", and"withAppointment"—are now numeric. This update ensures proper type consistency throughout the JSON.
82-82: Reminder Timestamp Conversion:
The"reminderTimestamp"is now a number (0), which complies with the schema's numeric field requirements.
84-102: Enhanced Process Scope in 10029 Fixture:
The process’s second"scope"object now includes a numeric"id", an enriched"provider"object with detailed information (including"displayName"and"contact"), as well as"shortName"and"source". This comprehensive update enhances both type safety and contextual richness.zmsapi/tests/Zmsapi/fixtures/GetNotification.json (7)
24-31: Appointment Scope Data Enhancement:
The"scope"object within the appointment now reflects a numeric"id"(167), an added"provider"object, and includes"shortName"and"source". This update strengthens consistency across the JSON fixtures.
64-64: Slot Count Conversion:
The"slotCount"field is correctly converted to a numeric value (1), ensuring schema compliance.
72-74: Client Notification Counters Update:
The client fields"emailSendCount","notificationsSendCount", and"surveyAccepted"are now numeric. This update aligns with the stricter type validations required by the new schema.
79-80: Process Identification Fields:
The"createTimestamp"and process"id"are updated to numeric values, maintaining consistency within the process metadata.
83-88: Queue Object Numeric Updates:
The"callCount"and"withAppointment"fields under"queue"are now numeric. Note that the"number"field remains a string ("10029"); please review whether this field should also be numeric to maintain consistency across all fixtures.
90-90: Reminder Timestamp as Numeric:
The"reminderTimestamp"has been successfully converted to a numeric value (0), in line with the updated schema.
92-99: Process Scope in Notification Fixture:
The"scope"object now includes a numeric"id", a fully detailed"provider"object (with"id","name", and"source"), and the additional properties"shortName"and"source". This enhances the overall data structure and type consistency.zmsapi/tests/Zmsapi/fixtures/GetFreeProcessList.json (9)
1-6: Overall JSON Structure and Numeric Conversion Consistency
The changes update numerous fields from string representations to numeric types, which aligns the fixture with stricter schema validation rules and improves data integrity. Please verify that every conversion (e.g. IDs, durations, numeric flags, and date timestamps) exactly matches what the schema expects downstream.
7-50: Scope and Appointment Preference Updates (Object "0")
Within the “scope” block of the first process object, key values such as"id": 141and nested fields under"preferences"—including the appointment settings (e.g."deallocationDuration": 10,"endInDaysDefault": 60,"multipleSlotsEnabled": 0,"reservationDuration": 20,"startInDaysDefault": 0), client flags (e.g."amendmentActivated": 0,"emailRequired": 0,"telephoneActivated": 0,"telephoneRequired": 0), notifications (e.g."enabled": 1,"headsUpTime": 30), and the pickup & queue information—have been converted to numbers as required. This change should ensure that the data now complies with the updated (and stricter) JSON schema validation rules.
68-100: Status and Provider Details (Object "0")
The"status"block (which includes emergency flags and queue counters) and the"provider"block (with numeric values such as"id": 122217and string values like"displayName": "001") have been updated. Please verify that the mix of numeric and string types matches the schema definitions—for example, if provider IDs should be numbers everywhere or if certain identifiers are meant to remain strings.
101-318: Dayoff Array Updates (Object "0")
Every"date"field in the"dayoff"array throughout the first process object has been converted to an integer timestamp. This uniformity should greatly help with schema validation and downstream processing. Ensure that all timestamp values (e.g. 1458860400, 1459116000, …) are in the expected format.
1002-1094: Scope and Preferences in Appointment (Object "1")
In the first appointment of object"1", the"scope"block mirrors the changes seen in object"0". The field"id": 141and nested"preferences"(appointment, client, notifications, pickup, queue, survey, ticketprinter, and workstation) have been updated to use numeric values where appropriate. This replication of changes across process objects helps maintain consistency.
1097-1314: Dayoff Array in Appointment (Object "1")
The"dayoff"array in this appointment now uses integer values for all"date"entries. The conversion is consistent with the updates in object"0", ensuring the timestamps are uniformly represented.
1315-1345: Availability Block in Appointment (Object "1")
Within the"availability"block, numeric scheduling parameters such as"slotTimeInMinutes","startDate", and"endDate"have been explicitly set as numbers. These adjustments comply with the stricter type requirements of the new schema.
1670-1675: Requests Array (Object "1")
In the"requests"array, the identifier remains a string (e.g."id": "120703"). This appears intentional; however, please verify that retaining string types here is consistent with the expected schema (especially since many other IDs were converted to numbers).
1678-1988: Final Scope Block in Object "1"
The final"scope"block (located after the"requests"array) repeats much of the structure found earlier in object"1", including nested"preferences","status","provider", and the extensive"dayoff"array. The numeric conversions in this section mirror previous changes. It is important to confirm that this redundancy is intentional and that the block’s structure meets the overall data requirements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
zmsapi/tests/Zmsapi/AvailabilityAddTest.php (1)
13-40: Extract duplicated test data into a fixture.The provider data is duplicated across test cases. Consider extracting it into a fixture file to improve maintainability.
Create a new fixture file
fixtures/test_provider.json:{ "id": 123456, "name": "Flughafen Schönefeld, Aufsicht", "source": "dldb" }Then update the test to use this fixture:
- "provider": { - "id": 123456, - "name": "Flughafen Schönefeld, Aufsicht", - "source": "dldb" - }, + "provider": ' . file_get_contents(__DIR__ . '/fixtures/test_provider.json') . ',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
zmsapi/tests/Zmsapi/AvailabilityAddTest.php(1 hunks)zmsapi/tests/Zmsapi/AvailabilitySlotsUpdateTest.php(2 hunks)zmsapi/tests/Zmsapi/AvailabilityUpdateTest.php(2 hunks)zmsapi/tests/Zmsapi/CounterGhostWorkstationTest.php(2 hunks)zmsapi/tests/Zmsapi/MailAddTest.php(1 hunks)zmsapi/tests/Zmsapi/fixtures/GetScope_lessData.json(2 hunks)zmsapi/tests/Zmsapi/fixtures/PostAppointment.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- zmsapi/tests/Zmsapi/fixtures/GetScope_lessData.json
- zmsapi/tests/Zmsapi/fixtures/PostAppointment.json
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: call-owasp-security-checks / security-scan (zmsclient, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsslim, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsticketprinter, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsstatistic, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsentities, 8.0)
- GitHub Check: call-php-unit-tests / zmsdb-test
- GitHub Check: call-owasp-security-checks / security-scan (zmscalldisplay, 8.0)
- GitHub Check: call-php-unit-tests / zmsclient-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-php-unit-tests / zmsapi-test
🔇 Additional comments (8)
zmsapi/tests/Zmsapi/AvailabilityAddTest.php (1)
13-40: Add assertions for new scope fields.The test data has been expanded with provider information and shortName, but there are no assertions to validate these new fields in the response.
Consider adding assertions to verify that the response contains the expected provider data and shortName:
$this->assertStringContainsString('availability.json', (string)$response->getBody()); +$responseData = json_decode((string)$response->getBody(), true); +$this->assertEquals(123456, $responseData['scope']['provider']['id']); +$this->assertEquals('Zentrale', $responseData['scope']['shortName']); $this->assertTrue(200 == $response->getStatusCode());zmsapi/tests/Zmsapi/AvailabilityUpdateTest.php (1)
18-32: Add assertions for new scope fields.The test data includes provider information and shortName, but there are no assertions to validate these fields in the response.
Consider adding assertions to verify the response data:
$this->assertStringContainsString('availability.json', (string)$response->getBody()); +$responseData = json_decode((string)$response->getBody(), true); +$this->assertEquals(123456, $responseData['scope']['provider']['id']); +$this->assertEquals('Zentrale', $responseData['scope']['shortName']); $this->assertTrue(200 == $response->getStatusCode());zmsapi/tests/Zmsapi/AvailabilitySlotsUpdateTest.php (1)
12-31: Same issues as in AvailabilityAddTest.php and AvailabilityUpdateTest.php.The same recommendations apply:
- Add assertions for new scope fields
- Extract duplicated provider data into a fixture
zmsapi/tests/Zmsapi/CounterGhostWorkstationTest.php (2)
15-15: String IDs correctly converted to integers.The conversion of string IDs to integers aligns with the stricter type validation requirements of Opis\JsonSchema.
Also applies to: 18-18
17-21:❓ Verification inconclusive
Inconsistent provider structure across test files.
The provider object structure here differs from other test files:
- Uses
displayNameinstead ofname- Includes
$reffield- Missing
sourcefieldThis inconsistency could lead to confusion and maintenance issues.
Consider standardizing the provider structure across all test files. Run this script to find all provider structures:
🏁 Script executed:
#!/bin/bash # Search for provider object structures in test files rg -U 'provider":\s*{[^}]+}' zmsapi/tests/Length of output: 161
Standardize provider object structure across test files.
The provider object in
zmsapi/tests/Zmsapi/CounterGhostWorkstationTest.php(lines 17–21) deviates from the structure used in other test files:
- It uses
displayNameinstead ofname- It includes a
$reffield- It is missing the
sourcefieldThese differences may lead to confusion and maintenance issues. Please standardize this structure across all test files. Also note that the initial regex search failed due to incorrect escaping. You can re-run the verification with the updated script below:
#!/bin/bash rg -U '"provider":\s*\{[^}]+\}' zmsapi/tests/Please verify that this command correctly identifies all provider object declarations and adjust the provider structures as needed.
zmsapi/tests/Zmsapi/MailAddTest.php (3)
38-39: LGTM! Type changes align with schema migration.The conversion of
queueIdfrom string to integer aligns with the stricter validation requirements of Opis\JsonSchema.Also applies to: 44-45
9-18:✅ Verification successful
Verify fixture file compatibility with schema changes.
Since we're migrating from League\JsonGuard to Opis\JsonSchema and updating
queueIdtypes, ensure that theGetMail.jsonfixture file is compatible with these changes.
🏁 Script executed:
#!/bin/bash # Description: Check if the fixture file needs updates for queueId type changes # Search for queueId in the fixture file rg -A 2 '"queueId"' zmsapi/tests/Zmsapi/fixtures/GetMail.jsonLength of output: 287
Fixture file already compliant with schema changes.
The
GetMail.jsonfixture file correctly uses an integer (1234) forqueueId, so it aligns with the updated schema requirements and the ongoing migration. No changes to this fixture are needed at this time. Just ensure that subsequent tests continue to pass with these settings.
27-53:❓ Verification inconclusive
Verify test expectations and error scenarios.
The test expects a
ClientWithoutEmailexception, but the test data doesn't include any email-related fields. This seems inconsistent with the test's purpose of validating input types.Consider:
- Adding test cases specifically for schema validation failures
- Separating the email validation test from type validation tests
🏁 Script executed:
#!/bin/bash # Description: Check for other test cases covering schema validation # Search for similar test methods ast-grep --pattern $'function test.*Input.*\(\) { $$$ expect.*Exception $$$ }'Length of output: 85
Action Required: Validate Test Intent and Consider Additional Schema Validations
I've confirmed that the JSON rendered in the test (lines 27–53 of
zmsapi/tests/Zmsapi/MailAddTest.php) does not include any email fields, which is why the test expects the\BO\Zmsdb\Exception\Mail\ClientWithoutEmailexception. However, the initial grep for similar test patterns (searching for methods with “test…Input” andexpectException) returned no additional cases—meaning that, as of now, there aren’t any dedicated tests targeting schema validation separately from input types.Please verify manually that:
- The absence of an email field here is intentional and aligns with your domain rules.
- There isn’t an overlap between schema validation tests and input type tests elsewhere in the codebase that might need decoupling.
- You consider adding dedicated test cases for schema validation failures and splitting the email validation from other input type validations if needed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
zmsentities/schema/process.json (1)
66-68:⚠️ Potential issueRemove Hardcoded API Key
The new"apiclient"object includes a hardcoded"clientKey", which poses a security risk. As noted in previous reviews, please replace the sensitive API key with a placeholder (or implement environment variable injection) to avoid exposing credentials.
Apply this diff:- "clientKey": "wMdVa5Nu1seuCRSJxhKl2M3yw8zqaAilPH2Xc2IZs" + "clientKey": "<api-key>"🧰 Tools
🪛 Gitleaks (8.21.2)
67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🧹 Nitpick comments (2)
zmsentities/src/Zmsentities/Session.php (1)
20-33: Consider implementing data type coercion for basket properties.Given the PR comments about migrating from League\JsonGuard to Opis\JsonSchema and the stricter validation rules, consider implementing consistent type coercion for the values returned by
getScope(),getProcess(), andgetSelectedDate()to maintain backward compatibility with code that expects specific types.You could implement this by adding type casting in the respective getter methods:
public function getProcess() { - return $this->toProperty()->content->basket->process->get(); + return (int)$this->toProperty()->content->basket->process->get(); } public function getScope() { - return $this->toProperty()->content->basket->scope->get(); + return (int)$this->toProperty()->content->basket->scope->get(); } public function getSelectedDate() { - return $this->toProperty()->content->basket->date->get(); + return (int)$this->toProperty()->content->basket->date->get(); }This would maintain backward compatibility while still allowing string values in the underlying storage to satisfy schema validation.
zmsentities/schema/process.json (1)
319-319: Relaxed Additional Properties – Verify Implications
Changing"additionalProperties"fromfalsetotrueincreases schema flexibility by permitting extra fields. Confirm that this decision does not unintentionally allow invalid or harmful data into processes. It may be worthwhile to document the rationale and any compensating validation measures elsewhere in the codebase.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
zmsentities/schema/dayoff.json(1 hunks)zmsentities/schema/process.json(2 hunks)zmsentities/src/Zmsentities/Process.php(3 hunks)zmsentities/src/Zmsentities/Session.php(2 hunks)zmsentities/src/Zmsentities/Workstation.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- zmsentities/src/Zmsentities/Workstation.php
- zmsentities/src/Zmsentities/Process.php
🧰 Additional context used
🪛 Gitleaks (8.21.2)
zmsentities/schema/process.json
67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: call-owasp-security-checks / security-scan (zmsclient, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsslim, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsentities, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdldb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsticketprinter, 8.0)
- GitHub Check: call-php-code-quality / module-code-quality (zmsstatistic, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsstatistic, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsmessaging, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscitizenapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscalldisplay, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsapi, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsentities, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsadmin, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsdldb, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmscitizenapi, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsadmin, 8.0)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
zmsentities/schema/dayoff.json (1)
13-13: Intentionally Allow Additional Properties
Changing"additionalProperties": falseto"additionalProperties": truealigns with the objectives of the refactoring, allowing extra properties to be included in the JSON objects. Please verify that any downstream consumers of this schema are prepared for the possibility of receiving extra fields, as this may require additional validation or sanitization.zmsentities/src/Zmsentities/Session.php (2)
14-14: Change to public visibility is appropriate.Making the PRIMARY constant public allows direct access via the class name, which aligns with the refactoring goals in this PR.
25-27: Ensure data type consistency with documentation.Changing the default values from integers to strings ('0' instead of 0) addresses the schema validation issues mentioned in the PR comments, helping with the migration from League\JsonGuard to Opis\JsonSchema. However, note that the
getSelectedDate()method on line 125 has a docblock specifying@return integerwhich now conflicts with the string default value.Consider either:
- Updating the docblock to reflect the new string type
- Using type coercion in the getter method to maintain integer return type consistency
/** * * Get selected date * * @return integer */ public function getSelectedDate() { - return $this->toProperty()->content->basket->date->get(); + return (int)$this->toProperty()->content->basket->date->get(); }zmsentities/schema/process.json (3)
7-40: Verify Availability Object Schema Structure
The introduction of the"availability"object within each appointment is well structured and provides detailed information (e.g., booking windows, time slots, repeat rules). Please verify that each property (such as timestamps in"endDate"and"startDate", booleans in"multipleSlotsAllowed", and the nested"weekday"object) accurately reflects the intended data types and usages in downstream processing.
43-62: Confirm Enhanced Provider Details in Scope
The revised"scope"block now contains a more detailed"provider"object (with nested"contact"details and a"displayName"field) as well as the"shortName"property. Ensure that these additions are consistent with related schema definitions (e.g., in department or appointment schemas) and that the data types (e.g., numeric IDs, string names) match the expected formats in consuming applications.
311-312: Ensure Consistency for Scope Short Name
The"shortName": "Zentrale"entry under the outer"scope"object in the example consistently mirrors its use in individual appointment entries. Please verify that this duplication is intentional and that the value remains consistent with business requirements across the entire schema.
…e-json-guard-dependency
Don't merge. This PR is just a draft.
Summary by CodeRabbit
Dependencies & Validation
Schema Enhancements
Code Refactoring
Test Improvements