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

Support Extra data models and subforms in validation #730

Merged
merged 80 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
80 commits
Select commit Hold shift + click to select a range
cf1bbf5
Removes form-data delete check
danielskovli Jul 9, 2024
7798803
Implements `AllowUserCreate` and `AllowUserDelete` methodology. Fixes…
danielskovli Jul 16, 2024
ad56781
Temp project ref to Storage.Interface
danielskovli Jul 16, 2024
5484854
Access testing for DataController -> `Create` and `Delete`
danielskovli Jul 17, 2024
87cae85
Verifies MaxCount before creating a new data element
danielskovli Jul 24, 2024
ddc9199
Merge branch 'main' into feature/subform
danielskovli Jul 31, 2024
8d9d77d
Implement support for multiple data models in expressions
ivarne Aug 15, 2024
a688536
Restructure validation to support incremental validation for multiple…
ivarne Aug 15, 2024
f7b91e0
Fix tests and cleanup
ivarne Aug 19, 2024
e6cda48
Fix sonar cloud issues
ivarne Aug 19, 2024
3c4ac0c
Add tests for shadow fields and hidden component data removal in proc…
ivarne Aug 20, 2024
c27f627
chore: update to new storage nuget
HauklandJ Aug 21, 2024
79a918b
Improve testing for multiple patch endpoint
ivarne Aug 22, 2024
9007d42
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Aug 22, 2024
62356fb
Merge branch 'main' into feat/multi-data-model-with-validation
ivarne Aug 26, 2024
e23d1d2
update Storage.Interface package reference to 3.33.0
HauklandJ Aug 28, 2024
ac353e4
Merge remote-tracking branch 'origin/main' into feature/subform
danielskovli Aug 29, 2024
af1200b
Fixes some tests that were double-creating the `default` data element
danielskovli Aug 29, 2024
07c1af7
Reorders DataController->MaxCount logic, adds missing ClassRef check …
danielskovli Aug 29, 2024
300faac
More changes to make tests work after merge
ivarne Aug 30, 2024
4734382
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Aug 30, 2024
45d2108
Introduces `LayoutSet.Type` and adds null checks for `LayoutSet.Tasks`
danielskovli Sep 3, 2024
e36b6cb
Almost make the tests pass
ivarne Sep 4, 2024
df3fa25
Fix last remaining known bugs
ivarne Sep 4, 2024
552b846
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Sep 4, 2024
eaf0af8
fix async warning
ivarne Sep 4, 2024
278e170
Don't filter fields to remove by doing a double lookup
ivarne Sep 4, 2024
ae2f6d9
Merge remote-tracking branch 'origin/feature/subform' into feat/multi…
ivarne Sep 4, 2024
269e0a3
Fix sonar suggestions and tests
ivarne Sep 4, 2024
c92d62b
fix another warning
ivarne Sep 4, 2024
46dafb4
Fix another batch of sonar cloud issues
ivarne Sep 4, 2024
7ea4e54
Implement SubFormComponent and use for validation
ivarne Sep 5, 2024
fae563f
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Sep 5, 2024
dea4fe9
Fix tests after merge with main
ivarne Sep 5, 2024
85fc76c
Update storage.interfaces
ivarne Sep 6, 2024
6d3fb4e
Fix tests after storage upgrade
ivarne Sep 6, 2024
9a3a35a
Add LayoutId to required validation descriptions
ivarne Sep 6, 2024
1d30664
Add tests for subform validation
ivarne Sep 6, 2024
ef8dcab
Add appMetadata to LayoutEvaluatorState
ivarne Sep 7, 2024
110b8e8
Fix typos and make sonar more happy
ivarne Sep 7, 2024
dfb250a
Revert changes to use scoped services
ivarne Sep 8, 2024
f71dad1
Remove unused usings
ivarne Sep 8, 2024
4c6f4d0
Remove implicit cast, as it was no longer used in tests (and hid bugs…
ivarne Sep 8, 2024
225f934
Remove file added by error
ivarne Sep 9, 2024
e7f3413
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Sep 9, 2024
82f859c
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Sep 9, 2024
cc68b17
Add ignoredValidators as a comma separated GET parameter to /validate…
ivarne Sep 10, 2024
259212c
Cleanup validatorService. Add new tests and telemetry
ivarne Sep 10, 2024
b1b9085
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Sep 10, 2024
1081d5f
Make sonar cloud complaints editor warnings instead of suggestions
ivarne Sep 10, 2024
18d4169
Activate CA1816: Call GC.SuppressFinalize correctly
ivarne Sep 10, 2024
93caa71
Fix more sonar cloud issues
ivarne Sep 10, 2024
54c5ffb
Make change to IProcessExclusiveGateway backwards compatible
ivarne Sep 10, 2024
450f53d
Add additional status codes to swagger
ivarne Sep 10, 2024
434528b
Update Instance.Data when writing copied data without shadow fields
ivarne Sep 11, 2024
32d5d15
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Sep 20, 2024
71b129f
Add concept of nonIncrementalValidators
ivarne Sep 23, 2024
f3ebc39
Add ModelSerializationService and other cleanup as a prerequisite to …
ivarne Sep 29, 2024
fdc4cb4
Add draft support for adding/removing/modifying other data elements i…
ivarne Sep 30, 2024
2e4cb52
Code review fixes
ivarne Sep 30, 2024
c77239f
Various cleanup tasks
ivarne Oct 3, 2024
29f2bb8
Rename DataElementId to DataElementIdentifier
ivarne Oct 3, 2024
1f75bb8
Merge pull request #809 from Altinn/feat/ivarne/multiDataProcessing
ivarne Oct 3, 2024
cc73fd4
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Oct 3, 2024
2ae6862
Fix issues and tests failing after merge
ivarne Oct 3, 2024
7ddd6d0
Rename id=>dataElementId in api
ivarne Oct 3, 2024
9e493c5
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Oct 3, 2024
a6ca853
Use async versions of File access methods in DataClientMock
ivarne Oct 3, 2024
165af46
Reorganize ProcessTaskFinalizer to use CachedInstanceDataAccessor
ivarne Oct 3, 2024
a31fba9
Use 'ProblemDetails' for new 409 response branch in DataController
martinothamar Oct 4, 2024
4e62860
Various fixes and api cleanup
ivarne Oct 5, 2024
1faedd2
Fix sonar issues
ivarne Oct 7, 2024
eab271b
Use new DataProcessing interface in PUT
ivarne Oct 8, 2024
b9ea9f8
Merge remote-tracking branch 'origin/main' into feat/multi-data-model…
ivarne Oct 8, 2024
7ff6782
Fix formatting and move the .gitignore file protecting test data
ivarne Oct 8, 2024
93e7ef3
Fix codeQL warning
ivarne Oct 8, 2024
12978b1
Add test for FormDataValidatorWrapper
ivarne Oct 8, 2024
e79b43c
Merge branch 'main' into feat/multi-data-model-with-validation
ivarne Oct 10, 2024
02aeef3
Update src/Altinn.App.Core/Helpers/Serialization/ModelSerializationSe…
ivarne Oct 10, 2024
aee3277
Add test for legacy LayoutEvaluatorInitializer
ivarne Oct 11, 2024
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
134 changes: 74 additions & 60 deletions src/Altinn.App.Api/Controllers/ActionsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Altinn.App.Core.Features.Action;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Internal.AppModel;
using Altinn.App.Core.Internal.Data;
using Altinn.App.Core.Internal.Instances;
using Altinn.App.Core.Internal.Validation;
Expand Down Expand Up @@ -33,23 +34,19 @@ public class ActionsController : ControllerBase
private readonly IValidationService _validationService;
private readonly IDataClient _dataClient;
private readonly IAppMetadata _appMetadata;
private readonly IAppModel _appModel;

/// <summary>
/// Create new instance of the <see cref="ActionsController"/> class
/// </summary>
/// <param name="authorization">The authorization service</param>
/// <param name="instanceClient">The instance client</param>
/// <param name="userActionService">The user action service</param>
/// <param name="validationService">Service for performing validations of user data</param>
/// <param name="dataClient">Client for accessing data in storage</param>
/// <param name="appMetadata">Service for getting application metadata</param>
public ActionsController(
IAuthorizationService authorization,
IInstanceClient instanceClient,
UserActionService userActionService,
IValidationService validationService,
IDataClient dataClient,
IAppMetadata appMetadata
IAppMetadata appMetadata,
IAppModel appModel
)
{
_authorization = authorization;
Expand All @@ -58,6 +55,7 @@ IAppMetadata appMetadata
_validationService = validationService;
_dataClient = dataClient;
_appMetadata = appMetadata;
_appModel = appModel;
}

/// <summary>
Expand Down Expand Up @@ -162,41 +160,53 @@ public async Task<ActionResult<UserActionResponse>> Perform(
);
}

var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel);
Dictionary<string, Dictionary<string, List<ValidationIssueWithSource>>>? validationIssues = null;

if (result.UpdatedDataModels is { Count: > 0 })
{
await SaveChangedModels(instance, result.UpdatedDataModels);
var changes = await SaveChangedModels(instance, dataAccessor, result.UpdatedDataModels);

validationIssues = await GetValidations(
instance,
dataAccessor,
changes,
actionRequest.IgnoredValidators,
language
);
}

return new OkObjectResult(
return Ok(
new UserActionResponse()
{
ClientActions = result.ClientActions,
UpdatedDataModels = result.UpdatedDataModels,
UpdatedValidationIssues = await GetValidations(
instance,
result.UpdatedDataModels,
actionRequest.IgnoredValidators,
language
),
UpdatedValidationIssues = validationIssues,
RedirectUrl = result.RedirectUrl,
}
);
}

private async Task SaveChangedModels(Instance instance, Dictionary<string, object> resultUpdatedDataModels)
private async Task<List<DataElementChange>> SaveChangedModels(
Instance instance,
CachedInstanceDataAccessor dataAccessor,
Dictionary<string, object> resultUpdatedDataModels
)
{
var changes = new List<DataElementChange>();
var instanceIdentifier = new InstanceIdentifier(instance);
foreach (var (elementId, newModel) in resultUpdatedDataModels)
{
if (newModel is null)
{
continue;
}
var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase));
var previousData = await dataAccessor.GetData(dataElement);

ObjectUtils.InitializeAltinnRowId(newModel);
ObjectUtils.PrepareModelForXmlStorage(newModel);

var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase));
await _dataClient.UpdateData(
newModel,
instanceIdentifier.InstanceGuid,
Expand All @@ -206,61 +216,65 @@ await _dataClient.UpdateData(
instanceIdentifier.InstanceOwnerPartyId,
Guid.Parse(dataElement.Id)
);
// update dataAccessor to use the changed data
dataAccessor.Set(dataElement, newModel);
// add change to list
changes.Add(
new DataElementChange
{
DataElement = dataElement,
PreviousValue = previousData,
CurrentValue = newModel,
}
);
}
return changes;
}

private async Task<Dictionary<string, Dictionary<string, List<ValidationIssue>>>?> GetValidations(
private async Task<Dictionary<string, Dictionary<string, List<ValidationIssueWithSource>>>?> GetValidations(
Instance instance,
Dictionary<string, object>? resultUpdatedDataModels,
IInstanceDataAccessor dataAccessor,
List<DataElementChange> changes,
List<string>? ignoredValidators,
string? language
)
{
if (resultUpdatedDataModels is null || resultUpdatedDataModels.Count < 1)
{
return null;
}

var instanceIdentifier = new InstanceIdentifier(instance);
var application = await _appMetadata.GetApplicationMetadata();
var taskId = instance.Process.CurrentTask.ElementId;
var validationIssues = await _validationService.ValidateIncrementalFormData(
instance,
taskId,
changes,
dataAccessor,
ignoredValidators,
language
);

var updatedValidationIssues = new Dictionary<string, Dictionary<string, List<ValidationIssue>>>();
// For historical reasons the validation issues from actions controller is separated per data element
// The easiest way was to keep this behaviour to improve compatibility with older frontend versions
return PartitionValidationIssuesByDataElement(validationIssues);
}

// TODO: Consider validating models in parallel
foreach (var (elementId, newModel) in resultUpdatedDataModels)
private static Dictionary<
string,
Dictionary<string, List<ValidationIssueWithSource>>
> PartitionValidationIssuesByDataElement(Dictionary<string, List<ValidationIssueWithSource>> validationIssues)
{
var updatedValidationIssues = new Dictionary<string, Dictionary<string, List<ValidationIssueWithSource>>>();
foreach (var (validationSource, issuesFromSource) in validationIssues)
{
if (newModel is null)
{
continue;
}

var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase));
var dataType = application.DataTypes.First(d =>
d.Id.Equals(dataElement.DataType, StringComparison.OrdinalIgnoreCase)
);

// TODO: Consider rewriting so that we get the original data the IUserAction have requested instead of fetching it again
var oldData = await _dataClient.GetFormData(
instanceIdentifier.InstanceGuid,
newModel.GetType(),
instance.Org,
instance.AppId.Split('/')[1],
instanceIdentifier.InstanceOwnerPartyId,
Guid.Parse(dataElement.Id)
);

var validationIssues = await _validationService.ValidateFormData(
instance,
dataElement,
dataType,
newModel,
oldData,
ignoredValidators,
language
);
if (validationIssues.Count > 0)
foreach (var issue in issuesFromSource)
{
updatedValidationIssues.Add(elementId, validationIssues);
if (!updatedValidationIssues.TryGetValue(issue.DataElementId ?? "", out var elementIssues))
{
elementIssues = [];
updatedValidationIssues[issue.DataElementId ?? ""] = elementIssues;
}
if (!elementIssues.TryGetValue(validationSource, out var sourceIssues))
{
sourceIssues = [];
elementIssues[validationSource] = sourceIssues;
}
sourceIssues.Add(issue);
}
}

Expand Down
125 changes: 100 additions & 25 deletions src/Altinn.App.Api/Controllers/DataController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,53 @@
[FromBody] DataPatchRequest dataPatchRequest,
[FromQuery] string? language = null
)
{
var request = new DataPatchRequestMultiple()
{
Patches = new() { [dataGuid] = dataPatchRequest.Patch },
IgnoredValidators = dataPatchRequest.IgnoredValidators
};
var response = await PatchFormDataMultiple(org, app, instanceOwnerPartyId, instanceGuid, request, language);

if (response.Result is OkObjectResult { Value: DataPatchResponseMultiple newResponse })
martinothamar marked this conversation as resolved.
Show resolved Hide resolved
{
// Map the new response to the old response
return Ok(
new DataPatchResponse()
{
ValidationIssues = newResponse.ValidationIssues,
NewDataModel = newResponse.NewDataModels[dataGuid],
}
);
}

// Return the error object unchanged
return response.Result ?? throw new InvalidOperationException("Response is null");
}

/// <summary>
/// Updates an existing form data element with patches to mulitple data elements.
/// </summary>
/// <param name="org">unique identfier of the organisation responsible for the app</param>
/// <param name="app">application identifier which is unique within an organisation</param>
/// <param name="instanceOwnerPartyId">unique id of the party that is the owner of the instance</param>
/// <param name="instanceGuid">unique id to identify the instance</param>
/// <param name="dataPatchRequest">Container object for the <see cref="JsonPatch" /> and list of ignored validators</param>
/// <param name="language">The language selected by the user.</param>
/// <returns>A response object with the new full model and validation issues from all the groups that run</returns>
[Authorize(Policy = AuthzConstants.POLICY_INSTANCE_WRITE)]
[HttpPatch("")]
[ProducesResponseType(typeof(DataPatchResponseMultiple), 200)]
[ProducesResponseType(typeof(ProblemDetails), 409)]
[ProducesResponseType(typeof(ProblemDetails), 422)]
martinothamar marked this conversation as resolved.
Show resolved Hide resolved
public async Task<ActionResult<DataPatchResponseMultiple>> PatchFormDataMultiple(
[FromRoute] string org,
[FromRoute] string app,
[FromRoute] int instanceOwnerPartyId,
[FromRoute] Guid instanceGuid,
[FromBody] DataPatchRequestMultiple dataPatchRequest,
[FromQuery] string? language = null
)
{
try
{
Expand All @@ -460,48 +507,76 @@
if (!InstanceIsActive(instance))
{
return Conflict(
$"Cannot update data element of archived or deleted instance {instanceOwnerPartyId}/{instanceGuid}"
new ProblemDetails()
ivarne marked this conversation as resolved.
Show resolved Hide resolved
{
Title = "Instance is not active",
Detail =
$"Cannot update data element of archived or deleted instance {instanceOwnerPartyId}/{instanceGuid}",
Status = (int)HttpStatusCode.Conflict,
}
);
}

var dataElement = instance.Data.First(m => m.Id.Equals(dataGuid.ToString(), StringComparison.Ordinal));

if (dataElement == null)
foreach (Guid dataGuid in dataPatchRequest.Patches.Keys)
{
return NotFound("Did not find data element");
}
var dataElement = instance.Data.Find(m => m.Id.Equals(dataGuid.ToString(), StringComparison.Ordinal));

var dataType = await GetDataType(dataElement);
if (dataElement is null)
{
return NotFound(
new ProblemDetails()
{
Title = "Did not find data element",
Detail =
$"Data element with id {dataGuid} not found on instance {instanceOwnerPartyId}/{instanceGuid}",
Status = (int)HttpStatusCode.NotFound,
}
);
}

if (dataType?.AppLogic?.ClassRef is null)
{
_logger.LogError(
"Could not determine if {dataType} requires app logic for application {org}/{app}",
dataType,
org,
app
);
return BadRequest($"Could not determine if data type {dataType?.Id} requires application logic.");
var dataType = await GetDataType(dataElement);

if (dataType?.AppLogic?.ClassRef is null)
{
_logger.LogError(
"Could not determine if {dataType} requires app logic for application {org}/{app}",
dataType,
org,
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
app
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
);
return BadRequest(
new ProblemDetails()
{
Title = "Could not determine if data type requires application logic",
Detail = $"Could not determine if data type {dataType?.Id} requires application logic."
}
);
}
}

ServiceResult<DataPatchResult, DataPatchError> res = await _patchService.ApplyPatch(
ServiceResult<DataPatchResult, DataPatchError> res = await _patchService.ApplyPatches(
instance,
dataType,
dataElement,
dataPatchRequest.Patch,
dataPatchRequest.Patches,
language,
dataPatchRequest.IgnoredValidators
);

if (res.Success)
{
await UpdateDataValuesOnInstance(instance, dataType.Id, res.Ok.NewDataModel);
await UpdatePresentationTextsOnInstance(instance, dataType.Id, res.Ok.NewDataModel);
foreach (var dataGuid in dataPatchRequest.Patches.Keys)
{
await UpdateDataValuesOnInstance(instance, dataGuid.ToString(), res.Ok.NewDataModels[dataGuid]);
await UpdatePresentationTextsOnInstance(
instance,
dataGuid.ToString(),
res.Ok.NewDataModels[dataGuid]
);
}

return Ok(
new DataPatchResponse
new DataPatchResponseMultiple()
{
NewDataModel = res.Ok.NewDataModel,
NewDataModels = res.Ok.NewDataModels,
ValidationIssues = res.Ok.ValidationIssues
}
);
Expand All @@ -513,7 +588,7 @@
{
return HandlePlatformHttpException(
e,
$"Unable to update data element {dataGuid} for instance {instanceOwnerPartyId}/{instanceGuid}"
$"Unable to update data element {string.Join(", ", dataPatchRequest.Patches.Keys)} for instance {instanceOwnerPartyId}/{instanceGuid}"
);
}
}
Expand Down
Loading
Loading