feat(ui): enhance merchant monitoring report status component #3061
Conversation
- Implement MerchantMonitoringReportStatus to display report statuses - Update Button variants and import adjustments for optimization - Refactor relevant table and business report components to utilize new status component
|
WalkthroughThe pull request introduces several focused changes. A new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as MerchantMonitoringReportStatus
participant D as DropdownMenu
participant C as Console
U->>M: Render component with status prop
M->>D: Render DropdownMenu with BadgeElement
U->>M: Click badge trigger
M->>D: Show status options
U->>D: Select a new status
D->>C: Log selected status
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/backoffice-v2/src/domains/business-reports/hooks/mutations/useCreateBusinessReportBatchMutation/useCreateBusinessReportBatchMutation.tsx (1)
3-3: LGTM! Consider enhancing error handling further.The changes look good:
- Import of
MerchantReportTypehas been updated to use the centralized definition.- Error handling has been improved using the
isObjecttype guard.Consider enhancing the error handling further by using optional chaining for a more concise implementation:
- errorMessage: isObject(error) && 'message' in error ? error.message : error, + errorMessage: isObject(error) ? error?.message ?? error : error,Also applies to: 48-48
apps/backoffice-v2/src/common/components/atoms/Button/Button.tsx (1)
22-22: Consider using theme variables for colors.While the implementation works, using hardcoded color values (
#F4F6FD) makes it harder to maintain theme consistency. Consider using theme variables instead.Apply this diff to use theme variables:
- status: 'focus-visible:ring-0 focus-visible:ring-offset-0 focus:!bg-[#F4F6FD] bg-[#F4F6FD]', + status: 'focus-visible:ring-0 focus-visible:ring-offset-0 focus:!bg-secondary/10 bg-secondary/10',apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringReportStatus.tsx (2)
59-64: Refactor hardcoded color values to use theme variables.The component uses multiple hardcoded color values which makes it harder to maintain theme consistency and support dark mode.
Consider moving these color values to a theme configuration and using CSS variables or Tailwind theme extensions.
- 'cursor-not-allowed bg-[#E3E2E0] text-[#32302C]/60 ': reportIsInProgress, - 'bg-[#D3E5EF] text-[#183347]': status === MERCHANT_REPORT_STATUSES_MAP['under-review'], - 'bg-[#DBEDDB] text-[#1C3829]': status === MERCHANT_REPORT_STATUSES_MAP['completed'], + 'cursor-not-allowed bg-gray-200 text-gray-700/60': reportIsInProgress, + 'bg-blue-100 text-blue-900': status === MERCHANT_REPORT_STATUSES_MAP['under-review'], + 'bg-green-100 text-green-900': status === MERCHANT_REPORT_STATUSES_MAP['completed'],Also applies to: 67-71
83-87: Add proper TypeScript props interface.The component's props are defined inline. Consider creating a proper interface for better maintainability and documentation.
Apply this diff to add a proper interface:
+interface MerchantMonitoringReportStatusProps { + status?: keyof typeof statusToData; + onStatusChange?: (status: keyof typeof statusToData) => void; +} + -export const MerchantMonitoringReportStatus = ({ - status, -}: { - status?: keyof typeof statusToData; -}) => { +export const MerchantMonitoringReportStatus = ({ + status, + onStatusChange, +}: MerchantMonitoringReportStatusProps) => {apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx (1)
120-127: LGTM! Good defensive programming practice.The addition of the nullish coalescing operator improves code robustness by ensuring
registryInfois always an object, preventing potential runtime errors.Consider adding type safety by explicitly typing the return value:
const registryInfo = useMemo( - () => + (): Record<string, unknown> => omitPropsFromObjectWhitelist({ object: workflow?.context?.pluginsOutput, whitelist: registryInfoWhitelist, }) ?? {}, [workflow?.context?.pluginsOutput], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/backoffice-v2/src/common/components/atoms/Button/Button.tsx(1 hunks)apps/backoffice-v2/src/domains/business-reports/hooks/mutations/useCreateBusinessReportBatchMutation/useCreateBusinessReportBatchMutation.tsx(1 hunks)apps/backoffice-v2/src/domains/business-reports/hooks/queries/useBusinessReportsQuery/useBusinessReportsQuery.tsx(1 hunks)apps/backoffice-v2/src/domains/business-reports/hooks/queries/useLatestBusinessReportQuery/useLatestBusinessReportQuery.tsx(1 hunks)apps/backoffice-v2/src/domains/business-reports/query-keys.ts(1 hunks)apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringReportStatus.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx(3 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/hooks/useMerchantMonitoringTableLogic/useMerchantMonitoringTableLogic.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx(4 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx(1 hunks)packages/common/src/consts/index.ts(1 hunks)packages/ui/src/components/atoms/Badge/Badge.tsx(1 hunks)services/workflows-service/prisma/data-migrations(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- services/workflows-service/prisma/data-migrations
- apps/backoffice-v2/src/domains/business-reports/hooks/queries/useLatestBusinessReportQuery/useLatestBusinessReportQuery.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: spell_check
- GitHub Check: lint
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (9)
apps/backoffice-v2/src/domains/business-reports/hooks/queries/useBusinessReportsQuery/useBusinessReportsQuery.tsx (1)
2-2:✅ Verification successful
Verify the import path consistency.
The import path for
MerchantReportTypehas been updated to@ballerine/common. This change aligns with similar updates in other files, indicating a consistent refactoring effort to centralize the type definition.Run the following script to verify the import path consistency across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify that all imports of MerchantReportType are from @ballerine/common. # Test: Search for any remaining imports from the old path. Expect: No results. rg "MerchantReportType.*@/domains/business-reports/constants" # Test: Search for imports from the new path. Expect: All imports should be from @ballerine/common. rg "MerchantReportType.*@ballerine/common"Length of output: 1596
Import Path Consistency Confirmed
All references toMerchantReportTypeacross the codebase now consistently use the updated path@ballerine/common. The verification script confirmed that there are no lingering imports from the old path.
- Verified files include:
apps/backoffice-v2/src/domains/business-reports/query-keys.tsapps/backoffice-v2/src/domains/business-reports/hooks/mutations/useCreateBusinessReportBatchMutation/useCreateBusinessReportBatchMutation.tsxapps/backoffice-v2/src/domains/business-reports/hooks/queries/useLatestBusinessReportQuery/useLatestBusinessReportQuery.tsxapps/backoffice-v2/src/domains/business-reports/hooks/queries/useBusinessReportsQuery/useBusinessReportsQuery.tsx- Additional service files within
services/workflows-service/No further changes are required.
apps/backoffice-v2/src/domains/business-reports/query-keys.ts (1)
1-1: LGTM!The import path for
MerchantReportTypehas been updated to use the centralized definition from@ballerine/common. This change is consistent with the updates in other files.apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/hooks/useMerchantMonitoringTableLogic/useMerchantMonitoringTableLogic.tsx (1)
23-29: LGTM! Enhanced status handling for clickable links.The expanded condition properly handles multiple report statuses, making the table more interactive by allowing links for 'pending-review', 'under-review', and 'completed' statuses.
apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx (2)
88-88: LGTM! Improved tooltip alignment.The tooltip trigger alignment change from
justify-centertojustify-startprovides better visual consistency with the content.
202-202: LGTM! Enhanced status display.The status column now uses the new
MerchantMonitoringReportStatuscomponent, which encapsulates the status display logic, improving modularity and maintainability.apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (1)
101-107: LGTM! Improved error handling.The error handling has been enhanced with:
- Toast notifications for user feedback
- Proper error message extraction using
isObjectutility- Consistent error handling pattern across monitoring state changes
Also applies to: 134-140
packages/common/src/consts/index.ts (1)
178-180: LGTM! Enhanced status granularity.The status array has been updated to split 'in-review' into more specific states:
- 'pending-review': Initial review state
- 'under-review': Active review state
- 'completed': Final state
This provides better tracking of the review process stages.
apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (2)
110-114: LGTM! Improved status validation.The status check has been enhanced to use an explicit array of valid statuses, making the logic more maintainable and easier to understand.
309-309: LGTM! Enhanced status display.The status display now uses the new
MerchantMonitoringReportStatuscomponent, providing a consistent status representation across the application.
| @@ -11,6 +11,7 @@ const badgeVariants = cva( | |||
| primary: 'bg-primary text-primary-foreground', | |||
| secondary: 'bg-secondary text-secondary-foreground', | |||
| info: 'bg-info/20 text-info', | |||
| gray: 'bg-[]/20 text-[]', | |||
There was a problem hiding this comment.
Fix incomplete color values in gray variant.
The gray variant has empty color values in the class names (bg-[]/20 text-[]). This will cause styling issues as the color values are missing.
Apply this diff to fix the color values:
- gray: 'bg-[]/20 text-[]',
+ gray: 'bg-gray-500/20 text-gray-700',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| gray: 'bg-[]/20 text-[]', | |
| gray: 'bg-gray-500/20 text-gray-700', |
| onClick={() => { | ||
| console.log(status); | ||
| }} |
There was a problem hiding this comment.
Remove console.log from production code.
The onClick handler contains a console.log statement which should not be in production code.
Apply this diff to handle the status change properly:
- onClick={() => {
- console.log(status);
- }}
+ onClick={() => {
+ // TODO: Implement status change handler
+ onStatusChange?.(status);
+ }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onClick={() => { | |
| console.log(status); | |
| }} | |
| onClick={() => { | |
| // TODO: Implement status change handler | |
| onStatusChange?.(status); | |
| }} |
| }; | ||
|
|
||
| const statusToData = { | ||
| [MERCHANT_REPORT_STATUSES_MAP['in-progress']]: reportInProgressData, |
| ![ | ||
| MERCHANT_REPORT_STATUSES_MAP['pending-review'], | ||
| MERCHANT_REPORT_STATUSES_MAP['under-review'], | ||
| MERCHANT_REPORT_STATUSES_MAP.completed, | ||
| ].includes(businessReport?.status) |
There was a problem hiding this comment.
I think this array of statuses should be in the @ballerine/common
- Add status update functionality to business reports - Implement user feedback for status updates in toast notifications - Create new UI components for report status and associated actions
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
services/workflows-service/src/business-report/dtos/business-report-status-update.dto.ts (2)
10-12: Use@IsEnum()for status validation.The
statusproperty is of typeUpdateableReportStatuswhich appears to be an enum, but it's validated using@IsString(). Consider using@IsEnum()for more precise validation.- @IsString() + @IsEnum(UpdateableReportStatus) @ApiProperty({ type: String, required: true }) status!: UpdateableReportStatus;
5-13: Consider alternatives to non-null assertions.The
!non-null assertion operator is used on both properties. Consider using optional properties or constructor initialization for better type safety.export class BusinessReportStatusUpdateRequestParamsDto { @IsString() @ApiProperty({ type: String, required: true }) - reportId!: string; + reportId: string; - @IsEnum(UpdateableReportStatus) - @ApiProperty({ type: String, required: true }) - status!: UpdateableReportStatus; + @IsEnum(UpdateableReportStatus) + @ApiProperty({ type: String, required: true }) + status: UpdateableReportStatus; + constructor(reportId: string, status: UpdateableReportStatus) { + this.reportId = reportId; + this.status = status; + } }apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/fetchers.ts (2)
16-16: Use absolute paths instead of relative paths.The endpoint path uses
../which could be fragile. Consider using an absolute path or a constant.- endpoint: `../external/business-reports/${reportId}/status/${status}`, + endpoint: `/api/external/business-reports/${reportId}/status/${status}`,
18-23: Consider enhancing the response schema validation and reducing timeout.The schema validation could be more specific by using
z.nativeEnum()for the status field, and the timeout of 5 minutes seems excessive.schema: z.object({ reportId: z.string(), - status: z.string(), + status: z.nativeEnum(UpdateableReportStatus), }), - timeout: 300_000, + timeout: 30_000, // 30 seconds should be sufficientapps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringReportStatusButton.tsx (2)
16-24: Simplify the click handler logic.The click handler can be simplified by using early return or combining conditions.
- onClick={e => { - if (disabled) { - e.stopPropagation(); - - return; - } - - onClick?.(e); - }} + onClick={e => { + if (!disabled) onClick?.(e); + else e.stopPropagation(); + }}
26-33: Extract styles to constants.Consider extracting the hardcoded styles and text colors to constants for better maintainability.
+const BUTTON_STYLES = { + base: 'flex h-16 w-80 flex-col items-start justify-center space-y-1 px-4 py-2', + disabled: '!cursor-not-allowed', +}; + +const TEXT_STYLES = 'text-xs font-semibold leading-5 text-[#94A3B8]'; + - className={ctw(`flex h-16 w-80 flex-col items-start justify-center space-y-1 px-4 py-2`, { - '!cursor-not-allowed': disabled, + className={ctw(BUTTON_STYLES.base, { + [BUTTON_STYLES.disabled]: disabled, })} > <MerchantMonitoringStatusBadge status={status} disabled={disabled} /> - <span className={`text-xs font-semibold leading-5 text-[#94A3B8]`}> + <span className={TEXT_STYLES}>apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/hooks/useUpdateReportStatusMutation/useUpdateReportStatusMutation.tsx (2)
18-32: Consider adding error message for missing parameters.The early return when
reportIdorstatusis missing could benefit from throwing an error to help with debugging.if (!reportId || !status) { + throw new Error('Missing required parameters: reportId and status must be provided'); return; }
33-38: Verify query invalidation scope.The current implementation invalidates all queries. Consider scoping the invalidation to only affected queries for better performance.
-void queryClient.invalidateQueries(); +void queryClient.invalidateQueries(['merchantReports', reportId]);apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringStatusBadge.tsx (2)
8-12: Consider moving constants to a separate file.The
reportInProgressDataconstant could be moved to a dedicated constants file for better organization.
34-73: Consider extracting color constants.The hardcoded color values could be moved to a theme configuration for better maintainability.
+const STATUS_COLORS = { + pending: { + bg: '#E3E2E0', + text: '#32302C', + }, + // ... other status colors +} as const; className={ctw(`h-6 cursor-pointer space-x-1 text-sm font-medium`, { 'cursor-not-allowed': disabled, 'hover:shadow-[0_0_2px_rgba(0,0,0,0.3)]': !disabled, - 'bg-[#E3E2E0] text-[#32302C]': status === MERCHANT_REPORT_STATUSES_MAP['pending-review'], + [`bg-[${STATUS_COLORS.pending.bg}] text-[${STATUS_COLORS.pending.text}]`]: status === MERCHANT_REPORT_STATUSES_MAP['pending-review'], // ... other styles })}apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringReportStatus.tsx (3)
41-43: Add validation constraints to the form schema.The form schema allows optional text input without any constraints. Consider adding validation rules for:
- Maximum length
- Minimum length (if required)
- Character type restrictions
const MerchantMonitoringCompletedStatusFormSchema = z.object({ - text: z.string().optional(), + text: z.string() + .min(1, 'Additional details are required') + .max(500, 'Additional details cannot exceed 500 characters') + .optional(), });
114-120: Improve modal escape key handling.The escape key handler for the dropdown menu content has complex logic. Consider extracting it into a separate function for better readability and maintainability.
+ const handleEscapeKey = (e: KeyboardEvent) => { + if (isCompleteReviewModalOpen) { + e.preventDefault(); + } + setIsCompleteReviewModalOpen(false); + }; <DropdownMenuContent align="start" className={`space-y-2 p-4`} - onEscapeKeyDown={e => { - if (isCompleteReviewModalOpen) { - e.preventDefault(); - } - setIsCompleteReviewModalOpen(false); - }} + onEscapeKeyDown={handleEscapeKey} >
191-196: Add loading state feedback during status update.The status update click handler should provide visual feedback during the loading state to prevent multiple clicks.
onClick={e => { e.preventDefault(); e.stopPropagation(); + if (isLoading) return; + mutateUpdateReportStatus({ reportId, status: selectableStatus }); }}apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (1)
111-115: Move status array to a constant.The array of statuses should be extracted to a constant or configuration object for better maintainability.
+const REPORT_VIEWABLE_STATUSES = [ + MERCHANT_REPORT_STATUSES_MAP['pending-review'], + MERCHANT_REPORT_STATUSES_MAP['under-review'], + MERCHANT_REPORT_STATUSES_MAP.completed, +] as const; if ( !isFetchingBusinessReport && businessReport?.status && - ![ - MERCHANT_REPORT_STATUSES_MAP['pending-review'], - MERCHANT_REPORT_STATUSES_MAP['under-review'], - MERCHANT_REPORT_STATUSES_MAP.completed, - ].includes(businessReport?.status) + !REPORT_VIEWABLE_STATUSES.includes(businessReport?.status) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/backoffice-v2/public/locales/en/toast.json(1 hunks)apps/backoffice-v2/src/domains/business-reports/fetchers.ts(0 hunks)apps/backoffice-v2/src/domains/entities/hooks/mutations/useDocumentOcr/useDocumentOcr.ts(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringReportStatus.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringReportStatusButton.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringStatusBadge.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/fetchers.ts(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/hooks/useUpdateReportStatusMutation/useUpdateReportStatusMutation.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx(3 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx(5 hunks)packages/common/src/consts/index.ts(2 hunks)packages/ui/src/components/atoms/Badge/Badge.tsx(1 hunks)services/workflows-service/prisma/data-migrations(1 hunks)services/workflows-service/src/business-report/business-report.controller.external.ts(2 hunks)services/workflows-service/src/business-report/dtos/business-report-status-update.dto.ts(1 hunks)services/workflows-service/src/business-report/merchant-monitoring-client.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/backoffice-v2/src/domains/business-reports/fetchers.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- services/workflows-service/prisma/data-migrations
- packages/ui/src/components/atoms/Badge/Badge.tsx
- packages/common/src/consts/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
🔇 Additional comments (6)
apps/backoffice-v2/src/domains/entities/hooks/mutations/useDocumentOcr/useDocumentOcr.ts (1)
18-18: LGTM!The changes to the
onSuccesscallback are minimal and maintain the existing functionality while simplifying the function signature.apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/hooks/useUpdateReportStatusMutation/useUpdateReportStatusMutation.tsx (2)
9-15: LGTM! Well-structured hook parameters.The hook's parameters are well-defined with proper TypeScript types and optional callbacks.
39-48: LGTM! Comprehensive error handling.The error handling is well-implemented with specific handling for HTTP 400 errors and a fallback for other errors.
apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringReportStatus/MerchantMonitoringStatusBadge.tsx (1)
14-32: LGTM! Well-structured status mapping.The
statusToDatamapping is comprehensive and well-organized with clear titles and descriptions.apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx (1)
89-90: LGTM! Improved tooltip alignment.The change from
justify-centertojustify-startimproves the tooltip trigger alignment.apps/backoffice-v2/public/locales/en/toast.json (1)
108-111: LGTM!The new toast messages are clear, consistent with existing messages, and properly categorized.
| onClick={e => { | ||
| console.log('in new onClick'); | ||
| e.stopPropagation(); | ||
| }} |
There was a problem hiding this comment.
Remove console.log statement.
The console.log statement should be removed from the production code.
onClick={e => {
- console.log('in new onClick');
e.stopPropagation();
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onClick={e => { | |
| console.log('in new onClick'); | |
| e.stopPropagation(); | |
| }} | |
| onClick={e => { | |
| e.stopPropagation(); | |
| }} |
| public async updateStatus({ | ||
| status, | ||
| reportId, | ||
| customerId, | ||
| }: { | ||
| reportId: string; | ||
| customerId: string; | ||
| status: UpdateableReportStatus; | ||
| }) { | ||
| await this.axios.put(`merchants/analysis/${reportId}/status`, { | ||
| status, | ||
| customerId, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add error handling to updateStatus method.
The method should include error handling similar to the findById method to maintain consistency and improve error reporting.
public async updateStatus({
status,
reportId,
customerId,
}: {
reportId: string;
customerId: string;
status: UpdateableReportStatus;
}) {
+ try {
await this.axios.put(`merchants/analysis/${reportId}/status`, {
status,
customerId,
});
+ } catch (error) {
+ console.log(error);
+
+ if (error instanceof AxiosError && error.response?.status === 404) {
+ throw new errors.NotFoundException(`No business report found for id ${reportId}`);
+ }
+
+ throw error;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async updateStatus({ | |
| status, | |
| reportId, | |
| customerId, | |
| }: { | |
| reportId: string; | |
| customerId: string; | |
| status: UpdateableReportStatus; | |
| }) { | |
| await this.axios.put(`merchants/analysis/${reportId}/status`, { | |
| status, | |
| customerId, | |
| }); | |
| } | |
| public async updateStatus({ | |
| status, | |
| reportId, | |
| customerId, | |
| }: { | |
| reportId: string; | |
| customerId: string; | |
| status: UpdateableReportStatus; | |
| }) { | |
| try { | |
| await this.axios.put(`merchants/analysis/${reportId}/status`, { | |
| status, | |
| customerId, | |
| }); | |
| } catch (error) { | |
| console.log(error); | |
| if (error instanceof AxiosError && error.response?.status === 404) { | |
| throw new errors.NotFoundException(`No business report found for id ${reportId}`); | |
| } | |
| throw error; | |
| } | |
| } |
| const selectableStatuses = [ | ||
| MERCHANT_REPORT_STATUSES_MAP['pending-review'], | ||
| MERCHANT_REPORT_STATUSES_MAP['under-review'], | ||
| MERCHANT_REPORT_STATUSES_MAP.completed, | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move selectableStatuses to @ballerine/common package.
The selectableStatuses array should be moved to the @ballerine/common package to maintain consistency and reusability across the codebase.
| void mutateCreateNote({ | ||
| content, | ||
| entityId: businessId ?? '', | ||
| entityType: 'Business', | ||
| noteableId: reportId ?? '', | ||
| noteableType: 'Report', | ||
| parentNoteId: null, | ||
| }); |
There was a problem hiding this comment.
Add error handling for mutateCreateNote.
The mutateCreateNote call is wrapped in a void operator without proper error handling. Consider handling potential errors to ensure data consistency.
- void mutateCreateNote({
+ try {
+ await mutateCreateNote({
content,
entityId: businessId ?? '',
entityType: 'Business',
noteableId: reportId ?? '',
noteableType: 'Report',
parentNoteId: null,
- });
+ });
+ } catch (error) {
+ // Handle error appropriately
+ console.error('Failed to create note:', error);
+ // Consider showing an error toast or rolling back the status update
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void mutateCreateNote({ | |
| content, | |
| entityId: businessId ?? '', | |
| entityType: 'Business', | |
| noteableId: reportId ?? '', | |
| noteableType: 'Report', | |
| parentNoteId: null, | |
| }); | |
| try { | |
| await mutateCreateNote({ | |
| content, | |
| entityId: businessId ?? '', | |
| entityType: 'Business', | |
| noteableId: reportId ?? '', | |
| noteableType: 'Report', | |
| parentNoteId: null, | |
| }); | |
| } catch (error) { | |
| // Handle error appropriately | |
| console.error('Failed to create note:', error); | |
| // Consider showing an error toast or rolling back the status update | |
| } |
| @swagger.ApiOperation({ | ||
| summary: 'Update business report status', | ||
| description: 'Update the status of a business report', | ||
| }) | ||
| @swagger.ApiParam({ | ||
| name: 'reportId', | ||
| required: true, | ||
| description: 'The ID of the report to update', | ||
| }) | ||
| @swagger.ApiParam({ | ||
| name: 'status', | ||
| required: true, | ||
| description: 'The status to update to', | ||
| }) | ||
| @swagger.ApiOkResponse({ | ||
| description: 'Report status updated successfully', | ||
| }) | ||
| @swagger.ApiForbiddenResponse({ | ||
| description: 'Forbidden access', | ||
| type: errors.ForbiddenException, | ||
| }) | ||
| @common.Put('/:reportId/status/:status') | ||
| async updateStatus( | ||
| @CurrentProject() currentProjectId: TProjectId, | ||
| @Param('reportId') reportId: BusinessReportStatusUpdateRequestParamsDto['reportId'], | ||
| @Param('status') status: BusinessReportStatusUpdateRequestParamsDto['status'], | ||
| ) { | ||
| const { id: customerId } = await this.customerService.getByProjectId(currentProjectId); | ||
|
|
||
| await this.merchantMonitoringClient.updateStatus({ | ||
| status, | ||
| reportId, | ||
| customerId, | ||
| }); | ||
|
|
||
| return merchantMonitoringMetrics; | ||
| return { | ||
| status, | ||
| reportId, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add input validation and error handling for status update endpoint.
The status update endpoint needs additional validation and error handling:
- Validate that the report exists before updating
- Add proper error responses for various failure scenarios
- Add rate limiting to prevent abuse
@common.Put('/:reportId/status/:status')
+@common.UseGuards(ThrottlerGuard)
+@Throttle(10, 60)
async updateStatus(
@CurrentProject() currentProjectId: TProjectId,
@Param('reportId') reportId: BusinessReportStatusUpdateRequestParamsDto['reportId'],
@Param('status') status: BusinessReportStatusUpdateRequestParamsDto['status'],
) {
const { id: customerId } = await this.customerService.getByProjectId(currentProjectId);
+ // Validate report exists and belongs to customer
+ const report = await this.businessReportService.findById({ id: reportId, customerId });
+ if (!report) {
+ throw new common.NotFoundException(`Report with ID ${reportId} not found`);
+ }
try {
await this.merchantMonitoringClient.updateStatus({
status,
reportId,
customerId,
});
return {
status,
reportId,
};
} catch (error) {
+ this.logger.error('Failed to update report status', { error, reportId, status });
+ throw new common.InternalServerErrorException('Failed to update report status');
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @swagger.ApiOperation({ | |
| summary: 'Update business report status', | |
| description: 'Update the status of a business report', | |
| }) | |
| @swagger.ApiParam({ | |
| name: 'reportId', | |
| required: true, | |
| description: 'The ID of the report to update', | |
| }) | |
| @swagger.ApiParam({ | |
| name: 'status', | |
| required: true, | |
| description: 'The status to update to', | |
| }) | |
| @swagger.ApiOkResponse({ | |
| description: 'Report status updated successfully', | |
| }) | |
| @swagger.ApiForbiddenResponse({ | |
| description: 'Forbidden access', | |
| type: errors.ForbiddenException, | |
| }) | |
| @common.Put('/:reportId/status/:status') | |
| async updateStatus( | |
| @CurrentProject() currentProjectId: TProjectId, | |
| @Param('reportId') reportId: BusinessReportStatusUpdateRequestParamsDto['reportId'], | |
| @Param('status') status: BusinessReportStatusUpdateRequestParamsDto['status'], | |
| ) { | |
| const { id: customerId } = await this.customerService.getByProjectId(currentProjectId); | |
| await this.merchantMonitoringClient.updateStatus({ | |
| status, | |
| reportId, | |
| customerId, | |
| }); | |
| return merchantMonitoringMetrics; | |
| return { | |
| status, | |
| reportId, | |
| }; | |
| } | |
| @swagger.ApiOperation({ | |
| summary: 'Update business report status', | |
| description: 'Update the status of a business report', | |
| }) | |
| @swagger.ApiParam({ | |
| name: 'reportId', | |
| required: true, | |
| description: 'The ID of the report to update', | |
| }) | |
| @swagger.ApiParam({ | |
| name: 'status', | |
| required: true, | |
| description: 'The status to update to', | |
| }) | |
| @swagger.ApiOkResponse({ | |
| description: 'Report status updated successfully', | |
| }) | |
| @swagger.ApiForbiddenResponse({ | |
| description: 'Forbidden access', | |
| type: errors.ForbiddenException, | |
| }) | |
| @common.Put('/:reportId/status/:status') | |
| @common.UseGuards(ThrottlerGuard) | |
| @Throttle(10, 60) | |
| async updateStatus( | |
| @CurrentProject() currentProjectId: TProjectId, | |
| @Param('reportId') reportId: BusinessReportStatusUpdateRequestParamsDto['reportId'], | |
| @Param('status') status: BusinessReportStatusUpdateRequestParamsDto['status'], | |
| ) { | |
| const { id: customerId } = await this.customerService.getByProjectId(currentProjectId); | |
| // Validate report exists and belongs to customer | |
| const report = await this.businessReportService.findById({ id: reportId, customerId }); | |
| if (!report) { | |
| throw new common.NotFoundException(`Report with ID ${reportId} not found`); | |
| } | |
| try { | |
| await this.merchantMonitoringClient.updateStatus({ | |
| status, | |
| reportId, | |
| customerId, | |
| }); | |
| return { | |
| status, | |
| reportId, | |
| }; | |
| } catch (error) { | |
| this.logger.error('Failed to update report status', { error, reportId, status }); | |
| throw new common.InternalServerErrorException('Failed to update report status'); | |
| } | |
| } |
| <MerchantMonitoringReportStatus | ||
| reportId={businessReport?.id} | ||
| status={businessReport?.status} | ||
| businessId={businessReport?.business.id} | ||
| /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add error handling for missing report ID or business ID.
The MerchantMonitoringReportStatus component should handle cases where reportId or businessId is undefined.
🏁 Script executed:
#!/bin/bash
# Check for potential undefined ID usage
rg -A 2 "reportId\?" .
rg -A 2 "businessId\?" .Length of output: 2794
Add Null-Check/Error Handling for Undefined IDs
In apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (Lines 310-314), the <MerchantMonitoringReportStatus> component is invoked with props derived from businessReport without ensuring both reportId and businessId are defined. Although the component’s prop types mark these fields as optional, there is no runtime safeguard—especially risky for businessReport?.business.id, which can throw an error if business is undefined.
Please update the code to handle these cases by either:
- Implementing error handling or fallback behavior within
MerchantMonitoringReportStatus(e.g., render a message or use default values when IDs are missing). - Or conditionally rendering
<MerchantMonitoringReportStatus>only when both IDs are available.
Summary by CodeRabbit
New Features
MerchantMonitoringReportStatuscomponent for managing merchant report statuses with dropdown and modal interfaces.MerchantMonitoringStatusButtonandMerchantMonitoringStatusBadgecomponents for enhanced status display.Style
Refactor & Improvements
Chores