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.

Sign up for GitHub

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

Jump to bottom

[JS sample] Add CoreBot with Application-Insights #1773

Merged
merged 36 commits into from
Jan 23, 2020
Merged

[JS sample] Add CoreBot with Application-Insights #1773

merged 36 commits into from
Jan 23, 2020

Conversation

gasper-az
Copy link
Contributor

Proposed Changes

  • Add sample 21.corebot-app-insights, based on JS Sample#13

More Information

This sample:

  • Uses LUIS to implement core AI capabilities
  • Implements a multi-turn conversation using Dialogs
  • Handles user interruptions for such things as Help or Cancel
  • Prompts for and validates requests for information from the user
  • Uses Application Insights to monitor your bot

Testing

To test this sample:

  1. Clone this branch
  2. In a terminal, navigate to samples/javascript_nodejs/21.corebot-app-insights
  3. Install modules with npm install
  4. Setup LUIS
  5. Run the sample with npm start

gasper-az and others added 27 commits September 11, 2019 11:41
@gasper-az
[AppInsights Sample] Init project [WIP]
f5643db
@Aliandi
add ErrorViewModel class
b6196bc
@Aliandi
add cancel dialog
271e30e
@Aliandi
add dateResolver dialog
24b9952
@Aliandi
Change ! to cast as boolean
3ca77fd
@gasper-az
[AppInsights sample] Add dialogBot
93db436
@gasper-az
[AppInsights sample] Export DialogBot
878adf6
@gasper-az
[AppInsights Sample] create DialogState
ceae8a3
@gasper-az
[AppInsight sample] dialogAndWelcomeBot + resource
0c1e07a
@gasper-az
[AppInsight Bot] Fix styling.
935ac9e
@Aliandi
add header
1fec03a
@gasper-az
[AppInsights-Bot] Add ErrorViewModel class
fbb6d0f
@gasper-az
[AppInsights Bot] Add Bot folder
f8186d2 
Add DialogBot
Add DialogAndWelcomeBot
Add Resources for DialogAndWelcomeBot
@Aliandi
add bookingDialog
29c9817
@gasper-az
[AppInsights Bot] Add new Dialogs
7e1f808 
Add BookingDialog
Add cancelAndHelpDialog
Add dateResolverDialog
@gasper-az
[AppInsights Bot] Add mainDialog [WIP]
7d9b209
@gasper-az
[AppInsights Bot] Add mainDialog
0e74c2d
@gasper-az
[AppInsights Bot] Add MainDialog
531c5f9
@gasper-az
[AppInsights Bot] Add index.js and flightBookingRecognizer
c323c54
@gasper-az
[AppInsight Bot] fix minor issues in dialogs
4ca9ba8
@gasper-az
[AppInsight Bot] Fix minor issues in dialogBot
b4d72c4
@gasper-az
[AppInsights] add index.js
d457060
@gasper-az
[AppInsights Bot] Add cognitiveModels
d54100c
@gasper-az
Add deployment templates
e41954c
@gasper-az
[ApiInsights Bot] Add index.js and folders
bbce2c9 
Add Index.js
Add cognitivesModels folder
Add deploymentTemplates folder
Fix minor issues in dialog & dialogBot
@gasper-az
[AppInsights Bot] Add README.md file
84e5a60
@gasper-az
Sample#21 - fix typo in mainDialog.js
793941d
@gasper-az
Update README.md file
409d5ef 
* Replace appsettings.json with .env file.

* Add information about appInsights instrumentation key.
@gasper-az
bookingDialog as parameter
3e6ce6a 
Add bookingDialog as a parameter of mainDialog instead of creating it.
daveta
daveta suggested changes Sep 16, 2019
View reviewed changes
Copy link
Contributor

@daveta daveta left a comment

Choose a reason for hiding this comment

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

Nice - thanks Gaspar!

@gasper-az
Apply feedback on Sample 21
b0d8ffe 
- index.js: uncommented line for using restify.plugins.bodyParser.

- welcomeCard.json: set isSubtle to true.

- README.md: replace references to sample 13 with references to Sample 21.

- README.md:  add link to AppInsights documentation.
daveta
daveta approved these changes Sep 17, 2019
View reviewed changes
Copy link
Contributor

@daveta daveta left a comment

Choose a reason for hiding this comment

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

Nice - thanks!!

@gasper-az
Sample21: Use TelemetryClient through ComponentDialog
055c3d7 
- As the mainDialog extends ComponentDialog, it s not necessary to pass the TelemetryClient as a parameter in its constructor. Instead, we can use the telemetryClient property from the ComponentDialog.
WashingtonKayaker
WashingtonKayaker reviewed Sep 30, 2019
View reviewed changes
Copy link
Contributor

@WashingtonKayaker WashingtonKayaker left a comment

Choose a reason for hiding this comment

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

Please make these changes to revert this code back to what it is in the coreBot sample app. The purposes of this sample app is very narrowly focused to show only what changes are required to update the coreBot sample app to add telemetry capabilities. Any changes beyond that will detract from that purpose.

I also noticed the string interpolation changes you made as well, and I think those are OK (in my opinion they are better, and virtually unnoticeable and I can get away with not documenting them)

samples/javascript_nodejs/21.corebot-app-insights/dialogs/bookingDialog.js Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/index.js Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/index.js Show resolved Hide resolved
WashingtonKayaker
WashingtonKayaker reviewed Sep 30, 2019
View reviewed changes
Copy link
Contributor

@WashingtonKayaker WashingtonKayaker left a comment

Choose a reason for hiding this comment

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

Please revert all non-critical changes not required to implement telemetry.

samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/mainDialog.js Outdated Show resolved Hide resolved
WashingtonKayaker
WashingtonKayaker reviewed Oct 1, 2019
View reviewed changes
Copy link
Contributor

@WashingtonKayaker WashingtonKayaker left a comment

Choose a reason for hiding this comment

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

Please remove brackets, unless there is an important reason to change, please revert these changes.

samples/javascript_nodejs/21.corebot-app-insights/dialogs/cancelAndHelpDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/cancelAndHelpDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/cancelAndHelpDialog.js Outdated Show resolved Hide resolved
samples/javascript_nodejs/21.corebot-app-insights/dialogs/cancelAndHelpDialog.js Show resolved Hide resolved
@gasper-az
Sample 21: Restored previous Dialog constructors.
2e215cd 
- Restore MainDialog previous constructor.

- Restore Booking Dialog previous contructor.

- Removed unnecessary brackets.
@garypretty
Copy link
Contributor

I will re-open this PR shortly once I have made updates to the sample to use latest telemetry pieces

@garypretty garypretty reopened this Jan 23, 2020
@garypretty garypretty merged commit 5b86f09 into master Jan 23, 2020
@garypretty garypretty mentioned this pull request Jan 23, 2020
Closed
@snyk-bot snyk-bot mentioned this pull request May 11, 2021
Open
@snyk-bot snyk-bot mentioned this pull request Sep 3, 2021
Open
This was referenced Jan 12, 2022
Open
Open
Open
Open
@tracyboehrer tracyboehrer deleted the feature/corebot-app-insights branch July 8, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scheyal scheyal scheyal left review comments

@WashingtonKayaker WashingtonKayaker WashingtonKayaker approved these changes

@munozemilio munozemilio munozemilio approved these changes

@daveta daveta daveta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@gasper-az @garypretty @daveta @scheyal @munozemilio @WashingtonKayaker @johnataylor @Aliandi