feat(alert): improves deployment alerts UI#1512
Conversation
WalkthroughThis set of changes refactors the deployment alert management UI to use unified form handling with react-hook-form and zod validation, introduces denomination-aware threshold conversions, and streamlines alert components to be controlled by external form context. It also adds comprehensive test coverage, a new deployment alert seeder, and a visual badge indicator for unsaved changes on the Alerts tab. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeploymentDetail
participant DeploymentAlerts
participant DeploymentAlertsContainer
participant API
User->>DeploymentDetail: Navigates to Alerts tab
DeploymentDetail->>DeploymentAlerts: Renders Alerts form
DeploymentAlerts->>DeploymentAlertsContainer: Requests current alert data
DeploymentAlertsContainer->>API: Fetch deployment alerts
API-->>DeploymentAlertsContainer: Returns alert data
DeploymentAlertsContainer-->>DeploymentAlerts: Provides alert data, max threshold
User->>DeploymentAlerts: Edits alert settings (enable, select channel, set threshold)
DeploymentAlerts->>DeploymentAlertsContainer: Calls upsert with new settings
DeploymentAlertsContainer->>API: Sends updated alert config
API-->>DeploymentAlertsContainer: Confirms update
DeploymentAlertsContainer-->>DeploymentAlerts: Notifies success, invalidates cache
DeploymentAlerts->>DeploymentDetail: Triggers badge update if changes are unsaved
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ygrishajev
force-pushed
the
feature/deployment-alerts-ui
branch
from
503a175 to
f28e2ae
Compare
TODO: - unit tests - prompt to confirm user action when leaving the page with unsaved changes
ygrishajev
force-pushed
the
feature/deployment-alerts-ui
branch
from
f28e2ae to
dd07644
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1512 +/- ##
==========================================
+ Coverage 40.33% 40.50% +0.16%
==========================================
Files 872 871 -1
Lines 21097 21115 +18
Branches 3838 3835 -3
==========================================
+ Hits 8510 8553 +43
- Misses 11857 12243 +386
+ Partials 730 319 -411
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx (1)
40-53: Verify disabled link accessibility.The test checks for
opacity-10andcursor-not-allowedclasses, but ensure the disabled link is also properly marked as disabled for screen readers. Consider addingaria-disabled="true"or similar accessibility attributes.apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.tsx (1)
108-114: Consider performance optimization for change detection.The deep comparison using
isEqualon every form value change could be performance-intensive for complex forms. Consider debouncing or optimizing this check if performance becomes an issue.+ import { useCallback, useMemo } from "react"; + import { debounce } from "lodash"; + const debouncedCompare = useMemo( + () => debounce((provided: any, current: any) => { + const hasChangesNext = !isEqual(provided, current); + if (hasChanges !== hasChangesNext) { + setHasChanges(hasChangesNext); + onStateChange({ hasChanges: hasChangesNext }); + } + }, 300), + [providedValues, onStateChange, hasChanges] + ); useEffect(() => { - const hasChangesNext = !isEqual(providedValues, values); - if (hasChanges !== hasChangesNext) { - setHasChanges(hasChangesNext); - onStateChange({ hasChanges: hasChangesNext }); - } + debouncedCompare(providedValues, values); }, [providedValues, onStateChange, values, hasChanges]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx(5 hunks)apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.tsx(3 hunks)apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx(1 hunks)apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx(2 hunks)apps/deploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.tsx(2 hunks)apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.spec.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentBalanceAlert/DeploymentBalanceAlert.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentBalanceContainer/DeploymentBalanceContainer.tsx(0 hunks)apps/deploy-web/src/components/deployments/DeploymentCloseAlert/DeploymentCloseAlert.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentDetail.tsx(7 hunks)apps/deploy-web/tests/seeders/deploymentAlert.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/deploy-web/src/components/deployments/DeploymentBalanceContainer/DeploymentBalanceContainer.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx (1)
Learnt from: baktun14
PR: akash-network/console#1432
File: apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentCloseAlert.tsx:38-38
Timestamp: 2025-06-05T21:07:51.985Z
Learning: The ContactPointSelect component in apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx uses the useFormContext hook internally to connect to React Hook Form, so it doesn't need to be wrapped in a FormField component.
🔇 Additional comments (37)
apps/deploy-web/tests/seeders/deploymentAlert.ts (1)
1-26: LGTM! Well-structured test data seeder.The seeder implementation is excellent:
- Proper TypeScript typing with imported schema types
- Realistic faker usage for generating test data
- Reasonable default values and threshold ranges
- Flexible override pattern for customization
- Supports both alert types with appropriate properties
apps/deploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.tsx (3)
11-11: Good addition of FCWithFnChildren import.The import properly supports the new function-as-children pattern being implemented.
40-40: Excellent architectural improvement with render prop pattern.The type change from
FCWithChildrentoFCWithFnChildren<object, ChildrenProps>properly implements the function-as-children pattern, providing better data flow and component flexibility.
45-45: Proper implementation of function-as-children pattern.The change from
{children}to{children(notificationChannelList)}correctly passes the notification channel data as a function argument, enabling consuming components to access this data directly.apps/deploy-web/src/components/deployments/DeploymentCloseAlert/DeploymentCloseAlert.tsx (4)
5-6: Good import changes for form context integration.The imports properly support the refactor to use
react-hook-formcontext and UI components for controlled form handling.
11-12: Excellent simplification to controlled component pattern.The component signature and context usage properly implement the refactor to consume external form context instead of managing internal form state.
17-32: Well-implemented checkbox integration in fieldset label.The "Enabled" checkbox is properly integrated using FormField with form context control. The positioning in the fieldset label provides good UX, and the disabled state flows through correctly.
37-39: Proper integration with controlled NotificationChannelSelect.The NotificationChannelSelect usage with explicit
nameanddisabledprops aligns well with the centralized form management approach and maintains proper form field binding.apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.spec.tsx (4)
1-11: Excellent test setup improvements.The updated imports and seeder usage provide more realistic and consistent test data, supporting the refactored component architecture.
13-47: Outstanding test refactor with realistic user interaction simulation.The test now properly simulates user interactions with
fireEventinstead of directly calling component methods. This provides much better test coverage and validates the actual user experience with the form.
54-78: Well-implemented mock components using form context.The mock alert components properly use
useFormContextand register form fields, accurately reflecting the refactored component architecture and enabling realistic form testing.
81-110: Improved test setup with seeded data and updated props.The component props setup uses seeded data builders for consistency and includes the new props structure (
maxBalanceThreshold,onStateChange) that align with the refactored component interface.apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx (5)
13-17: Good TypeScript interface for external props.The
ExternalPropstype properly defines the requirednameand optionaldisabledprops, supporting the refactor to make this a controlled component.
20-27: Excellent integration with form context and disabled state styling.The component properly uses form context and implements good UX with disabled state styling on the label (cursor-not-allowed, red color). The dynamic form field naming via the
nameprop is well-implemented.
32-56: Outstanding form field integration with validation and error handling.The FormField implementation is excellent:
- Proper controlled select with form context binding
- Visual error indication with red border on validation errors
- Error message display below the field
- Disabled state properly propagated to the select
- Good accessibility with proper id and data-testid attributes
59-73: Well-implemented disabled state handling for the action link.The link styling and click prevention when disabled provides good UX - users can clearly see the disabled state (opacity, cursor) and clicks are properly prevented.
79-83: Clean wrapper component implementation.The wrapper properly forwards external props to the view component while maintaining the data container pattern.
apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx (1)
63-86: Well-structured test setup with good flexibility.The new
setuphelper function provides excellent flexibility for testing different scenarios. The form context setup with conditional error handling is well-implemented.apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (3)
53-53: Good state management for badge functionality.The
badgedTabsstate structure allows for extensibility to other tabs in the future while maintaining clean organization.
191-198: Properly memoized callback with correct dependencies.The
recordAlertsChangecallback is correctly memoized withuseCallbackand includes the proper dependencies to prevent unnecessary re-renders.
265-267: Efficient conditional rendering approach.Using CSS classes to conditionally show/hide the DeploymentAlerts component is more efficient than conditional rendering, as it avoids component unmounting/remounting when switching tabs.
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.tsx (4)
43-51: Good dependency injection pattern for testability.The dependencies injection pattern makes the component more testable by allowing mock implementations of external dependencies like pricing context.
67-87: Robust error handling in conversion logic.The
convertfunction properly handles edge cases where conversion might fail, throwing descriptive errors. The null checks for AKT conversion are important for reliability.
130-147: Improved error handling with async upsert.Moving error handling inside the
upsertfunction and making it async is a good pattern. This centralizes error management and provides better user feedback through notifications.
89-104: To understand how deeply nestedalertscan be and whethermergemight overwrite or drop fields, let’s pull in the type and surrounding code:#!/bin/bash # 1. Print the top of the container file to find the ContainerInput definition/imports sed -n '1,200p' apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.tsx # 2. Search within that directory for where ContainerInput is defined rg -n "ContainerInput" -C 5 apps/deploy-web/src/components/alerts/DeploymentAlertsContainerapps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx (3)
238-254: Comprehensive pricing context mocking.The mock pricing implementation with realistic conversion rates (1.5 for AKT, 1 for USDC) properly simulates the pricing context functionality needed for testing denomination conversions.
73-186: Excellent test coverage for different alert configurations.The test cases cover all possible alert combinations (deployment closed only, balance only, and both) which ensures the container handles all expected scenarios correctly.
194-204: Good verification of side effects.Testing query invalidation on successful mutation ensures the component properly refreshes cached data, which is crucial for maintaining data consistency.
apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.tsx (4)
34-44: Well-structured zod schema for validation.The schema structure clearly defines validation rules for both alert types with appropriate error messages. This provides good user feedback.
77-83: Dynamic schema validation with runtime constraints.The dynamic schema extension with
maxBalanceThresholdvalidation is a good approach for ensuring thresholds don't exceed available balance. This prevents user errors.
85-97: Smart default values calculation.The logic for calculating default values (30% of max balance threshold, first notification channel) provides sensible defaults that improve user experience.
116-121: Robust form submission with error handling.The async submission with conditional form reset only on success is a good pattern that maintains form state if the operation fails.
apps/deploy-web/src/components/deployments/DeploymentBalanceAlert/DeploymentBalanceAlert.tsx (5)
5-5: LGTM: Clean integration with react-hook-formThe addition of
useFormContextaligns well with the refactoring goal of centralizing form management.
13-14: LGTM: Good addition of disabled propThe optional
disabledprop provides proper control over the component's interactive state, which is essential for form management.
24-36: LGTM: Well-structured checkbox integrationThe FormField integration with CheckboxWithLabel follows react-hook-form best practices. The controlled component pattern is properly implemented with proper type casting for the boolean value.
52-66: LGTM: Good accessibility and UX designThe form input implementation includes:
- Proper labeling with tooltip for user guidance
- Appropriate step value (0.000001) for USD precision
- Clear indication that the threshold is in USD
- Proper disabled state handling
The CustomTooltip with InfoCircle provides helpful context for users.
16-17: ```shell
#!/bin/bashVerify that DeploymentAlerts wraps its children in a FormProvider
grep -n "FormProvider" -n apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.tsx
grep -n "useForm" -n apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.tsx</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
apps/deploy-web/src/components/deployments/DeploymentBalanceAlert/DeploymentBalanceAlert.tsx
Show resolved
Hide resolved
...eploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx
Outdated
Show resolved
Hide resolved
...y-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx(3 hunks)apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx(1 hunks)apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx (1)
Learnt from: baktun14
PR: akash-network/console#1432
File: apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentCloseAlert.tsx:38-38
Timestamp: 2025-06-05T21:07:51.985Z
Learning: The ContactPointSelect component in apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx uses the useFormContext hook internally to connect to React Hook Form, so it doesn't need to be wrapped in a FormField component.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate-deploy-web
- GitHub Check: test-deploy-web-build
🔇 Additional comments (6)
apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx (2)
14-16: LGTM: Following testing best practices.Good use of
getByLabelTextas recommended in the past review comment, avoiding unnecessary test-ids for form field queries.
62-85: Well-structured test setup helper.The setup function provides good flexibility for testing different scenarios with configurable data, disabled state, and field errors. The form context setup with proper error handling is excellent.
apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx (4)
13-18: Clean props interface design.The separation of external props and internal props with proper TypeScript typing provides good component API design.
25-27: Good conditional styling for disabled state.The label properly reflects the disabled state with appropriate visual feedback using conditional classes.
55-69: Excellent disabled link behavior.The link properly handles disabled state with visual feedback and prevents navigation when disabled. The preventDefault approach ensures proper UX.
29-53: ```shell
#!/bin/bashVerify whether NotificationChannelSelect uses useFormContext internally
grep -R "useFormContext" -n apps/deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx || echo "NotificationChannelSelect: no useFormContext"
Confirm ContactPointSelect uses useFormContext and is not wrapped in FormField
grep -R "useFormContext" -n apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx || echo "ContactPointSelect: no useFormContext"
grep -R "FormField" -n apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx || echo "ContactPointSelect: no FormField wrapper"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...y-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.spec.tsx
Outdated
Show resolved
Hide resolved
...deploy-web/src/components/alerts/NotificationChannelSelectForm/NotificationChannelSelect.tsx
Show resolved
Hide resolved
ygrishajev
force-pushed
the
feature/deployment-alerts-ui
branch
from
3095cd9 to
ff59646
Compare
2 participants
Screen.Recording.2025-06-19.at.17.29.08.mov
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores