BotBuilderCommunity / botbuilder-community-dotnet

Part of the Bot Builder Community Project. Repository for extensions for the Bot Builder .NET SDK, including middleware, dialogs, recognizers and more.
MIT License
279 stars 173 forks source link

Webex Adapter: send messages in markdown format, too #479

Closed patatofries closed 2 years ago

patatofries commented 2 years ago

Hi all,

I sent markdown-formatted messages via the Webex Adapter, but they don't appear formatted. Webex expects formatted messages to be sent via the additional "markdown"-parameter, per their API docs.

To me, it looks like this functionality is prepared in the Webex Adapter Code (see WebexMessageRequest.cs, property Markdown) -- but not used in the code, e.g. in WebexClientWrapper.cs, line 127f.

My question is: how can I as a user of the Webex Adapter sent markdown-formatted messages? If not possible yet, I would propose to sent the same message within the text and markdown parameter as an easy fix.

patatofries commented 2 years ago

My understanding of required code adaption (as Webex Client not-markdown-capable will just use the text property for displaying - for all others the markdown has priority over text

var request = new WebexMessageRequest
            {
                RoomId = target == MessageTarget.SpaceId ? recipient : null,
                ToPersonId = target == MessageTarget.SpaceId ? null : recipient,
                Text = text ?? string.Empty,
++           Markdown = text ?? string.Empty,
                Attachments = attachmentsContent.Count > 0 ? attachmentsContent : null,
            };
bastian-thiede commented 2 years ago

Hi,

I can confirm this issue exists. However, just adding the markdown and the text might not be sufficient. I would suggest, we handle the MessageTextType properly and supply the markdown only if the MessageTextType matches Markdown. In this case, we should provide an alternative Text without markup for Text. From the API documents:

The message, in plain text. If markdown is specified this parameter may be optionally used to provide alternate text for UI clients that do not support rich text.

If my understanding is correct, we can make use of the Bot.Builder.Community.Adapters.Shared.MarkdownToPlaintextRenderer for this. We will further need to take a look at SendActivitiesAsync since Activity.TextFormat gets ignored here anyway and MessageTextType is always set to Text.

I might be able to work on a PR during the day or tomorrow.

bastian-thiede commented 2 years ago

Hi @patatofries, I do have #480 in production and alongside #482 with a couple of test users. I think this matches the expectations from a developer perspective. Maybe you have a look as well?

patatofries commented 2 years ago

Hello Bastian, your Pull Request #480 matches my problem description perfectly. Thus I will close this issue