Skip to content

Conversation

@jamesmanning
Copy link

@jamesmanning jamesmanning commented Aug 9, 2024

Currently the use of sysparm_display_value=all breaks the paging logic (the logic that happens when there's more than one page in the query result) which assumes that the property value found via the 'orderByField' will be something we can append a "Z" to then DateTimeOffset.Parse.

// At this point, we can be sure that we have the paging field in the data
maxDateTimeRetrieved = response.Items.Max(jObject =>
// Parse and enforce source as being UTC (Z)
DateTimeOffset.Parse((jObject[orderByField]?.ToString() ?? string.Empty) + "Z"));

This fails when you're using sysparm_display_value=all since the value of the orderByField will be a JObject with properties of value and display_value so you end up with an exception during the query similar to this:

String '{
"display_value": "2024-03-04 21:15:20",
"value": "2024-03-04 21:15:20"
}Z' was not recognized as a valid DateTime.

This changes the parsing to use DateTimeStyles.AssumeUniversal instead of appending a "Z" and to fall back to DateTimeOffset.MinValue for failure cases instead of throwing an exception. Since the current code throws an exception, it makes it difficult for query callers that require using sysparm_display_value=all

Currently the use of sysparm_display_value=all breaks the paging logic which assumes that the property value found via the 'orderByField' will be something we can append a "Z" to then DateTimeOffset.Parse.

This fails when you're using sysparm_display_value=all since the value of the orderByField will be a JObject with properties of value and display_value so you end up with an exception during the query similar to this:

String '{
  "display_value": "2024-03-04 21:15:20",
  "value": "2024-03-04 21:15:20"
}Z' was not recognized as a valid DateTime.

This changes the parsing to use DateTimeStyles.AssumeUniversal instead of appending a "Z" and to fall back to DateTimeOffset.MinValue for failure cases instead of throwing an exception.  Since it throws an exception, it makes it difficult for query callers that require using sysparm_display_value=all
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant