Add support for programmatic creation of property types providing the data type key

[AndyButland]

Prerequisites

• I have added steps to test this contribution in the description below

Resolves: #19715

Description

The linked issue shows that when programmatically creating a property type, only the integer DataTypeId is used when determining the data type to use. If DataTypeKey is provided, this isn't used and the data type is looked up by property alias. And as there can be more than one of these, you may not end up with the data type you expect.

This PR uses the IIdKeyMap to populate the integer Id from the GUID key if only the latter is provided.

Testing

Can be tested with the following surface controller.

Before the update this would create a property type using the "Label (decimal)" data type, even though "Label (jnteger)" was provided. With this update you should see the expected data type used:

using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Logging;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Web.Website.Controllers;

namespace Umbraco.Cms.Web.UI.Controllers;

public class MyTestController : SurfaceController
{
    private readonly IContentTypeService _contentTypeService;
    private readonly IShortStringHelper _shortStringHelper;

    public MyTestController(
        IUmbracoContextAccessor umbracoContextAccessor,
        IUmbracoDatabaseFactory databaseFactory,
        ServiceContext services,
        AppCaches appCaches,
        IProfilingLogger profilingLogger,
        IPublishedUrlProvider publishedUrlProvider,
        IContentTypeService contentTypeService,
        IShortStringHelper shortStringHelper)
        : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider)
    {
        _contentTypeService = contentTypeService;
        _shortStringHelper = shortStringHelper;
    }

    public async Task<IActionResult> AddPropertyType()
    {
        IContentType ct = _contentTypeService.Get("startPage") ?? throw new InvalidOperationException("Content type 'startPage' not found.");

        const string PropertyAlias = "MyTestPropertyAlias";
        if (!ct.PropertyTypeExists(PropertyAlias))
        {
            // Initialize a new tab
            var propertyGroup = new PropertyGroup(false)
            {
                Alias = "info",
                Name = "Information",
                Type = PropertyGroupType.Tab,
                SortOrder = 1
            };

            var propertyType = new PropertyType(_shortStringHelper, Core.Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer)
            {
                Alias = PropertyAlias,
                Name = "My Test Property",
                Description = "My test property description.",
                Mandatory = true,
                SortOrder = 1,
                DataTypeKey = Guid.Parse("8e7f995c-bd81-4627-9932-c40e568ec788"),
            };

            propertyGroup.PropertyTypes!.Add(propertyType);

            // Add the tab to the content type
            ct.PropertyGroups.Add(propertyGroup);
        }

        _contentTypeService.Save(ct);
        return Ok("Done");
    }
}

Merge and Release

Should be part of 13.10 and merged up to be included in 16.2. When applying for 16, check to see if the use of Lazy is needed (an initial look suggests it may not be).

[Copilot]

Pull Request Overview

Adds support for resolving a property type’s integer DataTypeId from its GUID DataTypeKey when programmatically creating property types.

• Injects IIdKeyMap into content-, media-, member-, and template-type repositories and updates constructor signatures accordingly
• Replaces AssignDataTypeFromPropertyEditor with AssignDataTypeIdFromProvidedKeyOrPropertyEditor, adding logic to map a provided DataTypeKey to DataTypeId or fall back to the editor alias
• Updates integration test base and repository tests to supply a Lazy<IIdKeyMap> for new constructor parameters

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/.../TemplateRepositoryTest.cs	Updated ContentTypeRepository instantiation to include IIdKeyMap
tests/.../MemberTypeRepositoryTest.cs	Added using and constructor call for IIdKeyMap
tests/.../MediaTypeRepositoryTest.cs	Added using and constructor call for IIdKeyMap
tests/.../MediaRepositoryTest.cs	Updated instantiation to include IIdKeyMap
tests/.../DocumentRepositoryTest.cs	Updated instantiation to include IIdKeyMap
tests/.../UmbracoIntegrationTest.cs	Exposed IIdKeyMap in the integration test base
src/Umbraco.Infrastructure/.../ContentTypeRepositoryBase.cs	Added _idKeyMap injection and new AssignDataTypeId… logic
src/Umbraco.Infrastructure/.../MemberTypeRepository.cs	Injected IIdKeyMap into constructor and base call
src/Umbraco.Infrastructure/.../MediaTypeRepository.cs	Injected IIdKeyMap into constructor and base call
src/Umbraco.Infrastructure/.../ContentTypeRepository.cs	Injected IIdKeyMap into constructor and base call

Comments suppressed due to low confidence (5)

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs:1450

• The new logic for resolving DataTypeId from DataTypeKey isn't currently covered by existing tests. Consider adding a test that sets DataTypeKey on a property type and verifies the correct DataTypeId is applied or that the fallback is used.

        if (propertyType.DataTypeKey != Guid.Empty)

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs:268

• This test references IIdKeyMap but the namespace Umbraco.Cms.Core.Services isn't imported at the top. Add using Umbraco.Cms.Core.Services; so the code compiles.

            var contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger<ContentTypeRepository>(), commonRepository, languageRepository, ShortStringHelper, new Lazy<IIdKeyMap>(() => IdKeyMap));

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs:58

• This test now uses IIdKeyMap but the import for Umbraco.Cms.Core.Services is missing. Add using Umbraco.Cms.Core.Services; at the top of this file.

        mediaTypeRepository = new MediaTypeRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger<MediaTypeRepository>(), commonRepository, languageRepository, ShortStringHelper, new Lazy<IIdKeyMap>(() => IdKeyMap));

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs:123

• The constructor call now requires IIdKeyMap, but this file lacks the Umbraco.Cms.Core.Services namespace import. Please add using Umbraco.Cms.Core.Services;.

        contentTypeRepository = new ContentTypeRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger<ContentTypeRepository>(), commonRepository, languageRepository, ShortStringHelper, new Lazy<IIdKeyMap>(() => IdKeyMap));

tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs:59

• The integration test base now references IIdKeyMap; ensure using Umbraco.Cms.Core.Services; is added so the type is recognized.

    protected IIdKeyMap IdKeyMap => Services.GetRequiredService<IIdKeyMap>();

[kjac]

Looks good 👍 I added a few tests to verify the fix