Removing invalid [FromForm] from TenantApiController.Setup (Lombiq Technologies: OCORE-191)
#16439
Conversation
| @@ -295,7 +295,7 @@ public async Task<IActionResult> Remove(string tenantName) | |||
|
|
|||
| [HttpPost] | |||
| [Route("setup")] | |||
| public async Task<ActionResult> Setup(SetupApiViewModel model, [FromForm] IFormFile recipe = null) | |||
There was a problem hiding this comment.
How this backward compatible while the old one accepts a file not a JSON string?
There was a problem hiding this comment.
The old one doesn't expect a file either. For that, as well as for the new one, the HTTP request is something like this, with the recipe passed in being part of the JSON body:
{"name":"ApiClientTenant638565081195196341","siteName":"Api Client Tenant Site","databaseProvider":"Sqlite","connectionString":"","tablePrefix":"apiclienttenant638565081195196341","userName":"admin","email":"admin@example.com","password":"Password1!","recipeName":null,"Recipe":"{\r\n \u0022name\u0022: \u0022Agency\u0022,}\r\n","siteTimeZone":"Europe/Budapest"}There was a problem hiding this comment.
I'm curious why the IFormFile was there :)
There was a problem hiding this comment.
I won't be able to answer that.
There was a problem hiding this comment.
This parameter was added in this PR, but before that it was a property in SetupApiViewModel ever since the tenant API was introduced. So I guess moving it into a standalone parameter was just for the sake of adding [FromForm]. I don't see any related changes to moving from SetupApiViewModel.Recipe to recipe in #14058, so it's a safe bet that this optional pathway was dead code.
|
Can we have a test for this one? |
|
I don't think it makes too much sense to test a controller action, apart from a UI test. How would you test it? |
|
It could be a functional test, anyhow let's merge if you are test it. I trust you ;) |
|
We don't have a framework set up for functional tests apart from the JS ones, which I won't be able to extend (nor see much point to it either). BTW I used https://github.com/Lombiq/Orchard-Core-API-Client to make sure that this works the same as before. Thanks! |
Nice, I want to point to it but thanks that you mentioned |
Fixes #16430.
This is backward compatible, the requests sent to the API remain the same.