Skip to content

Commit

Permalink
Ignore hidden rows on submit (#317)
Browse files Browse the repository at this point in the history
* ignore hidden rows on submit

* update unit tests
  • Loading branch information
bjosttveit authored Sep 26, 2023
1 parent 87fe4b9 commit 1aabc07
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 40 deletions.
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

0 comments on commit 1aabc07

Please sign in to comment.