Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore hidden rows on submit #317

Merged
merged 2 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Altinn.App.Core/Features/Validation/ValidationAppSI.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Altinn.App.Core.Configuration;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Interface;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Internal.AppModel;
Expand Down Expand Up @@ -230,8 +231,8 @@ public async Task<List<ValidationIssue>> ValidateDataElement(Instance instance,
{
var layoutSet = _appResourcesService.GetLayoutSetForTask(dataType.TaskId);
var evaluationState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSet?.Id);
// Remove hidden data before validation
LayoutEvaluator.RemoveHiddenData(evaluationState);
// Remove hidden data before validation, set rows to null to preserve indices
LayoutEvaluator.RemoveHiddenData(evaluationState, RowRemovalOption.SetToNull);
// Evaluate expressions in layout and validate that all required data is included and that maxLength
// is respected on groups
var layoutErrors = LayoutEvaluator.RunLayoutValidationsForRequired(evaluationState, dataElement.Id);
Expand Down
22 changes: 12 additions & 10 deletions src/Altinn.App.Core/Helpers/DataModel/DataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private static bool IsPropertyWithJsonName(PropertyInfo propertyInfo, string key
}

/// <inheritdoc />
public void RemoveField(string key, bool deleteRows = false)
public void RemoveField(string key, RowRemovalOption rowRemovalOption)
{
var keys_split = key.Split('.');
var keys = keys_split[0..^1];
Expand Down Expand Up @@ -242,16 +242,18 @@ public void RemoveField(string key, bool deleteRows = false)
throw new ArgumentException($"Tried to remove row {key}, ended in a non-list ({propertyValue?.GetType()})");
}

if (deleteRows)
switch (rowRemovalOption)
{
listValue.RemoveAt(lastGroupIndex.Value);
}
else
{

var genericType = listValue.GetType().GetGenericArguments().FirstOrDefault();
var nullValue = genericType?.IsValueType == true ? Activator.CreateInstance(genericType) : null;
listValue[lastGroupIndex.Value] = nullValue;
case RowRemovalOption.DeleteRow:
listValue.RemoveAt(lastGroupIndex.Value);
break;
case RowRemovalOption.SetToNull:
var genericType = listValue.GetType().GetGenericArguments().FirstOrDefault();
var nullValue = genericType?.IsValueType == true ? Activator.CreateInstance(genericType) : null;
listValue[lastGroupIndex.Value] = nullValue;
break;
case RowRemovalOption.Ignore:
return;
}
}
else
Expand Down
23 changes: 22 additions & 1 deletion src/Altinn.App.Core/Helpers/IDataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,34 @@ public interface IDataModelAccessor
/// <summary>
/// Remove a value from the wrapped datamodel
/// </summary>
void RemoveField(string key, bool deleteRows = false);
void RemoveField(string key, RowRemovalOption rowRemovalOption);

/// <summary>
/// Verify that a Key is a valid lookup for the datamodel
/// </summary>
bool VerifyKey(string key);
}

/// <summary>
/// Option for how to handle row removal
/// </summary>
public enum RowRemovalOption
{
/// <summary>
/// Remove the row from the data model
/// </summary>
DeleteRow,

/// <summary>
/// Set the row to null, used to preserve row indices
/// </summary>
SetToNull,

/// <summary>
/// Ignore row removal
/// </summary>
Ignore
}



4 changes: 2 additions & 2 deletions src/Altinn.App.Core/Implementation/DefaultTaskEvents.cs
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,10 @@ private async Task RemoveHiddenData(Instance instance, Guid instanceGuid, List<D

if (_appSettings?.RemoveHiddenDataPreview == true)
{
// Remove hidden data before validation
// Remove hidden data before validation, ignore hidden rows. TODO: Determine how hidden rows should be handled going forward.
var layoutSet = _appResources.GetLayoutSetForTask(dataType.TaskId);
var evaluationState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSet?.Id);
LayoutEvaluator.RemoveHiddenData(evaluationState, true);
LayoutEvaluator.RemoveHiddenData(evaluationState, RowRemovalOption.Ignore);
}

// save the updated data if there are changes
Expand Down
5 changes: 3 additions & 2 deletions src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Models.Expressions;
using Altinn.App.Core.Models.Layout.Components;
using Altinn.App.Core.Models.Validation;
Expand Down Expand Up @@ -89,12 +90,12 @@ private static void HiddenFieldsForRemovalRecurs(LayoutEvaluatorState state, Has
/// <summary>
/// Remove fields that are only refrenced from hidden fields from the data object in the state.
/// </summary>
public static void RemoveHiddenData(LayoutEvaluatorState state, bool deleteRows = false)
public static void RemoveHiddenData(LayoutEvaluatorState state, RowRemovalOption rowRemovalOption)
{
var fields = GetHiddenFieldsForRemoval(state);
foreach (var field in fields)
{
state.RemoveDataField(field, deleteRows);
state.RemoveDataField(field, rowRemovalOption);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ public ComponentContext GetComponentContext(string pageName, string componentId,
/// <summary>
/// Set the value of a field to null.
/// </summary>
public void RemoveDataField(string key, bool deleteRows = false)
public void RemoveDataField(string key, RowRemovalOption rowRemovalOption)
{
_dataModel.RemoveField(key, deleteRows);
_dataModel.RemoveField(key, rowRemovalOption);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion test/Altinn.App.Core.Tests/Helpers/JsonDataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public string AddIndicies(string key, ReadOnlySpan<int> indicies = default)
}

/// <inheritdoc />
public void RemoveField(string key, bool deleteRows = false)
public void RemoveField(string key, RowRemovalOption rowRemovalOption)
{
throw new NotImplementedException("Impossible to remove fields in a json model");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections.Generic;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Internal.Expressions;
using Altinn.App.Core.Models.Validation;
using FluentAssertions;
Expand Down Expand Up @@ -54,7 +55,7 @@ public async Task RemoveWholeGroup()
data.Some.Data[0].Binding2.Should().Be(0); // binding is not nullable, but will be reset to zero
data.Some.Data[1].Binding.Should().Be("binding");
data.Some.Data[1].Binding2.Should().Be(2);
LayoutEvaluator.RemoveHiddenData(state);
LayoutEvaluator.RemoveHiddenData(state, RowRemovalOption.SetToNull);

// Verify data was removed
data.Some.Data[0].Binding.Should().BeNull();
Expand Down Expand Up @@ -111,4 +112,4 @@ public class Data

[JsonPropertyName("binding3")]
public string Binding3 { get; set; }
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text.Json.Serialization;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Internal.Expressions;
using FluentAssertions;
using Xunit;
Expand Down Expand Up @@ -61,7 +62,7 @@ public async Task RemoveRowDataFromGroup()
data.Some.Data[2].Binding.Should().Be("hideRow");
data.Some.Data[2].Binding2.Should().Be(3);
data.Some.Data[2].Binding3.Should().Be("text");
LayoutEvaluator.RemoveHiddenData(state);
LayoutEvaluator.RemoveHiddenData(state, RowRemovalOption.SetToNull);

// Verify row not deleted but fields null
data.Some.Data.Should().HaveCount(3);
Expand All @@ -71,7 +72,7 @@ public async Task RemoveRowDataFromGroup()
data.Some.Data[1].Binding2.Should().Be(2);
data.Some.Data[2].Should().BeNull();
}

[Fact]
public async Task RemoveRowFromGroup()
{
Expand Down Expand Up @@ -118,9 +119,9 @@ public async Task RemoveRowFromGroup()
data.Some.Data[2].Binding.Should().Be("hideRow");
data.Some.Data[2].Binding2.Should().Be(3);
data.Some.Data[2].Binding3.Should().Be("text");

// Verify rows deleted
LayoutEvaluator.RemoveHiddenData(state, true);
LayoutEvaluator.RemoveHiddenData(state, RowRemovalOption.DeleteRow);
data.Some.Data.Should().HaveCount(2);
data.Some.Data[0].Binding.Should().BeNull();
data.Some.Data[0].Binding2.Should().Be(0); // binding is not nullable, but will be reset to zero
Expand Down Expand Up @@ -154,4 +155,4 @@ public class Data

[JsonPropertyName("binding3")]
public string Binding3 { get; set; }
}
}
26 changes: 13 additions & 13 deletions test/Altinn.App.Core.Tests/LayoutExpressions/TestDataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -226,22 +226,22 @@ public void TestRemoveFields()
};
IDataModelAccessor modelHelper = new DataModel(model);
model.Id.Should().Be(2);
modelHelper.RemoveField("id");
modelHelper.RemoveField("id", RowRemovalOption.SetToNull);
model.Id.Should().Be(default);

model.Name.Value.Should().Be("Ivar");
modelHelper.RemoveField("name");
modelHelper.RemoveField("name", RowRemovalOption.SetToNull);
model.Name.Should().BeNull();

model.Friends.First().Name!.Value.Should().Be("Første venn");
modelHelper.RemoveField("friends[0].name.value");
modelHelper.RemoveField("friends[0].name.value", RowRemovalOption.SetToNull);
model.Friends.First().Name!.Value.Should().BeNull();
modelHelper.RemoveField("friends[0].name");
modelHelper.RemoveField("friends[0].name", RowRemovalOption.SetToNull);
model.Friends.First().Name.Should().BeNull();
model.Friends.First().Age.Should().Be(1235);

model.Friends.First().Friends!.First().Age.Should().Be(233);
modelHelper.RemoveField("friends[0].friends");
modelHelper.RemoveField("friends[0].friends", RowRemovalOption.SetToNull);
model.Friends.First().Friends.Should().BeNull();
}

Expand Down Expand Up @@ -338,12 +338,12 @@ public void TestRemoveRows()
var model1 = System.Text.Json.JsonSerializer.Deserialize<Model>(serializedModel)!;
IDataModelAccessor modelHelper1 = new DataModel(model1);

modelHelper1.RemoveField("friends[0].friends[0]");
modelHelper1.RemoveField("friends[0].friends[0]", RowRemovalOption.SetToNull);
model1.Friends![0].Friends![0].Should().BeNull();
model1.Friends![0].Friends!.Count.Should().Be(3);
model1.Friends[0].Friends![1].Name!.Value.Should().Be("Første venn sin andre venn");

modelHelper1.RemoveField("friends[1]");
modelHelper1.RemoveField("friends[1]", RowRemovalOption.SetToNull);
model1.Friends[1].Should().BeNull();
model1.Friends.Count.Should().Be(3);
model1.Friends[2].Name!.Value.Should().Be("Tredje venn");
Expand All @@ -352,11 +352,11 @@ public void TestRemoveRows()
var model2 = System.Text.Json.JsonSerializer.Deserialize<Model>(serializedModel)!;
IDataModelAccessor modelHelper2 = new DataModel(model2);

modelHelper2.RemoveField("friends[0].friends[0]", true);
modelHelper2.RemoveField("friends[0].friends[0]", RowRemovalOption.DeleteRow);
model2.Friends![0].Friends!.Count.Should().Be(2);
model2.Friends[0].Friends![0].Name!.Value.Should().Be("Første venn sin andre venn");

modelHelper2.RemoveField("friends[1]", true);
modelHelper2.RemoveField("friends[1]", RowRemovalOption.DeleteRow);
model2.Friends.Count.Should().Be(2);
model2.Friends[1].Name!.Value.Should().Be("Tredje venn");
}
Expand Down Expand Up @@ -454,16 +454,16 @@ public void RemoveField_WhenValueDoesNotExist_DoNothing()
var modelHelper = new DataModel(new Model());

// real fields works, no error
modelHelper.RemoveField("id");
modelHelper.RemoveField("id", RowRemovalOption.SetToNull);

// non-existant-fields works, no error
modelHelper.RemoveField("doesNotExist");
modelHelper.RemoveField("doesNotExist", RowRemovalOption.SetToNull);

// non-existant-fields in subfield works, no error
modelHelper.RemoveField("friends.doesNotExist");
modelHelper.RemoveField("friends.doesNotExist", RowRemovalOption.SetToNull);

// non-existant-fields in subfield works, no error
modelHelper.RemoveField("friends[0].doesNotExist");
modelHelper.RemoveField("friends[0].doesNotExist", RowRemovalOption.SetToNull);
}
}

Expand Down
Loading