diff --git a/src/Altinn.App.Analyzers/IncrementalGenerator/FormDataWrapperGenerator.cs b/src/Altinn.App.Analyzers/FormDataWrapperGenerator.cs similarity index 98% rename from src/Altinn.App.Analyzers/IncrementalGenerator/FormDataWrapperGenerator.cs rename to src/Altinn.App.Analyzers/FormDataWrapperGenerator.cs index 4fc5b01228..9aa68a0883 100644 --- a/src/Altinn.App.Analyzers/IncrementalGenerator/FormDataWrapperGenerator.cs +++ b/src/Altinn.App.Analyzers/FormDataWrapperGenerator.cs @@ -2,7 +2,7 @@ using Altinn.App.Analyzers.Utils; using NanoJsonReader; -namespace Altinn.App.Analyzers.IncrementalGenerator; +namespace Altinn.App.Analyzers; /// /// Generate IFormDataWrapper implementations for classes in models/*.cs in the app. @@ -249,6 +249,6 @@ private static void GenerateFromNode(SourceProductionContext context, Result> ValidateFormData( gatewayAction: null, language ); - var hiddenFields = await LayoutEvaluator.GetHiddenFieldsForRemoval(evaluatorState); + var hiddenFields = await LayoutEvaluator.GetHiddenFieldsForRemoval( + evaluatorState, + evaluateRemoveWhenHidden: false + ); var validationIssues = new List(); DataElementIdentifier dataElementIdentifier = dataElement; diff --git a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs index ab572c7ea7..4166749f5f 100644 --- a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs @@ -59,7 +59,7 @@ public CleanInstanceDataAccessor( _hiddenFieldsTask = new(() => { using var activity = telemetry?.StartRemoveHiddenDataForValidation(); - return LayoutEvaluator.GetHiddenFieldsForRemoval(state); + return LayoutEvaluator.GetHiddenFieldsForRemoval(state, evaluateRemoveWhenHidden: false); }); } } diff --git a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs index c0df1f91aa..2b63076047 100644 --- a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs @@ -29,6 +29,7 @@ bool defaultReturn { "hidden" => context.Component.Hidden, "required" => context.Component.Required, + "removeWhenHidden" => context.Component.RemoveWhenHidden, _ => throw new ExpressionEvaluatorTypeErrorException($"unknown boolean expression property {property}"), }; @@ -268,7 +269,7 @@ LayoutEvaluatorState state { throw new ArgumentException("component lookup requires the target component to have a simpleBinding"); } - if (await targetContext.IsHidden()) + if (await targetContext.IsHidden(evaluateRemoveWhenHidden: false)) { return ExpressionValue.Null; } diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs index 7f467662d4..547fc81ff6 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs @@ -13,7 +13,17 @@ public static class LayoutEvaluator /// /// Get a list of fields that are only referenced in hidden components in /// - public static async Task> GetHiddenFieldsForRemoval(LayoutEvaluatorState state) + [Obsolete("Use the overload with evaluateRemoveWhenHidden parameter")] + public static async Task> GetHiddenFieldsForRemoval(LayoutEvaluatorState state) => + await GetHiddenFieldsForRemoval(state, evaluateRemoveWhenHidden: false); + + /// + /// Get a list of fields that are only referenced in hidden components in + /// + public static async Task> GetHiddenFieldsForRemoval( + LayoutEvaluatorState state, + bool evaluateRemoveWhenHidden + ) { var hiddenModelBindings = new HashSet(); var nonHiddenModelBindings = new HashSet(); @@ -21,7 +31,14 @@ public static async Task> GetHiddenFieldsForRemoval(LayoutEv var pageContexts = await state.GetComponentContexts(); foreach (var pageContext in pageContexts) { - await HiddenFieldsForRemovalRecurs(state, hiddenModelBindings, nonHiddenModelBindings, pageContext, []); + await HiddenFieldsForRemovalRecurs( + state, + hiddenModelBindings, + nonHiddenModelBindings, + pageContext, + evaluateRemoveWhenHidden, + [] + ); } var forRemoval = hiddenModelBindings.Except(nonHiddenModelBindings).ToList(); @@ -34,6 +51,7 @@ private static async Task HiddenFieldsForRemovalRecurs( HashSet hiddenModelBindings, HashSet nonHiddenModelBindings, ComponentContext context, + bool evaluateRemoveWhenHidden, IReadOnlyList ignoredPrefixes ) { @@ -45,7 +63,7 @@ IReadOnlyList ignoredPrefixes ); } - var isHidden = await context.IsHidden(); + var isHidden = await context.IsHidden(evaluateRemoveWhenHidden); List childIgnoredPrefixes = [.. ignoredPrefixes]; @@ -77,6 +95,7 @@ await HiddenFieldsForRemovalRecurs( hiddenModelBindings, nonHiddenModelBindings, childContext, + evaluateRemoveWhenHidden, childIgnoredPrefixes ); } @@ -88,15 +107,26 @@ await HiddenFieldsForRemovalRecurs( [Obsolete("Use the async version of this method RemoveHiddenDataAsync")] public static void RemoveHiddenData(LayoutEvaluatorState state, RowRemovalOption rowRemovalOption) { - RemoveHiddenDataAsync(state, rowRemovalOption).GetAwaiter().GetResult(); + RemoveHiddenDataAsync(state, rowRemovalOption, evaluateRemoveWhenHidden: false).GetAwaiter().GetResult(); } /// /// Remove fields that are only referenced from hidden fields from the data object in the state. /// - public static async Task RemoveHiddenDataAsync(LayoutEvaluatorState state, RowRemovalOption rowRemovalOption) + [Obsolete("Use the overload with evaluateRemoveWhenHidden parameter")] + public static async Task RemoveHiddenDataAsync(LayoutEvaluatorState state, RowRemovalOption rowRemovalOption) => + await RemoveHiddenDataAsync(state, rowRemovalOption, evaluateRemoveWhenHidden: false); + + /// + /// Remove fields that are only referenced from hidden fields from the data object in the state. + /// + public static async Task RemoveHiddenDataAsync( + LayoutEvaluatorState state, + RowRemovalOption rowRemovalOption, + bool evaluateRemoveWhenHidden + ) { - var fields = await GetHiddenFieldsForRemoval(state); + var fields = await GetHiddenFieldsForRemoval(state, evaluateRemoveWhenHidden); // Ensure fields with higher row numbers are removed before fields with lower row numbers. foreach (var dataReference in OrderByListIndexReverse(fields)) @@ -127,7 +157,7 @@ ComponentContext context ) { ArgumentNullException.ThrowIfNull(context.Component); - var hidden = await context.IsHidden(); + var hidden = await context.IsHidden(evaluateRemoveWhenHidden: false); if (!hidden) { foreach (var childContext in context.ChildContexts) diff --git a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs index 7b5c16af5d..325037dab4 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs @@ -101,7 +101,11 @@ private async Task RemoveFieldsOnTaskComplete( gatewayAction: null, language ); - await LayoutEvaluator.RemoveHiddenDataAsync(evaluationState, RowRemovalOption.DeleteRow); + await LayoutEvaluator.RemoveHiddenDataAsync( + evaluationState, + RowRemovalOption.DeleteRow, + evaluateRemoveWhenHidden: true + ); } // Remove shadow fields diff --git a/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs b/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs index b0d829faf2..b66c08c880 100644 --- a/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs +++ b/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs @@ -53,20 +53,44 @@ public ComponentContext( /// public int[]? RowIndices { get; } + /// + /// Memoization for evaluation of hidden + /// private bool? _isHidden; + /// + /// Memoization for evaluation of removeWhenHidden + /// + private bool? _removeWhenHidden; + /// /// Memoized way to check if the component is hidden /// - public async Task IsHidden() + public async Task IsHidden(bool evaluateRemoveWhenHidden) { - if (_isHidden.HasValue) + if (evaluateRemoveWhenHidden) { - return _isHidden.Value; + // We will check parent removeWhenHidden when we check parent hidden + var removeWhenHidden = await GetMemoizedRemoveWhenHidden(); + if (!removeWhenHidden) + { + return false; + } + } + + // If the parent is hidden, this is also hidden + if (Parent is not null && await Parent.IsHidden(evaluateRemoveWhenHidden)) + { + return true; } - if (Parent is not null && await Parent.IsHidden()) + + return await GetMemoizedIsHidden(); + } + + private async Task GetMemoizedIsHidden() + { + if (_isHidden.HasValue) { - _isHidden = true; return _isHidden.Value; } @@ -86,12 +110,31 @@ public async Task IsHidden() } } - var hidden = await ExpressionEvaluator.EvaluateBooleanExpression(State, this, "hidden", false); - - _isHidden = hidden; + var isHidden = await ExpressionEvaluator.EvaluateBooleanExpression(State, this, "hidden", false); + _isHidden = isHidden; return _isHidden.Value; } + private async Task GetMemoizedRemoveWhenHidden() + { + if (_removeWhenHidden.HasValue) + { + return _removeWhenHidden.Value; + } + + var removeWhenHidden = await ExpressionEvaluator.EvaluateBooleanExpression( + State, + this, + "removeWhenHidden", + // Default return should match AppSettings.RemoveHiddenData, + // but currently we only run removal when it is true, so we set it to true here + defaultReturn: true + ); + + _removeWhenHidden = removeWhenHidden; + return _removeWhenHidden.Value; + } + /// /// Indicates whether this context was initialized with child contexts /// diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs index 61d54b747b..131446aa53 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs @@ -49,7 +49,12 @@ public async Task ProcessDataWrite( var id = new DataElementIdentifier(dataId.Value); hidden .Should() - .BeEquivalentTo([new DataReference() { DataElementIdentifier = id, Field = "melding.hidden" }]); + .BeEquivalentTo( + [ + new DataReference() { DataElementIdentifier = id, Field = "melding.hidden" }, + new DataReference() { DataElementIdentifier = id, Field = "melding.hiddenNotRemove" }, + ] + ); if (data is Skjema { Melding: { } melding }) { melding.Toggle = !melding.Toggle; diff --git a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs index 47ac5547d6..9cffbad289 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs @@ -422,6 +422,10 @@ public async Task RunProcessNext_DataFromHiddenComponents_GetsRemoved() PatchOperation.Add( JsonPointer.Create("melding", "hidden"), JsonNode.Parse("\"value that is hidden\"") + ), + PatchOperation.Add( + JsonPointer.Create("melding", "hiddenNotRemove"), + JsonNode.Parse("\"value that is not removed\"") ) ), IgnoredValidators = [], @@ -441,6 +445,7 @@ public async Task RunProcessNext_DataFromHiddenComponents_GetsRemoved() OutputHelper.WriteLine("Data before process next:"); OutputHelper.WriteLine(dataString); dataString.Should().Contain("value that is hidden"); + dataString.Should().Contain("value that is not removed"); // Run process next var nextResponse = await client.PutAsync($"{Org}/{App}/instances/{_instanceId}/process/next", null); @@ -453,6 +458,7 @@ public async Task RunProcessNext_DataFromHiddenComponents_GetsRemoved() OutputHelper.WriteLine("Data after process next:"); OutputHelper.WriteLine(dataString); dataString.Should().NotContain("value that is hidden"); + dataString.Should().Contain("value that is not removed"); _dataProcessorMock.Verify(); } diff --git a/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/models/Skjema.cs b/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/models/Skjema.cs index 9a10f272b0..0ac66581fb 100644 --- a/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/models/Skjema.cs +++ b/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/models/Skjema.cs @@ -66,6 +66,11 @@ public bool ShouldSerializeTagWithAttribute() [JsonProperty("SF_test")] [JsonPropertyName("SF_test")] public string? SF_test { get; set; } + + [XmlElement("hiddenNotRemove", Order = 10)] + [JsonProperty("hiddenNotRemove")] + [JsonPropertyName("hiddenNotRemove")] + public string? HiddenNotRemove { get; set; } } public class TagWithAttribute diff --git a/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/ui/default/layouts/page.json b/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/ui/default/layouts/page.json index 9b586a8e2e..dee6b80222 100644 --- a/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/ui/default/layouts/page.json +++ b/test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/ui/default/layouts/page.json @@ -25,6 +25,15 @@ "dataModelBindings": { "simpleBinding": "melding.hidden" } + }, + { + "id": "hiddenNotRemove", + "type": "Input", + "hidden": true, + "removeWhenHidden": false, + "dataModelBindings": { + "simpleBinding": "melding.hiddenNotRemove" + } } ] } diff --git a/test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveCustomOpenApiSpec.verified.json b/test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveCustomOpenApiSpec.verified.json index ea59bb831d..f2e56a5113 100644 --- a/test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveCustomOpenApiSpec.verified.json +++ b/test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveCustomOpenApiSpec.verified.json @@ -3258,6 +3258,10 @@ "SF_test": { "type": "string", "nullable": true + }, + "hiddenNotRemove": { + "type": "string", + "nullable": true } }, "additionalProperties": false diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RemoveHiddenData/RemoveHiddenDataTests.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RemoveHiddenData/RemoveHiddenDataTests.cs index 670be798bf..c18373e999 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RemoveHiddenData/RemoveHiddenDataTests.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RemoveHiddenData/RemoveHiddenDataTests.cs @@ -11,7 +11,8 @@ public class RemoveHiddenDataTests [Fact] public async Task TestRemoveHiddenData() { - var jsonData = """ + using var jsonDoc = JsonDocument.Parse( + """ { "root": { "fornavn": null, @@ -107,8 +108,8 @@ public async Task TestRemoveHiddenData() "vedlegg": [] } } - """; - using var jsonDoc = JsonDocument.Parse(jsonData); + """ + ); IInstanceDataAccessor dataAccessor = DynamicClassBuilder.DataAccessorFromJsonDocument( new Instance(), jsonDoc.RootElement diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.Test1.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.Test1.verified.txt index 980ce7f94b..4d695665d6 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.Test1.verified.txt +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.Test1.verified.txt @@ -116,4 +116,4 @@ Source: Required, NoIncrementalUpdates: false } -] +] \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt index fd030bed18..efa34d528d 100644 --- a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt +++ b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt @@ -3058,10 +3058,14 @@ namespace Altinn.App.Core.Internal.Expressions } public static class LayoutEvaluator { + [System.Obsolete("Use the overload with evaluateRemoveWhenHidden parameter")] public static System.Threading.Tasks.Task> GetHiddenFieldsForRemoval(Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState state) { } + public static System.Threading.Tasks.Task> GetHiddenFieldsForRemoval(Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState state, bool evaluateRemoveWhenHidden) { } [System.Obsolete("Use the async version of this method RemoveHiddenDataAsync")] public static void RemoveHiddenData(Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState state, Altinn.App.Core.Helpers.RowRemovalOption rowRemovalOption) { } + [System.Obsolete("Use the overload with evaluateRemoveWhenHidden parameter")] public static System.Threading.Tasks.Task RemoveHiddenDataAsync(Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState state, Altinn.App.Core.Helpers.RowRemovalOption rowRemovalOption) { } + public static System.Threading.Tasks.Task RemoveHiddenDataAsync(Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState state, Altinn.App.Core.Helpers.RowRemovalOption rowRemovalOption, bool evaluateRemoveWhenHidden) { } public static System.Threading.Tasks.Task> RunLayoutValidationsForRequired(Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState state) { } } public class LayoutEvaluatorState @@ -4276,7 +4280,7 @@ namespace Altinn.App.Core.Models.Expressions public Altinn.App.Core.Models.Expressions.ComponentContext? Parent { get; } public int[]? RowIndices { get; } public Altinn.App.Core.Internal.Expressions.LayoutEvaluatorState State { get; } - public System.Threading.Tasks.Task IsHidden() { } + public System.Threading.Tasks.Task IsHidden(bool evaluateRemoveWhenHidden) { } } [System.Text.Json.Serialization.JsonConverter(typeof(Altinn.App.Core.Models.Expressions.ExpressionConverter?))] public readonly struct Expression : System.IEquatable diff --git a/test/Altinn.App.SourceGenerator.Integration.Tests/Altinn.App.SourceGenerator.Integration.Tests.csproj b/test/Altinn.App.SourceGenerator.Integration.Tests/Altinn.App.SourceGenerator.Integration.Tests.csproj index 3796e2b28e..10dcc1a816 100644 --- a/test/Altinn.App.SourceGenerator.Integration.Tests/Altinn.App.SourceGenerator.Integration.Tests.csproj +++ b/test/Altinn.App.SourceGenerator.Integration.Tests/Altinn.App.SourceGenerator.Integration.Tests.csproj @@ -7,7 +7,7 @@ false true true - Generated + gen diff --git a/test/Altinn.App.SourceGenerator.Integration.Tests/Generated/Altinn.App.Analyzers/Altinn.App.Analyzers.IncrementalGenerator.FormDataWrapperGenerator/Altinn_App_SourceGenerator_Integration_Tests_Models_SkjemaFormDataWrapper.g.cs b/test/Altinn.App.SourceGenerator.Integration.Tests/gen/Altinn.App.Analyzers/Altinn.App.Analyzers.FormDataWrapperGenerator/SkjemaFormDataWrapper.g.cs similarity index 100% rename from test/Altinn.App.SourceGenerator.Integration.Tests/Generated/Altinn.App.Analyzers/Altinn.App.Analyzers.IncrementalGenerator.FormDataWrapperGenerator/Altinn_App_SourceGenerator_Integration_Tests_Models_SkjemaFormDataWrapper.g.cs rename to test/Altinn.App.SourceGenerator.Integration.Tests/gen/Altinn.App.Analyzers/Altinn.App.Analyzers.FormDataWrapperGenerator/SkjemaFormDataWrapper.g.cs diff --git a/test/Altinn.App.SourceGenerator.Tests/DiagnosticTests.cs b/test/Altinn.App.SourceGenerator.Tests/DiagnosticTests.cs index 8aa54eec4a..e170c9e6ec 100644 --- a/test/Altinn.App.SourceGenerator.Tests/DiagnosticTests.cs +++ b/test/Altinn.App.SourceGenerator.Tests/DiagnosticTests.cs @@ -1,7 +1,7 @@ using System.Collections.Immutable; using System.Reflection; using System.Text.Json.Serialization; -using Altinn.App.Analyzers.IncrementalGenerator; +using Altinn.App.Analyzers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; diff --git a/test/Altinn.App.SourceGenerator.Tests/FullTests.cs b/test/Altinn.App.SourceGenerator.Tests/FullTests.cs index f491ef322d..7d3472afe2 100644 --- a/test/Altinn.App.SourceGenerator.Tests/FullTests.cs +++ b/test/Altinn.App.SourceGenerator.Tests/FullTests.cs @@ -1,6 +1,6 @@ using System.Reflection; using System.Text.Json.Serialization; -using Altinn.App.Analyzers.IncrementalGenerator; +using Altinn.App.Analyzers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp;