Added Unit tests, Passing rawData along with messages now #14
Conversation
mrajatttt
requested review from
mliao95 and
doreechi
and removed request for
a team
| } | ||
|
|
||
| public class TranscriptItem: TranscriptItemProtocol { | ||
| public var id = UUID() | ||
| public var timeStamp: String | ||
| public var contentType: String | ||
| public var rawData: [String: Any]? |
There was a problem hiding this comment.
For future reference, we should split the rawData changes into a separate PR.
| @@ -0,0 +1,50 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Is this file used anywhere or does it have to be run manually?
There was a problem hiding this comment.
Not used as of now, might consider running github actions using it later but for now, just wanted it for safe keep. I will remove it if i found it useless.
| @@ -55,8 +41,8 @@ public class Message: TranscriptItem, MessageProtocol { | |||
| case ContentType.interactiveText.rawValue: | |||
| return decodeInteractiveContent(from: text) | |||
| default: | |||
| // Handle or log unsupported content types | |||
| return nil | |||
| // Handle or log unsupported content types - Sending as Plain Text for now. | |||
There was a problem hiding this comment.
is this the correct default for unsupported content type?
There was a problem hiding this comment.
For the fallback option i have added plain text instead of throwing an error.
There was a problem hiding this comment.
I don’t think translating message to plaintext if content type isn’t recognized is what UI should be doing. I think we should just be ignoring content types that aren’t explicitly expected
There was a problem hiding this comment.
so return an error?
There was a problem hiding this comment.
I believe we’re already doing this in chat interface but we should be ignoring/not doing anything if there’s a content type we don’t recognize. There shouldn’t be any translation or error thrown since new content types can be added at any time we should explicitly have to make changes to start handling those messages on the client
| @@ -20,10 +20,10 @@ public class Metadata: TranscriptItem, MetadataProtocol { | |||
| public var messageId: String? | |||
| public var eventDirection: MessageDirection? | |||
|
|
|||
| init(status: MessageStatus? = nil, messageId: String? = nil, timeStamp: String, contentType: String, eventDirection: MessageDirection? = .Common) { | |||
| init(status: MessageStatus? = nil, messageId: String? = nil, timeStamp: String, contentType: String, eventDirection: MessageDirection? = .Common, rawData: [String: Any]) { | |||
There was a problem hiding this comment.
in general, what's the purpose of rawData? Is it the message content? If so, should we name it content instead of rawData
There was a problem hiding this comment.
Yes its the message content as we receive from websocket. We could rename it to content but in rawData we are also including "contentType": application/json, "topic": aws/chat
For example:
raw data for transcript Item/message looks like this
Optional(["content": {"AbsoluteTime":"2024-05-23T19:41:26.578Z","ContentType":"application/
vnd.amazonaws.connect.event.participant.joined","Id":"6b532881-2835-4981-abb0-
a7a4b422d013","Type":"EVENT","ParticipantId":"48f25338-d51c-41ae-
b10a-7bd55f6e2406","DisplayName":"C","ParticipantRole":"CUSTOMER","InitialContactId":"ec34
9471-1139-4fa4-af6c-c277a903d3e7"}, "contentType": application/json, "topic": aws/chat])
There was a problem hiding this comment.
oh, so it's an item? https://docs.aws.amazon.com/connect/latest/APIReference/API_connect-participant_Item.html can we rename it that. I feel like rawData isn't specific enough
There was a problem hiding this comment.
But it says An item - message or event - that has been sent. and the params of that matches to the params of content. @mliao95 any opinion here?
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.