-
Notifications
You must be signed in to change notification settings - Fork 87
Add mocks and boilerplate UI for Okta monitors #7017
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Greptile OverviewGreptile SummaryThis PR adds boilerplate UI components and mock data for Okta infrastructure monitors in the action center. Created reusable hooks ( Key changes:
Issues found:
Confidence Score: 3/5
Important Files ChangedFile Analysis
|
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.
8 files reviewed, 3 comments
...ta-discovery-and-detection/action-center/hooks/useDiscoveredInfrastructureSystemsColumns.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| interface UseDiscoveredInfrastructureSystemsColumnsConfig { | ||
| monitorId: string; | ||
| onTabChange: (tab: ActionCenterTabHash) => Promise<void>; |
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.
style: onTabChange parameter is unused and should be removed
| const handleAddSystem = useCallback( | ||
| async (systemId: string) => { | ||
| await addMonitorResultSystemsMutation({ | ||
| monitor_config_key: monitorId, | ||
| resolved_system_ids: [systemId], | ||
| }); | ||
| }, | ||
| [addMonitorResultSystemsMutation, monitorId], | ||
| ); | ||
|
|
||
| const handleIgnoreSystem = useCallback( | ||
| async (systemId: string) => { | ||
| await ignoreMonitorResultSystemsMutation({ | ||
| monitor_config_key: monitorId, | ||
| resolved_system_ids: [systemId], | ||
| }); | ||
| }, | ||
| [ignoreMonitorResultSystemsMutation, monitorId], | ||
| ); |
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.
logic: missing error handling for mutation failures - these functions should check isErrorResult() and display error toasts like the similar pattern in DiscoveredSystemActionsCell.tsx:54-74
| onCell: (record: SystemStagedResourcesAggregateRecord) => ({ | ||
| onClick: () => { | ||
| window.location.href = rowClickUrl(record); | ||
| }, | ||
| style: { cursor: "pointer" }, | ||
| }), |
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.
We've been moving away from this row click pattern in favor of the main column text being a link. Are you sure we should be using it in this boilerplate?
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.
nice catch, I'll remove that
Ticket ENG-2088
Description Of Changes
Setups mocks responses for an Okta monitor. Creates boilerplate component and hook to handle action center results for infrastructure monitors.
Code Changes
Steps to Confirm
npm run msw:mockPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works