Umbraco Engage UmbracoUrlAlias Fix - Fixes #19654#19827
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the variation context system to include property alias awareness, enabling packages like Umbraco Engage to blacklist specific properties from segmentation. This fixes an issue where the umbracoUrlAlias property would cause infinite loops in A/B testing scenarios.
- Added property alias parameter to
ContextualizeVariationmethods across the variation context system - Extended
GetSegmentmethod to accept an optional property alias parameter for property-specific segmentation logic - Updated all calls to variation context methods to pass the property alias information
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Property.cs | Updated all variation context calls to pass PropertyType.Alias parameter |
| VariationContextAccessorExtensions.cs | Modified extension methods to accept and forward property alias parameter |
| VariationContext.cs | Extended GetSegment method signature to include optional propertyAlias parameter |
| PublishedValueFallback.cs | Updated variation context calls to pass property alias from various fallback scenarios |
Ensures the `GetSegment` method overload is not hidden when a null `propertyAlias` is passed.
AndyButland
left a comment
There was a problem hiding this comment.
This seems good to me @cornehoskam. I've just made some changes to resolve the backward compatibility issues.
Can you let me know if you think this needs any more? I.e. if you need to do any more testing to prepare the fix for Engage? Otherwise, we can merge this in for 13.11, and I'll look at cherry-picking it up to 16.
|
Thank you for that @AndyButland! |
|
@AndyButland Done some more testing on the Engage side of things and all seems fine from my end! |
|
OK, great, let's merge this in then and it'll be in 13.11 (not yet scheduled). I'll look at cherry-picking it up to 16. |
* Fixes #19654 Adds the propertyAlias to the VariationContext so that products implementing the GetSegment method are aware which propertyAlias it's being called for * Re-implement original variation context for backwards compatibility * Fixes hidden overload method Ensures the `GetSegment` method overload is not hidden when a null `propertyAlias` is passed. * Resolve backward compatibility issues. * Improved comments. --------- Co-authored-by: Andy Butland <[email protected]> # Conflicts: # src/Umbraco.PublishedCache.NuCache/Property.cs
* Fixes #19654 Adds the propertyAlias to the VariationContext so that products implementing the GetSegment method are aware which propertyAlias it's being called for * Re-implement original variation context for backwards compatibility * Fixes hidden overload method Ensures the `GetSegment` method overload is not hidden when a null `propertyAlias` is passed. * Resolve backward compatibility issues. * Improved comments. --------- # Conflicts: # src/Umbraco.PublishedCache.NuCache/Property.cs
This issue attempts to resolve issue #19654 by adding property awareness to the VariationContext, allowing packages like Umbraco Engage to blacklist certain propertyAliasses from being attempted to be segmented, like the umbracoUrlAlias property.
This code should be used as an example as it has no true backwards compatible as it stands right now, and has been tested with a local setup using Engage 13 and Umbraco 13, using a page with the 'umbracoUrlAlias' property configured. With the Umbraco CMS code that passes the propertyAlias to the variation context GetSegment method, and the code in Engage to blacklist segmenting of the 'umbracoUrlAlias' property, A/B Testing now properly works again on routes that would otherwise cause infinite loops due to routing issues and would fallback on not segmenting the entire page.
This has only been tested against Engage A/B Testing and Personalization, and not against the full scope of all other implications where the PublishedValue & NuCache GetValue may be used in the Core CMS.