Label (decimal) used when creating property type programmatically using integer value type

[bjarnef]

Which Umbraco version are you using?

13.9.2

Bug summary

When I create a property type programmatically using ValueStorageType.Integer, the property is using Label (decimal), but I would expect Label (integer). There's another ValueStorageType.Decimal intended for Label (decimal).

Consider the following example inspired from https://skrift.io/issues/working-with-content-types-and-data-types-programmatically/

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

    var propertyType = new PropertyType(_shortStringHelper, Umbraco.Cms.Core.Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer)
    {
        Alias = "courseId",
        Name = "Course ID",
        Description = "The reference of the course.",
        Mandatory = true,
        SortOrder = 1,
    };

    propertyGroup.PropertyTypes!.Add(propertyType);

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

_contentTypeService.Save(ctCourse);

I tried explicit to assign LabelIntGuid, but didn't seem to make a difference.

var propertyType = new PropertyType(_shortStringHelper, Umbraco.Cms.Core.Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer)
{
    Alias = "courseId",
    Name = "Course ID",
    Description = "The reference of the course.",
    Mandatory = true,
    SortOrder = 1,
    DataTypeKey = Umbraco.Cms.Core.Constants.DataTypes.Guids.LabelIntGuid
};

It works if I set DataTypeId instead.

var propertyType = new PropertyType(_shortStringHelper, Umbraco.Cms.Core.Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer)
{
    Alias = "courseId",
    Name = "Course ID",
    Description = "The reference of the course.",
    Mandatory = true,
    SortOrder = 1,
    DataTypeId = Umbraco.Cms.Core.Constants.DataTypes.LabelInt,
    //DataTypeKey = Umbraco.Cms.Core.Constants.DataTypes.Guids.LabelIntGuid,
};

or both, but setting DataTypeKey only doesn't have any impact on this:

var propertyType = new PropertyType(_shortStringHelper, Umbraco.Cms.Core.Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer)
{
    Alias = "courseId",
    Name = "Course ID",
    Description = "The reference of the course.",
    Mandatory = true,
    SortOrder = 1,
    DataTypeId = Umbraco.Cms.Core.Constants.DataTypes.LabelInt,
    DataTypeKey = Umbraco.Cms.Core.Constants.DataTypes.Guids.LabelIntGuid,
};

Alternatively it can of course use the overload method of PropertyType() and pass in the specific IDataType instance.
However it isn't really needed to fetch datatype here as these are locked and referenced in core as well.

Specifics

No response

Steps to reproduce

Use the code example above.

Expected result / actual result

I would expect ValueStorageType to be respected and use the proper Label datatype matching value type as the other way around here: https://github.com/umbraco/Umbraco-CMS/blob/543b644ed19112ab8b401679579485522a9e05df/src/Umbraco.Core/PropertyEditors/ValueTypes.cs#L87C18-L91

[github-actions]

Hi there @bjarnef!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[AndyButland]

I've traced this through, and the reason this happens isn't related to the storage type. Rather there's a lookup to find one of the appropriate data type for the provided property editor alias.

Given there are several data types associated with Umbraco.Cms.Core.Constants.PropertyEditors.Aliases.Label, it's taking the first it finds in the database and using that. As it orders by ID, and these data types have negative numbers, the last added - the decimal one - comes first.

Perhaps we could improve that logic but it'll be difficult to cover all cases and expectations, so I think really it is best solved by being explicit in your code. If you populate the DataTypeId or DataTypeKey property on your propertyType you'll be able to select the data type you want to use.

[bjarnef]

@AndyButland okay, but then it should probably respect use of DataTypeKey - currently this has no effect and still set Label (decimal). When I set DataTypeId from Umbraco core constant it does set Label (integer) datatype.

The integer will most likely be the same across environments as these are installed initially, but perhaps different when migrated from a older Umbraco version, where these didn't exist.
Anyway I would prefer to be able to set it use the datatype guid.

[AndyButland]

Yes, agree we should support setting this via the data type key.

[kjac]

Fixed in #19720 for V13.10

Cherry-picked to V16 in #19735, slated for V16.2