GroupNode.subscribe() should also subscribe on all of its children?

[sadiqkhoja]

Issue:

When a node depends on a group node (by using string function) and state of a child of that group changes, the state of the target node is not recomputed. Because subscribe of group function doesn't subscribes to the children.

I don't know how much practical this scenario is, based on that we can prioritize.

In the following example value of /data/result is computed using /data/g1 (group) and /data/g1/t1 (text input), it works when /data/g1/t1 is updated but not when /data/g1/t2 is updated. Note that calculate expression of result is using implicity string() function.

Example:

describe('Subscribe all children of group when group-node is the argument to an XPath function', () => {
	it.only('should work', async () => {
		
		const scenario = await Scenario.init(
			'get-value-of-all-questions-in-a-group',
			html(
				head(
					title('get-value-of-all-questions-in-a-group'),
					model(
						mainInstance(
							t('data id="get-value-of-all-questions-in-a-group"', 
								t('g1',
									t('t1'),
									t('t2')),
								t('result'))), 
						bind('/data/g1/t1').type('string'),
						bind('/data/g1/t2').type('string'),
						bind('/data/result').type('string').readonly('true()').calculate('concat( /data/g1, /data/g1/t1 )')
					)
				),
				body(group('/data/g1',
					input('/data/g1/t1'),
					input('/data/g1/t2')
				))
			)
		);

		scenario.answer('/data/g1/t1', 'test1 ');
		expect(scenario.answerOf('/data/result')).toEqualAnswer(stringAnswer('test1 test1 '));
		
		scenario.answer('/data/g1/t2', 'test2 ');
		expect(scenario.answerOf('/data/result')).toEqualAnswer(stringAnswer('test1 test1 ')); // This should be 'test1 test2 test1 '

		scenario.answer('/data/g1/t1', 'test1again ');
		expect(scenario.answerOf('/data/result')).toEqualAnswer(stringAnswer('test1again test2 test1again '));
	});
});

[eyelidlessness]

A few notes on this:

0. Tests are super helpful when filing issues like this, it's a great way to jumpstart a fix! That said, when I ran the test I was surprised to see it pass. I thought maybe there the issue could be mistaken, until I spotted the trailing comment after the second assertions. It'd be more helpful to post such tests with the actual expectation/assertion failing (and then helpful reading in-issue to see the actual behavior in the comment instead).

1. I'm inclined to mark this as a bug. I don't know how common the pattern (calculate referencing a non-leaf node as a value) is in real world forms, but it does break expectations I would have from a spec perspective.

2. I would like to consider editing the issue to be less specific to implementation details. The details are partially¹ correct now, but the bug may still be present/worth testing for even if the implementation details change. A likely change would be if we take on Decouple @odk/xpath API from XML/browser DOM #39.

3. I don't think it's entirely correct to say that the calculate is implicitly using the string() function, but it is using the same casting logic. Pertinent language from the XPath 1.0 spec:

   An argument is converted to type string as if by calling the string function. An argument is converted to type number as if by calling the number function. An argument is converted to type boolean as if by calling the boolean function. An argument that is not of type node-set cannot be converted to a node-set.

Footnotes

1. Partially because the GroupNode interface does not expose a subscribe method (it's internal on Group), and because I would currently expect the same behavior when referencing a node implemented by Subtree. ↩