HotChocolate 16 Milestone 11 Migration#3136
Conversation
michaelstaib
requested review from
Alekhya-Polavarapu,
Aniruddh25,
RubenCerna2079,
aaronburtle,
anushakolan,
neeraj-sharma2592,
rusamant,
sourabh1007,
souvikghosh04 and
vadeveka
as code owners
| authorizationResolver, | ||
| ctx.Selection.Field, | ||
| ctx.Selection.SyntaxNode, | ||
| ctx.Selection.SyntaxNodes[0].Node, |
There was a problem hiding this comment.
Is SyntaxNodes guaranteed to have at least 1 node?
There was a problem hiding this comment.
Perhaps using Single() would be more clear?
| DATETIME_TYPE => new(DATETIME_TYPE, new DateTimeType().ValueToLiteral( | ||
| DateTime.Parse(defaultValueFromConfig, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal))), | ||
| BYTEARRAY_TYPE => new(BYTEARRAY_TYPE, new ByteArrayType().ParseValue(Convert.FromBase64String(defaultValueFromConfig))), | ||
| LOCALTIME_TYPE => new(LOCALTIME_TYPE, new HotChocolate.Types.NodaTime.LocalTimeType().ParseResult(LocalTimePattern.ExtendedIso.Parse(defaultValueFromConfig).Value)), |
There was a problem hiding this comment.
Do both ParseResult and ParseValue map to ValueToLiteral now?
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
@copilot, fix formatting error: /home/vsts/work/1/s/src/Core/Services/ExecutionHelper.cs(21,1): warning IDE0005: Using directive is unnecessary. [/home/vsts/work/1/s/src/Core/Azure.DataApiBuilder.Core.csproj] |
looks like copilot wont work cross repos, this needs to be fixed manually. |
| authorizationResolver, | ||
| ctx.Selection.Field, | ||
| ctx.Selection.SyntaxNode, | ||
| ctx.Selection.SyntaxNodes[0].Node, |
There was a problem hiding this comment.
Perhaps using Single() would be more clear?
| { | ||
| FieldNode? fieldNode = ExtractQueryField(selection.SyntaxNode); | ||
| FieldNode? fieldNode = ExtractQueryField(selection.SyntaxNodes[0].Node); | ||
|
|
There was a problem hiding this comment.
Same as the SQL Structure, should we consider a guard or helper around SyntaxNodes in case it is empty or to indicate that we expect just one?
|
|
||
| ObjectField schemaField = _ctx.Selection.Field; | ||
| FieldNode? queryField = _ctx.Selection.SyntaxNode; | ||
| FieldNode? queryField = _ctx.Selection.SyntaxNodes[0].Node; |
There was a problem hiding this comment.
Does this accessor need a guard or helper?
| else | ||
| { | ||
| Columns.AddRange(GenerateQueryColumns(selection.SyntaxNode.SelectionSet!, _context.Operation.Document, SourceAlias)); | ||
| Columns.AddRange(GenerateQueryColumns(selection.SyntaxNodes[0].Node.SelectionSet!, _context.Operation.Document, SourceAlias)); |
There was a problem hiding this comment.
Does this accessor need a guard or helper?
|
|
||
| contextData[ExecutionContextData.HttpStatusCode] = HttpStatusCode.BadRequest; | ||
| context.Result = singleResult.WithContextData(contextData.ToImmutable()); | ||
| singleResult.ContextData = contextData.ToImmutable(); |
There was a problem hiding this comment.
Can you confirm this is the intended HC16 approach and that the mutated result is guaranteed to be the one returned on context.Result?
3 participants
Why make this change?
This upgraded Data API Builder to the near final preview of Hot Chocolate 16