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

Support creating alert in the chat window #217

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

qianheng-aws
Copy link
Contributor

@qianheng-aws qianheng-aws commented Jul 17, 2024

Description

Support creating alert in the chat window as a follow-up question of alert analysis

Expected results of creating alert in chatbot:
image
image

Issues Resolved

#216

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test.
  • New functionality has user manual doc added.
  • Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
server/parsers/ParserHelper.ts Outdated Show resolved Hide resolved
server/parsers/ParserHelper.ts Outdated Show resolved Hide resolved
server/parsers/ParserHelper.ts Outdated Show resolved Hide resolved
@@ -86,52 +87,55 @@ export const ChatFlyout = (props: ChatFlyoutProps) => {
>
<>
<div className={cs('llm-chat-flyout-header')}>
<ChatWindowHeader />
{props.overrideComponent ? <ChatOverrideHeader /> : <ChatWindowHeader />}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this ChatOverrideHeader suitable for all override component header? Shall we just hide the ChatWindowHeader and let overrideComponent take over the whole screen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think this is a standard header for all override component. As UX required, we should change the title, add a back button and hide the history button. There is a mock up for AD, which also applies to alert.

image

server/parsers/create_monitor_parser.ts Outdated Show resolved Hide resolved
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.40%. Comparing base (3991de2) to head (327b68d).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #217      +/-   ##
==========================================
- Coverage   90.02%   89.40%   -0.62%     
==========================================
  Files          54       60       +6     
  Lines        1433     1623     +190     
  Branches      347      394      +47     
==========================================
+ Hits         1290     1451     +161     
- Misses        141      169      +28     
- Partials        2        3       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Heng Qian <qianheng@amazon.com>
@qianheng-aws qianheng-aws requested a review from wanglam July 23, 2024 08:14
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.

2 participants