Skip to content

Property source level variation should only be applied when configured#16270

Merged
bergmania merged 1 commit intov13/devfrom
v13/fix/variance-issue-16186
May 13, 2024
Merged

Property source level variation should only be applied when configured#16270
bergmania merged 1 commit intov13/devfrom
v13/fix/variance-issue-16186

Conversation

@kjac
Copy link
Copy Markdown
Contributor

@kjac kjac commented May 13, 2024

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #16186

Description

This PR fixes #16186

The property level variance has been a problem child for a while now. These issues are related to the same thing:

Testing this PR

First and foremost, make sure everything works in respect to the original issue (#16186). I have attached testing resources at the end of this PR description, so you don't have to set everything up from scratch. The "Home" item is setup for testing #16186.

Second make sure that property level expansion still works (see #14758 for details). Remember to enable the Content Delivery API 😉 If you're using the test DB attached below, this content item (f9f25aed-4a73-4ed2-aa6e-efb4359a39ce) can be used to verify:

image

image

Database

This DB has everything setup for testing:

Umbraco.sqlite.16186.db.zip

Admin login: admin@localhost / SuperSecret123

Note

You may have to republish everything in all cultures and segments.

Segments setup

Include this code to enable the "customer" and "visitor" segments:

Issue16186.cs
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;

namespace Umbraco.Cms.Web.UI.Custom;

public class VariantsComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.AddNotificationHandler<ServerVariablesParsingNotification, VariantsServerVariablesParser>();
        builder.AddNotificationHandler<SendingContentNotification, CreateSegmentsEvent>();
    }

    public class CreateSegmentsEvent : INotificationHandler<SendingContentNotification>
    {
        private readonly IContentTypeService contentTypeService;

        public CreateSegmentsEvent(IContentTypeService contentTypeService)
        {
            this.contentTypeService = contentTypeService;
        }

        public void Handle(SendingContentNotification notification)
        {
            /* There are several rules for segmentation
                *  - For each original version, we should have additional versions for each segment
             *  - A segment should be added if no version already exists on the content model
             */


            var segments = new List<string>() { "customer", "visitor" };

            var contentType = this.contentTypeService.Get(notification.Content.ContentTypeId ?? -1);
            if (contentType is null) return;

            var props = contentType.PropertyTypes.Where(p => p.VariesBySegment()).Select(p => p.Alias).ToHashSet();
            props.UnionWith(contentType.CompositionPropertyGroups.SelectMany(p => p.PropertyTypes ?? Enumerable.Empty<IPropertyType>()).Where(p => p.VariesBySegment()).Select(p => p.Alias));

            if (!props.Any()) return;

            // The frontend depends on the order of the segments,
            //   therefore it is important to ensure a consistent order of all different variations.
            var newVariants = new List<ContentVariantDisplay>(notification.Content.Variants);
            newVariants = EnsureProperDisplayNames(newVariants, segments);
            newVariants = EnsureSegmentForEachVariant(newVariants, segments, props);
            notification.Content.Variants = newVariants
                .OrderBy(v => IsDefaultLanguage(v) ? 0 : 1)
                .ThenBy(v => IsDefaultSegment(v) ? 0 : 1)
                .ThenBy(v => v?.Language?.Name)
                .ThenBy(v => v.Segment)
                .ToList();
        }

        private static bool IsDefaultLanguage(ContentVariantDisplay variant) =>
            variant.Language == null || variant.Language.IsDefault;

        private static bool IsDefaultSegment(ContentVariantDisplay variant) => variant.Segment == null;

        private List<ContentVariantDisplay> EnsureSegmentForEachVariant(List<ContentVariantDisplay> newVariants, IEnumerable<string> segments, ISet<string> variantProps)
        {
            var additionalVariants = new List<ContentVariantDisplay>();

            // A variant is original if the value for segment is empty
            foreach (var variant in newVariants.Where(v => string.IsNullOrWhiteSpace(v.Segment)))
            {
                // A variant is the same if their language and segment are equal.
                foreach (var segment in segments.Where(s => !newVariants.Any(v => v.Language?.Id == variant.Language?.Id && v.Segment == s)))
                {
                    var newVariant = new ContentVariantDisplay
                    {
                        AllowedActions = new List<string>(variant.AllowedActions),
                        DisplayName = segment,
                        Language = variant.Language,
                        Segment = segment,
                        State = ContentSavedState.NotCreated,
                        Tabs = variant.Tabs.Select(x => new Tab<ContentPropertyDisplay>
                        {
                            Alias = x.Alias,
                            IsActive = x.IsActive,
                            Expanded = x.Expanded,
                            Id = x.Id,
                            Key = x.Key,
                            Label = x.Label,
                            Type = x.Type,
                            Properties = x.Properties?.Select(p => new ContentPropertyDisplay
                            {
                                Alias = p.Alias,
                                Config = p.Config,
                                ConfigNullable = p.ConfigNullable,
                                Culture = p.Culture,
                                DataTypeKey = p.DataTypeKey,
                                Description = p.Description,
                                Editor = p.Editor,
                                HideLabel = p.HideLabel,
                                Id = p.Id,
                                IsSensitive = p.IsSensitive,
                                Label = p.Label,
                                LabelOnTop = p.LabelOnTop,
                                PropertyEditor = p.PropertyEditor,
                                Readonly = p.Readonly,
                                Segment = variantProps.Contains(p.Alias) ? segment : null,
                                SupportsReadOnly = p.SupportsReadOnly,
                                Validation = p.Validation,
                                Value = variantProps.Contains(p.Alias) ? null : p.Value,
                                Variations = p.Variations,
                                View = p.View,
                            }),
                        })
                    };

                    additionalVariants.Add(newVariant);
                }
            }

            newVariants.AddRange(additionalVariants);
            return newVariants;
        }

        private List<ContentVariantDisplay> EnsureProperDisplayNames(List<ContentVariantDisplay> newVariants, IEnumerable<string> segments)
        {
            // Always assign the current name of the segment to the displayname to ensure that the frontend always shows an up-to-date name
            foreach (var variant in newVariants)
            {
                if (string.IsNullOrWhiteSpace(variant.Segment)) continue;
                var segment = segments.FirstOrDefault(s => s == variant.Segment);
                if (segment is null) continue;

                variant.DisplayName = segment;
            }

            return newVariants;
        }
    }

    public class VariantsServerVariablesParser : INotificationHandler<ServerVariablesParsingNotification>
    {
        public void Handle(ServerVariablesParsingNotification notification)
        {
            if (!notification.ServerVariables.TryGetValue("umbracoSettings", out object? umbracoSettingsObject)
                || umbracoSettingsObject is not IDictionary<string, object> umbracoSettings)
            {
                umbracoSettings = new Dictionary<string, object>();
                notification.ServerVariables.Add("umbracoSettings", umbracoSettings);
            }

            umbracoSettings["showAllowSegmentationForDocumentTypes"] = true;
        }
    }
}

Template

Use this template to test the PR:

home.cshtml
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage<IPublishedContent>
@inject IVariationContextAccessor VariationContextAccessor
@inject IPublishedValueFallback PublishedValueFallback
@{
    Layout = null;

    var segment = Context.Request.Query["s"].ToString();

    if (string.IsNullOrWhiteSpace(segment) == false)
    {
        VariationContextAccessor.VariationContext = new VariationContext(VariationContextAccessor.VariationContext?.Culture, segment);
    }

    var cultureAlias = "cultureOnlyText";
    var segmentAlias = "segmentOnlyText";
    var cultureAndSegmentAlias = "segmentAndCultureText";
    var invariantAlias = "invariantText";
}

<h2>Specifiying culture and segment for getting values</h2>

<ul>
    <li>Invariant text :  @Model.Value(invariantAlias, VariationContextAccessor.VariationContext?.Culture, segment)</li>
    <li>Culture text :  @Model.Value(cultureAlias, VariationContextAccessor.VariationContext?.Culture, segment)</li>
    <li>Segment text :  @Model.Value(segmentAlias, VariationContextAccessor.VariationContext?.Culture, segment)</li>
    <li>Culture and segment text : @Model.Value(cultureAndSegmentAlias, VariationContextAccessor.VariationContext?.Culture, segment)</li>
</ul>

<h2>Getting values without specifying anything</h2>

<ul>
    <li>Invariant text :  @Model.Value(invariantAlias)</li>
    <li>Culture text :  @Model.Value(cultureAlias)</li>
    <li>Segment text :  @Model.Value(segmentAlias)</li>
    <li>Culture and segment text : @Model.Value(cultureAndSegmentAlias)</li>
</ul>

<h2>Using publishedFallback (the modelsbuilder way)</h2>

<ul>
    <li>Invariant text :  @Model.Value(PublishedValueFallback, invariantAlias)</li>
    <li>Culture text :  @Model.Value(PublishedValueFallback, cultureAlias)</li>
    <li>Segment text :  @Model.Value(PublishedValueFallback, segmentAlias)</li>
    <li>Culture and segment text : @Model.Value(PublishedValueFallback, cultureAndSegmentAlias)</li>
</ul>

@bergmania bergmania merged commit ab32bac into v13/dev May 13, 2024
@bergmania bergmania deleted the v13/fix/variance-issue-16186 branch May 13, 2024 13:44
bergmania pushed a commit that referenced this pull request May 13, 2024
Zeegaan added a commit that referenced this pull request May 22, 2024
* Updates JSON schema for Umbraco 10 with latest references for Forms and Deploy (#15918)

* Ported over #15928 changes for 13.3 RC (#16023)

* Ported over #15928 changes for 13.3 RC

* Use GetOrAdd()

* Lock dictionary initialization

---------

Co-authored-by: Jason Elkin <jasonelkin86@gmail.com>

* Make the API content response builder extendable (#16056)

* Make the API content response builder extendable

* DeliveryApiJsonTypeResolver needs to be extendable too

* bump rc to regular

* Bump to next minor

* Add blocks in RTE telemetry (#16104)

* Add blocks telemetry

* Use constants and update tests

* V13: Add property type information to telemetry (#16109)

* Add property type counts to telemetry

* Use constants and fix tests

* Update description

* V10: Fix for fallback file upload (#14892) (#15868)

* Fix for fallback file upload (#14892)

* Added check for file type

* Removed unneeded null checks and fixed tabs

* Cleaning

* Cleanups, cleanups, and removal of unneeded null checks

* Reverted removal of relationshipservice

* Revert null check removals (too risky)

---------

Co-authored-by: Ambert van Unen <AvanUnen@ilionx.com>
Co-authored-by: Laura Neto <12862535+lauraneto@users.noreply.github.com>

(cherry picked from commit 0b5d1f8)

* Fix up formatting

---------

Co-authored-by: Ambert van Unen <ambertvu@gmail.com>

* Implementors using Umbraco.Tests.Integration won't have to override GetLocalizedTextService

(cherry picked from commit b001668)
(cherry picked from commit 2bb56f1)

* Fix logic for retrieving lastKnownElement

(cherry picked from commit cae106b)

* bump version

* Bump version

* Bump version

* Since v13 properties can sometimes be of type IRichTextEditorIntermediateValue - this was unexpected in the XPath navigator code (#16121)

* Webhook log improvements (#16200)

* fix: include all headers in webhook log

* feat: return webhook log status from server

* feat: make webhook logs deep linkable

* feat: add webhook log pagination

* feat: improve webhook request/response body preview

* V13: Optimize custom MVC routing (#16218)

* Introduce EagerMatcherPolicy to conditionally bypass content routing

* Ensure that the candidate we disable dynamic routing for is valid

* Skip Umbraco endpoints

* Simplify logic a bit

* Move install logic to matcher

* Ensure that dynamic routing is still skipped when in upgrade state

* Fixup comments

* Reduce nesting a bit

* Don't show maintenance page when statically routed controllers are hít

* Remove excess check, since installer requests are statically routed

* V13: Optimize custom MVC routing (#16218)

* Introduce EagerMatcherPolicy to conditionally bypass content routing

* Ensure that the candidate we disable dynamic routing for is valid

* Skip Umbraco endpoints

* Simplify logic a bit

* Move install logic to matcher

* Ensure that dynamic routing is still skipped when in upgrade state

* Fixup comments

* Reduce nesting a bit

* Don't show maintenance page when statically routed controllers are hít

* Remove excess check, since installer requests are statically routed

(cherry picked from commit ba9ddd1)

* Property source level variation should only be applied when configured (#16270)

* Property source level variation should only be applied when configured (#16270)

(cherry picked from commit ab32bac)

* Merge pull request from GHSA-j74q-mv2c-rxmp

* Merge pull request from GHSA-j74q-mv2c-rxmp

* Merge pull request from GHSA-j74q-mv2c-rxmp

* Fix up after merge

* Remove obselete test

---------

Co-authored-by: Andy Butland <abutland73@gmail.com>
Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>
Co-authored-by: Jason Elkin <jasonelkin86@gmail.com>
Co-authored-by: Sven Geusens <sge@umbraco.dk>
Co-authored-by: Mole <nikolajlauridsen@protonmail.ch>
Co-authored-by: Ambert van Unen <ambertvu@gmail.com>
Co-authored-by: Lars-Erik <lars-erik@aabech.no>
Co-authored-by: Joshua Daniel Pratt Nielsen <jdpnielsen@gmail.com>
Co-authored-by: Bjarke Berg <mail@bergmania.dk>
Co-authored-by: Sebastiaan Janssen <sebastiaan@umbraco.com>
Co-authored-by: Rasmus John Pedersen <mail@rjp.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants