Fixes issue where time values gets stripped from DateTime data in Xpath calculations

[shubham1g5]

String casts dateTime data while unpacking a Xpath reference to avoid an issue where time values gets stripped from dateTime values because of Date casted values getting converted to DateData in recalculate expressions by default in case no explicit dataType is specificed for the Xpath node. Including a test case for sake of clarity.

Product Note: Fixes an issue with values from a date-time question type not accurately propagating to a case.

[shubham1g5]

@ctsims Revisiting this and I unfortunately don't recollect what we decided as the outcome here. This is a data error and if we think fixing this might break more things, I would like to note this issue somewhere explicitly in the product docs as a limitation.

[ctsims]

@shubham1g5 Yeah, I think we decided that this introduced ambiguous behavior we don't have the ability to resolve right now.

I'm inclined to say that this github wiki is my preferred place to describe technical limitations like this, maybe we should make a page about Date and Time handling / limitations in general? I think there's a lot to write down on that topic, and putting it all in one place would be helpful

[shubham1g5]

Thanks. Do you recall explicitly what was the ambiguous behaviour associated with this change ? Is it that people might be relying on the incorrect behaviour today ?

Agree that it would be good to add a page on date time limitations. I can try putting it together though would you be able to give me some pointers our outline of other limitations you are talking about.

[ctsims]

Hi Shubham,

Sorry for the delay. I thought I remembered this a bit more clearly than I seem to in reviewing the code.

I think the underlying issue you are describing is that DateTime's don't propagated in full, their time components end up stripped if you try to refer to them through untyped data. I think that is currently an intentionally enforced limitation, where the engine 'fuzzes' datetimes in most contexts to ensure that a calendar date stays a calendar date regardless of timezones, but I'm not 100% sure I can trace the logic for why this change breaks that without actually stepping through in more detail.

[shubham1g5]

@ctsims I see, I am happy to close this out if this issue is not a big concern. Just want to flag though this issue exists in behaviour today when a CommCare App tries to save the value of a dateTime question as a case property. Probably a better change here would be on HQ to bind the case properties operating on datetime nodes with the type="xsd:dateTime" flag.

[ctsims]

One idea: maybe it would be good to at least encode the current behavior very specifically in the regression / unit tests as the place in-code that we document what happens? That would make it clear what the current behavior actually is, and if anyone makes an update that changes that behavior, we can add notes or a link to the explanation inline

[shubham1g5]

maybe it would be good to at least encode the current behavior very specifically in the regression / unit tests as the place in-code that we document what happens?

You mean in the mobile code ? This PR has a test that checks for this behaviour. So would a similar test on current datetime behaviour is what you are referring to ?