Merge "REST: Replace patched-invalid-sitelink-type with generic error"

wikimedia · Oct 13, 2024 · 046997a · 046997a
2 parents 954e903 + b147a49
commit 046997a
Show file tree

Hide file tree

Showing 8 changed files with 7 additions and 36 deletions.
diff --git a/repo/rest-api/specs/global/examples.json b/repo/rest-api/specs/global/examples.json
@@ -138,15 +138,6 @@
 			}
 		}
 	},
-	"PatchedInvalidSitelinkTypeExample": {
-		"value": {
-			"code": "patched-invalid-sitelink-type'",
-			"message": "Not a valid Sitelink type in patched Sitelinks",
-			"context": {
-				"site_id": "{site_id}"
-			}
-		}
-	},
 	"PatchResultResourceNotFoundExample": {
 		"value": {
 			"code": "patch-result-referenced-resource-not-found",

diff --git a/repo/rest-api/specs/global/responses.json b/repo/rest-api/specs/global/responses.json
@@ -402,7 +402,6 @@
 					"patched-statement-group-property-id-mismatch": {
 						"$ref": "./examples.json#/PatchedStatementGroupPropertyIdMismatchExample"
 					},
-					"patched-invalid-sitelink-type": { "$ref": "./examples.json#/PatchedInvalidSitelinkTypeExample"},
 					"patch-result-referenced-resource-not-found": { "$ref": "./examples.json#/PatchResultResourceNotFoundExample" },
 					"patch-result-missing-field": { "$ref": "./examples.json#/PatchResultMissingFieldExample" },
 					"patch-result-invalid-key": { "$ref": "./examples.json#/PatchResultInvalidKeyExample" },

diff --git a/repo/rest-api/src/Application/UseCases/PatchItem/PatchedItemValidator.php b/repo/rest-api/src/Application/UseCases/PatchItem/PatchedItemValidator.php
@@ -301,10 +301,9 @@ private function handleSitelinksValidationError( ValidationError $validationErro
 		$siteId = fn() => $context[ SitelinkValidator::CONTEXT_SITE_ID ];
 		switch ( $validationError->getCode() ) {
 			case SitelinksValidator::CODE_INVALID_SITELINK:
-				throw new UseCaseError(
-					UseCaseError::PATCHED_INVALID_SITELINK_TYPE,
-					'Not a valid Sitelink type in patched Sitelinks',
-					[ UseCaseError::CONTEXT_SITE_ID => $context[ SitelinksValidator::CONTEXT_SITE_ID ] ]
+				throw UseCaseError::newPatchResultInvalidValue(
+					"/sitelinks/{$context[SitelinksValidator::CONTEXT_SITE_ID]}",
+					$sitelinksSerialization[$context[SitelinksValidator::CONTEXT_SITE_ID]]
 				);
 			case SitelinksValidator::CODE_SITELINKS_NOT_ASSOCIATIVE:
 				throw UseCaseError::newPatchResultInvalidValue( '/sitelinks', $sitelinksSerialization );

diff --git a/repo/rest-api/src/Application/UseCases/UseCaseError.php b/repo/rest-api/src/Application/UseCases/UseCaseError.php
@@ -26,7 +26,6 @@ class UseCaseError extends UseCaseException {
 	public const PATCH_RESULT_VALUE_TOO_LONG = 'patch-result-value-too-long';
 	public const PATCH_TARGET_NOT_FOUND = 'patch-target-not-found';
 	public const PATCH_TEST_FAILED = 'patch-test-failed';
-	public const PATCHED_INVALID_SITELINK_TYPE = 'patched-invalid-sitelink-type';
 	public const PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH = 'patched-statement-group-property-id-mismatch';
 	public const PERMISSION_DENIED = 'permission-denied';
 	public const PERMISSION_DENIED_REASON_UNAUTHORIZED_BOT_EDIT = 'unauthorized-bot-edit';
@@ -95,7 +94,6 @@ class UseCaseError extends UseCaseException {
 		self::PATCH_RESULT_VALUE_TOO_LONG => [ self::CONTEXT_PATH, self::CONTEXT_LIMIT ],
 		self::PATCH_TARGET_NOT_FOUND => [ self::CONTEXT_PATH ],
 		self::PATCH_TEST_FAILED => [ self::CONTEXT_PATH, self::CONTEXT_ACTUAL_VALUE ],
-		self::PATCHED_INVALID_SITELINK_TYPE => [ self::CONTEXT_SITE_ID ],
 		self::PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH => [
 			self::CONTEXT_PATH,
 			self::CONTEXT_STATEMENT_GROUP_PROPERTY_ID,

diff --git a/repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php b/repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php
@@ -44,7 +44,6 @@ class ErrorResponseToHttpStatus {
 		UseCaseError::PATCH_RESULT_MODIFIED_READ_ONLY_VALUE => 422,
 		UseCaseError::PATCH_RESULT_REFERENCED_RESOURCE_NOT_FOUND => 422,
 		UseCaseError::PATCH_RESULT_VALUE_TOO_LONG => 422,
-		UseCaseError::PATCHED_INVALID_SITELINK_TYPE => 422,
 		UseCaseError::PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH => 422,
 
 		// 429 errors:

diff --git a/repo/rest-api/src/RouteHandlers/openapi.json b/repo/rest-api/src/RouteHandlers/openapi.json
@@ -5434,9 +5434,6 @@
               "patched-statement-group-property-id-mismatch": {
                 "$ref": "#/components/examples/PatchedStatementGroupPropertyIdMismatchExample"
               },
-              "patched-invalid-sitelink-type": {
-                "$ref": "#/components/examples/PatchedInvalidSitelinkTypeExample"
-              },
               "patch-result-referenced-resource-not-found": {
                 "$ref": "#/components/examples/PatchResultResourceNotFoundExample"
               },
@@ -6887,15 +6884,6 @@
           }
         }
       },
-      "PatchedInvalidSitelinkTypeExample": {
-        "value": {
-          "code": "patched-invalid-sitelink-type'",
-          "message": "Not a valid Sitelink type in patched Sitelinks",
-          "context": {
-            "site_id": "{site_id}"
-          }
-        }
-      },
       "PatchResultResourceNotFoundExample": {
         "value": {
           "code": "patch-result-referenced-resource-not-found",

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js
@@ -670,8 +670,9 @@ describe( newPatchItemRequestBuilder().getRouteDescription(), () => {
 				[ { op: 'add', path: '/sitelinks', value: { [ siteId ]: invalidSitelinkType } } ]
 			).assertValidRequest().makeRequest();
 
-			assertValidError( response, 422, 'patched-invalid-sitelink-type', { site_id: siteId } );
-			assert.strictEqual( response.body.message, 'Not a valid Sitelink type in patched Sitelinks' );
+			const context = { path: `/sitelinks/${siteId}`, value: invalidSitelinkType };
+			assertValidError( response, 422, 'patch-result-invalid-value', context );
+			assert.strictEqual( response.body.message, 'Invalid value in patch result' );
 		} );
 
 		it( 'sitelinks not an object', async () => {

diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/PatchItem/PatchedItemValidatorTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/PatchItem/PatchedItemValidatorTest.php
@@ -827,11 +827,7 @@ public static function sitelinksValidationErrorProvider(): Generator {
 
 		yield 'invalid sitelink type' => [
 			[ $validSiteId => 'invalid-sitelink' ],
-			new UseCaseError(
-				UseCaseError::PATCHED_INVALID_SITELINK_TYPE,
-				'Not a valid Sitelink type in patched Sitelinks',
-				[ UseCaseError::CONTEXT_SITE_ID => $validSiteId ]
-			),
+			UseCaseError::newPatchResultInvalidValue( "/sitelinks/$validSiteId", 'invalid-sitelink' ),
 		];
 
 		$invalidSitelinks = [ 'invalid-sitelinks' ];