Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using Asp.Versioning;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Api.Management.ViewModels.Template;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.OperationStatus;

namespace Umbraco.Cms.Api.Management.Controllers.Template;

[ApiVersion("1.0")]
public class CreateTemplateForDocumentTypeController : TemplateControllerBase
{
private readonly ITemplateService _templateService;
private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor;

public CreateTemplateForDocumentTypeController(
ITemplateService templateService,
IBackOfficeSecurityAccessor backOfficeSecurityAccessor)
{
_templateService = templateService;
_backOfficeSecurityAccessor = backOfficeSecurityAccessor;
}

[HttpPost("for-document-type")]
[MapToApiVersion("1.0")]
[ProducesResponseType(StatusCodes.Status201Created)]
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
public async Task<IActionResult> CreateForDocumentType(CancellationToken cancellationToken, CreateTemplateForDocumentTypeRequestModel requestModel)
Copy link
Contributor

Choose a reason for hiding this comment

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

Another thought - would it be better to remove this new endpoint and instead extend the existing create one? So have CreateTemplateRequestModel take an optional public ReferenceByIdModel? DocumentType { get; set; }, and depending on whether that is provided call _templateService.CreateAsync() or _templateService.CreateForContentTypeAsync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess that depends on whether we want to allow users to create the template after the document type has been created.
Not sure if that was an option in v13 or not, I will double check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just checked how this worked in v13.
You had both options:

  • Create with the content type - which would add a parameter to the content type creation so it knew to create the template as well.
  • Do it after - which would do a request to POST 'https://localhost:44340/umbraco/backoffice/umbracoapi/contenttype/PostCreateDefaultTemplate?id=1055', with id being the id of the content type.
    image

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't necessarily have to expose it in the UI even if the management API supports it (there are other clients of the API these days of course!).

My comment here though was more just about whether it's cleaner to have a single endpoint for creating templates, with a model that optionally allows you to provide the document type to create it for. Or to have, as you currently do in this PR, two endpoints, with two models - one for create a standalone template, and one for creating one for a document type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my confusion.
For a moment I was thinking that it was about adding an option to the endpoint that creates the document type.
I think that makes sense, my only doubt is what to do if you also provide content in the request, I'm guessing we would then ignore the provided content type key?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see your point, and had missed that possibility. In which case I think it's better how you have it as two discrete endpoints. It's clear then you either have the option to a) create and populate a template or b) create an empty template for a document type. And avoids otherwise messy logic of the nature of "if this is provided ignore that".

{
Attempt<ITemplate?, TemplateOperationStatus> result = await _templateService.CreateForContentTypeAsync(
requestModel.DocumentType.Id,
CurrentUserKey(_backOfficeSecurityAccessor));

return result.Success
? CreatedAtId<ByKeyTemplateController>(controller => nameof(controller.ByKey), result.Result!.Key)
: TemplateOperationStatusResult(result.Status);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ protected IActionResult TemplateOperationStatusResult(TemplateOperationStatus st
.WithTitle("Master template cannot be deleted")
.WithDetail("The master templates cannot be deleted. Please ensure the template is not a master template before you delete.")
.Build()),
TemplateOperationStatus.ContentTypeNotFound => BadRequest(problemDetailsBuilder
.WithTitle("Content type not found")
.WithDetail("The provided content type was not found.")
.Build()),
_ => StatusCode(StatusCodes.Status500InternalServerError, problemDetailsBuilder
.WithTitle("Unknown template operation status.")
.Build()),
Expand Down
139 changes: 138 additions & 1 deletion src/Umbraco.Cms.Api.Management/OpenApi.json
Original file line number Diff line number Diff line change
Expand Up @@ -28944,6 +28944,126 @@
]
}
},
"/umbraco/management/api/v1/template/for-document-type": {
"post": {
"tags": [
"Template"
],
"operationId": "PostTemplateForDocumentType",
"requestBody": {
"content": {
"application/json": {
"schema": {
"oneOf": [
{
"$ref": "#/components/schemas/CreateTemplateForDocumentTypeRequestModel"
}
]
}
},
"text/json": {
"schema": {
"oneOf": [
{
"$ref": "#/components/schemas/CreateTemplateForDocumentTypeRequestModel"
}
]
}
},
"application/*+json": {
"schema": {
"oneOf": [
{
"$ref": "#/components/schemas/CreateTemplateForDocumentTypeRequestModel"
}
]
}
}
}
},
"responses": {
"201": {
"description": "Created",
"headers": {
"Umb-Generated-Resource": {
"description": "Identifier of the newly created resource",
"schema": {
"type": "string",
"description": "Identifier of the newly created resource"
}
},
"Location": {
"description": "Location of the newly created resource",
"schema": {
"type": "string",
"description": "Location of the newly created resource",
"format": "uri"
}
},
"Umb-Notifications": {
"description": "The list of notifications produced during the request.",
"schema": {
"type": "array",
"items": {
"$ref": "#/components/schemas/NotificationHeaderModel"
},
"nullable": true
}
}
}
},
"400": {
"description": "Bad Request",
"headers": {
"Umb-Notifications": {
"description": "The list of notifications produced during the request.",
"schema": {
"type": "array",
"items": {
"$ref": "#/components/schemas/NotificationHeaderModel"
},
"nullable": true
}
}
},
"content": {
"application/json": {
"schema": {
"oneOf": [
{
"$ref": "#/components/schemas/ProblemDetails"
}
]
}
}
}
},
"401": {
"description": "The resource is protected and requires an authentication token"
},
"403": {
"description": "The authenticated user does not have access to this resource",
"headers": {
"Umb-Notifications": {
"description": "The list of notifications produced during the request.",
"schema": {
"type": "array",
"items": {
"$ref": "#/components/schemas/NotificationHeaderModel"
},
"nullable": true
}
}
}
}
},
"security": [
{
"Backoffice-User": [ ]
}
]
}
},
"/umbraco/management/api/v1/template/query/execute": {
"post": {
"tags": [
Expand Down Expand Up @@ -37175,6 +37295,22 @@
},
"additionalProperties": false
},
"CreateTemplateForDocumentTypeRequestModel": {
"required": [
"documentType"
],
"type": "object",
"properties": {
"documentType": {
"oneOf": [
{
"$ref": "#/components/schemas/ReferenceByIdModel"
}
]
}
},
"additionalProperties": false
},
"CreateTemplateRequestModel": {
"required": [
"alias",
Expand Down Expand Up @@ -38444,7 +38580,8 @@
"type": "boolean"
},
"allowNonExistingSegmentsCreation": {
"type": "boolean"
"type": "boolean",
"deprecated": true
}
},
"additionalProperties": false
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.ComponentModel.DataAnnotations;

namespace Umbraco.Cms.Api.Management.ViewModels.Template;

public class CreateTemplateForDocumentTypeRequestModel
{
[Required]
public required ReferenceByIdModel DocumentType { get; set; }
}
12 changes: 12 additions & 0 deletions src/Umbraco.Core/Services/ITemplateService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ Task<Attempt<ITemplate, TemplateOperationStatus>> CreateForContentTypeAsync(
string? contentTypeName,
Guid userKey);

/// <summary>
/// Creates a template for a content type.
/// </summary>
/// <param name="contentTypeKey">The content type key.</param>
/// <param name="userKey">The key of the performing user.</param>
/// <returns>
/// An attempt with the created template.
/// </returns>
Task<Attempt<ITemplate?, TemplateOperationStatus>> CreateForContentTypeAsync(
Guid contentTypeKey,
Guid userKey) => throw new NotImplementedException();

/// <summary>
/// Creates a new template
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ public enum TemplateOperationStatus
MasterTemplateNotFound,
CircularMasterTemplateReference,
MasterTemplateCannotBeDeleted,
ContentTypeNotFound,
}
60 changes: 56 additions & 4 deletions src/Umbraco.Core/Services/TemplateService.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
using Microsoft.Extensions.DependencyInjection;

Check notice on line 1 in src/Umbraco.Core/Services/TemplateService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 58.62% to 51.43%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Persistence.Querying;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services.ContentTypeEditing;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Cms.Core.Strings;
using Umbraco.Extensions;
Expand All @@ -20,6 +23,7 @@
private readonly ITemplateContentParserService _templateContentParserService;
private readonly IUserIdKeyResolver _userIdKeyResolver;
private readonly IDefaultViewContentProvider _defaultViewContentProvider;
private readonly IContentTypeService _contentTypeService;

public TemplateService(
ICoreScopeProvider provider,
Expand All @@ -30,7 +34,8 @@
IAuditRepository auditRepository,
ITemplateContentParserService templateContentParserService,
IUserIdKeyResolver userIdKeyResolver,
IDefaultViewContentProvider defaultViewContentProvider)
IDefaultViewContentProvider defaultViewContentProvider,
IContentTypeService contentTypeService)
: base(provider, loggerFactory, eventMessagesFactory)
{
_shortStringHelper = shortStringHelper;
Expand All @@ -39,13 +44,43 @@
_templateContentParserService = templateContentParserService;
_userIdKeyResolver = userIdKeyResolver;
_defaultViewContentProvider = defaultViewContentProvider;
_contentTypeService = contentTypeService;
}

[Obsolete("Use the non-obsolete constructor. Scheduled for removal in v18.")]
public TemplateService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IShortStringHelper shortStringHelper,
ITemplateRepository templateRepository,
IAuditRepository auditRepository,
ITemplateContentParserService templateContentParserService,
IUserIdKeyResolver userIdKeyResolver,
IDefaultViewContentProvider defaultViewContentProvider)
: this(
provider,
loggerFactory,
eventMessagesFactory,
shortStringHelper,
templateRepository,
auditRepository,
templateContentParserService,
userIdKeyResolver,
defaultViewContentProvider,
StaticServiceProvider.Instance.GetRequiredService<IContentTypeService>())
{
}

/// <inheritdoc />
public async Task<Attempt<ITemplate, TemplateOperationStatus>> CreateForContentTypeAsync(
string contentTypeAlias, string? contentTypeName, Guid userKey)
string contentTypeAlias,
string? contentTypeName,
Guid userKey)
{
ITemplate template = new Template(_shortStringHelper, contentTypeName,
ITemplate template = new Template(
_shortStringHelper,
contentTypeName,

// NOTE: We are NOT passing in the content type alias here, we want to use it's name since we don't
// want to save template file names as camelCase, the Template ctor will clean the alias as
Expand All @@ -71,7 +106,7 @@
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
var savingEvent = new TemplateSavingNotification(template, eventMessages, true, contentTypeAlias!);
if (scope.Notifications.PublishCancelable(savingEvent))
if (await scope.Notifications.PublishCancelableAsync(savingEvent))
{
scope.Complete();
return Attempt.FailWithStatus(TemplateOperationStatus.CancelledByNotification, template);
Expand All @@ -89,6 +124,23 @@
return Attempt.SucceedWithStatus(TemplateOperationStatus.Success, template);
}

/// <inheritdoc />
public async Task<Attempt<ITemplate?, TemplateOperationStatus>> CreateForContentTypeAsync(
Guid contentTypeKey,
Guid userKey)
{
IContentType? contentType = await _contentTypeService.GetAsync(contentTypeKey);
if (contentType is null)
{
return Attempt.FailWithStatus(TemplateOperationStatus.ContentTypeNotFound, default(ITemplate));
}

Attempt<ITemplate, TemplateOperationStatus> result = await CreateForContentTypeAsync(contentType.Alias, contentType.Name, userKey);
return result.Success
? Attempt.SucceedWithStatus<ITemplate?, TemplateOperationStatus>(result.Status, result.Result)
: Attempt.FailWithStatus<ITemplate?, TemplateOperationStatus>(result.Status, result.Result);
}

/// <inheritdoc />
public async Task<Attempt<ITemplate, TemplateOperationStatus>> CreateAsync(
string name,
Expand Down

Large diffs are not rendered by default.

Loading
Loading