Feat/conversation event transcoding capability endpoints #43
Conversation
move agent to conversation/common namespace add test of Transcoding
Dovchik
requested review from
650elx,
krogers0607,
asein-sinch and
JPPortier
as code owners
src/Sinch/Conversation/Conversations/InjectEvent/InjectEventRequest.cs
Outdated
Show resolved
Hide resolved
thanks you system.text.json
|
Latest commit makes InjectEvent to take conversationId as a param and not included in the request object due to system.text.json limitations |
| @@ -244,20 +245,19 @@ public Task Stop(string conversationId, CancellationToken cancellationToken = de | |||
| } | |||
|
|
|||
| /// <inheritdoc /> | |||
| public Task<InjectEventResponse> InjectEvent(InjectEventRequest injectEventRequest, | |||
| public Task<InjectEventResponse> InjectEvent(string conversationId, InjectEventRequest injectEventRequest, | |||
There was a problem hiding this comment.
Why do you need to extract this parameter from InjectEventRequest ? I see the comment about the limitations of system.text.json, but it becomes inconsistent with "injectMessage"
There was a problem hiding this comment.
The behavior is counterintuitive, If field is marked required and JsonIgnore at the same type this throws a runtime error from json serializer. I'll include conversationId in request class. Also, there will be task to harmonize approach inside all over the solution
There was a problem hiding this comment.
For the curious - dotnet/runtime#82879
| if (string.IsNullOrEmpty(conversationId)) | ||
| throw new ArgumentNullException(nameof(conversationId)); | ||
| if (string.IsNullOrEmpty(injectEventRequest.ConversationId)) | ||
| throw new NullReferenceException($"{nameof(injectEventRequest.ConversationId)} is required."); |
There was a problem hiding this comment.
Nit. the exception message is different from InjectMessage
Dovchik
deleted the
feat/conversation-event-transcoding-capability-endpoints
branch
Covers: conversation/events get, list, delete, send
conversation/conversation inject-event
Add APPLEBC into list of Channels
CI blocked by https://github.com/sinch/doppelganger/pull/13