Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#2042] Changed convo infrastructure to use Web Sockets. #2034

Merged
merged 3 commits into from
Jan 23, 2020

Conversation

tonyanziano
Copy link
Contributor

@tonyanziano tonyanziano commented Jan 8, 2020

related to #1405 , #2042

===

Highlights

  • Activities from the bot are now sent to Web Chat over a Web Socket connection (no more endless polling / Network tab spam!!! 🎉🎉🎉)
  • Pulled heavy-duty conversation initialization logic out of emulator.tsx component
    • this logic has been moved to botSagas.ts and chatSagas.ts so now they can be called more declaratively instead of relying on changes to props satisfying the right criteria
  • Consolidated conversation creation logic so that opening a bot via .bot file, URL, or opening a transcript all flow through (more-or-less) the same code path
  • Consolidated code path for opening a transcript via a .chat file versus a .transcript file
    • they essentially call the same code, except when handling a .chat file, we first use chatdown to convert the file to a transcript before extracting the activities
  • Removed global user state
    • users are now scoped to the conversation level
    • no more having to call the SetCurrentUser command and manually maintaining the state
    • removed the users property from persisted server.json file (no more endless list of randomly generated user IDs from previous sessions)

Passing E2E tests:

image


Diagram of High-level Changes

image

@tonyanziano tonyanziano changed the title Changed convo infrastructure to use Web Sockets. [#2042] Changed convo infrastructure to use Web Sockets. Jan 10, 2020
@tonyanziano tonyanziano marked this pull request as ready for review January 14, 2020 22:37
@tonyanziano tonyanziano force-pushed the toanzian/ws-final branch 2 times, most recently from 960a6d0 to c026480 Compare January 14, 2020 23:26
@coveralls
Copy link

coveralls commented Jan 14, 2020

Coverage Status

Coverage decreased (-0.4%) to 68.472% when pulling b29388f on toanzian/ws-final into cea9a86 on master.

@tonyanziano
Copy link
Contributor Author

I think the coverage number has gone down as a result of removing a lot of tested code, but I will try to write some more to lessen the drop.

@tonyanziano
Copy link
Contributor Author

Opened another PR with a bunch of new tests to compensate for drop in code coverage:

#2048

srinaath
srinaath previously approved these changes Jan 22, 2020
Copy link
Contributor

@srinaath srinaath left a comment

Choose a reason for hiding this comment

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

Just a few comments or nice to have's.. Looks awesome 👍

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.

3 participants