leekelleher / umbraco-contentment

Contentment for Umbraco - a state of happiness and satisfaction
https://marketplace.umbraco.com/package/umbraco.community.contentment
Mozilla Public License 2.0
157 stars 72 forks source link

$parent xpath breaks on new nodes #307

Closed glombek closed 1 year ago

glombek commented 1 year ago

Which Contentment version are you using?

4.4.1

Which Umbraco version are you using? For example: 8.14.1 - don't just write v8

11

Bug summary

When creating a node which contains a Contentment Data List with a Umbraco Content Property Values data source, using the $parent xpath, the creation of a new node causes an exception:

System.NullReferenceException, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e: Object reference not set to an instance of an object.
at Umbraco.Community.Contentment.DataEditors.DataList.DataSources.UmbracoContentPropertyValueDataListSource.GetItems(Dictionary`2 config)
   at Umbraco.Community.Contentment.DataEditors.DataListConfigurationEditor.ToValueEditor(Object configuration)
   at Umbraco.Cms.Core.Models.Mapping.ContentPropertyDisplayMapper.Map(IProperty originalProp, ContentPropertyDisplay dest, MapperContext context)
   at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, Type sourceType, MapperContext context)
   at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TSource,TTarget](TSource source, MapperContext context)
   at System.Linq.Enumerable.SelectIPartitionIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.ToList()
   at Umbraco.Cms.Core.Models.Mapping.TabsAndPropertiesMapper.MapProperties(IContentBase content, List`1 properties, MapperContext context)
   at Umbraco.Cms.Core.Models.Mapping.TabsAndPropertiesMapper`1.Map(TSource source, MapperContext context)
   at Umbraco.Cms.Web.BackOffice.Mapping.ContentMapDefinition.Map(IContent source, ContentVariantDisplay target, MapperContext context)
   at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, Type sourceType, MapperContext context)
   at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, MapperContext context)
   at Umbraco.Cms.Core.Models.Mapping.ContentVariantMapper.Map[TVariant](IContent source, MapperContext context)
   at Umbraco.Cms.Web.BackOffice.Mapping.ContentMapDefinition.Map[TVariant](IContent source, ContentItemDisplay`1 target, MapperContext context)
   at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, Type sourceType, MapperContext context)
   at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, MapperContext context)
   at Umbraco.Cms.Web.BackOffice.Controllers.ContentController.MapToDisplay(IContent content, Action`1 contextOptions)
   at Umbraco.Cms.Web.BackOffice.Controllers.ContentController.MapToDisplay(IContent content)
   at Umbraco.Cms.Web.BackOffice.Controllers.ContentController.GetEmptyInner(IContentType contentType, Int32 parentId)
   at lambda_method2820(Closure, Object, Object[])
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.SyncObjectResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeActionMethodAsync()
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeNextActionFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|26_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)

Steps to reproduce

Expected result / actual result

The $parent should select the page where the new node is being created.

We've come across a similar problem in adjacent code where we manually replace these values before the Umbraco XPath logic kicks in:

config["contentNode"] = xpath?.ToString()
    .Replace("$parent", $"id({content.Id})/parent::*")
    .Replace("$current", $"id({content.Id})");

It would be very helpful to have this wrapped in a service (ContentmentXPathPathSyntaxParser?) wrapping the UmbracoXPathPathSyntaxParser to provide this extra logic. Perhaps this could accept a IContentmentContentContext as a paremeter?

Do you have Umbraco ModelsBuilder enabled?

What browsers are you seeing the problem on?

No response

leekelleher commented 1 year ago

@glombek Thanks for raising this. I wont get chance to look at this until later in the week. In the meantime, I'd be open to having a ContentmentXPathPathSyntaxParser wrapper.