Merge "REST: Replace url-not-modifiable with generic error"

wikimedia · Oct 13, 2024 · 954e903 · 954e903
2 parents ac7b46f + d289181
commit 954e903
Show file tree

Hide file tree

Showing 12 changed files with 15 additions and 56 deletions.
diff --git a/repo/rest-api/specs/global/examples.json b/repo/rest-api/specs/global/examples.json
@@ -157,15 +157,6 @@
 			}
 		}
 	},
-	"SitelinkUrlNotModifiableExample": {
-		"value": {
-			"code": "url-not-modifiable",
-			"message": "URL of Sitelink cannot be modified",
-			"context": {
-				"site_id": "{site_id}"
-			}
-		}
-	},
 	"PatchResultInvalidKeyExample": {
 		"value": {
 			"code": "patch-result-invalid-key",

diff --git a/repo/rest-api/specs/global/responses.json b/repo/rest-api/specs/global/responses.json
@@ -410,7 +410,6 @@
 					"patch-result-modified-read-only-value": {
 						"$ref": "./examples.json#/PatchResultModifiedReadOnlyValue"
 					},
-					"url-not-modifiable": { "$ref": "./examples.json#/SitelinkUrlNotModifiableExample" },
 					"data-policy-violation": { "$ref": "./examples.json#/DataPolicyViolationExample" }
 				}
 			}

diff --git a/repo/rest-api/specs/resources/sitelinks/responses.json b/repo/rest-api/specs/resources/sitelinks/responses.json
@@ -9,7 +9,9 @@
 					"patch-result-invalid-value": { "$ref": "../../global/examples.json#/PatchResultInvalidValueExample" },
 					"patch-result-missing-field": { "$ref": "../../global/examples.json#/PatchResultMissingFieldExample" },
 					"patch-result-invalid-key": { "$ref": "../../global/examples.json#/PatchResultInvalidKeyExample" },
-					"url-not-modifiable": { "$ref": "../../global/examples.json#/SitelinkUrlNotModifiableExample" },
+					"patch-result-modified-read-only-value": {
+						"$ref": "../../global/examples.json#/PatchResultModifiedReadOnlyValue"
+					},
 					"data-policy-violation": { "$ref": "../../global/examples.json#/DataPolicyViolationExample" }
 				}
 			}

diff --git a/repo/rest-api/src/Application/UseCases/PatchItem/PatchedItemValidator.php b/repo/rest-api/src/Application/UseCases/PatchItem/PatchedItemValidator.php
@@ -348,11 +348,7 @@ private function assertUrlsNotModified( Sitelinks $originalSitelinks, array $pat
 				isset( $originalSitelinks[ $siteId ] ) &&
 				$originalSitelinks[ $siteId ]->getUrl() !== $sitelink[ 'url' ]
 			) {
-				throw new UseCaseError(
-					UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE,
-					'URL of Sitelink cannot be modified',
-					[ UseCaseError::CONTEXT_SITE_ID => $siteId ]
-				);
+				throw UseCaseError::newPatchResultModifiedReadOnlyValue( "/sitelinks/$siteId/url" );
 			}
 		}
 	}

diff --git a/repo/rest-api/src/Application/UseCases/PatchSitelinks/PatchedSitelinksValidator.php b/repo/rest-api/src/Application/UseCases/PatchSitelinks/PatchedSitelinksValidator.php
@@ -98,11 +98,7 @@ private function assertUrlsNotModified( array $originalSitelinksSerialization, a
 				isset( $originalSitelinksSerialization[ $siteId ] ) &&
 				$originalSitelinksSerialization[ $siteId ][ 'url' ] !== $sitelink[ 'url' ]
 			) {
-				throw new UseCaseError(
-					UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE,
-					'URL of Sitelink cannot be modified',
-					[ UseCaseError::CONTEXT_SITE_ID => $siteId ]
-				);
+				throw UseCaseError::newPatchResultModifiedReadOnlyValue( "/$siteId/url" );
 			}
 		}
 	}

diff --git a/repo/rest-api/src/Application/UseCases/UseCaseError.php b/repo/rest-api/src/Application/UseCases/UseCaseError.php
@@ -27,7 +27,6 @@ class UseCaseError extends UseCaseException {
 	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_SITELINK_URL_NOT_MODIFIABLE = 'url-not-modifiable';
 	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';
@@ -97,7 +96,6 @@ class UseCaseError extends UseCaseException {
 		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_SITELINK_URL_NOT_MODIFIABLE => [ 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
@@ -45,7 +45,6 @@ class ErrorResponseToHttpStatus {
 		UseCaseError::PATCH_RESULT_REFERENCED_RESOURCE_NOT_FOUND => 422,
 		UseCaseError::PATCH_RESULT_VALUE_TOO_LONG => 422,
 		UseCaseError::PATCHED_INVALID_SITELINK_TYPE => 422,
-		UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE => 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
@@ -390,8 +390,8 @@
                   "patch-result-invalid-key": {
                     "$ref": "#/components/examples/PatchResultInvalidKeyExample"
                   },
-                  "url-not-modifiable": {
-                    "$ref": "#/components/examples/SitelinkUrlNotModifiableExample"
+                  "patch-result-modified-read-only-value": {
+                    "$ref": "#/components/examples/PatchResultModifiedReadOnlyValue"
                   },
                   "data-policy-violation": {
                     "$ref": "#/components/examples/DataPolicyViolationExample"
@@ -5452,9 +5452,6 @@
               "patch-result-modified-read-only-value": {
                 "$ref": "#/components/examples/PatchResultModifiedReadOnlyValue"
               },
-              "url-not-modifiable": {
-                "$ref": "#/components/examples/SitelinkUrlNotModifiableExample"
-              },
               "data-policy-violation": {
                 "$ref": "#/components/examples/DataPolicyViolationExample"
               }
@@ -6909,15 +6906,6 @@
           }
         }
       },
-      "SitelinkUrlNotModifiableExample": {
-        "value": {
-          "code": "url-not-modifiable",
-          "message": "URL of Sitelink cannot be modified",
-          "context": {
-            "site_id": "{site_id}"
-          }
-        }
-      },
       "PatchResultInvalidKeyExample": {
         "value": {
           "code": "patch-result-invalid-key",

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js
@@ -825,8 +825,9 @@ describe( newPatchItemRequestBuilder().getRouteDescription(), () => {
 				} ]
 			).assertValidRequest().makeRequest();
 
-			assertValidError( response, 422, 'url-not-modifiable', { site_id: siteId } );
-			assert.equal( response.body.message, 'URL of Sitelink cannot be modified' );
+			const path = `/sitelinks/${siteId}/url`;
+			assertValidError( response, 422, 'patch-result-modified-read-only-value', { path } );
+			assert.strictEqual( response.body.message, 'Read only value in patch result cannot be modified' );
 		} );
 
 		it( 'rejects statement with non-existent property', async () => {

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchSitelinksTest.js b/repo/rest-api/tests/mocha/api-testing/PatchSitelinksTest.js
@@ -308,8 +308,9 @@ describe( newPatchSitelinksRequestBuilder().getRouteDescription(), () => {
 				]
 			).assertValidRequest().makeRequest();
 
-			assertValidError( response, 422, 'url-not-modifiable', { site_id: siteId } );
-			assert.equal( response.body.message, 'URL of Sitelink cannot be modified' );
+			const path = `/${siteId}/url`;
+			assertValidError( response, 422, 'patch-result-modified-read-only-value', { path } );
+			assert.strictEqual( response.body.message, 'Read only value in patch result cannot be modified' );
 		} );
 
 	} );

diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/PatchItem/PatchedItemValidatorTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/PatchItem/PatchedItemValidatorTest.php
@@ -1056,12 +1056,6 @@ public function testSitelinkUrlModification_throws(): void {
 			'sitelinks' => [ $validSiteId => [ 'title' => $title, 'url' => 'https://en.wikipedia.org/wiki/.example' ] ],
 		];
 
-		$expectedError = new UseCaseError(
-			UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE,
-			'URL of Sitelink cannot be modified',
-			[ UseCaseError::CONTEXT_SITE_ID => $validSiteId ]
-		);
-
 		try {
 			$this->newValidator()->validateAndDeserialize(
 				$item,
@@ -1071,7 +1065,7 @@ public function testSitelinkUrlModification_throws(): void {
 			);
 			$this->fail( 'this should not be reached' );
 		} catch ( UseCaseError $error ) {
-			$this->assertEquals( $expectedError, $error );
+			$this->assertEquals( UseCaseError::newPatchResultModifiedReadOnlyValue( "/sitelinks/$validSiteId/url" ), $error );
 		}
 	}
 

diff --git a/...t-api/tests/phpunit/Application/UseCases/PatchSitelinks/PatchedSitelinksValidatorTest.php b/...t-api/tests/phpunit/Application/UseCases/PatchSitelinks/PatchedSitelinksValidatorTest.php
@@ -202,12 +202,6 @@ public function testSitelinkUrlModification_throws(): void {
 		$validSiteId = TestValidatingRequestDeserializer::ALLOWED_SITE_IDS[0];
 		$title = 'test_title';
 
-		$expectedError = new UseCaseError(
-			UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE,
-			'URL of Sitelink cannot be modified',
-			[ UseCaseError::CONTEXT_SITE_ID => $validSiteId ]
-		);
-
 		try {
 			$this->newValidator( new SameTitleSitelinkTargetResolver() )->validateAndDeserialize(
 				self::SITELINK_ITEM_ID,
@@ -217,7 +211,7 @@ public function testSitelinkUrlModification_throws(): void {
 
 			$this->fail( 'this should not be reached' );
 		} catch ( UseCaseError $error ) {
-			$this->assertEquals( $expectedError, $error );
+			$this->assertEquals( UseCaseError::newPatchResultModifiedReadOnlyValue( "/$validSiteId/url" ), $error );
 		}
 	}