search

umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
Other
4.49k stars 2.69k forks source link

Models Builder fails on Member after deleting 'Date Picker with time' data type #7919

Closed ronaldbarendse closed 4 years ago

ronaldbarendse commented 4 years ago

After removing the default 'Date Picker with time' data type, Models Builder fails to build the Member model with this error:

Failed to build models.
Could not find a datatype with identifier -36.
Parameter name: id

   at Umbraco.Core.Models.PublishedContent.PublishedContentTypeFactory.GetDataType(Int32 id) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedContentTypeFactory.cs:line 95
   at Umbraco.Core.Models.PublishedContent.PublishedPropertyType..ctor(String propertyTypeAlias, Int32 dataTypeId, Boolean isUserProperty, ContentVariation variations, PropertyValueConverterCollection propertyValueConverters, IPublishedModelFactory publishedModelFactory, IPublishedContentTypeFactory factory) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:line 69
   at Umbraco.Core.Models.PublishedContent.PublishedPropertyType..ctor(IPublishedContentType contentType, String propertyTypeAlias, Int32 dataTypeId, Boolean isUserProperty, ContentVariation variations, PropertyValueConverterCollection propertyValueConverters, IPublishedModelFactory publishedModelFactory, IPublishedContentTypeFactory factory) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:line 47
   at Umbraco.Core.Models.PublishedContent.PublishedContentTypeFactory.CreateCorePropertyType(IPublishedContentType contentType, String propertyTypeAlias, Int32 dataTypeId, ContentVariation variations) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedContentTypeFactory.cs:line 67
   at Umbraco.Core.Models.PublishedContent.PublishedContentType.EnsureMemberProperties(List`1 propertyTypes, IPublishedContentTypeFactory factory) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:line 98
   at Umbraco.Core.Models.PublishedContent.PublishedContentType..ctor(IContentTypeComposition contentType, IPublishedContentTypeFactory factory) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:line 30
   at Umbraco.Core.Models.PublishedContent.PublishedContentTypeFactory.CreateContentType(IContentTypeComposition contentType) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedContentTypeFactory.cs:line 31
   at Umbraco.ModelsBuilder.Umbraco.UmbracoServices.GetTypes(PublishedItemType itemType, IContentTypeComposition[] contentTypes) in D:\d\Zbu ModelsBuilder\src\Umbraco.ModelsBuilder\Umbraco\UmbracoServices.cs:line 125
   at Umbraco.ModelsBuilder.Umbraco.UmbracoServices.GetAllTypes() in D:\d\Zbu ModelsBuilder\src\Umbraco.ModelsBuilder\Umbraco\UmbracoServices.cs:line 41
   at Umbraco.ModelsBuilder.Umbraco.ModelsBuilderBackOfficeController.GenerateModels(UmbracoServices umbracoServices, String modelsDirectory, String bin, String modelsNamespace) in D:\d\Zbu ModelsBuilder\src\Umbraco.ModelsBuilder\Umbraco\ModelsBuilderBackOfficeController.cs:line 121
   at Umbraco.ModelsBuilder.Umbraco.ModelsBuilderBackOfficeController.BuildModels() in D:\d\Zbu ModelsBuilder\src\Umbraco.ModelsBuilder\Umbraco\ModelsBuilderBackOfficeController.cs:line 45

I can verify on a new install that this data type isn't used on the default member type: Date Picker with time

Member type

Umbraco version

I am seeing this issue on Umbraco 8.6.0 and Models Builder 8.1.0.

Reproduction

Bug summary

Looking at the stack trace, the issue comes from Umbraco (not Models Builder). specifically the EnsureMemberProperties method that contains the wrong data types: https://github.com/umbraco/Umbraco-CMS/blob/280f4e7cbef8a1c11b8ef92db8e923d0a3ef8903/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs#L88-L115

Steps to reproduce

  1. Remove the default 'Date Picker with time' data type (id -36)
  2. Ensure you have at least 1 member type (this should also save without errors)
  3. Generate models in Models Builder: this should show the error

Expected result

The EnsureMemberProperties method should not throw the exception, as the property is present and the value type correct (DateTime, although not from the Umbraco.Textbox editor, but Umbraco.Label).

As the 'Label (datetime)' can't be removed from the backoffice, it's practically always available and ensuring on that data type should not throw exceptions.

nul800sebastiaan commented 4 years ago

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/7920

ronaldbarendse commented 4 years ago

This will need additional changes to be correctly fixed, as we discovered another situation where the default Textbox data type (with ID -88) was mistakenly deleted (on an empty instance) and later re-added (getting a new ID). Although some data types can't be deleted (from the back-office), but Textbox isn't included:

https://github.com/umbraco/Umbraco-CMS/blob/ecb6f93e54de1f37e83f4c85bffe81d32c86d917/src/Umbraco.Web/Trees/DataTypeTreeController.cs#L81-L100

Besides EnsureMemberProperties, there's a method to get the default member property types that contains a different ID for the PasswordQuestion (LabelString instead of Textbox):

https://github.com/umbraco/Umbraco-CMS/blob/a9d45413c89d67280ce54c7fd4182e90323bab6c/src/Umbraco.Core/Constants-Conventions.cs#L209-L284

IMHO these IDs should only be used to create the default data types on install (in the DatabaseDataCreator) and we should never assume these IDs always exist (especially when deleting them from the back-office is possible). If this isn't possible, we might also consider making all negative system IDs non-removable...