{"action":"created","issue":{"url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/issues/286","repository_url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet","labels_url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/issues/286/labels{/name}","comments_url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/issues/286/comments","events_url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/issues/286/events","html_url":"https://github.com/graphql-dotnet/graphql-dotnet/issues/286","id":211085867,"number":286,"title":"UTC Dates in Mutation Input fields are broken in 0.14.7 and upards","user":{"login":"lngr","id":1508058,"avatar_url":"https://avatars3.githubusercontent.com/u/1508058?v=3","gravatar_id":"","url":"https://api.github.com/users/lngr","html_url":"https://github.com/lngr","followers_url":"https://api.github.com/users/lngr/followers","following_url":"https://api.github.com/users/lngr/following{/other_user}","gists_url":"https://api.github.com/users/lngr/gists{/gist_id}","starred_url":"https://api.github.com/users/lngr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lngr/subscriptions","organizations_url":"https://api.github.com/users/lngr/orgs","repos_url":"https://api.github.com/users/lngr/repos","events_url":"https://api.github.com/users/lngr/events{/privacy}","received_events_url":"https://api.github.com/users/lngr/received_events","type":"User","site_admin":false},"labels":[{"id":232986994,"url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/labels/bug","name":"bug","color":"fc2929","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2017-03-01T13:47:56Z","updated_at":"2017-03-01T21:35:44Z","closed_at":null,"body":"The recent changes to the DateTime output (allow DateTime in StringType-fields, https://github.com/graphql-dotnet/graphql-dotnet/commit/67bf2ed015753801787f9f13931be7ba0c3b7186 ) unfortunately breaks UTC dates again for input fields/query arguments.\r\n\r\nMore specifically, we have the following resolver for a Relay mutation:\r\n\r\n```\r\n.Resolve(args =>\r\n                {\r\n                    var input = args.GetArgument<MyInputType>(\"input\");\r\n                    ...\r\n```\r\n\r\nlooking at the source of `ResolveFieldContext.GetArgument`, it will call `arg.GetPropertyValue<TType>();` in \r\nhttps://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Types/ResolveFieldContext.cs#L85\r\n\r\nHere it will eventually call `ConvertValue()` in https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/ObjectExtensions.cs#L105\r\nwhich was unfortunately modified in the pull request.\r\n\r\nSince the test against `DateTime` has been removed in https://github.com/graphql-dotnet/graphql-dotnet/pull/272/files#diff-ffca66d73ea1b01c7a2ab35b3b63a4f9L126 , the string is converted via \"ToString() again, which was to root case in #197, see the fix in\r\nhttps://github.com/graphql-dotnet/graphql-dotnet/pull/198\r\n\r\nSince `DateTime.ToString()` removes the `Z`-suffix, the \"UTC\" information is lost again.\r\n\r\nTBH, if someone wants to send date strings in `StringGraphType`, I'd recommend to do the conversion manually before feeding the object to GraphQL and not let the framework handle such use cases. I therefore recommend to revert the changes in https://github.com/graphql-dotnet/graphql-dotnet/pull/272\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/issues/comments/283477937","html_url":"https://github.com/graphql-dotnet/graphql-dotnet/issues/286#issuecomment-283477937","issue_url":"https://api.github.com/repos/graphql-dotnet/graphql-dotnet/issues/286","id":283477937,"user":{"login":"cstlaurent","id":213276,"avatar_url":"https://avatars0.githubusercontent.com/u/213276?v=3","gravatar_id":"","url":"https://api.github.com/users/cstlaurent","html_url":"https://github.com/cstlaurent","followers_url":"https://api.github.com/users/cstlaurent/followers","following_url":"https://api.github.com/users/cstlaurent/following{/other_user}","gists_url":"https://api.github.com/users/cstlaurent/gists{/gist_id}","starred_url":"https://api.github.com/users/cstlaurent/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cstlaurent/subscriptions","organizations_url":"https://api.github.com/users/cstlaurent/orgs","repos_url":"https://api.github.com/users/cstlaurent/repos","events_url":"https://api.github.com/users/cstlaurent/events{/privacy}","received_events_url":"https://api.github.com/users/cstlaurent/received_events","type":"User","site_admin":false},"created_at":"2017-03-01T21:35:44Z","updated_at":"2017-03-01T21:35:44Z","body":"Hi, @lngr, I worked with @mamasselin on the pull request #272 ans I would advise against reversing it. The goal of this merge request was to fix the unexpected behavior of receiving `null` in your `StringGraphType` field value if you had a string that happened to look like an ISO date. I think this correction puts this library in line with the GraphQL standard.\r\n\r\nDate handling is not in the standard since it's not native to json, but I still agree with you that losing timezone information when you have a `DateGraphType` is very bad.\r\n\r\nI was pretty sure we covered the issue you mentioned while doing the modifications but maybe we missed something.\r\n\r\nAnyway, @mamasselin and me are looking into it."}}
{"actor":{"display_login":"cstlaurent"}}