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

Conversation

ivarne
Copy link
Member

@ivarne ivarne commented Aug 19, 2024

Work on supporting multiple data models and subforms in backend validations with expressions.

Related Issue(s)

Main changes

The main goal for this PR is to add

New single interface for validation IValidator

With changes in one data element affects the validity of another element, the current hierarchy of validators and only IFormDataValidator supporting incremental validation break down. Adaptors for the old interfaces has been provided so no changes to current apps are required, but they might want to update in order to support incremental validation and read instance data with the IInstanceDataAccessor.

New interface IInstanceDataAccessor

When validation has dependencies across data models, we need a cached way to access the content of data elements that does not require us to download the same data element multiple times for the same request. The implementation is not available from DI, but always passed as an argument in order to eliminate risk of it being reused cross requests because of Singleton services or other unintentionally shared global state.

New DataElementId struct to ensure type safety for a wrapped Guid

36 places in the updated code need to reference the Guid of a data element, and adding type safety is an important step to ensure correctness. An implicit cast is provided from DataElement to simplify usage

New ModelBinding struct in expressions engine

To support the new syntax for referring to a non default data element in components and expressions

{
    "dataModelBindings": {"field": "path.to.data", "dataType": "default"}
    "hidden": ["equals", null, ["dataModel", "path.to.other", "prefill"]]
}

When handling references to a specific fields (eg. for removal) we needed to also add DataReference to combine a Field (path) with a specific DataElementId

New PATCH api endpoint that allows frontend to update multiple data models at the same time

Saving and triggering incremental validation is expensive and some apps might want to batch multiple changes to reduce cost. The code in frontend would also be simplified if it could do a single request, instead of multiple.

ValidationIssueWithSource class for external apis

Previously we had a single class ValidationIssue that included a Source property. Service owners sometimes tries to set this in their validators (because it shows up in intellisense), but as we override it based on IValidator.ValidationSource in our ValidationService it would be good if it wasn't there to confuse. This PR creates a new type ValidationIssueWithSource for external apis (where the source is included) and marks it [Obsolete] in the type returned in the validator interfaces.

Respect new app logic settings for data elements DisallowUserCreate and DisallowUserDelete

This means that users can not create or delete this types. Typically used for prefill, when you want only the org token to be able to modify the number of data elements on an instance.

Minor changes

Used IL code to generate real C# classes based on the json data models in shared tests. Previously different code was used to access properties of the test code as in production, which is not really good for tests.

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@ivarne ivarne force-pushed the feat/multi-data-model-with-validation branch from c2bfbae to f7b91e0 Compare August 19, 2024 12:22
@ivarne ivarne force-pushed the feat/multi-data-model-with-validation branch from 1703585 to 9007d42 Compare August 22, 2024 10:05
* Refactor DataController and separate out DataElementAccessChecker so that it is easier to reuse logic and return consistent ProblemDetails across APIs
* Fix error in compatibility code with ActionController not returning data from UpdatedDataModels when IInstanceDataMutator is not used.
* Return Instance from UserAction endpoint in case data elements were added or removed.
* Check that instance is active when updating metadata.
* Use PatchListItem in multiplePatchEndpoint instead of dictionary for better OpenApi documentation
@ivarne ivarne mentioned this pull request Oct 6, 2024
5 tasks
* use IInstanceMutator
* Make IInstanceMutator update dataValues and PresentationTexts and remove from places that uses it
@ivarne ivarne mentioned this pull request Oct 9, 2024
5 tasks
Copy link
Contributor

@HauklandJ HauklandJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some minor things to consider

)
{
var layoutSetId = "default";
var layoutEvaluatorState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSetId);

Check warning

Code scanning / CodeQL

Call to obsolete method Warning test

Call to obsolete method
Init
.
Copy link

sonarcloud bot commented Oct 11, 2024

@ivarne ivarne changed the title Feat/multi data model with validation Support Extra data models and subforms in validation Oct 11, 2024
@ivarne ivarne merged commit f9a6d24 into main Oct 11, 2024
9 of 10 checks passed
@ivarne ivarne deleted the feat/multi-data-model-with-validation branch October 11, 2024 06:49
@ivarne ivarne added the feature Label Pull requests with new features. Used when generation releasenotes label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Label Pull requests with new features. Used when generation releasenotes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants