Fix: item parameter_list dynamic context

[chrismclarke]

PR Checklist

• PR title descriptive (can be used in release notes)

TODO

• Builds on fix: data-items underscore properties #2323 should merge after

Description

Add support for using dynamic values within the parameter list of items and data_items components. E.g.

filter: @item.number < @local.test_value
filter: @item.string === @local.some_string

Author Notes

Should confirm if passing tests cases from example_items_pipe_2 (see current output in screenshot below)

I expect this won't solve the issue in #2303 (but could be tested). If confirmed not working should probably handle in a follow-up instead of this PR

Review Notes

Worth testing against both items and data_items syntax to test if working as expected. The fix did have to change a couple parts of the items code that isn't very well documented or tested, and so does have some scope for unexpected errors. So would be good to test against current debug sheets and with cases where the item lists are dynamically updated

Dev Notes

As all the code changes are made within the template-row.service there weren't any more tests to to items or data-items specs, although I have tested that existing specs still pass. The template-row.service has no existing infrastructure, and given this is mostly a workaround it seemed beyond scope to start adding (but could be done as a follow-up).

Unpicking the issue was a bit of a challenge given the complexity of the template-row service and item implementation. See more details in the comments on #1598. This PR goes for the first proposed solution, replacing the app-string evaluator with a pure JS evaluator and attempting to pre-evaluate all non-item dynamic expressions in advance.

I also independently noticed an issue when using the app-string evaluator to evaluator any sort of string comparison (not-working). I've added a placeholder test to identify but beyond scope of this PR to fix (the more I unpick the more that starts to unravel - I think really a lot of this would be better-off with templating system refactor)

The part of the fix I'm least certain about is the parameter list revert hack that was included in both items and data-items component. The comments suggested this is because parameter lists would have item context replaced, although looking at the code it seems that it wouldn't be until render, so not sure if either the fix was legacy or I'm missing something (might be worth confirming against cases where the items get updated)

I'm not sure if related to this PR or not but starting to see issue with modifying final fields

(update - seeing on main - possibly may want to address after merge #2323)

Git Issues

Closes #1598

Screenshots/Videos

output from example_items_pipe_2

[esmeetewinkel]

Issue #2303 was closed by PR #2314, but we forgot to mark it as such when moving from the original PR #2304 to the refactor.

[esmeetewinkel]

Functional test passed for items and data_items (see additional debug sheet example_data_items_pipe_2). I also added a case with two parameters (filter + limit) and that worked fine.

I tested several other templates that use items or data_items, all seem to still be working as expected.

[jfmcquade]

Looks good and appears to be working. So I think we're good to merge, @esmeetewinkel I know you said you might make use of this functionality immediately.

A few points:

• I've added clarity to a couple of comments here: 5f46a3f

• I also independently noticed an issue when using the app-string evaluator to evaluator any sort of string comparison (not-working). I've added a placeholder test to identify but beyond scope of this PR to fix (the more I unpick the more that starts to unravel - I think really a lot of this would be better-off with templating system refactor)

  I think Fix/string evaluation js #2146 is relevant here. But agree that it might be best to wait for a wholesale templating system refactor rather than patching up.

• I agree that the comment explaining the parameter list hack that is removed in this PR seems wrong, so the hack itself is presumably outdated. Inspecting the processed sheet json for example_items_pipe_2 suggests that it is not true that "When parsing item parameter lists filter references to @item will be replaced before processing". So it does seem like we should be fine to remove this hack.

• I haven't been able to recreate the bug you noted regarding modifying final fields, but I've pulled it out into an issue here: [BUG] Data items: error trying to modify final fields #2327.