Add function select to dynamic expressions

mikaelrss commented 3 weeks ago

Description

This is a feature request, and I would like some feedback as to whether you think this should be implemented or not.

This PR introduces a new function _experimentalSelectAndMap to the dynamic expression language. The function selects string properties in objects of an array in the data model and concatenates them to a single string. The reason for this is to be able to create a list of something in a form's data model.

Usage:

Say you have this underlying data structure:

{
  "path": {
    "to": {
      "arrayOfObjects": [
        {
          "name": "Prop1",
          "value": "Value1"
        },
        {
          "name": "Prop2",
          "value": "Value2"
        },
        {
          "name": "Prop3",
          "value": "Value3"
        }
      ]
    }
  }
}

["_experimentalSelectAndMap", "path.to.arrayOfObjects", "name"]

Would result in the string Prop1 Prop2 Prop3

The function accepts two optional parameters that are string constants to prepend and append to each item in the list.

["_experimentalSelectAndMap", "path.to.arrayOfObjects", "name", "<li>", "</li>"]

Would result in the string <li>Prop1</li> <li>Prop2</li> <li>Prop3</li>

Appending to the last element in the list is optional but on by default.

["_experimentalSelectAndMap", "path.to.arrayOfObjects", "name", "", ",", false]

Would result in the string Prop1, Prop2, Prop3

A typical use case for this would be to show a list of names from some data structure. The need arose in Digital gravferdsmelding, where there is a need to be able to list deceased people who already have somebody to take care of their funeral.

I realize I can probably already achieve this by creating the output string on the backend and show the string in clear text in the form. However, I think it would be nice to have this kind of function as this would allow service owners to implement something like this in Altinn Studio without having to write C# code.

Verification/QA

Manual functionality testing
- [x] I have tested these changes manually
- [ ] Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
- [ ] No testing done/necessary
Automated tests
- [x] Unit test(s) have been added/updated
- [ ] Cypress E2E test(s) have been added/updated
- [ ] No automatic tests are needed here (no functional changes/additions)
- [ ] I want someone to help me make some tests
UU/WCAG (follow these guidelines until we have our own)
- [ ] I have tested with a screen reader/keyboard navigation/automated wcag validator
- [x] No testing done/necessary (no DOM/visual changes)
- [ ] I want someone to help me perform accessibility testing
User documentation @ altinn-studio-docs
- [ ] Has been added/updated
- [ ] No functionality has been changed/added, so no documentation is needed
- [x] I will do that later/have created an issue - This feature will be undocumented as it is not intended to be removed in the future. It will be replaced by a more comprehensive list operation workflow in the dynamic expression language.
Support in Altinn Studio
- [ ] Issue(s) created for support in Studio
- [x] This change/feature does not require any changes to Altinn Studio
Sprint board
- [ ] The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
- [ ] I don't have permissions to do that, please help me out
Labels
- [x] I have added a kind/* label to this PR for proper release notes grouping
- [ ] I don't have permissions to add labels, please help me out

bjosttveit commented 3 weeks ago

Have you thought about what the function should look like with multiple data models?

For example, the dataModel expression will have this syntax for referencing different data models explicitly: ["dataModel", "path.to.field", "dataType"]

mikaelrss commented 3 weeks ago

Have you thought about what the function should look like with multiple data models?

For example, the dataModel expression will have this syntax for referencing different data models explicitly: ["dataModel", "path.to.field", "dataType"]

Ah, no I did not think of this. But if the dataModel function is going to have an extra field to specify which data model to read from, then I guess the select function would need some way to do this as well. Is the explicit dataType argument required in the new function? If so, we could just add it to select as the second argument as well, and require 3 args instead of 2.

bjosttveit commented 3 weeks ago

Is the explicit dataType argument required in the new function? If so, we could just add it to select as the second argument as well, and require 3 args instead of 2.

For the dataModel expression the data type is an optional parameter to maintain backwards compatibility. Not specifying the data type will use the one defined in layout-sets as before. But in this case, since the function does a lot and there are already many optional args, it will get very cluttered. Maybe this could be split into multiple simpler expressions?

Edit: What you suggested in slack was exactly what I was thinking 😅

mikaelrss commented 3 weeks ago

Is the explicit dataType argument required in the new function? If so, we could just add it to select as the second argument as well, and require 3 args instead of 2.

For the dataModel expression the data type is an optional parameter to maintain backwards compatibility. Not specifying the data type will use the one defined in layout-sets as before. But in this case, since the function does a lot and there are already many optional args, it will get very cluttered. Maybe this could be split into multiple simpler expressions?

Edit: What you suggested in slack was exactly what I was thinking 😅

I think something like this would be the best solution. ["map", ["pluck", "[path.to](http://path.to/).dataModel", "name"], ["concat", "<li>", ["argv", 0], "</li>]] but as @olemartinorg pointed out. This would be a larger job to implement. I don't think we have the time to spare to implement something like this right now. But since my select (which is going to be renamed) function is a temporary solution, do you still think it is necessary to implement support for multiple data models? Could we get by with only operating on the data model of the current layout-set or are there any issues with this when we start to support multiple data models?

bjosttveit commented 3 weeks ago

But since my select (which is going to be renamed) function is a temporary solution, do you still think it is necessary to implement support for multiple data models?

For a "temporary" thing like this, that we do not want to support in the future, but rather replace with something more general. Could we maybe leave this as an undocumented feature, and require a feature flag for it to work? Maybe also a warning in the schema that this is an experimental feature not to be used in prod by anyone else?

maybe give it a disgusting name like _experimentalSelectAndMap to really discourage its use 😅

That way we could replace it before the next major version without it being a breaking change. And also not require an auto-fix in the altinn-studio-cli

Could we get by with only operating on the data model of the current layout-set or are there any issues with this when we start to support multiple data models?

Should not be a problem to still support the default model when multiple data models are released. Should only be a 1-2 line change 😃

mikaelrss commented 3 weeks ago

For a "temporary" thing like this, that we do not want to support in the future, but rather replace with something more general. Could we maybe leave this as an undocumented feature, and require a feature flag for it to work? Maybe also a warning in the schema that this is an experimental feature not to be used in prod by anyone else?

I think this is a great solution! Do we need to put the functionality behind a feature flag if we're naming it _experimentalSelectAndMap? :sweat_smile:

bjosttveit commented 3 weeks ago