-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: changed get alert by fingerprint #5258
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
Conversation
|
@DolevNe is attempting to deploy a commit to the KeepHQ Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@DolevNe can you add a quick unit test to make sure we don't fall here again? also - what element in the ui uses this api? lets make sure it works properly too |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5258 +/- ##
=======================================
Coverage 30.73% 30.73%
=======================================
Files 101 101
Lines 11494 11494
=======================================
Hits 3533 3533
Misses 7961 7961 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at the test - wouldn't it pass regardless of the fix? if I understand correctly - the bug occur when > 1000 alerts?
Closes #5255
📑 Description
the function that gets the alert by fingerprint uses a function that fetches the last 1000 alerts (get_last_alerts)
the route should use the function get_last_alerts_by_fingerprints
✅ Checks