Relations not updated/removed after updating picker references

[bjarnef]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.2.1

Bug summary

I was working on a custom property editor using UmbracoEntityReference similar to content picker and MNTP picker in core, but using a custom relation.

https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Core/PropertyEditors/ContentPickerPropertyEditor.cs#L80

https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Infrastructure/PropertyEditors/MultiNodeTreePickerPropertyEditor.cs#L75

Although the relations are created between category and course node, I later get an error inserting record which already exists.

I tried the same with core Content Picker referencing a picker from Page A to B and from Page B to A, which seems to work without throwing this error.

I also tried implementing IDataValueReferenceFactory but it didn't seem to make a difference: https://our.umbraco.com/documentation/Extending/Property-Editors/Tracking/

public class CourseRelationPickerPropertyEditor : DataEditor
{
    private readonly IEditorConfigurationParser _editorConfigurationParser;
    private readonly IIOHelper _ioHelper;
    private readonly IJsonSerializer _jsonSerializer;

    public CourseRelationPickerPropertyEditor(
        IDataValueEditorFactory dataValueEditorFactory,
        IIOHelper ioHelper,
        IJsonSerializer jsonSerializer,
        IEditorConfigurationParser editorConfigurationParser,
        EditorType type = EditorType.PropertyValue)
        : base(dataValueEditorFactory, type)
    {
        _ioHelper = ioHelper;
        _jsonSerializer = jsonSerializer;
        _editorConfigurationParser = editorConfigurationParser;
        SupportsReadOnly = true;
    }

    /// <inheritdoc />
    protected override IConfigurationEditor CreateConfigurationEditor() =>
        new CourseRelationPickerConfigurationEditor(_ioHelper, _jsonSerializer, _editorConfigurationParser);

    protected override IDataValueEditor CreateValueEditor() =>
        DataValueEditorFactory.Create<CourseRelationPickerPropertyValueEditor>(Attribute!);

    public class CourseRelationPickerPropertyValueEditor : DataValueEditor, IDataValueReferenceFactory, IDataValueReference
    {
        public CourseRelationPickerPropertyValueEditor(
            ILocalizedTextService localizedTextService,
            IShortStringHelper shortStringHelper,
            IJsonSerializer jsonSerializer,
            IIOHelper ioHelper,
            DataEditorAttribute attribute)
            : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute)
        {
        }

        public IDataValueReference GetDataValueReference() => this;

        public bool IsForEditor(IDataEditor? dataEditor) => dataEditor?.Alias == Constants.PropertyEditors.Aliases.CourseRelationPicker;

        public IEnumerable<UmbracoEntityReference> GetReferences(object? value)
        {
            var asString = value == null ? string.Empty : value is string str ? str : value.ToString();

            var udiPaths = asString!.Split(',');

            foreach (var udiPath in udiPaths)
            {
                if (UdiParser.TryParse(udiPath, out Udi? udi))
                {
                    yield return new UmbracoEntityReference(udi, Constants.RelationTypes.RelatedCourseAlias);
                }
            }
        }
    }
}

Not sure if I am missing something here or if it doesn't work with a custom relation type or depends on the configuration of the relation type?

Specifics

No response

Steps to reproduce

Add a custom property editor implementing IDataValueReference and returning UmbracoEntityReference similar to content picker and MNTP in core, but using a custom relation type.

Expected result / actual result

No response

---

_This item has been added to our backlog AB#24047_

[bjarnef]

@Zeegaan @kjac do you know how/where the relations are updated/removed? I my use-case they still exists after the selections has been removed.

[bjarnef]

@bergmania do you know how this works in core. I think this should be pretty much like Content Picker and MNTP, but it doesn't seem to remove or update existing relations.

I guess it should work with a custom relation type as well although Umbraco core doesn't use it, but decides the relation type from the udi. https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs#L20-L28 (maybe there should be one for member as well, e.g. member picker and MNTP (member)

I guess member based pickers currently don't create relations.

What am I missing? 🙈

[bjarnef]

I am correct that the relations are deleted and new created here? https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs#L1038-L1086

but it seems this only happen to umbContent and umbMedia relations. https://github.com/umbraco/Umbraco-CMS/blob/v11/contrib/src/Umbraco.Core/Constants-Conventions.cs#L285

So I guess UmbracoEntityReference doesn't work as expected with a custom relation type, as it only creates relations, but doesn't handle the deletion first.

I guess I would need to handle this myself in the class inheriting DataValueEditor, IDataValueReferenceFactory, IDataValueReference.

[bjarnef]

My workaround was to hook into ContentSavingNotification to remove existing relations, before creating the new ones. I tried ContentSavedNotification as well, but it seems it was too later at the it already tried to insert the duplicate relations and break so it didn't hit that notification handler.

public class CourseCategorySavingNotification : INotificationHandler<ContentSavingNotification>
{
    private readonly IRelationService _relationService;

    public CourseCategorySavingNotification(
        IRelationService relationService)
    {
        _relationService = relationService;
    }

    public void Handle(ContentSavingNotification notification)
    {
        foreach (var node in notification.SavedEntities)
        {
            if (node.ContentType.Alias.Equals(Course.ModelTypeAlias) || node.ContentType.Alias.Equals(Category.ModelTypeAlias))
            {
                var relations = _relationService.GetByParentId(node.Id, Constants.RelationTypes.RelatedCourseAlias);

                if (relations != null)
                {
                    foreach (var relation in relations)
                    {
                        _relationService.Delete(relation);
                    }
                }
            }
        }
    }
}

@Zeegaan @bergmania I think Umbraco need to handle this as when using this overload in implementation of GetReferences() in a property editor.

https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs#L10-L14

and make this part work as well for other relation types: https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs#L1045-L1046

[bjarnef]

The PersistRelations method is called for document here for new entity here https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs#L1057

and updated entity here https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs#L1308

[bjarnef]

@Zeegaan @bergmania could we fetch unique relation types from the collection:

https://github.com/umbraco/Umbraco-CMS/blob/b69afe81f3f6fcd37480b3b0295a62af44ede245/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs#L1043

and then pass it into the DeleteByParent method, e.g.

var trackedRelations = new List<UmbracoEntityReference>();
trackedRelations.AddRange(_dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors));

var relationTypes = trackedRelations.Where(x => !x.IsEmpty()).Select(x => x.RelationTypeAlias).Distinct();

// First delete all auto-relations for this entity
//RelationRepository.DeleteByParent(entity.Id, Constants.Conventions.RelationTypes.AutomaticRelationTypes);
RelationRepository.DeleteByParent(entity.Id, relationTypes);

instead of the current array. https://github.com/umbraco/Umbraco-CMS/blob/46414f2c646d9a04e2d3614876a7a311cba8653a/src/Umbraco.Core/Constants-Conventions.cs#L285

[bjarnef]

@AndyButland I think this cause issues with restore as well.

On restore it trigger the ContentCacheRefresherNotification, but not the ContentSavingNotification (where we handle the deletion on the existing relations), so we get this.

-- EXCEPTION ---------------------------------------------------
 (proper exception traces at the end of this log)
Microsoft.Data.SqlClient.SqlException: Cannot insert duplicate key row in object 'dbo.umbracoRelation' with unique index 'IX_umbracoRelation_parentChildType'. The duplicate key value is (1152, 1149, 9).
The statement has been terminated.
Umbraco.Deploy.Core.Exceptions.ProcessArtifactException: Process pass #4 failed for artifact umb://document/76fd31bb700f41898f0fb435a96cefb7.
----------------------------------------------------------------

and have to clear the existing pickers/relations on the mentioned nodes and do another restore.. but later we end up with same issue again.