mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	fix: allow empty appName as it may come in the url (#3953)
## About the changes Edit application under https://app.unleash-hosted.com/demo/applications/test-app is currently not working as the appName is expected to come in the request body, but it's actually part of the url. We have two options here: 1. We change the UI to adapt to the expectations of the request by adding appName to the request body (and eventually removing appName from the URL, which would be a breaking change) 2. We remove the restriction of only sending the appName in the body and take the one that comes in the URL. We have a validation that verifies that at least one of the two sets the appName ([here](e376088668/src/lib/services/client-metrics/instance-service.ts (L208)) we validate using [this schema](e376088668/src/lib/services/client-metrics/schema.ts (L55-L70))) In terms of REST API, we can assume that the appName will be present in the resource `/api/admin/metrics/applications` (an endpoint we don't have), but when we're updating an application we should refer to that application by its URL: `/api/admin/metrics/applications/<appName>` and the presence of an appName in the body might indicate that we're trying to change the name of the application (something we currently not support) Based on the above, I believe going with the second option is best, as it adheres to REST principles and does not require a breaking change. Despite that, we only support updating applications as the creation is done from metrics ingestion Fixes: #3580
This commit is contained in:
		
							parent
							
								
									9f0d94287e
								
							
						
					
					
						commit
						4e0678dfb1
					
				| @ -5,7 +5,6 @@ export const createApplicationSchema = { | ||||
|     type: 'object', | ||||
|     additionalProperties: true, | ||||
|     description: 'Reported application information from Unleash SDKs', | ||||
|     required: ['appName'], | ||||
|     properties: { | ||||
|         appName: { | ||||
|             description: 'Name of the application', | ||||
|  | ||||
| @ -84,6 +84,20 @@ test('should store application', () => { | ||||
|         .expect(202); | ||||
| }); | ||||
| 
 | ||||
| test('should store application coming from edit application form', () => { | ||||
|     expect.assertions(0); | ||||
|     const appName = '123!23'; | ||||
| 
 | ||||
|     return request | ||||
|         .post(`/api/admin/metrics/applications/${appName}`) | ||||
|         .send({ | ||||
|             url: 'http://test.com', | ||||
|             description: 'This is an optional description', | ||||
|             icon: 'arrow-down', | ||||
|         }) | ||||
|         .expect(202); | ||||
| }); | ||||
| 
 | ||||
| test('should store application details without strategies', () => { | ||||
|     expect.assertions(0); | ||||
|     const appName = '123!23'; | ||||
|  | ||||
| @ -1576,9 +1576,6 @@ The provider you choose for your addon dictates what properties the \`parameters | ||||
|             "type": "string", | ||||
|           }, | ||||
|         }, | ||||
|         "required": [ | ||||
|           "appName", | ||||
|         ], | ||||
|         "type": "object", | ||||
|       }, | ||||
|       "createFeatureSchema": { | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user