-
Notifications
You must be signed in to change notification settings - Fork 425
RI-7776 Fix cloud discovery search and notifications #5296
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
base: main
Are you sure you want to change the base?
RI-7776 Fix cloud discovery search and notifications #5296
Conversation
…discovery - Calculate notification counts from original data instead of filtered items to prevent re-triggering toast when search filters change - Expand search fields in subscriptions page (type, provider, region, status) - Add statusAdded field to search in databases result page - Add comprehensive search tests for subscriptions page Fixes #RI-7776
Use empty string fallback to prevent false positives when statusAdded is undefined
Code Coverage - Integration Tests
|
Code Coverage - Backend unit tests
Test suite run success2990 tests passing in 287 suites. Report generated by 🧪jest coverage report action from 93f8edb |
Code Coverage - Frontend unit tests
Test suite run success5475 tests passing in 703 suites. Report generated by 🧪jest coverage report action from 93f8edb |
| (item: RedisCloudSubscription) => | ||
| item.name?.toLowerCase()?.indexOf(value) !== -1 || | ||
| item.id?.toString()?.toLowerCase().indexOf(value) !== -1, | ||
| item.id?.toString()?.indexOf(value) !== -1 || |
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.
I'm wondering can't we add such util:
export const filterBySearchTerm = <T extends Record<string, unknown>>(
items: T[],
searchTerm: string,
fields: (keyof T)[],
): T[] => {
const term = searchTerm.toLowerCase()
return items.filter((item) =>
fields.some((field) => {
const value = item[field]
return String(value ?? '').toLowerCase().includes(term)
}),
)
}
or
export const matchesSearch = (
fieldValue: unknown,
searchTerm: string, // expected to be already lowercased
): boolean => String(fieldValue ?? '').toLowerCase().includes(searchTerm)
and then this syntax would be easier to be read and moreover we have it in the RedisCloudDatabasesResult.tsx as well.
Maybe not for this PR, but actually, why not 😸
What
Fixes two issues in the cloud database discovery flow:
Root Cause
When search filtered items, the notification counts were recalculated from filtered data. This caused the
variantprop to change (e.g., from "attention" to "success" when filtered results had no failures), which triggered the MessageBar's useEffect to show a new toast.Solution
subscriptionsprop instead of filtereditemsstateTesting
Fixes #RI-7776
Note
Stabilizes notifications by deriving counts from original data and broadens search across subscriptions and database results, with tests added for search behavior.
countStatusActive/countStatusFailedfromsubscriptions(not filtereditems) inRedisCloudSubscriptions.tsxto avoid re-triggering toasts on search.RedisCloudSubscriptions.tsx: Search now matchesname,id,type,provider,region,status.RedisCloudDatabasesResult.tsx: Search now includesstatusAddedin addition to existing fields.RedisCloudSubscriptions.spec.tsx: Add comprehensive search tests (name, id, provider, region, status, type; case-insensitive) and test scaffolding withfaker.Written by Cursor Bugbot for commit 93f8edb. This will update automatically on new commits. Configure here.