Merge "REST: Fix the path context of propertyIdMismatch error"

wikimedia · Oct 31, 2024 · d2a5552 · d2a5552
2 parents 3cfb7fb + 539b4dc
commit d2a5552
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 14 deletions.
diff --git a/repo/rest-api/src/Application/Validation/StatementsValidator.php b/repo/rest-api/src/Application/Validation/StatementsValidator.php
@@ -71,7 +71,7 @@ public function validateModifiedStatements(
 				$statementPropertyId = $statement['property']['id'] ?? null;
 				if ( $statementPropertyId && $statementPropertyId !== (string)$propertyId ) {
 					return new ValidationError( self::CODE_PROPERTY_ID_MISMATCH, [
-						self::CONTEXT_PATH => "$propertyId/$groupIndex/property/id",
+						self::CONTEXT_PATH => "$basePath/$propertyId/$groupIndex/property/id",
 						self::CONTEXT_PROPERTY_ID_KEY => $propertyId,
 						self::CONTEXT_PROPERTY_ID_VALUE => $statementPropertyId,
 					] );

diff --git a/repo/rest-api/tests/mocha/api-testing/CreateItemTest.js b/repo/rest-api/tests/mocha/api-testing/CreateItemTest.js
@@ -465,7 +465,7 @@ describe( newCreateItemRequestBuilder().getRouteDescription(), () => {
 				400,
 				'statement-group-property-id-mismatch',
 				{
-					path: `${propertyIdKey}/0/property/id`,
+					path: `/item/statements/${propertyIdKey}/0/property/id`,
 					statement_group_property_id: propertyIdKey,
 					statement_property_id: predicatePropertyId
 				}

diff --git a/repo/rest-api/tests/mocha/api-testing/CreatePropertyTest.js b/repo/rest-api/tests/mocha/api-testing/CreatePropertyTest.js
@@ -301,7 +301,7 @@ describe( newCreatePropertyRequestBuilder().getRouteDescription(), () => {
 				400,
 				'statement-group-property-id-mismatch',
 				{
-					path: `${propertyIdKey}/0/property/id`,
+					path: `/property/statements/${propertyIdKey}/0/property/id`,
 					statement_group_property_id: propertyIdKey,
 					statement_property_id: predicatePropertyId
 				}

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js
@@ -587,7 +587,7 @@ describe( newPatchItemRequestBuilder().getRouteDescription(), () => {
 				.assertValidRequest().makeRequest();
 
 			const context = {
-				path: `${propertyIdKey}/0/property/id`,
+				path: `/statements/${propertyIdKey}/0/property/id`,
 				statement_group_property_id: propertyIdKey,
 				statement_property_id: predicatePropertyId
 			};

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchPropertyTest.js b/repo/rest-api/tests/mocha/api-testing/PatchPropertyTest.js
@@ -593,7 +593,7 @@ describe( newPatchPropertyRequestBuilder().getRouteDescription(), () => {
 				.assertValidRequest().makeRequest();
 
 			const context = {
-				path: `${propertyIdKey}/0/property/id`,
+				path: `/statements/${propertyIdKey}/0/property/id`,
 				statement_group_property_id: propertyIdKey,
 				statement_property_id: predicatePropertyId
 			};

diff --git a/...plication/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializerTest.php b/...plication/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializerTest.php
@@ -493,6 +493,26 @@ public function itemStatementsValidationErrorProvider(): Generator {
 			),
 			UseCaseError::newReferencedResourceNotFound( '/some-path-to-non-existing-property' ),
 		];
+
+		yield 'property id mismatch' => [
+			new ValidationError(
+				StatementsValidator::CODE_PROPERTY_ID_MISMATCH,
+				[
+					StatementsValidator::CONTEXT_PATH => '/item/statements/P123/0/property/id',
+					StatementsValidator::CONTEXT_PROPERTY_ID_KEY => 'P123',
+					StatementsValidator::CONTEXT_PROPERTY_ID_VALUE => 'P567',
+				]
+			),
+			new UseCaseError(
+				UseCaseError::STATEMENT_GROUP_PROPERTY_ID_MISMATCH,
+				"Statement's Property ID does not match the Statement group key",
+				[
+					UseCaseError::CONTEXT_PATH => '/item/statements/P123/0/property/id',
+					UseCaseError::CONTEXT_STATEMENT_GROUP_PROPERTY_ID => 'P123',
+					UseCaseError::CONTEXT_STATEMENT_PROPERTY_ID => 'P567',
+				]
+			),
+		];
 	}
 
 	public static function sitelinksValidationErrorProvider(): Generator {

diff --git a/...est-api/tests/phpunit/Application/UseCases/CreateProperty/CreatePropertyValidatorTest.php b/...est-api/tests/phpunit/Application/UseCases/CreateProperty/CreatePropertyValidatorTest.php
@@ -520,7 +520,7 @@ public function statementsValidationErrorProvider(): Generator {
 				UseCaseError::STATEMENT_GROUP_PROPERTY_ID_MISMATCH,
 				"Statement's Property ID does not match the Statement group key",
 				[
-					UseCaseError::CONTEXT_PATH => self::EXISTING_STRING_PROPERTY_IDS[0] . '/0/property/id',
+					UseCaseError::CONTEXT_PATH => '/property/statements/' . self::EXISTING_STRING_PROPERTY_IDS[0] . '/0/property/id',
 					UseCaseError::CONTEXT_STATEMENT_GROUP_PROPERTY_ID => self::EXISTING_STRING_PROPERTY_IDS[0],
 					UseCaseError::CONTEXT_STATEMENT_PROPERTY_ID => 'P122',
 				]

diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/PatchItem/PatchedItemValidatorTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/PatchItem/PatchedItemValidatorTest.php
@@ -770,7 +770,7 @@ public function providePatchInvalidStatements(): Generator {
 				UseCaseError::PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH,
 				"Statement's Property ID does not match the Statement group key",
 				[
-					UseCaseError::CONTEXT_PATH => "$propertyIdKey/0/property/id",
+					UseCaseError::CONTEXT_PATH => "/statements/$propertyIdKey/0/property/id",
 					UseCaseError::CONTEXT_STATEMENT_GROUP_PROPERTY_ID => $propertyIdKey,
 					UseCaseError::CONTEXT_STATEMENT_PROPERTY_ID => $propertyIdValue,
 				]

diff --git a/...est-api/tests/phpunit/Application/UseCases/PatchProperty/PatchedPropertyValidatorTest.php b/...est-api/tests/phpunit/Application/UseCases/PatchProperty/PatchedPropertyValidatorTest.php
@@ -742,7 +742,7 @@ public function statementsProvider(): Generator {
 				UseCaseError::PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH,
 				"Statement's Property ID does not match the Statement group key",
 				[
-					UseCaseError::CONTEXT_PATH => self::EXISTING_STRING_PROPERTY_IDS[0] . '/0/property/id',
+					UseCaseError::CONTEXT_PATH => '/statements/' . self::EXISTING_STRING_PROPERTY_IDS[0] . '/0/property/id',
 					UseCaseError::CONTEXT_STATEMENT_GROUP_PROPERTY_ID => self::EXISTING_STRING_PROPERTY_IDS[0],
 					UseCaseError::CONTEXT_STATEMENT_PROPERTY_ID => 'P122',
 				]

diff --git a/repo/rest-api/tests/phpunit/Application/Validation/StatementsValidatorTest.php b/repo/rest-api/tests/phpunit/Application/Validation/StatementsValidatorTest.php
@@ -88,25 +88,31 @@ public function testMissingStatementValue_returnsValidationError(): void {
 	/**
 	 * @dataProvider provideInvalidStatements
 	 */
-	public function testInvalidStatements_returnsValidationError( array $invalidStatements, ValidationError $expectedError ): void {
-		$this->assertEquals( $expectedError, $this->newValidator()->validate( $invalidStatements ) );
+	public function testInvalidStatements_returnsValidationError(
+		string $basePath,
+		array $invalidStatements,
+		ValidationError $expectedError
+	): void {
+		$this->assertEquals( $expectedError, $this->newValidator()->validate( $invalidStatements, $basePath ) );
 	}
 
 	public function provideInvalidStatements(): Generator {
 		$invalidStatements = [ 'not a valid statements array' ];
 		yield 'statements field is not an associative array' => [
+			'/statements',
 			$invalidStatements,
 			new ValidationError(
 				StatementsValidator::CODE_STATEMENTS_NOT_ASSOCIATIVE,
 				[
-					StatementsValidator::CONTEXT_PATH => '',
+					StatementsValidator::CONTEXT_PATH => '/statements',
 					StatementsValidator::CONTEXT_VALUE => $invalidStatements,
 				]
 			),
 		];
 
 		$invalidStatementGroup = $this->newSomeValueSerialization( 'P123' );
 		yield 'statement group is not a sequential array (list)' => [
+			'',
 			[ 'P123' => $invalidStatementGroup ],
 			new ValidationError(
 				StatementsValidator::CODE_STATEMENT_GROUP_NOT_SEQUENTIAL,
@@ -119,23 +125,25 @@ public function provideInvalidStatements(): Generator {
 
 		$invalidStatement = 'somevalue';
 		yield 'statement in statement group is not an array' => [
+			'/property/statements',
 			[ 'P123' => [ $invalidStatement ] ],
 			new ValidationError(
 				StatementsValidator::CODE_STATEMENT_NOT_ARRAY,
 				[
-					StatementsValidator::CONTEXT_PATH => '/P123/0',
+					StatementsValidator::CONTEXT_PATH => '/property/statements/P123/0',
 					StatementsValidator::CONTEXT_VALUE => $invalidStatement,
 				]
 			),
 		];
 
 		$invalidStatement = [ 'statement not an associate array' ];
 		yield 'statement in statement group is not an associative array' => [
+			'/item/statements',
 			[ 'P123' => [ $invalidStatement ] ],
 			new ValidationError(
 				StatementValidator::CODE_INVALID_FIELD_TYPE,
 				[
-					StatementValidator::CONTEXT_PATH => '/P123/0',
+					StatementValidator::CONTEXT_PATH => '/item/statements/P123/0',
 					StatementValidator::CONTEXT_VALUE => $invalidStatement,
 				]
 			),
@@ -147,6 +155,7 @@ public function provideInvalidStatements(): Generator {
 			'rank' => 'not-a-valid-rank',
 		];
 		yield 'rank field in a statement is incorrect' => [
+			'',
 			[ 'P567' => [ $invalidStatement ] ],
 			new ValidationError(
 				StatementValidator::CODE_INVALID_FIELD,
@@ -159,13 +168,14 @@ public function provideInvalidStatements(): Generator {
 		];
 
 		yield 'property id mismatch' => [
+			'/statements',
 			[
 				'P123' => [ $this->newSomeValueSerialization( 'P567' ) ],
 			],
 			new ValidationError(
 				StatementsValidator::CODE_PROPERTY_ID_MISMATCH,
 				[
-					StatementsValidator::CONTEXT_PATH => 'P123/0/property/id',
+					StatementsValidator::CONTEXT_PATH => '/statements/P123/0/property/id',
 					StatementsValidator::CONTEXT_PROPERTY_ID_KEY => 'P123',
 					StatementsValidator::CONTEXT_PROPERTY_ID_VALUE => 'P567',
 				]