Merge "REST: Test supported media types in edit routes"

wikimedia · Sep 16, 2024 · 17afcde · 17afcde
2 parents a3c13cd + 9d1e213
commit 17afcde
Show file tree

Hide file tree

Showing 12 changed files with 47 additions and 226 deletions.
diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemAliasesTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemAliasesTest.js
@@ -67,19 +67,6 @@ describe( newPatchItemAliasesRequestBuilder().getRouteDescription(), () => {
 			assert.include( response.body.en, alias );
 		} );
 
-		it( 'allows content-type application/json-patch+json', async () => {
-			const alias = 'Brand new English alias';
-			const response = await newPatchItemAliasesRequestBuilder(
-				testItemId,
-				[ { op: 'add', path: '/en/-', value: alias } ]
-			)
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			expect( response ).to.have.status( 200 );
-			assert.include( response.body.en, alias );
-		} );
-
 		it( 'can patch aliases providing edit metadata', async () => {
 			const newDeAlias = `de-alias-${utils.uniq()}`;
 			const user = await getOrCreateBotUser();

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemDescriptionsTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemDescriptionsTest.js
@@ -63,19 +63,6 @@ describe( newPatchItemDescriptionsRequestBuilder().getRouteDescription(), () =>
 			assert.strictEqual( response.body.de, description );
 		} );
 
-		it( 'allows content-type application/json-patch+json', async () => {
-			const description = `Neues Deutsches Beschreibung ${utils.uniq()}`;
-			const response = await newPatchItemDescriptionsRequestBuilder(
-				testItemId,
-				[ { op: 'add', path: '/de', value: description } ]
-			)
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			expect( response ).to.have.status( 200 );
-			assert.strictEqual( response.body.de, description );
-		} );
-
 		it( 'can patch descriptions with edit metadata', async () => {
 			const description = `${utils.uniq()} وصف عربي جديد`;
 			const user = await getOrCreateBotUser();

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemLabelsTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemLabelsTest.js
@@ -66,21 +66,6 @@ describe( newPatchItemLabelsRequestBuilder().getRouteDescription(), () => {
 			assert.strictEqual( response.body.de, label );
 		} );
 
-		it( 'allows content-type application/json-patch+json', async () => {
-			const label = `neues deutsches label ${utils.uniq()}`;
-			const response = await newPatchItemLabelsRequestBuilder(
-				testItemId,
-				[
-					{ op: 'add', path: '/de', value: label }
-				]
-			)
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			expect( response ).to.have.status( 200 );
-			assert.strictEqual( response.body.de, label );
-		} );
-
 		it( 'can patch labels with edit metadata', async () => {
 			const label = `new arabic label ${utils.uniq()}`;
 			const user = await getOrCreateBotUser();

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemStatementTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemStatementTest.js
@@ -88,22 +88,6 @@ describe( 'PATCH statement tests', () => {
 					assert.strictEqual( response.body.value.content, expectedValue );
 				} );
 
-				it( 'allows content-type application/json-patch+json', async () => {
-					const expectedValue = 'i been patched again!!';
-					const response = await newPatchRequestBuilder( testStatementId, [
-						{
-							op: 'replace',
-							path: '/value/content',
-							value: expectedValue
-						}
-					] )
-						.withHeader( 'content-type', 'application/json-patch+json' )
-						.assertValidRequest().makeRequest();
-
-					assertValid200Response( response );
-					assert.strictEqual( response.body.value.content, expectedValue );
-				} );
-
 				it( 'can patch a statement with edit metadata', async () => {
 					const user = await getOrCreateBotUser();
 					const tag = await action.makeTag( 'e2e test tag', 'Created during e2e test', true );

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js b/repo/rest-api/tests/mocha/api-testing/PatchItemTest.js
@@ -100,19 +100,6 @@ describe( newPatchItemRequestBuilder().getRouteDescription(), () => {
 			);
 		} );
 
-		it( 'allows content-type application/json-patch+json', async () => {
-			const label = `some-label-${utils.uniq()}`;
-			const response = await newPatchItemRequestBuilder(
-				testItemId,
-				[ { op: 'add', path: '/labels/de', value: label } ]
-			)
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			expect( response ).to.have.status( 200 );
-			assert.strictEqual( response.body.labels.de, label );
-		} );
-
 		it( 'can patch other fields even if there is a statement using a deleted property', async () => {
 			const propertyToDelete = ( await entityHelper.createUniqueStringProperty() ).entity.id;
 			await newAddItemStatementRequestBuilder(

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchPropertyAliasesTest.js b/repo/rest-api/tests/mocha/api-testing/PatchPropertyAliasesTest.js
@@ -89,38 +89,6 @@ describe( newPatchPropertyAliasesRequestBuilder().getRouteDescription(), () => {
 				formatTermsEditSummary( 'update-languages-short', 'de', editSummary )
 			);
 		} );
-
-		it( 'allows content-type application/json-patch+json', async () => {
-			const expectedValue = `en-alias-${utils.uniq()}`;
-			const response = await newPatchPropertyAliasesRequestBuilder( testPropertyId, [
-				{
-					op: 'add',
-					path: '/en',
-					value: [ expectedValue ]
-				}
-			] )
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.deepEqual( response.body.en, [ expectedValue ] );
-		} );
-
-		it( 'allows content-type application/json', async () => {
-			const expectedValue = `en-alias-${utils.uniq()}`;
-			const response = await newPatchPropertyAliasesRequestBuilder( testPropertyId, [
-				{
-					op: 'add',
-					path: '/en',
-					value: [ expectedValue ]
-				}
-			] )
-				.withHeader( 'content-type', 'application/json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.deepEqual( response.body.en, [ expectedValue ] );
-		} );
 	} );
 
 	describe( '400 Bad Request', () => {

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchPropertyDescriptionsTest.js b/repo/rest-api/tests/mocha/api-testing/PatchPropertyDescriptionsTest.js
@@ -93,38 +93,6 @@ describe( newPatchPropertyDescriptionsRequestBuilder().getRouteDescription(), ()
 			assertValid200Response( response );
 			assert.strictEqual( response.body.de, description );
 		} );
-
-		it( 'allows content-type application/json-patch+json', async () => {
-			const expectedValue = `new arabic description ${utils.uniq()}`;
-			const response = await newPatchPropertyDescriptionsRequestBuilder( testPropertyId, [
-				{
-					op: 'replace',
-					path: '/ar',
-					value: expectedValue
-				}
-			] )
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.strictEqual( response.body.ar, expectedValue );
-		} );
-
-		it( 'allows content-type application/json', async () => {
-			const expectedValue = `new arabic description ${utils.uniq()}`;
-			const response = await newPatchPropertyDescriptionsRequestBuilder( testPropertyId, [
-				{
-					op: 'replace',
-					path: '/ar',
-					value: expectedValue
-				}
-			] )
-				.withHeader( 'content-type', 'application/json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.strictEqual( response.body.ar, expectedValue );
-		} );
 	} );
 
 	describe( '400 error response', () => {

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchPropertyLabelsTest.js b/repo/rest-api/tests/mocha/api-testing/PatchPropertyLabelsTest.js
@@ -85,38 +85,6 @@ describe( newPatchPropertyLabelsRequestBuilder().getRouteDescription(), () => {
 				formatTermsEditSummary( 'update-languages-short', 'ar', editSummary )
 			);
 		} );
-
-		it( 'allows content-type application/json-patch+json', async () => {
-			const expectedValue = `new english label ${utils.uniq()}`;
-			const response = await newPatchPropertyLabelsRequestBuilder( testPropertyId, [
-				{
-					op: 'replace',
-					path: '/en',
-					value: expectedValue
-				}
-			] )
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.strictEqual( response.body.en, expectedValue );
-		} );
-
-		it( 'allows content-type application/json', async () => {
-			const expectedValue = `new english label ${utils.uniq()}`;
-			const response = await newPatchPropertyLabelsRequestBuilder( testPropertyId, [
-				{
-					op: 'replace',
-					path: '/en',
-					value: expectedValue
-				}
-			] )
-				.withHeader( 'content-type', 'application/json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.strictEqual( response.body.en, expectedValue );
-		} );
 	} );
 
 	describe( '400 error response', () => {

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchPropertyStatementTest.js b/repo/rest-api/tests/mocha/api-testing/PatchPropertyStatementTest.js
@@ -90,22 +90,6 @@ describe( 'PATCH property statement', () => {
 					assert.strictEqual( response.body.value.content, expectedValue );
 				} );
 
-				it( 'allows content-type application/json-patch+json', async () => {
-					const expectedValue = 'i been patched again!!';
-					const response = await newPatchRequestBuilder( testStatementId, [
-						{
-							op: 'replace',
-							path: '/value/content',
-							value: expectedValue
-						}
-					] )
-						.withHeader( 'content-type', 'application/json-patch+json' )
-						.assertValidRequest().makeRequest();
-
-					assertValid200Response( response );
-					assert.strictEqual( response.body.value.content, expectedValue );
-				} );
-
 				it( 'can patch a statement with edit metadata', async () => {
 					const user = await getOrCreateBotUser();
 					const tag = await action.makeTag( 'e2e test tag', 'Created during e2e test', true );

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchPropertyTest.js b/repo/rest-api/tests/mocha/api-testing/PatchPropertyTest.js
@@ -95,38 +95,6 @@ describe( newPatchPropertyRequestBuilder().getRouteDescription(), () => {
 			);
 		} );
 
-		it( 'allows content-type application/json-patch+json', async () => {
-			const expectedValue = `new english label ${utils.uniq()}`;
-			const response = await newPatchPropertyRequestBuilder( testPropertyId, [
-				{
-					op: 'add',
-					path: '/labels/en',
-					value: expectedValue
-				}
-			] )
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.strictEqual( response.body.labels.en, expectedValue );
-		} );
-
-		it( 'allows content-type application/json', async () => {
-			const expectedValue = `new english label ${utils.uniq()}`;
-			const response = await newPatchPropertyRequestBuilder( testPropertyId, [
-				{
-					op: 'add',
-					path: '/labels/en',
-					value: expectedValue
-				}
-			] )
-				.withHeader( 'content-type', 'application/json' )
-				.assertValidRequest().makeRequest();
-
-			assertValid200Response( response );
-			assert.strictEqual( response.body.labels.en, expectedValue );
-		} );
-
 		it( 'can patch other fields even if there is a statement using a deleted property', async () => {
 			const propertyToDelete = ( await entityHelper.createUniqueStringProperty() ).entity.id;
 			await newAddPropertyStatementRequestBuilder(

diff --git a/repo/rest-api/tests/mocha/api-testing/PatchSitelinksTest.js b/repo/rest-api/tests/mocha/api-testing/PatchSitelinksTest.js
@@ -58,18 +58,6 @@ describe( newPatchSitelinksRequestBuilder().getRouteDescription(), () => {
 			assertValidResponse( response, 200, sitelink.title, sitelink.badges );
 		} );
 
-		it( 'allows content-type application/json-patch+json', async () => {
-			const sitelink = { title: linkedArticle, badges: [ allowedBadges[ 0 ] ] };
-			const response = await newPatchSitelinksRequestBuilder(
-				testItemId,
-				[ { op: 'add', path: `/${siteId}`, value: sitelink } ]
-			)
-				.withHeader( 'content-type', 'application/json-patch+json' )
-				.assertValidRequest().makeRequest();
-
-			assertValidResponse( response, 200, sitelink.title, sitelink.badges );
-		} );
-
 		it( 'can patch sitelinks with edit metadata', async () => {
 			const sitelink = { title: linkedArticle, badges: [ allowedBadges[ 1 ] ] };
 			const user = await getOrCreateBotUser();

diff --git a/repo/rest-api/tests/mocha/api-testing/SupportedMediaTypeTest.js b/repo/rest-api/tests/mocha/api-testing/SupportedMediaTypeTest.js
@@ -0,0 +1,47 @@
+'use strict';
+
+const { expect } = require( '../helpers/chaiHelper' );
+const {
+	getItemEditRequests,
+	getPropertyEditRequests,
+	getItemCreateRequest
+} = require( '../helpers/happyPathRequestBuilders' );
+const { describeWithTestData } = require( '../helpers/describeWithTestData' );
+
+describeWithTestData(
+	'supported media type requests',
+	( itemRequestInputs, propertyRequestInputs, describeEachRouteWithReset ) => {
+		const requestsToTest = [
+			...getItemEditRequests( itemRequestInputs ),
+			...getPropertyEditRequests( propertyRequestInputs ),
+			getItemCreateRequest( itemRequestInputs )
+		];
+		describeEachRouteWithReset( requestsToTest, ( newRequestBuilder ) => {
+			// We implicitly check that edit routes support application/json
+			// So no need to add additional test for it.
+
+			// Accept DELETE endpoints with no content-type
+			if ( newRequestBuilder().getMethod() === 'DELETE' ) {
+				it( `${newRequestBuilder().getRouteDescription()} responds OK with no content type`,
+					async () => {
+						const response = await newRequestBuilder().assertValidRequest().makeRequest();
+						expect( response.status ).to.be.within( 200, 299 );
+					}
+				);
+			}
+
+			// Accept 'application/json-patch+json' content-type for PATCH endpoints
+			if ( newRequestBuilder().getMethod() === 'PATCH' ) {
+				it( `${newRequestBuilder().getRouteDescription()} responds OK for application/json-patch+json`, async () => {
+					const contentType = 'application/json-patch+json';
+					const response = await newRequestBuilder()
+						.withHeader( 'content-type', contentType )
+						.assertValidRequest()
+						.makeRequest();
+
+					expect( response.status ).to.be.within( 200, 299 );
+				} );
+			}
+		} );
+	}
+);