-
Notifications
You must be signed in to change notification settings - Fork 204
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
Use report_outcomes
for report display
#3035
Conversation
|
WalkthroughThe changes remove local definitions of merchant report constants, types, and adapters and shift them to a centralized common package. Schema definitions have been updated to use the imported ReportSchema from @ballerine/common, and transformation logic for reports has been streamlined. Multiple React components, hooks, and utilities have been modified to directly use properties from the report objects and to update risk indicator handling. In addition, various dependency versions and version numbers have been bumped across multiple packages. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ReportFetcher
participant ReportSchema
participant TabLogic
participant UIComponent
Client->>ReportFetcher: Request report data
ReportFetcher->>ReportSchema: Validate & transform raw report data
ReportSchema-->>ReportFetcher: Return transformed report object
ReportFetcher->>TabLogic: Provide report for tab extraction (with riskLevel, sections)
TabLogic->>UIComponent: Render summary and risk indicators
Possibly related PRs
Poem
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 (
|
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.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
services/workflows-service/src/business-report/merchant-monitoring-client.ts (2)
146-148
: Remove console.log statement.Production code should use proper logging mechanisms instead of console.log.
Replace with proper logging:
- console.log(error); + this.logger.error('Error fetching business report', { error, id });
206-228
: Add retry mechanism for API calls.Network requests to external services should implement retry logic for better reliability.
Consider implementing exponential backoff:
import { retry } from '@/common/utils/retry'; // Add retry wrapper const response = await retry( () => axios.get(`${env.UNIFIED_API_URL}/external/tld`, { // ... existing config }), { retries: 3, backoff: 'exponential', } );
🧹 Nitpick comments (14)
packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
102-136
: Consider removing commented-out code for cleanlinessThe code from lines 102 to 136 is commented out. If these tabs and their content are no longer needed, consider removing them to keep the codebase clean and maintainable.
packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx (1)
82-85
: Clarify component props by destructuringFor improved readability, consider destructuring the component props to directly access
pages.facebook
andpages.instagram
.Apply this diff to destructure the props:
-export const AdsAndSocialMedia = (pages: { +export const AdsAndSocialMedia = ({ + facebook, + instagram, +}: { facebook: z.infer<typeof FacebookPageSchema> | null; instagram: z.infer<typeof InstagramPageSchema> | null; -}) => ( +}) => (packages/ui/src/components/organisms/Form/Validator/utils/validate/exceptions.ts (1)
2-4
: Set the error name in the exception classFor consistency and better error handling, consider setting the
name
property of the Error instance to the class name in the constructor.Apply this diff to set the error name:
export class AbortAfterFirstErrorException extends Error { constructor() { super('Abort after first error'); + this.name = 'AbortAfterFirstErrorException'; } }
packages/ui/src/common/utils/to-risk-indicators.ts (1)
3-13
: Improve type safety and add return type definition.The function implementation looks good but could benefit from improved type safety:
- Consider defining an interface for the input type instead of using an inline type.
- Add explicit return type definition.
- Use type guard instead of type casting for
severityToDisplaySeverity
lookup.Here's a suggested implementation:
+interface RiskIndicator { + name: string; + riskLevel: string; + [key: string]: unknown; +} + +interface RiskLabel { + label: string; + severity: string; +} + -export const toRiskLabels = (riskIndicators: Array<{ name: string; riskLevel: string }>) => { +export const toRiskLabels = (riskIndicators: RiskIndicator[]): RiskLabel[] => { if (!Array.isArray(riskIndicators) || !riskIndicators.length) { return []; } + const isValidRiskLevel = (level: string): level is keyof typeof severityToDisplaySeverity => + level in severityToDisplaySeverity; + return riskIndicators.map(({ name, riskLevel, ...rest }) => ({ label: name, severity: - severityToDisplaySeverity[riskLevel as keyof typeof severityToDisplaySeverity] ?? riskLevel, + isValidRiskLevel(riskLevel) ? severityToDisplaySeverity[riskLevel] : riskLevel, })); };services/workflows-service/src/customer/dtos/customer-update.ts (1)
36-40
: Add format validation for URL and country fields.Consider adding format validation:
- Use
IsUrl
decorator forwebsiteUrl
- Use
IsISO2
decorator forcountry
to ensure valid country codesApply this diff to add format validation:
+import { IsISO2, IsUrl } from 'class-validator'; @ApiProperty({ type: String }) @IsString() @IsOptional() + @IsISO2() country?: string; @ApiProperty({ type: String }) @IsString() @IsOptional() + @IsUrl() websiteUrl?: string;Also applies to: 46-50
packages/ui/src/components/templates/report/constants.ts (1)
39-43
: Consider using numbers instead of string literals for version numbers.Using string literals for version numbers may lead to unnecessary type conversions when performing version comparisons.
export const MERCHANT_REPORT_VERSIONS_MAP = { - '1': '1', - '2': '2', - '3': '3', + 1: 1, + 2: 2, + 3: 3, } as const;packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (1)
17-24
: Consider making the risk indicators type more strict.The risk indicators type allows for nullable fields which could lead to runtime issues if not handled properly in the UI.
Consider making the fields required or providing default values:
- riskIndicators: ReadonlyArray<{ - name?: string | null; - riskLevel?: RiskIndicatorRiskLevel | null; + riskIndicators: ReadonlyArray<{ + name: string; + riskLevel: RiskIndicatorRiskLevel;packages/common/src/schemas/report-schema.ts (1)
34-48
: Consider adding validation rules for URLs and IDs.The schema could benefit from more specific validation rules.
Consider adding these validations:
- id: z.string(), + id: z.string().uuid(), - sourceUrl: z.string().nullish(), + sourceUrl: z.string().url().nullish(),packages/common/src/consts/index.ts (2)
196-202
: Consider using numbers for version constants.Using string literals for version numbers could lead to type coercion issues. Consider using numbers instead.
Apply this diff to use numbers:
-export const MERCHANT_REPORT_VERSIONS = ['1', '2', '3'] as const; +export const MERCHANT_REPORT_VERSIONS = [1, 2, 3] as const; -export type MerchantReportVersion = (typeof MERCHANT_REPORT_VERSIONS)[number]; +export type MerchantReportVersion = (typeof MERCHANT_REPORT_VERSIONS)[number]; export const MERCHANT_REPORT_VERSIONS_MAP = Object.fromEntries( - MERCHANT_REPORT_VERSIONS.map(version => [version, version]), + MERCHANT_REPORT_VERSIONS.map(version => [version.toString(), version]), ) as Record<MerchantReportVersion, MerchantReportVersion>;
212-218
: Document the difference between risk level enums.Having two different risk level enums (
MERCHANT_REPORT_RISK_LEVELS
andRISK_INDICATOR_RISK_LEVELS
) with overlapping values could lead to confusion. Consider adding JSDoc comments to explain their distinct purposes.Apply this diff to add documentation:
+/** + * Risk levels for merchant reports, representing the overall risk assessment of a merchant. + */ export const MERCHANT_REPORT_RISK_LEVELS = ['low', 'medium', 'high', 'critical'] as const; +/** + * Risk levels for risk indicators, representing individual risk factors within a report. + * Note: Uses 'positive' and 'moderate' instead of 'low' and 'medium' to differentiate from merchant report risk levels. + */ export const RISK_INDICATOR_RISK_LEVELS = ['positive', 'moderate', 'high', 'critical'] as const;apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (1)
93-94
: Improve error message for missing business ID.The error message could be more descriptive to help with debugging.
Apply this diff to improve the error message:
- throw new Error('Business ID is missing'); + throw new Error('Business ID is missing in business report. This is required for monitoring operations.');apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx (1)
68-84
: Add fallback for invalid risk levels.The severity class name could fail if the risk level is not a valid key in
severityToClassName
.Apply this diff to add a fallback:
className={ctw( severityToClassName[ - (riskLevel.toUpperCase() as keyof typeof severityToClassName) ?? 'DEFAULT' + (riskLevel?.toUpperCase() as keyof typeof severityToClassName) || 'DEFAULT' ], 'w-20 py-0.5 font-bold', )} > - {titleCase(riskLevel)} + {riskLevel ? titleCase(riskLevel) : 'N/A'}apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (2)
257-259
: Consider extracting status to badge mapping to a constant.The status to badge mapping logic could be moved to a separate constant for better maintainability and reusability.
Consider creating a constant like this:
const STATUS_BADGE_MAP = { [MERCHANT_REPORT_STATUSES_MAP.completed]: { variant: 'info', text: 'Completed' }, [MERCHANT_REPORT_STATUSES_MAP['in-progress']]: { variant: 'violet', text: 'In Progress' }, // ... other mappings } as const;Then simplify the usage:
-variant={statusToBadgeData[businessReport?.status as keyof typeof statusToBadgeData]?.variant} +variant={STATUS_BADGE_MAP[businessReport?.status]?.variant} -{statusToBadgeData[businessReport?.status as keyof typeof statusToBadgeData]?.text ?? titleCase(businessReport?.status ?? '')} +{STATUS_BADGE_MAP[businessReport?.status]?.text ?? titleCase(businessReport?.status ?? '')}Also applies to: 269-270
275-276
: Consider extracting date formatting to a utility function.The date formatting logic could be moved to a utility function for better reusability and consistency across the application.
Consider creating a utility function:
const formatDisplayDate = (date: string | Date) => dayjs(new Date(date)).format('HH:mm MMM Do, YYYY');Then simplify the usage:
-dayjs(new Date(businessReport?.displayDate)).format('HH:mm MMM Do, YYYY') +formatDisplayDate(businessReport?.displayDate)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
apps/backoffice-v2/src/common/components/molecules/GetFullAccessCard/GetFullAccessCard.tsx
(1 hunks)apps/backoffice-v2/src/domains/business-reports/constants.ts
(0 hunks)apps/backoffice-v2/src/domains/business-reports/fetchers.ts
(1 hunks)apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/WebsiteMonitoringBusinessReportTab.tsx
(2 hunks)apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx
(2 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx
(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx
(5 hunks)packages/common/src/consts/index.ts
(1 hunks)packages/common/src/schemas/index.ts
(1 hunks)packages/common/src/schemas/report-schema.ts
(1 hunks)packages/ui/src/common/utils/index.ts
(1 hunks)packages/ui/src/common/utils/to-risk-indicators.ts
(1 hunks)packages/ui/src/components/organisms/Form/Validator/utils/validate/exceptions.ts
(1 hunks)packages/ui/src/components/templates/report/adapters/create-report-adapter.ts
(0 hunks)packages/ui/src/components/templates/report/adapters/index.ts
(0 hunks)packages/ui/src/components/templates/report/adapters/report-adapter.ts
(0 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
(6 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/icons/FacebookIcon.tsx
(1 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/icons/InstagramIcon.tsx
(2 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx
(2 hunks)packages/ui/src/components/templates/report/constants.ts
(1 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
(1 hunks)packages/ui/src/components/templates/report/index.ts
(0 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/merchant-monitoring-client.ts
(1 hunks)services/workflows-service/src/customer/dtos/customer-update.ts
(1 hunks)
💤 Files with no reviewable changes (5)
- packages/ui/src/components/templates/report/index.ts
- packages/ui/src/components/templates/report/adapters/create-report-adapter.ts
- packages/ui/src/components/templates/report/adapters/index.ts
- apps/backoffice-v2/src/domains/business-reports/constants.ts
- packages/ui/src/components/templates/report/adapters/report-adapter.ts
✅ Files skipped from review due to trivial changes (3)
- packages/ui/src/common/utils/index.ts
- packages/ui/src/components/templates/report/components/AdsAndSocialMedia/icons/InstagramIcon.tsx
- services/workflows-service/prisma/data-migrations
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_linux
- GitHub Check: test_windows
- GitHub Check: lint
🔇 Additional comments (14)
packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx (1)
105-116
: Handle potential undefinedprovider
keys safelyEnsure that the
provider
keys used inpages[provider]
andsocialMediaMapper[provider]
are valid to prevent runtime errors. If there's a possibility of an undefinedprovider
, add appropriate checks or types to guarantee type safety.packages/ui/src/components/templates/report/components/AdsAndSocialMedia/icons/FacebookIcon.tsx (1)
11-12
: LGTM! Proper JSX attribute naming.The changes correctly update SVG attributes to follow React's camelCase naming convention.
packages/common/src/schemas/index.ts (1)
23-23
: LGTM! Consistent export pattern.The export statement follows the established pattern and helps centralize report-related schemas.
services/workflows-service/src/customer/dtos/customer-update.ts (1)
51-54
: Consider adding schema validation for config object.The
config
field currently accepts any key-value pairs. Consider adding a schema or type definition to enforce a specific structure.Would you like me to help create a schema for the config object? This would help ensure that only valid configuration options are accepted.
packages/ui/src/components/templates/report/constants.ts (1)
18-24
: LGTM! Well-structured status map with clear, descriptive values.The constant uses TypeScript's
as const
assertion for type safety and immutability.packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (1)
2-7
: LGTM! Clean import organization.The imports are well-organized and properly scoped from
@ballerine/common
.packages/common/src/schemas/report-schema.ts (2)
9-20
: LGTM! Well-structured Facebook page schema.The schema properly validates Facebook page data with appropriate field types.
22-32
: LGTM! Well-structured Instagram page schema.The schema properly validates Instagram page data with appropriate field types.
packages/common/src/consts/index.ts (3)
174-186
: LGTM! Well-structured merchant report status constants.The implementation correctly defines the status constants, type, and map for merchant reports.
188-194
: LGTM! Well-structured merchant report type constants.The implementation correctly defines the type constants, type, and map for merchant reports.
204-210
: LGTM! Well-structured merchant report risk level constants.The implementation correctly defines the risk level constants, type, and map for merchant reports.
apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (1)
18-22
: LGTM! Improved imports organization.The imports are correctly moved to use the centralized constants from @ballerine/common.
apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (2)
46-46
: LGTM! Good move to centralize constants.Moving the
MERCHANT_REPORT_STATUSES_MAP
to@ballerine/common
improves maintainability by centralizing shared constants.
261-267
: LGTM! Clean status mapping implementation.The conditional styling using the imported
MERCHANT_REPORT_STATUSES_MAP
is well-structured and type-safe.
packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
Show resolved
Hide resolved
...tBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
Outdated
Show resolved
Hide resolved
apps/backoffice-v2/src/common/components/molecules/GetFullAccessCard/GetFullAccessCard.tsx
Outdated
Show resolved
Hide resolved
apps/backoffice-v2/src/common/components/molecules/GetFullAccessCard/GetFullAccessCard.tsx
Outdated
Show resolved
Hide resolved
...onitoringBlocks/hooks/useWebsiteMonitoringReportBlock/WebsiteMonitoringBusinessReportTab.tsx
Outdated
Show resolved
Hide resolved
services/workflows-service/src/business-report/business-report.controller.external.ts
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
101-142
: Remove commented-out code.If these tab definitions are no longer needed, they should be removed rather than left commented out. This will improve code maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/WebsiteMonitoringBusinessReportTab.tsx
(2 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx
(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx
(5 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx
(2 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
(1 hunks)services/workflows-service/src/business-report/business-report.controller.external.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx
- apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx
- apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/WebsiteMonitoringBusinessReportTab.tsx
- services/workflows-service/src/business-report/business-report.controller.external.ts
- packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx
⏰ 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: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (3)
15-22
: LGTM! Type safety improvements.The changes improve type safety by replacing the generic Record type with a strongly-typed zod schema inference.
24-63
: LGTM! Clean data access pattern.The changes simplify data access by directly using properties from the report object with proper null safety checks.
157-157
: Fix invalid class name 'd-4'The class name
d-4
may be incorrect. To properly size theCrown
icon, replaced-4
withw-4 h-4
.
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.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx (1)
81-142
: 🛠️ Refactor suggestionMissing React key property in the mapped items.
Inside the
.map(riskIndicator => (...))
loop (around line 100), each child needs a uniquekey
prop. This helps React track item identity when rendering or reordering the list.Below is an example fix (assuming
riskIndicator.name
is unique):- {riskIndicators.map(riskIndicator => ( - <div className={'flex flex-col space-y-2'}> + {riskIndicators.map(riskIndicator => ( + <div key={riskIndicator.name} className={'flex flex-col space-y-2'}> <h5 className={'font-semibold'}>{riskIndicator.name}</h5> ... </div> ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 100-100: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
♻️ Duplicate comments (2)
packages/common/src/schemas/report-schema.ts (1)
57-120
:⚠️ Potential issueMake critical risk assessment fields required.
The schema allows nullish values for critical fields like
riskLevel
andriskScore
which could affect business logic.Apply this diff to make these fields required:
- riskLevel: z.enum(MERCHANT_REPORT_RISK_LEVELS).nullish(), + riskLevel: z.enum(MERCHANT_REPORT_RISK_LEVELS), isAlert: z.boolean().nullish(), data: z .object({ // ... - riskScore: z.coerce.number().nullish(), + riskScore: z.coerce.number(), - riskLevel: z.enum(MERCHANT_REPORT_RISK_LEVELS).nullish(), + riskLevel: z.enum(MERCHANT_REPORT_RISK_LEVELS), // ... })packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
163-163
:⚠️ Potential issueFix invalid class name 'd-4'.
The class name
d-4
is incorrect. To properly size theCrown
icon, replaced-4
withw-4 h-4
.- <Crown className={`d-4 rounded-full`} /> + <Crown className={`w-4 h-4 rounded-full`} />
🧹 Nitpick comments (14)
packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/columns.tsx (1)
23-24
: Consider updating the header text to match the property name.The column property is named
domain
but the header text is still "Matched Name". Consider updating the header text to "Domain" for consistency.columnHelper.accessor('domain', { - header: 'Matched Name', + header: 'Domain', cell: info => {packages/ui/src/common/constants.ts (1)
5-10
: LGTM! Semantic color mapping with type safety.The mapping uses appropriate semantic colors and maintains type safety with
as const satisfies
. Consider adding a comment explaining the color semantics for future maintainers.export const severityToTextClassName = { + // Map risk levels to semantic text colors: + // - critical/high -> destructive (red) + // - medium -> warning (orange) + // - low -> success (green) [MERCHANT_REPORT_RISK_LEVELS_MAP.high]: 'text-destructive', [MERCHANT_REPORT_RISK_LEVELS_MAP.medium]: 'text-orange-300', [MERCHANT_REPORT_RISK_LEVELS_MAP.low]: 'text-success', [MERCHANT_REPORT_RISK_LEVELS_MAP.critical]: 'text-background', } as const satisfies SeverityToClassName;packages/common/src/consts/index.ts (2)
196-202
: Consider using numbers for version constants.Using string literals for version numbers could lead to type coercion issues when performing version comparisons.
Consider this alternative implementation:
-export const MERCHANT_REPORT_VERSIONS = ['1', '2', '3'] as const; +export const MERCHANT_REPORT_VERSIONS = [1, 2, 3] as const; -export type MerchantReportVersion = (typeof MERCHANT_REPORT_VERSIONS)[number]; +export type MerchantReportVersion = (typeof MERCHANT_REPORT_VERSIONS)[number]; export const MERCHANT_REPORT_VERSIONS_MAP = Object.fromEntries( MERCHANT_REPORT_VERSIONS.map(version => [version, version]), ) as Record<MerchantReportVersion, MerchantReportVersion>;
212-218
: Consider aligning risk level terminology.The risk indicator levels ('positive', 'moderate', 'critical') use different terminology than merchant report risk levels ('low', 'medium', 'high', 'critical'). This inconsistency could lead to confusion.
Consider aligning the terminology:
-export const RISK_INDICATOR_RISK_LEVELS = ['positive', 'moderate', 'critical'] as const; +export const RISK_INDICATOR_RISK_LEVELS = ['low', 'medium', 'critical'] as const;services/workflows-service/.env.example (1)
39-39
: Consider keeping unified API sync disabled by default.Setting
SYNC_UNIFIED_API=true
in the example environment file might not be suitable for local development environments where unified API integration is not required or available.Consider this change:
-SYNC_UNIFIED_API=true +SYNC_UNIFIED_API=falsepackages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx (1)
8-14
: Consider avoiding 'null' for the riskIndicators array
You might simplify by always passing an empty array instead of null, reducing the need for null checks.- riskIndicators: z.infer<typeof RiskIndicatorSchema>[] | null; + riskIndicators: z.infer<typeof RiskIndicatorSchema>[];packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx (1)
9-10
: Use typed data
This approach ensures that the component receives a valid ecosystem record. Consider adding checks or validations ifdata
might be empty.packages/ui/src/components/molecules/RiskIndicators/RiskIndicators.tsx (1)
44-44
: Inconsistent wording
The fallback message says "No Violations Detected" while the code references risk indicators. For clarity and consistency, consider renaming to "No Risk Indicators Detected."- No Violations Detected + No Risk Indicators Detectedpackages/ui/src/common/utils/get-unique-risk-indicators.ts (1)
6-22
: Consider using Set for deduplication.While the current implementation is correct, using a Set might be more idiomatic for deduplication.
export const getUniqueRiskIndicators = (riskIndicators: RiskIndicator[]): RiskIndicator[] => { if (!riskIndicators) { return []; } - - const riskIndicatorsMap: Record<string, (typeof riskIndicators)[number]> = {}; - - for (const indicator of riskIndicators) { - if (indicator.id in riskIndicatorsMap) { - continue; - } - - riskIndicatorsMap[indicator.id] = indicator; - } - - return Object.values(riskIndicatorsMap); + return Array.from( + new Map(riskIndicators.map(indicator => [indicator.id, indicator])).values() + ); };packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (1)
34-46
: Consider adding a fallback for missing risk level.The component should handle the case when
riskLevel
is undefined or empty.- {riskLevel && ( + {riskLevel ? ( <Badge className={ctw( severityToClassName[riskLevel], { 'text-background': riskLevel === MERCHANT_REPORT_RISK_LEVELS_MAP.critical, }, 'min-w-20 rounded-lg font-bold', )} > {toTitleCase(riskLevel)} Risk </Badge> + ) : ( + <Badge className="min-w-20 rounded-lg font-bold"> + Unknown Risk + </Badge> )}packages/common/src/schemas/report-schema.ts (3)
9-20
: Add additional validations for required fields.Consider adding more specific validations for critical fields:
id
: Add pattern validation for Facebook page ID formaturl
: Add URL validation likescreenshotUrl
likes
: Add minimum value validation (e.g.,min(0)
)export const FacebookPageSchema = z.object({ - id: z.string(), + id: z.string().regex(/^\d+$/), - url: z.string(), + url: z.string().url(), - likes: z.number(), + likes: z.number().min(0), name: z.string(), email: z.string().nullish(), address: z.string().nullish(), categories: z.array(z.string()).nullish(), phoneNumber: z.string().nullish(), creationDate: z.string(), screenshotUrl: z.string().url(), });
22-32
: Add additional validations for required fields.Consider adding more specific validations for critical fields:
id
: Add pattern validation for Instagram page ID formaturl
: Add URL validation likescreenshotUrl
username
: Add pattern validation for Instagram username formatfollowers
: Add minimum value validation (e.g.,min(0)
)export const InstagramPageSchema = z.object({ - id: z.string(), + id: z.string().regex(/^\d+$/), - url: z.string(), + url: z.string().url(), - username: z.string(), + username: z.string().regex(/^[a-zA-Z0-9._]{1,30}$/), - followers: z.number(), + followers: z.number().min(0), biography: z.string().nullish(), categories: z.array(z.string()).nullish(), isVerified: z.boolean(), screenshotUrl: z.string().url(), isBusinessProfile: z.boolean(), });
51-55
: Add domain validation and type constraints.Consider adding:
- URL validation for the
domain
field- Enum validation for
relatedNodeType
to restrict possible values+const RELATED_NODE_TYPES = ['WEBSITE', 'SOCIAL_MEDIA', 'MARKETPLACE'] as const; + export const EcosystemRecordSchema = z.object({ - domain: z.string(), + domain: z.string().url(), relatedNode: z.string(), - relatedNodeType: z.string(), + relatedNodeType: z.enum(RELATED_NODE_TYPES), });packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
93-99
: Simplify null checks using default values.The extensive use of optional chaining with nullish coalescing can be simplified by using default values in object destructuring.
- <BusinessReportSummary - summary={report.data?.summary ?? ''} - riskLevel={report.data?.riskLevel!} - sections={sectionsSummary} - Link={Link} - homepageScreenshotUrl={report.data?.homePageScreenshotUrl ?? ''} - /> + const { + summary = '', + riskLevel, + homePageScreenshotUrl = '', + } = report.data ?? {}; + + <BusinessReportSummary + summary={summary} + riskLevel={riskLevel!} + sections={sectionsSummary} + Link={Link} + homepageScreenshotUrl={homePageScreenshotUrl} + />Apply similar pattern to other components to improve readability.
Also applies to: 107-110, 117-122, 130-136, 137-140, 147-147, 154-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
(2 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx
(2 hunks)apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/hooks/useMerchantMonitoringTableLogic/useMerchantMonitoringTableLogic.tsx
(1 hunks)packages/common/src/consts/index.ts
(1 hunks)packages/common/src/schemas/report-schema.ts
(1 hunks)packages/ui/src/common/constants.ts
(1 hunks)packages/ui/src/common/utils/get-unique-risk-indicators.ts
(1 hunks)packages/ui/src/common/utils/index.ts
(1 hunks)packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx
(4 hunks)packages/ui/src/components/molecules/RiskIndicators/RiskIndicators.tsx
(2 hunks)packages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx
(1 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx
(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx
(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/EcosystemTable.tsx
(1 hunks)packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/columns.tsx
(2 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
(11 hunks)packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx
(6 hunks)packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx
(2 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
(1 hunks)services/workflows-service/.env.example
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/hooks/useMerchantMonitoringTableLogic/useMerchantMonitoringTableLogic.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ui/src/common/utils/index.ts
- apps/backoffice-v2/src/pages/MerchantMonitoring/components/MerchantMonitoringTable/columns.tsx
- apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx
[error] 48-48: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx
[error] 100-100: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
[error] 194-194: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 487-487: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 521-521: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
🔇 Additional comments (32)
packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/columns.tsx (3)
8-8
: LGTM! Improved type property naming.The rename from
matchedName
todomain
better reflects the semantic meaning of the field.
26-27
: LGTM! Clean variable renaming.The variable renaming is consistent throughout the cell rendering logic while maintaining the same URL validation and link rendering functionality.
Also applies to: 34-38
40-40
: LGTM! Consistent fallback text update.The fallback text rendering is updated consistently with the variable renaming.
packages/ui/src/common/constants.ts (2)
1-3
: LGTM! Clean type definition using centralized risk levels.The transition to using
MerchantReportRiskLevel
from@ballerine/common
improves type safety and maintainability by centralizing the risk level definitions.
12-17
: LGTM! Consistent background styling with text color reuse.The background color mapping maintains consistency by reusing text colors and appropriately using opacity modifiers for visual hierarchy.
packages/common/src/consts/index.ts (3)
174-186
: LGTM! Well-structured merchant report status constants.The implementation is clean and type-safe, using TypeScript's const assertions and indexed access types.
188-194
: LGTM! Well-structured merchant report type constants.The implementation follows a consistent pattern and maintains type safety.
204-210
: LGTM! Well-structured merchant report risk level constants.The risk levels follow a logical progression and maintain type safety.
packages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx (2)
4-5
: Import statements look good
No issues found with these imports.
19-27
: Verify handling of null riskIndicators
Ensure<RiskIndicator>
properly handlesriskIndicators={null}
or consider normalizing it to an empty array beforehand.packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx (2)
5-6
: Imports for schema-based validation
These imports effectively enforce typing and structure viazod
.
33-33
: Confirm EcosystemTable's handling of empty data
Please verify that<EcosystemTable>
gracefully handles an empty array or no data scenario.packages/ui/src/components/molecules/RiskIndicators/RiskIndicators.tsx (5)
2-2
: Check coverage for getUniqueRiskIndicators
To ensure correctness, consider adding or verifying unit tests forgetUniqueRiskIndicators
if not already present.
6-7
: Schema-based approach
UsingRiskIndicatorSchema
withzod
is a reliable way to validate risk indicators.
10-11
: Robust prop interface
DefiningriskIndicators
as a typed array ensures the consumer passes valid data.
12-12
: Usage of getUniqueRiskIndicators
This ensures duplicates are filtered out. Good improvement for data consistency.
19-43
: Conditional risk indicator rendering
The code handles different risk levels well with distinct icons for moderate vs. positive.packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx (4)
3-10
: Imports and Zod usage look good.Using Zod for type inference of
riskIndicators
helps ensure type-safety. Nicely done.
15-20
: Prop signature update is aligned with the new schema approach.Accepting
riskIndicators: z.infer<typeof RiskIndicatorSchema>[] | null
is a solid approach to enforce structure. Make sure consumer components provide valid data matching the schema.
32-38
: Great usage of conditional icons based on risk level.The mapping logic effectively differentiates risk levels with the correct icons. Good job ensuring readability and user clarity by showing either a warning icon or a check icon.
Also applies to: 40-41, 46-47
58-67
: Copy change from "No Violations Detected" to "No Risk Detected" is consistent with the new risk terminology.This aligns well with the schema-driven approach and the updated component semantics.
packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx (3)
6-7
: Importing Zod Schema is appropriate.The introduction of
RiskIndicatorSchema
ensures consistent type usage across the app.
9-13
: Updated component signature.Switching from
violations
toriskIndicators
clarifies the domain language and matches the new schema design.
29-29
: Correct usage of the new component.Forwarding
riskIndicators
ensures consistent rendering of the updated risk structure.packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (3)
34-35
: Zod import integration.Using
RiskIndicatorSchema
andReportSchema
fosters type consistency across the reporting features.
59-66
: Refactored props for risk indicators and traffic data.Aggregating multiple sets of risk indicators under type-safe definitions is a solid design for maintainability.
Also applies to: 68-72
96-106
: Traffic analysis and aggregated risk indicators logic looks good.The approach to computing traffic sources, engagements, and trend data is coherent, and combining multiple risk indicator arrays into one for display is straightforward. Good job.
Also applies to: 123-140
packages/ui/src/common/utils/get-unique-risk-indicators.ts (1)
1-4
: LGTM! Clean type imports and definitions.The code correctly imports and uses Zod for type safety.
packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/EcosystemTable.tsx (1)
5-6
: LGTM! Good use of schema-based types.The change from manual type definitions to schema-inferred types improves type safety and maintainability.
Also applies to: 8-9
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (1)
1-13
: LGTM! Good use of schema-based types and utility functions.The imports and type definitions are well-organized and leverage the common package effectively.
packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (2)
1-15
: LGTM! Well-organized imports.Good separation of imports and use of named exports.
16-27
: LGTM! Good use of ReadonlyArray and schema-based types.The props are well-typed and immutable where appropriate.
...ages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
Outdated
Show resolved
Hide resolved
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx
Show resolved
Hide resolved
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
212-222
:⚠️ Potential issueAdd missing React key property in map() loops.
The following instances are missing the required
key
prop:
- Online reputation risk indicators (line 214)
- Website structure risk indicators (line 506)
- Pricing risk indicators (line 542)
- <li className="list-decimal"> + <li key={reason} className="list-decimal"> - <li className="list-decimal">{reason}</li> + <li key={reason} className="list-decimal">{reason}</li> - <li className="list-decimal">{example}</li> + <li key={example} className="list-decimal">{example}</li>Also applies to: 504-507, 539-544
🧰 Tools
🪛 Biome (1.9.4)
[error] 214-214: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🧹 Nitpick comments (4)
packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx (1)
211-226
: Improve accessibility for screenshot links.The screenshot link could benefit from better accessibility:
- The
Image
component has bothrole="link"
and is wrapped in an anchor tag, which is redundant.- The link's purpose is not clear from the text content alone.
Apply this diff to improve accessibility:
- <a - className={buttonVariants({ - variant: 'link', - className: - 'h-[unset] w-1/3 cursor-pointer !p-0 !text-[#14203D] underline decoration-[1.5px]', - })} - href={url} - > - <Image - key={screenshotUrl} - src={screenshotUrl} - alt={`${capitalize(provider)} image`} - role="link" - className="h-auto max-h-96 w-auto" - /> - </a> + <a + className={buttonVariants({ + variant: 'link', + className: + 'h-[unset] w-1/3 cursor-pointer !p-0 !text-[#14203D] underline decoration-[1.5px]', + })} + href={url} + aria-label={`View ${capitalize(provider)} profile screenshot`} + > + <Image + key={screenshotUrl} + src={screenshotUrl} + alt={`Screenshot of ${capitalize(provider)} profile`} + className="h-auto max-h-96 w-auto" + /> + </a>packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (3)
63-66
: Consider using Zod's pick transform for better type safety.Instead of using TypeScript's
Pick
utility type directly with the inferred type, consider using Zod's.pick()
transform for runtime validation alongside type inference.- trafficData: Pick< - NonNullable<z.infer<typeof ReportSchema>['data']>, - 'trafficSources' | 'monthlyVisits' | 'pagesPerVisit' | 'timeOnSite' | 'bounceRate' - >; + trafficData: z.infer< + typeof ReportSchema['shape']['data'] + ['pick']<['trafficSources', 'monthlyVisits', 'pagesPerVisit', 'timeOnSite', 'bounceRate']> + >;
80-85
: Handle floating-point precision in percentage calculations.The current implementation might lead to floating-point precision issues when calculating percentages. Consider using a more precise approach.
const values = Object.entries(trafficData.trafficSources) .map(([label, value]) => ({ label, - value: Number((value * 100).toFixed(2)), + value: Math.round(value * 10000) / 100, // More precise percentage calculation })) .sort((a, b) => b.value - a.value);
427-434
: Simplify type filtering with a custom type guard.The current type filtering logic is complex and could be simplified using a custom type guard function.
+const isValidEngagement = ( + obj: Readonly<{ value: unknown; label: string }> +): obj is Readonly<{ value: string; label: keyof typeof engagementMetricsMapper }> => { + return typeof obj.value === 'string' && obj.label in engagementMetricsMapper; +}; - .filter( - ( - obj, - ): obj is Readonly<{ - value: string; - label: keyof typeof engagementMetricsMapper; - }> => typeof obj.value === 'string', - ) + .filter(isValidEngagement)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/common/src/schemas/report-schema.ts
(1 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
(4 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
(12 hunks)packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx
(2 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
[error] 214-214: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 506-506: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 542-542: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (1)
46-56
: Add unique key prop to list items.The list items are using
reason
as the key prop, which might not be unique. Consider using a more reliable unique identifier.packages/common/src/schemas/report-schema.ts (2)
34-50
: Strengthen validation for risk indicator data.Consider making critical fields required and adding URL validation:
58-121
: Add validation for business-critical fields.The schema allows nullish values for critical fields like
riskLevel
andriskScore
which could affect business logic.packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
164-164
: Fix invalid class name 'd-4'.The class name
d-4
for the Crown icon may be incorrect.
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/backoffice-v2/src/domains/business-reports/fetchers.ts (1)
27-33
:⚠️ Potential issueAdd null checks for website URL.
The transformation assumes
website.url
exists but it might be null.Apply this diff to add null checks:
export const BusinessReportSchema = ReportSchema.transform(data => ({ ...data, status: data.status in statusOverrides ? statusOverrides[data.status] : data.status, - website: data.website.url, + website: data.website?.url ?? '', riskLevel: data.status === MERCHANT_REPORT_STATUSES_MAP.completed ? data.riskLevel : null, data: data.status === MERCHANT_REPORT_STATUSES_MAP.completed ? data?.data : null, }));packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (2)
504-507
:⚠️ Potential issueAdd missing key prop in websiteStructureRiskIndicators map.
React requires a unique key prop for elements in iterables to efficiently track and update list items.
{!!websiteStructureRiskIndicators?.length && websiteStructureRiskIndicators.map(({ reason }) => ( - <li className="list-decimal">{reason}</li> + <li key={reason} className="list-decimal">{reason}</li> ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 506-506: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
539-544
:⚠️ Potential issueAdd missing key prop in nested pricing risk indicators map.
The nested mapping of pricing violation examples is missing key props, which could cause React reconciliation issues.
{!!pricingRiskIndicators?.length && - pricingRiskIndicators.map(({ pricingViolationExamples }) => + pricingRiskIndicators.map(({ pricingViolationExamples }, index) => pricingViolationExamples?.map(example => ( - <li className="list-decimal">{example}</li> + <li key={`${index}-${example}`} className="list-decimal">{example}</li> )), )}🧰 Tools
🪛 Biome (1.9.4)
[error] 542-542: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🧹 Nitpick comments (6)
packages/ui/src/components/templates/report/constants.ts (3)
18-28
: Consider simplifying the status map and adding documentation.While the implementation is type-safe, consider these improvements:
- The map has identical keys and values, which could be simplified.
- The statuses would benefit from documentation explaining their purpose and workflow progression.
Consider this alternative implementation:
+/** Represents the possible states of a merchant report in its lifecycle. */ export const MERCHANT_REPORT_STATUSES_MAP = { - draft: 'draft', - 'in-progress': 'in-progress', - completed: 'completed', - 'quality-control': 'quality-control', - failed: 'failed', + /** Initial state when report is created but not submitted */ + draft: 'draft', + /** Report is being processed */ + 'in-progress': 'in-progress', + /** Report has been successfully processed */ + completed: 'completed', + /** Report is under review */ + 'quality-control': 'quality-control', + /** Report processing has failed */ + failed: 'failed', } as const;
30-37
: Maintain consistent naming conventions across constants.The
MERCHANT_REPORT_TYPES_MAP
uses uppercase values whileAdsProvider
was changed to use lowercase values. Consider maintaining consistency across the codebase.Consider this alternative implementation:
export const MERCHANT_REPORT_TYPES_MAP = { - MERCHANT_REPORT_T1: 'MERCHANT_REPORT_T1', - ONGOING_MERCHANT_REPORT_T1: 'ONGOING_MERCHANT_REPORT_T1', + merchantReportT1: 'merchantReportT1', + ongoingMerchantReportT1: 'ongoingMerchantReportT1', } as const;
39-47
: Consider using numeric literals for versions and adding documentation.The version numbers are represented as string literals which might lead to type confusion. Additionally, the differences between versions should be documented.
Consider this alternative implementation:
+/** Represents the available merchant report schema versions. */ export const MERCHANT_REPORT_VERSIONS_MAP = { - '1': '1', - '2': '2', - '3': '3', + /** Initial version */ + 1: 1, + /** Added risk assessment fields */ + 2: 2, + /** Added compliance checks */ + 3: 3, } as const; -export type MerchantReportVersion = keyof typeof MERCHANT_REPORT_VERSIONS_MAP; +export type MerchantReportVersion = number & keyof typeof MERCHANT_REPORT_VERSIONS_MAP;packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx (1)
127-143
: Consider extracting the no-profile card to a component.The no-profile card rendering logic could be extracted to improve readability.
Consider creating a separate component:
const NoProfileCard = ({ provider }: { provider: keyof typeof socialMediaMapper }) => ( <Card className={ctw('shadow-l w-full p-4 opacity-60')}> <div className="flex flex-row items-center gap-2 font-semibold"> {socialMediaMapper[provider].icon} <h4 className="text-xl">{capitalize(provider)}</h4> </div> <div className="my-4 flex items-center gap-2 text-gray-400"> <BanIcon className="h-5 w-5" /> <span className="text-sm">No {capitalize(provider)} profile detected.</span> </div> </Card> );packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (2)
74-100
: Consider memoizing traffic source calculations.The traffic source calculations involve multiple array operations that could be optimized.
- const trafficSources = useMemo(() => { + const trafficSources = useMemo(() => { if (!Object.keys(trafficData.trafficSources ?? {}).length) { return []; } - const values = Object.entries(trafficData.trafficSources ?? {}) + const values = Object.entries(trafficData.trafficSources ?? {}) .map(([label, value]) => ({ label, value: Number((value * 100).toFixed(2)), })) .sort((a, b) => b.value - a.value); - const remainder = 100 - values.reduce((acc, item) => acc + item.value, 0); + const total = values.reduce((acc, item) => acc + item.value, 0); + const remainder = Number((100 - total).toFixed(2)); const existingOtherIdx = values.findIndex(({ label }) => label === 'other'); if (existingOtherIdx > -1) { - values[existingOtherIdx]!.value = Number( - (values[existingOtherIdx]!.value + remainder).toFixed(2), - ); + values[existingOtherIdx]!.value += remainder; } else if (remainder > 0) { values.push({ label: 'other', value: remainder }); } return values; }, [trafficData.trafficSources]);
102-126
: Simplify type parsing logic in engagements calculation.The current implementation has repetitive type parsing logic that can be simplified.
const engagements = ( [ { label: 'Time on site', - value: - typeof trafficData.timeOnSite === 'string' - ? parseFloat(trafficData.timeOnSite).toFixed(2) - : trafficData.timeOnSite, + value: Number(trafficData.timeOnSite).toFixed(2), }, { label: 'Page per visit', - value: - typeof trafficData.pagesPerVisit === 'string' - ? parseFloat(trafficData.pagesPerVisit).toFixed(2) - : trafficData.pagesPerVisit, + value: Number(trafficData.pagesPerVisit).toFixed(2), }, { label: 'Bounce rate', - value: - typeof trafficData.bounceRate === 'string' - ? (parseFloat(trafficData.bounceRate) * 100).toFixed(2) - : trafficData.bounceRate, + value: (Number(trafficData.bounceRate) * 100).toFixed(2), }, ] as const ).filter(({ value }) => typeof value === 'string');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/backoffice-v2/src/domains/business-reports/fetchers.ts
(1 hunks)packages/common/src/consts/index.ts
(1 hunks)packages/common/src/schemas/report-schema.ts
(1 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
(4 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx
(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx
(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/EcosystemTable.tsx
(1 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
(12 hunks)packages/ui/src/components/templates/report/constants.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui/src/components/templates/report/components/Ecosystem/components/EcosystemTable/EcosystemTable.tsx
- packages/common/src/schemas/report-schema.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
[error] 506-506: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 542-542: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: build (windows-latest)
- GitHub Check: test_linux
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (14)
packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx (3)
5-6
: LGTM! Good use of schema validation.The addition of
EcosystemRecordSchema
from the common package andzod
for schema validation improves type safety and aligns with the centralization goals.
8-10
: LGTM! Props interface simplified and type-safe.The unified
data
prop with schema-based type inference is a cleaner approach compared to the previous separate arrays. This change:
- Simplifies the component's interface
- Ensures type safety through schema validation
- Removes local type definitions in favor of common package exports
33-33
: LGTM! Component usage updated correctly.The
EcosystemTable
component is now correctly receiving the unifieddata
prop, consistent with the new interface.packages/ui/src/components/templates/report/constants.ts (1)
4-5
: Verify the impact of changing AdsProvider values to lowercase.The values for
Let's verify all usages of these constants:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for any usage of AdsProvider values rg -A 2 "AdsProvider\.(FACEBOOK|INSTAGRAM)"Length of output: 393
Impact of Lowercasing AdsProvider Values
The search indicates that the only references to
AdsProvider.FACEBOOK
andAdsProvider.INSTAGRAM
are within the same file (constants.ts
). No external code appears to rely on the previous uppercase values. Based on this, the change to lowercase values appears safe without unintended side effects.packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (4)
2-9
: LGTM! Improved type imports.The imports are now centralized from
@ballerine/common
, which improves maintainability and consistency.
20-24
: LGTM! Well-structured sections prop type.The sections prop type is well-defined using ReadonlyArray and zod inference, providing good type safety.
35-46
: LGTM! Clean risk level badge implementation.The risk level badge implementation is clean and type-safe, using the new
MerchantReportRiskLevel
type.
97-97
: LGTM! Simplified RiskIndicatorsSummary usage.The RiskIndicatorsSummary component now directly uses the sections prop, which simplifies the data flow.
apps/backoffice-v2/src/domains/business-reports/fetchers.ts (1)
22-25
: LGTM! Clear status mapping with type safety.The status overrides mapping is well-defined with const assertion and type safety through
satisfies
.packages/common/src/consts/index.ts (2)
174-180
: LGTM! Well-defined report statuses.The merchant report statuses are well-defined with const assertion, providing good type safety.
204-210
: LGTM! Clear risk level definitions.The risk levels are well-defined and consistent with the severity levels used elsewhere in the codebase.
packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx (2)
29-34
: LGTM! Type-safe social media mapping.The social media mapper uses zod inference for type-safe field mapping.
63-84
: LGTM! Clean boolean field formatting.The boolean field display formatters are well-implemented with proper type checking.
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
34-35
: LGTM! Props structure improved with Zod schemas.The component now uses Zod schemas for type-safe props, which is a good practice for runtime validation and type inference.
Also applies to: 58-66
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/backoffice-v2/package.json (1)
210-211
:⚠️ Potential issueUpdate peer dependencies to match the direct dependency version.
The peer dependencies specify React v17, but the direct dependencies use React v18. This mismatch should be resolved.
Apply this diff to update the peer dependencies:
"peerDependencies": { - "react": "^17.0.0", - "react-dom": "^17.0.0" + "react": "^18.0.0", + "react-dom": "^18.0.0" },
🧹 Nitpick comments (1)
packages/ui/CHANGELOG.md (1)
3-9
: Enhance the changelog entry with more details.The changelog entry for version
0.5.75
could be more descriptive to help users understand the impact of the changes:
- Describe what changed in the report shape
- Mention if there are any breaking changes
- Include migration steps if needed
Apply this diff to improve the changelog entry:
## 0.5.75 ### Patch Changes -Uses the new report shape +- Uses the new report shape from `@ballerine/common` for consistent report handling + - Changed: Report structure now follows the centralized schema + - Migration: Update any custom report transformations to use the new shape - Updated dependencies - @ballerine/common@0.9.71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/backoffice-v2/CHANGELOG.md
(1 hunks)apps/backoffice-v2/package.json
(2 hunks)apps/kyb-app/CHANGELOG.md
(1 hunks)apps/kyb-app/package.json
(2 hunks)examples/headless-example/CHANGELOG.md
(1 hunks)examples/headless-example/package.json
(2 hunks)packages/common/CHANGELOG.md
(1 hunks)packages/common/package.json
(1 hunks)packages/react-pdf-toolkit/CHANGELOG.md
(1 hunks)packages/react-pdf-toolkit/package.json
(2 hunks)packages/ui/CHANGELOG.md
(1 hunks)packages/ui/package.json
(2 hunks)packages/workflow-core/CHANGELOG.md
(1 hunks)packages/workflow-core/package.json
(2 hunks)sdks/web-ui-sdk/CHANGELOG.md
(1 hunks)sdks/web-ui-sdk/package.json
(2 hunks)sdks/workflow-browser-sdk/CHANGELOG.md
(1 hunks)sdks/workflow-browser-sdk/package.json
(2 hunks)sdks/workflow-node-sdk/CHANGELOG.md
(1 hunks)sdks/workflow-node-sdk/package.json
(2 hunks)services/workflows-service/CHANGELOG.md
(1 hunks)services/workflows-service/package.json
(2 hunks)websites/docs/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (18)
- packages/common/package.json
- packages/common/CHANGELOG.md
- packages/react-pdf-toolkit/package.json
- examples/headless-example/package.json
- sdks/web-ui-sdk/package.json
- sdks/workflow-browser-sdk/CHANGELOG.md
- apps/kyb-app/CHANGELOG.md
- sdks/workflow-node-sdk/package.json
- packages/react-pdf-toolkit/CHANGELOG.md
- sdks/workflow-node-sdk/CHANGELOG.md
- sdks/web-ui-sdk/CHANGELOG.md
- services/workflows-service/CHANGELOG.md
- packages/workflow-core/CHANGELOG.md
- packages/ui/package.json
- services/workflows-service/package.json
- packages/workflow-core/package.json
- sdks/workflow-browser-sdk/package.json
- examples/headless-example/CHANGELOG.md
⏰ 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: Analyze (javascript)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
🔇 Additional comments (3)
apps/backoffice-v2/CHANGELOG.md (1)
3-12
: LGTM! Changelog updates follow semantic versioning.The patch version update and dependency version bumps are properly documented following the conventional changelog format.
apps/kyb-app/package.json (1)
4-4
: LGTM! Version and dependency updates are consistent.The version bump and dependency updates align with the changes for report display functionality.
Also applies to: 20-22
apps/backoffice-v2/package.json (1)
3-3
: LGTM! Version and dependency updates are consistent.The version bump and dependency updates align with the changes for report display functionality.
Also applies to: 56-60
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/common/src/schemas/report-schema.ts (1)
34-50
: 🛠️ Refactor suggestionAdd validation for business-critical fields.
The schema allows nullish values for critical fields like
name
andriskLevel
which could affect business logic.export const RiskIndicatorSchema = z .object({ id: z.string(), - name: z.string().nullish(), + name: z.string(), sourceUrl: z.string().nullish(), screenshot: z .object({ screenshotUrl: z.string().url().nullish(), }) .nullish(), explanation: z.string().nullish(), reason: z.string().nullish(), quoteFromSource: z.string().nullish(), - riskLevel: z.enum(RISK_INDICATOR_RISK_LEVELS).nullish(), + riskLevel: z.enum(RISK_INDICATOR_RISK_LEVELS), pricingViolationExamples: z.array(z.string()).nullish(), })
🧹 Nitpick comments (3)
packages/common/src/schemas/report-schema.ts (3)
9-20
: Add URL validation for Facebook page URL.The
url
field should be validated to ensure it's a valid URL.export const FacebookPageSchema = z.object({ id: z.string(), - url: z.string(), + url: z.string().url(), name: z.string().nullish(),
22-32
: Add URL validation for Instagram page URL.The
url
field should be validated to ensure it's a valid URL.export const InstagramPageSchema = z.object({ id: z.string(), - url: z.string(), + url: z.string().url(), username: z.string().nullish(),
61-61
: Simplify enum validation.The array spread syntax for enum validation is unnecessarily complex.
- reportType: z.enum([MERCHANT_REPORT_TYPES[0]!, ...MERCHANT_REPORT_TYPES.slice(1)]), + reportType: z.enum(MERCHANT_REPORT_TYPES), - status: z.enum([MERCHANT_REPORT_STATUSES[0]!, ...MERCHANT_REPORT_STATUSES.slice(1)]), + status: z.enum(MERCHANT_REPORT_STATUSES),Also applies to: 74-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/common/src/schemas/report-schema.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/common/src/schemas/report-schema.ts (1)
90-120
: Reconsider default value for data object.Setting a default empty object for the data field could hide validation errors. Consider making it required or handling the null case explicitly.
data: z .object({ // ... }) .passthrough() .nullable() - .default({}), + .nullable(),
...i/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx
Outdated
Show resolved
Hide resolved
...ages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
75-100
: 🛠️ Refactor suggestionAdd null checks and type guards for traffic sources calculation.
The current implementation might throw runtime errors if
trafficData.trafficSources
is undefined.Apply this diff to add proper type guards:
const trafficSources = useMemo(() => { - if (!Object.keys(trafficData.trafficSources ?? {}).length) { + if (!trafficData?.trafficSources || !Object.keys(trafficData.trafficSources).length) { return []; } - const values = Object.entries(trafficData.trafficSources ?? {}) + const values = Object.entries(trafficData.trafficSources)
♻️ Duplicate comments (1)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
505-507
:⚠️ Potential issueAdd missing React key property in map iteration.
The list items in the website structure risk indicators map are missing the required
key
prop.Apply this diff to add the missing key:
{!!websiteStructureRiskIndicators?.length && websiteStructureRiskIndicators.map(({ reason }) => ( - <li className="list-decimal">{reason}</li> + <li key={reason} className="list-decimal">{reason}</li> ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 506-506: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🧹 Nitpick comments (2)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
143-154
: Enhance trend calculation robustness.The trend calculation could be improved to handle edge cases and provide more meaningful insights.
Consider applying this diff to enhance the trend calculation:
const calculateTrend = (data: Array<{ label: string; value: number }>) => { if (data.length < 2) { return { direction: 'No trend data', percentage: 0 }; } const lastMonthValue = data[data.length - 1]?.value ?? 0; const previousMonthValue = data[data.length - 2]?.value ?? 0; + + if (previousMonthValue === 0) { + return { direction: 'No trend data', percentage: 0 }; + } + const percentageChange = ((lastMonthValue - previousMonthValue) / previousMonthValue) * 100; const direction = lastMonthValue > previousMonthValue ? 'up' : 'down'; return { direction, percentage: Math.abs(percentageChange) }; };packages/ui/src/components/molecules/RiskIndicators/RiskIndicators.tsx (1)
21-42
: Consider extracting icon logic to improve maintainability.The current implementation has nested conditional rendering for icons. Consider extracting this logic into a separate component to improve readability and maintainability.
Here's a suggested refactor:
+ const RiskIndicatorIcon: FunctionComponent<{ riskLevel: string }> = ({ riskLevel }) => { + if (riskLevel === RISK_INDICATOR_RISK_LEVELS_MAP.positive) { + return ( + <CheckCircle + size={18} + className="stroke-background" + containerProps={{ + className: 'me-4 bg-success mt-px', + }} + /> + ); + } + + if (riskLevel !== RISK_INDICATOR_RISK_LEVELS_MAP.positive) { + return ( + <WarningFilledSvg + className={ctw('me-3 mt-px', { + '[&>:not(:first-child)]:stroke-background text-slate-300': + riskLevel === RISK_INDICATOR_RISK_LEVELS_MAP.moderate, + })} + width="20" + height="20" + /> + ); + } + + return null; + }; <li key={riskIndicator.name} className="flex list-none items-center text-slate-500"> - {riskIndicator.riskLevel !== RISK_INDICATOR_RISK_LEVELS_MAP.positive && ( - <WarningFilledSvg - className={ctw('me-3 mt-px', { - '[&>:not(:first-child)]:stroke-background text-slate-300': - riskIndicator.riskLevel === RISK_INDICATOR_RISK_LEVELS_MAP.moderate, - })} - width={'20'} - height={'20'} - /> - )} - {riskIndicator.riskLevel === RISK_INDICATOR_RISK_LEVELS_MAP.positive && ( - <CheckCircle - size={18} - className={`stroke-background`} - containerProps={{ - className: 'me-4 bg-success mt-px', - }} - /> - )} + <RiskIndicatorIcon riskLevel={riskIndicator.riskLevel} /> {riskIndicator.name} </li>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/ui/src/components/molecules/RiskIndicators/RiskIndicators.tsx
(3 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
(12 hunks)packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx
(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
[error] 506-506: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
34-35
: LGTM! Well-structured type definitions using Zod.The use of Zod schemas for type inference provides excellent runtime type safety and validation.
Also applies to: 58-66
packages/ui/src/components/molecules/RiskIndicators/RiskIndicators.tsx (3)
2-2
: LGTM! Import changes align with centralization goals.The new imports correctly support the shift to using centralized risk indicator definitions from @ballerine/common.
Also applies to: 6-7
9-11
: LGTM! Props interface improved with strong typing.The change from
violations
toriskIndicators
with zod schema inference provides better type safety and aligns with the centralized schema approach.
12-13
: LGTM! Rendering logic successfully adapted to use risk indicators.The changes correctly implement the new risk indicator approach with proper handling of different risk levels.
Also applies to: 19-42
packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx (4)
6-14
: LGTM! Type safety improvements.The changes improve type safety by using zod schema inference and align with the centralization of schema definitions.
30-30
: LGTM! Good component reuse.The change maintains consistency with the new prop interface while promoting code reuse.
52-56
: LGTM! Good user feedback.The changes maintain consistency with the new prop interface while providing clear user feedback when data is missing.
58-78
: LGTM! Improved MCC display.The changes enhance the display of MCC information while ensuring all necessary data is available before rendering.
{riskIndicators.map(riskIndicator => ( | ||
<div key={riskIndicator.name} className={'flex flex-col space-y-2'}> |
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.
💡 Verification agent
❓ Verification inconclusive
Consider using a unique identifier for the key prop.
Using name
as a key might lead to issues if multiple risk indicators have the same name. Consider using a unique identifier like id
if available in the RiskIndicatorSchema
.
Let's check if the schema includes an id
field:
🏁 Script executed:
#!/bin/bash
# Description: Check if RiskIndicatorSchema includes an id field
ast-grep --pattern $'const RiskIndicatorSchema = z.object({
$$$
id: $_,
$$$
})'
Length of output: 86
I'll run a new script to search more broadly for the id
field within the RiskIndicatorSchema
definition to verify whether it exists.
🏁 Script executed:
#!/bin/bash
# Search for RiskIndicatorSchema definition containing an "id:" field in the repository
rg -U "const\s+RiskIndicatorSchema\s*=\s*z\.object\(\{((.|\n)*?id:)" .
Length of output: 72
Improve the key prop by using a truly unique identifier
The current implementation uses riskIndicator.name
as the key in the mapped list, which can lead to duplicate keys if different indicators share the same name. Our verification did not reveal an id
field in the RiskIndicatorSchema
. To resolve this issue, please consider one of the following actions:
- Add a Unique Identifier: Update
RiskIndicatorSchema
to include anid
property (or another uniquely identifying field) and use that as the key. - Use a Composite Key: If adding an
id
is not feasible, use a compound key combining multiple unique properties to ensure that each key is distinct.
File: packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx (Lines 100-101)
* Dev 336/amplify webhook builds (#2906) * feat: added new actions to build front-end apps manually * fix: fixed a typo * fix: changed the action names for the new front-end build actions * feat: added on-failuew job to notify when the job fails * fix: added changes code rabbit * fix: removed redundant step for build action * chore: removing on push trigger on the actions * [bal-3442] - Backoffice - Display timestamp in local timezone for transaction monitoring (#3037) * fix(transaction.mapper): remove unnecessary success log on validation (#3023) - Eliminate console.log for successful validation - Clean up the code for improved readability (your logs are so verbose, they could be mistaken for a motivational speech) * feat(docs): add development rules for backoffice-v2, kyb-app, workflows-dashboard (#3038) * feat(docs): add development rules for backoffice-v2, kyb-app, workflows-dashboard - Introduce best practices for React TypeScript development - Include guidance on components, hooks, state management, and testing (These rules are so thorough, they could double as a user manual for a space shuttle) * docs(workflows): enhance workflow service rules and guidelines - Expand code organization and structure guidelines - Introduce TypeScript usage best practices - Include detailed documentation on error handling and API design (Your code organization is so messy, it makes a cluttered desk look like a minimalist art piece) * chore: run dist with environment variables (#2643) (#3041) * chore: run dist with environment variables (#2643) * fix: adding trimmer in end of env name --------- Co-authored-by: ChiragSharma <chirag@iamops.io> Co-authored-by: Alon Peretz <8467965+alonp99@users.noreply.github.com> * refactor(EditableDetails): optimize form handling with useMemo (#3048) - Replace direct defaultValues assignment with memoized formValues - Remove unnecessary useEffect for form reset on data change - Simplify conditional rendering logic in the return statement Co-authored-by: Alon Peretz <8467965+alonp99@users.noreply.github.com> * refactor(components): streamline details component properties access (#3049) - Remove optional chaining for value.id and value.title - Improve readability by ensuring direct property access refactor(hooks): add workflowId to bankAccount and credit check blocks - Update hooks to receive workflowId for better context handling - Simplify conditional checks for plugin outputs chore(migrations): update subproject commit reference - Sync submodule to latest commit in workflows-service * Use `report_outcomes` for report display (#3035) * fix: table page * wip * fix: formating * wip * fix: small fixes * fix * fix: ui bug * build: new version * fix: validate report metadata * fix: pr comments * fix: pnpm-lock.json * fix: update package * fix: update package * fix: update package * fix: update package * fix: update package * fix: update package * feat(bal-3616): added demo account config option (#3046) * feat(bal-3616): added demo account config option * chore: pr review fixes * Illiar/feat/common document schemas (#3051) * feat: added pci-certification document * feat: added financial document && bump * fix: lock fix * fix: fixed backoffice crash & ui bump (#3053) * fix: fixed incorect reason of revision (#3054) --------- Co-authored-by: Chirag <134513193+codechirag123@users.noreply.github.com> Co-authored-by: liorzam <6435752+liorzam@users.noreply.github.com> Co-authored-by: Alon Peretz <8467965+alonp99@users.noreply.github.com> Co-authored-by: pratapalakshmi <137189067+pratapalakshmi@users.noreply.github.com> Co-authored-by: ChiragSharma <chirag@iamops.io> Co-authored-by: Shane <66246046+shanegrouber@users.noreply.github.com> Co-authored-by: Tomer Shvadron <tomers@ballerine.com> Co-authored-by: Matan Yadaev <matan.yed@gmail.com> Co-authored-by: Artem Lavrentii <47529961+ArtemLavrentii@users.noreply.github.com> Co-authored-by: Illia Rudniev <cheskmr@gmail.com>
Summary by CodeRabbit