communication-ui-library icon indicating copy to clipboard operation
communication-ui-library copied to clipboard

[Bugfix] add RoomCallLocator to the types for CallWithChatComposite

Open realjesset opened this issue 3 months ago • 3 comments

What

missing the RoomCallLocator from CallAndChatLocator in AzureCommunicationCallWithChatAdapter.ts

Why

adds the RoomCallLocator to the CallAndChatLocator types

How Tested

ran up an instance of the CallWithChatComposite and it works, it was working without this change, but added types to make it consistent.

Process & policy checklist

  • [ ] I have updated the project documentation to reflect my changes if necessary.
  • [x] I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • [ ] This change causes current functionality to break.

realjesset avatar Apr 03 '24 12:04 realjesset

Hi @realjesset, thanks for being proactive and creating this PR. We did not intend for Rooms call to have chat. Technically, it should be possible to add a chat thread to a Rooms call. I will try this out myself to test if this works. I'm wondering if you were able to make this work?

mgamis-msft avatar Apr 04 '24 17:04 mgamis-msft

Hi @realjesset, thanks for being proactive and creating this PR.

Hello @mgamis-msft! No worries at all, I use this library heavily, its my pleasure!

We did not intend for Rooms call to have chat. Technically, it should be possible to add a chat thread to a Rooms call. I will try this out myself to test if this works. I'm wondering if you were able to make this work?

so passing the roomId to the locator just works out of the box, apart from it missing types. As for the chat, once you've created a chat thread with a topic name then you can pass it like this and it works:

// ...rest of your args for adapter
locator: {
  callLocator: {
    // @ts-expect-error - this is a bug in the types
    roomId: room.id,
  },
  chatThreadId: room.threadId, // threadId we create via `${endpoint}/chat/threads?api-version=2024-03-15-preview` endpoint
},

realjesset avatar Apr 04 '24 17:04 realjesset

Hi @realjesset, thanks for being proactive and creating this PR.

Hello @mgamis-msft! No worries at all, I use this library heavily, its my pleasure!

We did not intend for Rooms call to have chat. Technically, it should be possible to add a chat thread to a Rooms call. I will try this out myself to test if this works. I'm wondering if you were able to make this work?

so passing the roomId to the locator just works out of the box, apart from it missing types. As for the chat, once you've created a chat thread with a topic name then you can pass it like this and it works:

// ...rest of your args for adapter
locator: {
  callLocator: {
    // @ts-expect-error - this is a bug in the types
    roomId: room.id,
  },
  chatThreadId: room.threadId, // threadId we create via `${endpoint}/chat/threads?api-version=2024-03-15-preview` endpoint
},

Cool! That is great to hear to have such a heavy user.

I guess the concern here is someone who is not invited to the room can be added to the chat thread. The Rooms API intends to add chat in the future and have a secure way of inviting someone to both a room and a chat thread. They will most likely provide the chat thread id for us which we can add to the locator. Your code change here is spot on but at this time we aren't prepared to officially add it into our API for security reasons. You are just a little bit ahead of us :P

mgamis-msft avatar Apr 04 '24 19:04 mgamis-msft