- (Lava) Updated Dynamic Shortcode processing to only enter a child scope if indicated by the newly-added Shortcode Scope Behavior. Also updated the Workflow Activate Lava block to no longer enter a child scope while preserving any provided merge field values that use reserved keys. Added unit and integration tests to support these changes.

jasonhendee · jasonhendee · commit 853dae538e1b · 2025-12-10T14:27:58.000-07:00
diff --git a/Rock.Lava.Fluid/FluidRenderContext.cs b/Rock.Lava.Fluid/FluidRenderContext.cs
@@ -32,7 +32,6 @@ public class FluidRenderContext : LavaRenderContextBase
         private TemplateContext _context;
         private const string _InternalFieldKeyPrefix = "$_";
         private const string _InternalFieldKeyEnabledCommands = "EnabledCommands";
-        private const string _InternalFieldKeyIsChildScope = "_isChildScope";
 
         #region Constructors
 
@@ -180,9 +179,7 @@ public override void SetEnabledCommands( IEnumerable<string> commands )
         {
             var commandString = commands == null ? string.Empty : commands.JoinStrings( "," );
 
-            SetInternalField( _InternalFieldKeyEnabledCommands,
-                commandString,
-                LavaContextRelativeScopeSpecifier.Local );
+            SetInternalField( _InternalFieldKeyEnabledCommands, commandString );
         }
 
         /// <summary>
@@ -191,23 +188,7 @@ public override void SetEnabledCommands( IEnumerable<string> commands )
         /// </summary>
         public override void EnterChildScope()
         {
-            // In Fluid, the ChildScope has the following behavior:
-            // 1. GetValue(var) gets the value of "var" from the innermost scope in which the variable is currently defined.
-            // 2. SetValue(var) sets the value of "var" for the innermost scope.
-            // ... and the ForLoopScope has the following behavior:
-            // 1. GetValue(var) gets the value of "var" from the innermost scope in which the variable is currently defined.
-            // 2. SetValue(var) sets the value of "var" for the parent scope.
-            // 3. SetOwnValue(var) sets the value of "var" for the innermost scope.
-
-            // In Lava, the child scope has the following behavior:
-            // 1. GetValue(var) gets the value of "var" from the innermost scope in which the variable is currently defined.
-            // 2. SetValue(var) sets the value of "var" in the parent scope if it is already defined;
-            //    if not, the variable is created in the child scope.
-
-            _context.EnterForLoopScope();
-
-            var localScope = _contextScopeInternalField.GetValue( _context ) as Scope;
-            localScope.SetOwnValue( _InternalFieldKeyIsChildScope, FluidValue.Create( true, _context.Options ) );
+            _context.EnterChildScope();
         }
 
         /// <summary>
@@ -279,19 +260,9 @@ private void SetFieldPrivate( string key, object value, bool allowInternalFieldA
                 value = new FluidRawValueProxy( value );
             }
 
-            if ( scope == LavaContextRelativeScopeSpecifier.Local )
+            if ( scope == LavaContextRelativeScopeSpecifier.Default )
             {
-                var isChildScope = localScope.Properties.Contains( _InternalFieldKeyIsChildScope );
-                if ( isChildScope )
-                {
-                    // We are using the Fluid "ForLoopScope", which would set the value in the parent scope by default.
-                    // Here we must specify that we want to set the value in the current child scope.
-                    localScope.SetOwnValue( key, FluidValue.Create( value, _context.Options ) );
-                }
-                else
-                {
-                    _context.SetValue( key, value );
-                }
+                _context.SetValue( key, value );
             }
             else if ( scope == LavaContextRelativeScopeSpecifier.Parent )
             {
diff --git a/Rock.Lava.Shared/Core/DynamicShortcodeDefinition.cs b/Rock.Lava.Shared/Core/DynamicShortcodeDefinition.cs
@@ -17,6 +17,8 @@
 using System;
 using System.Collections.Generic;
 
+using Rock.Enums.Cms;
+
 namespace Rock.Lava
 {
     /// <summary>
@@ -62,6 +64,11 @@ public DynamicShortcodeDefinition()
         /// </summary>
         public LavaShortcodeTypeSpecifier ElementType { get; set; }
 
+        /// <summary>
+        /// The scope behavior for variables that are defined within this shortcode.
+        /// </summary>
+        public ShortcodeScopeBehavior ShortcodeScopeBehavior { get; set; }
+
         #endregion
     }
 }
diff --git a/Rock.Lava.Shared/Core/ILavaRenderContext.cs b/Rock.Lava.Shared/Core/ILavaRenderContext.cs
@@ -41,9 +41,10 @@ public enum LavaContextRelativeScopeSpecifier
         /// <summary>
         /// The current local scope.
         /// </summary>
+        [Obsolete( "[v19] This scope specifier is no longer used." )]
         Local = 3,
 
-        [Obsolete("The behavior of this setting is equivalent to Default. (v16.1).")]
+        [Obsolete( "[v16.1] The behavior of this setting is equivalent to Default." )]
         Root = 2
     }
 
diff --git a/Rock.Lava.Shared/Rock.Lava.Shared.csproj b/Rock.Lava.Shared/Rock.Lava.Shared.csproj
@@ -13,4 +13,8 @@
     </Compile>
   </ItemGroup>
 
+  <ItemGroup>
+    <ProjectReference Include="..\Rock.Enums\Rock.Enums.csproj" />
+  </ItemGroup>
+
 </Project>
diff --git a/Rock.Lava/Core/Shortcodes/DynamicShortcode.cs b/Rock.Lava/Core/Shortcodes/DynamicShortcode.cs
@@ -192,7 +192,6 @@ public override void OnRender( ILavaRenderContext context, TextWriter result )
             // Parameters declared on child elements can be referenced in the shortcode template as <childElementName>.<paramName>.
 
             string residualMarkup = string.Empty;
-            var keysToCleanup = new List<string>();
 
             if ( parms["processchilditems"].ToString().AsBoolean() )
             {
@@ -210,7 +209,6 @@ public override void OnRender( ILavaRenderContext context, TextWriter result )
                 // Add the collections of child to the set of parameters that will be passed to the shortcode template.
                 foreach ( var item in childElements )
                 {
-                    keysToCleanup.Add( item.Key );
                     parms.AddOrReplace( item.Key, item.Value );
                 }
             }
@@ -277,15 +275,11 @@ public override void OnRender( ILavaRenderContext context, TextWriter result )
                 }
             }
 
-            // This is a partial fix for issue #6470. Due to variable leakage between
-            // shortcode executions, some shortcodes were failing if you used multiple
-            // instances of the same shortcode on a page with different parameters.
-            // In the future we will fix the EnterChildScope() implementation to
-            // completely isolate the merge fields of the child scope from the parent.
-            var savedMergeFields = SaveOriginalMergeFields( context, keysToCleanup );
-
-            // Render the shortcode template in a child scope that includes the shortcode parameters.
-            context.EnterChildScope();
+            var isIsolatedScope = _shortcode.ShortcodeScopeBehavior == Enums.Cms.ShortcodeScopeBehavior.Isolated;
+            if ( isIsolatedScope )
+            {
+                context.EnterChildScope();
+            }
 
             LavaRenderResult results;
             try
@@ -312,11 +306,12 @@ public override void OnRender( ILavaRenderContext context, TextWriter result )
             }
             finally
             {
-                context.ExitChildScope();
+                if ( isIsolatedScope )
+                {
+                    context.ExitChildScope();
+                }
             }
 
-            RestoreOriginalMergeFields( context, savedMergeFields );
-
             // Reset the original parameters in the context
             context.SetMergeField( "blockContent", originalBlockContent );
             context.SetMergeField( "RecursionDepth", originalRecursionDepth );
@@ -513,40 +508,5 @@ private void AssertShortcodeIsInitialized()
                 throw new Exception( $"Shortcode configuration error. \"{_tagName}\" is not initialized." );
             }
         }
-
-        /// <summary>
-        /// Saves the current merge field values for the specified keys.
-        /// </summary>
-        /// <param name="context">The lava context to get the values from.</param>
-        /// <param name="keys">The keys whose values will be returned.</param>
-        /// <returns>A dictionary of keys and values that can be later restored.</returns>
-        private Dictionary<string, object> SaveOriginalMergeFields( ILavaRenderContext context, IEnumerable<string> keys )
-        {
-            var saved = new Dictionary<string, object>();
-
-            foreach ( var key in keys )
-            {
-                saved[key] = context.GetMergeField( key, null );
-            }
-
-            return saved;
-        }
-
-        /// <summary>
-        /// Restores the saved merge fields to the specified context. This
-        /// is used to make a best effor to reset the context to its original
-        /// state after rendering a shortcode. The shortcode will have modified
-        /// the merge fields in order to pass the parameters to the shortcode
-        /// template. We attempt to undo that.
-        /// </summary>
-        /// <param name="context">The lava context to restore the values on.</param>
-        /// <param name="savedMergeFields">The saved merge fields.</param>
-        private void RestoreOriginalMergeFields( ILavaRenderContext context, Dictionary<string, object> savedMergeFields )
-        {
-            foreach ( var kvp in savedMergeFields )
-            {
-                context.SetMergeField( kvp.Key, kvp.Value );
-            }
-        }
     }
 }
diff --git a/Rock.Lava/Rock.Lava.csproj b/Rock.Lava/Rock.Lava.csproj
@@ -54,6 +54,7 @@
 
   <ItemGroup>
     <ProjectReference Include="..\Rock.Common\Rock.Common.csproj" />
+    <ProjectReference Include="..\Rock.Enums\Rock.Enums.csproj" />
     <ProjectReference Include="..\Rock.Lava.Shared\Rock.Lava.Shared.csproj" />
   </ItemGroup>
 
diff --git a/Rock.Tests.Integration/Core/Lava/Shortcodes/ShortcodeCodeTests.cs b/Rock.Tests.Integration/Core/Lava/Shortcodes/ShortcodeCodeTests.cs
@@ -73,6 +73,7 @@ public void Shortcode_WithMergeFieldAsParameter_CorrectlyResolvesParameters()
         }
 
         [TestMethod]
+        [TestCategory( "ShortcodeScopeBehavior" )]
         public void Shortcode_ReferencingItemFromParentScope_CorrectlyResolvesItem()
         {
             var shortcodeTemplate = @"
@@ -274,6 +275,56 @@ public void ScripturizeShortcode_WithInvalidLandingSite_ProducesErrorMessage()
 
         #endregion
 
+        #region Workflow Activate
+
+        [TestMethod]
+        [DataRow( "Workflow", "some workflow" )]
+        [DataRow( "Activity", "some activity" )]
+        public void WorkflowActivate_WithPreexistingReservedMergeField_SavesAndRestoresMergeFieldValue( string mergeFieldKey, string mergeFieldValue )
+        {
+            var input = $@"
+{{% workflowactivate workflowtype:'51FE9641-FB8F-41BF-B09E-235900C3E53E' %}}
+{{% endworkflowactivate %}}
+Restored Value: {{{{{mergeFieldKey}}}}}";
+
+            var expectedOutput = $"Restored Value: {mergeFieldValue}";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                var options = new LavaTestRenderOptions()
+                    .WithContextVariable( mergeFieldKey, mergeFieldValue )
+                    .WithEnabledCommands( "workflowactivate" );
+
+                var output = TestHelper.GetTemplateOutput( engine, input, options );
+
+                Assert.Contains( expectedOutput, output, $"Reserved merge field with key '{mergeFieldKey}' was not restored." );
+            } );
+        }
+
+        [TestMethod]
+        public void WorkflowActivate_WithPreexistingErrorMergeField_SavesAndRestoresMergeFieldValue()
+        {
+            var input = @"
+{% workflowactivate workflowtype:'some-invalid-workflow-type-guid' %}
+{% endworkflowactivate %}
+Restored Value: {{Error}}";
+
+            var expectedOutput = $"Restored Value: some error";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                var options = new LavaTestRenderOptions()
+                    .WithContextVariable( "Error", "some error" )
+                    .WithEnabledCommands( "workflowactivate" );
+
+                var output = TestHelper.GetTemplateOutput( engine, input, options );
+
+                Assert.Contains( expectedOutput, output, $"Reserved merge field with key 'Error' was not restored." );
+            } );
+        }
+
+        #endregion
+
         /// <summary>
         /// Verify that an invalid shortcode name correctly throws a shortcode parsing error when embedded in an if/endif block.
         /// </summary>
diff --git a/Rock.Tests.Integration/Issues/Issues_v16.cs b/Rock.Tests.Integration/Issues/Issues_v16.cs
@@ -172,6 +172,7 @@ Did you see those comments ^^^
         }
 
         [TestMethod]
+        [TestCategory( "ShortcodeScopeBehavior" )]
         public void Issue5102_VariableScopingInWorkflowActivateTag()
         {
             /* The WorkflowActivate tag does not allow persistent changes to variables declared outside the block in Fluid.
diff --git a/Rock.Tests/Lava/Shortcodes/ShortcodeScopeBehaviorTests.cs b/Rock.Tests/Lava/Shortcodes/ShortcodeScopeBehaviorTests.cs
diff --git a/Rock/Lava/Blocks/WorkflowActivateBlock.cs b/Rock/Lava/Blocks/WorkflowActivateBlock.cs
diff --git a/Rock/Lava/WebsiteLavaShortcodeProvider.cs b/Rock/Lava/WebsiteLavaShortcodeProvider.cs

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,8 @@`
`17`	`17`	`using System;`
`18`	`18`	`using System.Collections.Generic;`
`19`	`19`
	`20`	`+using Rock.Enums.Cms;`
	`21`	`+`
`20`	`22`	`namespace Rock.Lava`
`21`	`23`	`{`
`22`	`24`	`/// <summary>`
`@@ -62,6 +64,11 @@ public DynamicShortcodeDefinition()`
`62`	`64`	`/// </summary>`
`63`	`65`	`public LavaShortcodeTypeSpecifier ElementType { get; set; }`
`64`	`66`
	`67`	`+ /// <summary>`
	`68`	`+ /// The scope behavior for variables that are defined within this shortcode.`
	`69`	`+ /// </summary>`
	`70`	`+ public ShortcodeScopeBehavior ShortcodeScopeBehavior { get; set; }`
	`71`	`+`
`65`	`72`	`#endregion`
`66`	`73`	`}`
`67`	`74`	`}`
Original file line number	Diff line number	Diff line change
`@@ -172,6 +172,7 @@ Did you see those comments ^^^`
`172`	`172`	`}`
`173`	`173`
`174`	`174`	`[TestMethod]`
	`175`	`+ [TestCategory( "ShortcodeScopeBehavior" )]`
`175`	`176`	`public void Issue5102_VariableScopingInWorkflowActivateTag()`
`176`	`177`	`{`
`177`	`178`	`/* The WorkflowActivate tag does not allow persistent changes to variables declared outside the block in Fluid.`