Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Removing invalid [FromForm] from TenantApiController.Setup (Lombiq Technologies: OCORE-191) #16439

Merged
merged 3 commits into from
Jul 14, 2024
Merged

Removing invalid [FromForm] from TenantApiController.Setup (Lombiq Technologies: OCORE-191) #16439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42dd11d
Moving Recipe into the model
Piedone Jul 13, 2024
c723382
Making the recipe a simple string, not a file
Piedone Jul 13, 2024
617e6ca
Moving the recipe to the viewmodel
Piedone Jul 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/OrchardCore.Modules/OrchardCore.Tenants/Controllers/TenantApiController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How this backward compatible while the old one accepts a file not a JSON string?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why the IFormFile was there :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't be able to answer that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

public async Task<ActionResult> Setup(SetupApiViewModel model)
{
if (!_currentShellSettings.IsDefaultShell())
{
Expand Down Expand Up @@ -393,17 +393,14 @@ public async Task<ActionResult> Setup(SetupApiViewModel model, [FromForm] IFormF

if (string.IsNullOrEmpty(recipeName))
{
if (recipe == null)
if (model.Recipe == null)
{
return BadRequest(S["Either a 'recipe' file or 'RecipeName' is required."]);
}

var tempFilename = Path.GetTempFileName();

using (var fs = System.IO.File.Create(tempFilename))
{
await recipe.CopyToAsync(fs);
}
await System.IO.File.WriteAllTextAsync(tempFilename, model.Recipe);

var fileProvider = new PhysicalFileProvider(Path.GetDirectoryName(tempFilename));

Expand Down
8 changes: 8 additions & 0 deletions src/OrchardCore.Modules/OrchardCore.Tenants/ViewModels/SetupApiViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,16 @@ public class SetupApiViewModel
[DataType(DataType.Password)]
public string Password { get; set; }

/// <summary>
/// The name of a recipe available in the app.
/// </summary>
public string RecipeName { get; set; }

/// <summary>
/// A JSON string representing a custom recipe.
/// </summary>
public string Recipe { get; set; }

public string SiteTimeZone { get; set; }

public string Schema { get; set; }
Expand Down
Loading