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

feat(FE): dashboard alerts is added #3908

Merged
merged 21 commits into from
Nov 13, 2023
Merged

feat(FE): dashboard alerts is added #3908

merged 21 commits into from
Nov 13, 2023

Conversation

palashgdev
Copy link
Contributor

@palashgdev palashgdev commented Nov 6, 2023

Close #3733

@palashgdev palashgdev self-assigned this Nov 6, 2023
@github-actions github-actions bot added the enhancement New feature or request label Nov 6, 2023
@palashgdev
Copy link
Contributor Author

need to remove the apm to alerts changes as backend is not ready

@palashgdev palashgdev marked this pull request as ready for review November 8, 2023 19:43
@palashgdev palashgdev requested a review from YounixM as a code owner November 8, 2023 19:43
Rajat-Dabade
Rajat-Dabade previously approved these changes Nov 9, 2023
Copy link
Contributor

@Rajat-Dabade Rajat-Dabade left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@srikanthccv
Copy link
Member

srikanthccv commented Nov 9, 2023

  • When the user is creating a new alert i.e not editing the existing alert. Do not allow alert creation with an empty selected query
  • Invalid api/v1/dashboards/ GET request polling is happening from the alerts listing page

@srikanthccv
Copy link
Member

  • Show only enabled queries in the dropdown
  • If there is only one enabled query from the dashboard to alert, select it by default without needing user selection

@srikanthccv
Copy link
Member

srikanthccv commented Nov 9, 2023

  • Dashboard can create multiple ClickHouse/PromQL queries but the alerts shows only one. We should show all the queries.

There are two queries here but only a is shown.
Screenshot 2023-11-09 at 3 39 06 PM

@srikanthccv
Copy link
Member

  • Do not show create alerts for table panel type.

@srikanthccv
Copy link
Member

@palashgdev these are the main issues that I could find in testing

@palashgdev
Copy link
Contributor Author

Thanks @srikanthccv will take a look

@srikanthccv
Copy link
Member

Let's wrap up this today

@palashgdev
Copy link
Contributor Author

@srikanthccv

  1. fixed promql issue which we discussed offline
  2. enabled the default selected which is enabled query as default option
  3. for table panel widget create alert is not visible
Screenshot 2023-11-10 at 9 37 42 PM

srikanthccv
srikanthccv previously approved these changes Nov 10, 2023
@YounixM
Copy link
Member

YounixM commented Nov 13, 2023

@palashgdev : the down arrows next query names should be at the end of the row.

@palashgdev
Copy link
Contributor Author

@palashgdev : the down arrows next query names should be at the end of the row.

saw the comments seems like you are suggesting the design change
would recommend to change the figma then will change the design

@palashgdev palashgdev merged commit 5a9f626 into develop Nov 13, 2023
8 checks passed
@palashgdev palashgdev deleted the feat/dashboard-alerts branch November 13, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to create alert from dashboard panel
4 participants