Skip to content

Fix activity without "from"#1405

Merged
compulim merged 2 commits intomicrosoft:masterfrom
compulim:fix-1397
Nov 28, 2018
Merged

Fix activity without "from"#1405
compulim merged 2 commits intomicrosoft:masterfrom
compulim:fix-1397

Conversation

@compulim
Copy link
Copy Markdown
Contributor

@compulim compulim commented Nov 28, 2018

Will fix #1397.

Changes

  • Patch activity.from.role
    • "user" if from.id is user ID, otherwise,
    • "bot" if from.id is anything otherwise,
    • "channel", because either from or from.id was not defined (e.g. in ConversationUpdate activity)
  • Should not set suggested actions if there are nothing on the transcript

@coveralls
Copy link
Copy Markdown

coveralls commented Nov 28, 2018

Pull Request Test Coverage Report for Build 506

  • 4 of 6 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 46.598%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/core/src/sagas/incomingActivitySaga.js 4 6 66.67%
Totals Coverage Status
Change from base Build 483: 0.08%
Covered Lines: 713
Relevant Lines: 1370

💛 - Coveralls

Copy link
Copy Markdown
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, pending nitpick CHANGELOG.md update

Comment thread CHANGELOG.md
@compulim compulim merged commit 7d59219 into microsoft:master Nov 28, 2018
@compulim compulim deleted the fix-1397 branch November 28, 2018 21:12
@hansmbakker
Copy link
Copy Markdown

hansmbakker commented Apr 10, 2019

@compulim @corinagum is this supposed to work in the embedded WebChat in the Azure Portal?
(at https://webchat.botframework.com/embed/<botId>?s=<secret>)

I was seeing the role being empty for activities sent from the bot, and I was wondering whether that is a bug or not?

I had the preview checkbox enabled in the webchat channel configuration.

@hansmbakker
Copy link
Copy Markdown

Also using the following HTML taken from the Full bundle sample the role property is not set in from for bot messages:

<!DOCTYPE html>
<html>
  <body>
    <div id="webchat" role="main"></div>
    <script src="https://cdn.botframework.com/botframework-webchat/latest/webchat.js"></script>
    <script>
      window.WebChat.renderWebChat({
        directLine: window.WebChat.createDirectLine({ token: 'somesecret' }),
        userID: 'myusername'
      }, document.getElementById('webchat'));
    </script>
  </body>
</html>

@hansmbakker
Copy link
Copy Markdown

This works correctly in the Bot Emulator though.

@hansmbakker
Copy link
Copy Markdown

Tracking in #1886.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Greeting Message Web Chat v4 - Sending events from bot

4 participants