Skip to content
Merged
Changes from all commits
Commits
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
46 changes: 41 additions & 5 deletions src/Umbraco.Web.BackOffice/Controllers/MacroRenderingController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
[HttpGet]
public async Task<IActionResult> GetMacroResultAsHtmlForEditor(string macroAlias, int pageId,
[FromQuery] IDictionary<string, object> macroParams) =>
await GetMacroResultAsHtml(macroAlias, pageId, macroParams);
await GetMacroResultAsHtml(macroAlias, pageId.ToString(), macroParams);

/// <summary>
/// Gets a rendered macro as HTML for rendering in the rich text editor.
Expand All @@ -98,24 +98,37 @@
/// <param name="model"></param>
/// <returns></returns>
[HttpPost]
[NonAction]
[Obsolete("This endpoint is no longer used.")]
public async Task<IActionResult> GetMacroResultAsHtmlForEditor(MacroParameterModel model) =>
await GetMacroResultAsHtml(model.MacroAlias, model.PageId.ToString(), model.MacroParams);

/// <summary>
/// Gets a rendered macro as HTML for rendering in the rich text editor.
/// Using HTTP POST instead of GET allows for more parameters to be passed as it's not dependent on URL-length
/// limitations like GET.
/// The method using GET is kept to maintain backwards compatibility
/// </summary>
/// <param name="model"></param>
/// <returns></returns>
[HttpPost]
public async Task<IActionResult> GetMacroResultAsHtmlForEditor(MacroParameterModel2 model) =>
await GetMacroResultAsHtml(model.MacroAlias, model.PageId, model.MacroParams);

private async Task<IActionResult> GetMacroResultAsHtml(string? macroAlias, int pageId,
IDictionary<string, object>? macroParams)
private async Task<IActionResult> GetMacroResultAsHtml(string? macroAlias, string pageId, IDictionary<string, object>? macroParams)
{
IMacro? m = macroAlias is null ? null : _macroService.GetByAlias(macroAlias);
if (m == null)
{
return NotFound();
}

IUmbracoContext umbracoContext = _umbracoContextAccessor.GetRequiredUmbracoContext();
IPublishedContent? publishedContent = umbracoContext.Content?.GetById(true, pageId);
IPublishedContent? publishedContent = GetPagePublishedContent(pageId, umbracoContext);

//if it isn't supposed to be rendered in the editor then return an empty string
//currently we cannot render a macro if the page doesn't yet exist
if (pageId == -1 || publishedContent == null || m.DontRender)
if (publishedContent == null || m.DontRender)

Check notice on line 131 in src/Umbraco.Web.BackOffice/Controllers/MacroRenderingController.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ No longer an issue: Complex Method

GetMacroResultAsHtml is no longer above the threshold for cyclomatic complexity

Check notice on line 131 in src/Umbraco.Web.BackOffice/Controllers/MacroRenderingController.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ No longer an issue: Complex Conditional

GetMacroResultAsHtml no longer has a complex conditional
{
//need to create a specific content result formatted as HTML since this controller has been configured
//with only json formatters.
Expand Down Expand Up @@ -149,6 +162,21 @@
}
}

private static IPublishedContent? GetPagePublishedContent(string pageId, IUmbracoContext umbracoContext)
{
if (int.TryParse(pageId, NumberStyles.Integer, CultureInfo.InvariantCulture, out int pageIdAsInt))
{
return umbracoContext.Content?.GetById(true, pageIdAsInt);
}

if (Guid.TryParse(pageId, out Guid pageIdAsGuid))
{
return umbracoContext.Content?.GetById(true, pageIdAsGuid);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@AndyButland would it be better to have Guid.TryParse first.. if page id is Guid there is no reason trying parsing to int.

if (Guid.TryParse(pageId, out Guid pageIdAsGuid))
{
    return umbracoContext.Content?.GetById(true, pageIdAsGuid);
} 

if (int.TryParse(pageId, NumberStyles.Integer, CultureInfo.InvariantCulture, out int pageIdAsInt))
{
    return umbracoContext.Content?.GetById(true, pageIdAsInt);
}

I guess int is mainly for legacy/fallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually int is the expected case. For 13 back-office routes use integers.

Copy link
Contributor

@bjarnef bjarnef Mar 17, 2025

Choose a reason for hiding this comment

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

Yes, and it will continue to that as well, but it pageId is already Guid it won't try parsing to int.

But I see your point as most backoffice routes in v13 are using int id, besides linking in custom extension or packages e.g.

Copy link
Contributor

Choose a reason for hiding this comment

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

It also never fully supported Guid in v13 😅
#15289


return null;
}

[HttpPost]
public IActionResult CreatePartialViewMacroWithFile(CreatePartialViewMacroWithFileModel model)
{
Expand Down Expand Up @@ -180,13 +208,21 @@
return Ok();
}

[Obsolete("This model is no longer used and has been replaced with MacroParameterModel2 that changes the type of the PageId property.")]
public class MacroParameterModel
{
public string? MacroAlias { get; set; }
public int PageId { get; set; }
public IDictionary<string, object>? MacroParams { get; set; }
}

public class MacroParameterModel2
{
public string? MacroAlias { get; set; }
public string PageId { get; set; } = string.Empty;
public IDictionary<string, object>? MacroParams { get; set; }
}

public class CreatePartialViewMacroWithFileModel
{
public string? Filename { get; set; }
Expand Down
Loading