Skip to content
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

Update Get-ServiceNowTable.ps1 #22

Merged
merged 2 commits into from
Jan 12, 2018
Merged

Conversation

GilmondRoss
Copy link
Contributor

Added correct string to datatime casting using the local computer's culture settings.

Added correct string to datatime casting using the local computer's culture settings.
@GilmondRoss
Copy link
Contributor Author

We might want to consider merging this as soon as possible as this broke existing functionality for me.

@Rick-2CA
Copy link
Member

Hey, Gildmond, thanks for the PR. First off my apologies for not considering culture with the change.

I've been testing the code in my environment and the string returned by the REST API doesn't match the pattern you're proposing. I've got a yyyy-MM-dd hh:mm:ss format which doesn't match the d/M/yyyy H:mm:ss format my culture returns.

What format are your DateTime fields in?
Which ServiceNow version are you in?

I'm not a ServiceNow admin. I consulted with one here and they're not aware of culture settings within ServiceNow. If you happen to be aware of them could you point me in the right direction? Perhaps we'd be able to get the proper format out of ServiceNow for the conversion.

@Rick-2CA Rick-2CA self-assigned this Jan 11, 2018
@Rick-2CA Rick-2CA added the bug label Jan 11, 2018
@GilmondRoss
Copy link
Contributor Author

GilmondRoss commented Jan 11, 2018

Hi Rick,

Here is the error I get when running the original code.

PSMessageDetails : Exception : System.Management.Automation.RuntimeException: Cannot convert value "13/12/2017 09:22:16" to type "System.DateTime". Error: "String was not recognized as a valid DateTime." ---> System.Management.Automation.PSInvalidCastException: Cannot convert value "13/12/2017 09:22:16" to type "System.DateTime". Error: "String was not recognized as a valid DateTime." ---> System.FormatException: String was not recognized as a valid DateTime. at System.DateTimeParse.Parse(String s, DateTimeFormatInfo dtfi, DateTimeStyles styles) --- End of inner exception stack trace --- at System.Management.Automation.LanguagePrimitives.ConvertViaParseMethod.ConvertWithCulture(Object valueToConvert, Type resultType, Boolean recursion, PSObject originalValueToConvert, IFormatProvider formatProvider, TypeTable backupTable) at CallSite.Target(Closure , CallSite , Object ) at lambda_method(Closure , Object[] , StrongBox1[] , InterpretedFrame )
--- End of inner exception stack trace --- TargetObject : CategoryInfo : InvalidArgument: (:) [], RuntimeException FullyQualifiedErrorId : InvalidCastParseTargetInvocationWithFormatProvider ErrorDetails : InvocationInfo : System.Management.Automation.InvocationInfo ScriptStackTrace : at Get-ServiceNowTable, C:\Modules\ServiceNow\1.0.0\Public\Get-ServiceNowTable.ps1: line 89 at Get-ServiceNowIncident, C:\Modules\ServiceNow\1.0.0\Public\Get-ServiceNowIncident.ps1: line 95 at <ScriptBlock>, C:\: line 94 PipelineIterationInfo : {}

I'm afraid I'm not sure of Service-Now's culture settings myself.

@Rick-2CA
Copy link
Member

@GilmondRoss - Short term I'm going to suggest the code change below. Would you mind testing it on your end and let me know if it makes sense? Basically if your code to use the local culture doesn't work then we attempt to use a universal format. If that doesn't work then the variable stays a string.

Long term I'd like to find out how to query ServiceNow for the proper format for whatever tenant this is run against and then convert that to the local culture.

Try {
    # Extract the default Date/Time formatting from the local computer's "Culture" settings, and then create the format to use when parsing the date/time from Service-Now
    $CultureDateTimeFormat = (Get-Culture).DateTimeFormat
    $DateFormat = $CultureDateTimeFormat.ShortDatePattern
    $TimeFormat = $CultureDateTimeFormat.LongTimePattern
    $DateTimeFormat = "$DateFormat $TimeFormat"
    $SNResult.$Property = [DateTime]::ParseExact($($SNResult.$Property), $DateTimeFormat, [System.Globalization.DateTimeFormatInfo]::InvariantInfo, [System.Globalization.DateTimeStyles]::None)
}
Catch {
    Try {
        # Universal Format
        $DateTimeFormat = 'yyyy-MM-dd HH:mm:ss'
        $SNResult.$Property = [DateTime]::ParseExact($($SNResult.$Property), $DateTimeFormat, [System.Globalization.DateTimeFormatInfo]::InvariantInfo, [System.Globalization.DateTimeStyles]::None)
    }
    Catch {
        # If the local culture and universal formats both fail keep the property as a string (Do nothing)
    }
}

Feel free to edit your PR or let me know if you want me to do it.

Updated with Rick's additional changes.
@GilmondRoss
Copy link
Contributor Author

Hi @Rick-2CA,

That seems logical, I've tested and merged your changes.

Let me know if you require anything else from me.

@Rick-2CA Rick-2CA merged commit 33b72ea into Snow-Shell:master Jan 12, 2018
@Rick-2CA
Copy link
Member

Appreciate the contribution. I'll get this put onto the PSGallery soon.

@GilmondRoss GilmondRoss deleted the patch-1 branch January 12, 2018 16:23
Rick-2CA added a commit that referenced this pull request Jan 16, 2018
@X-Guardian
Copy link

Hi guys, the Service-Now Instance date format is stored in the sys_properties table, property glide.sys.data_format. Reference: https://docs.servicenow.com/bundle/jakarta-platform-administration/page/administer/time/reference/r_FormatDateAndTimeFields.html

@GilmondRoss
Copy link
Contributor Author

@X-Guardian Confirmed. Our instance date format is set in the following table property. glide.sys.date_format.

I'd be cautious about getting this property from there because it's a system table, and the chances of many people having access to that is questionable. If we wanted to improve on the current code, perhaps we could try getting the property from there, and if that fails, fall back to the current method?

@X-Guardian
Copy link

Sorry, yes the sys_properties table is not readable by a normal user, so not very useful.

Looking at the Get-ServiceNowTable code, you are setting the sysparm_display_value to the $DisplayValues parameter. This defaults to false in Get-ServiceNowTable but defaults to true in Get-ServiceNowconfigurationItem for example. According to Table API, this setting defines whether fields are returned with their actual database values or their display values. Date fields are stored in the database as yyyy-mm-dd hh:mm:ss, so with sysparm_display_value set to false (which is also the API default) will return the date in a consistent format, irrelevant of the Instance date format settings.

@GilmondRoss
Copy link
Contributor Author

@X-Guardian Would you be happy to make this change?

@X-Guardian
Copy link

OK, to start with I'll change the $DisplayValues to default to false for all the higher level functions, then tweak the date processing code in Get-ServiceNowTable to cope with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants