-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add separate context for rows in expression evaluation #979
Conversation
…repeating groups Previously this was implemented as a special case, but this makes the code simpler and fixes bugs I did not understand.
92145ea
to
fd5d9c2
Compare
if (!includeHiddenRowChildren && isHidden) | ||
{ | ||
if (context.Component is RepeatingGroupRowComponent or RepeatingGroupComponent) | ||
{ | ||
var hiddenRows = await childContext.GetHiddenRows(state); | ||
var currentRow = childContext.RowIndices?[^1]; | ||
var rowIsHidden = currentRow is not null && hiddenRows[currentRow.Value]; | ||
if (rowIsHidden) | ||
if (context.Component.DataModelBindings.TryGetValue("group", out var groupBinding)) | ||
{ | ||
continue; | ||
var indexedBinding = await state.AddInidicies(groupBinding, context); | ||
hiddenModelBindings.Add(indexedBinding); | ||
} | ||
return; | ||
} | ||
|
||
} |
Check notice
Code scanning / CodeQL
Nested 'if' statements can be combined Note
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I don't know where it failed before, but this way of handling repeating group rows seems more robust 💪
|
Frontend tests in ExpressionValidation app discovered that Expression validation failed on empty lists in data model.
This PR adds a check for empty list.
Related Issue(s)
Verification
Documentation