-
Notifications
You must be signed in to change notification settings - Fork 271
Bal 4094 #3447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Bal 4094 #3447
Conversation
|
WalkthroughThis update expands the KYB and Ownership assessment schema to include new error and risk fields, refactors frontend type usage to infer from the schema, introduces a warning flag system with a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KybAndOwnershipAssessmentPage
participant useKybAndOwnershipAssessmentPageLogic
participant useAssessmentWarningFlags
participant WarningBanner
User->>KybAndOwnershipAssessmentPage: Visit assessment page
KybAndOwnershipAssessmentPage->>useKybAndOwnershipAssessmentPageLogic: Invoke logic hook
useKybAndOwnershipAssessmentPageLogic->>useAssessmentWarningFlags: Generate warning flags from assessment
useAssessmentWarningFlags-->>useKybAndOwnershipAssessmentPageLogic: Return warningFlags
useKybAndOwnershipAssessmentPageLogic-->>KybAndOwnershipAssessmentPage: Return warningFlags and other data
KybAndOwnershipAssessmentPage->>WarningBanner: Render if warningFlags present
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (1)
apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useAssessmentChecks.tsx (1)
133-187: Well-implemented warning flag system with minor improvement opportunities.The warning flag implementation is solid with proper TypeScript types and React patterns. However, consider these refinements:
- Potential string fragility: The severity mapping logic uses hardcoded string checks that could break if error message formats change.
- Code duplication: Similar error checking pattern is repeated for registry and structure.
Consider extracting the error checking logic into a reusable function:
+const createWarningFlag = ( + status: string | undefined, + error: string | undefined, + defaultTitle: string, + errorSeverityMap: Record<string, 'info' | 'warning' | 'error'> = {} +): WarningFlag | null => { + if (status === 'failed' || error) { + const severity = error + ? Object.entries(errorSeverityMap).find(([key]) => + error.toLowerCase().includes(key) + )?.[1] || 'info' + : 'info'; + + return { + title: error ?? defaultTitle, + severity, + }; + } + return null; +}; export const getWarningFlags = (assessment: Assessment | undefined): WarningFlag[] => { if (!assessment) { return []; } const flags: WarningFlag[] = []; + const registryFlag = createWarningFlag( + assessment.companyRegistryInformation?.status, + assessment.companyRegistryInformation?.errors, + 'Registry Information Not Available At The Moment', + { 'not supported': 'error', 'does not exist': 'error' } + ); + if (registryFlag) flags.push(registryFlag); + const structureFlag = createWarningFlag( + assessment.companyStructure?.status, + assessment.companyStructure?.errors, + 'Ownership Information Not Available At The Moment', + { 'not supported': 'error' } + ); + if (structureFlag) flags.push(structureFlag); const isHighRiskJurisdiction = assessment.companyJurisdictionRisk?.output?.level === 'HIGH'; if (isHighRiskJurisdiction) { flags.push({ title: 'High Risk Company Jurisdiction', severity: 'error', }); } return flags; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/backoffice-v2/src/domains/assessments/fetchers.ts(1 hunks)apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/KybAndOwnershipAssessmentPage.tsx(5 hunks)apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useAssessmentChecks.tsx(2 hunks)apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useKybAndOwnershipAssessmentPageLogic.tsx(3 hunks)apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/types.ts(1 hunks)services/workflows-service/src/common/utils/unified-api-client/unified-api-client.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`apps/backoffice-v2/**/*.{ts,tsx}`: Use functional components with TypeScript. Implement smart/dumb component pattern. Place components in feature-based directories. Use compound c...
apps/backoffice-v2/**/*.{ts,tsx}: Use functional components with TypeScript.
Implement smart/dumb component pattern.
Place components in feature-based directories.
Use compound components for complex UIs.
Follow atomic design principles.
Use React Query for server state and API calls.
Use Context for shared state.
Implement state machines for complex flows.
Use local state for UI-only state.
Follow unidirectional data flow.
Use strict TypeScript configuration.
Define interfaces for all props.
Use discriminated unions for state.
Leverage type inference.
Use Radix UI for accessible components.
Implement proper ARIA attributes.
Follow consistent styling patterns.
Use composition over inheritance.
Keep components small and focused.
Use React Hook Form for forms.
Implement Zod for validation.
Handle form submission states.
Show validation feedback.
Use controlled inputs when needed.
Implement proper loading states for data fetching.
Handle error states gracefully.
Cache responses appropriately.
Type API responses.
Use error boundaries.
Implement fallback UI.
Handle async errors.
Show user-friendly error messages.
Log errors appropriately.
Use React.memo wisely.
Implement proper code splitting.
Use lazy loading for routes.
Optimize re-renders.
Profile performance regularly.
Write unit tests for components.
Use React Testing Library.
Mock external dependencies in tests.
Maintain good test coverage.
Follow feature-based organization.
Use index files for exports.
Keep related files together.
Use consistent naming.
Implement barrel exports.
Use Tailwind CSS for styling.
Follow utility-first approach for styling.
Use CSS variables for theming.
Keep styles maintainable.
Use CSS modules when needed.
Document complex logic.
Write clear component documentation.
Keep documentation up to date.
Use JSDoc when helpful.
Follow ESLint rules.
Use consistent formatting.
Write clear variable names.
Keep functions pure.
Use meaningful types.
Validate user input.
Implement proper authentication.
Handle sensitive data carefully.
Follow security best practices.
Use HTTPS for API calls.
Follow WCAG guidelines for accessibility.
Use semantic HTML.
Test with screen readers.
Ensure keyboard navigation.
Provide proper focus management.
apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/types.tsapps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useKybAndOwnershipAssessmentPageLogic.tsxapps/backoffice-v2/src/domains/assessments/fetchers.tsapps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/KybAndOwnershipAssessmentPage.tsxapps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useAssessmentChecks.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_linux
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (18)
services/workflows-service/src/common/utils/unified-api-client/unified-api-client.ts (3)
215-215: LGTM! API endpoint refactoring looks correct.The change from path parameter to query parameter for assessment type is consistent with REST API best practices for filtering resources.
224-224: LGTM! Simplified endpoint structure.Removing the
/by-idsegment simplifies the endpoint structure while maintaining the same functionality through query parameters.
237-237: LGTM! Consistent payload-based approach.Moving the assessment type to the payload instead of the URL path is consistent with the other endpoint changes and follows REST conventions.
apps/backoffice-v2/src/domains/assessments/fetchers.ts (3)
47-47: LGTM! Error field addition follows consistent pattern.The
errorsfield as nullable string is appropriately typed for optional error information in the registry data.
57-57: LGTM! Consistent error field pattern.The
errorsfield incompanyStructuremaintains consistency with the pattern established incompanyRegistryInformation.
62-77: LGTM! Well-structured jurisdiction risk schema.The
companyJurisdictionRiskobject follows the established pattern with:
- Optional timestamps (
createdAt,updatedAt)- Nested
outputobject with appropriate risk level enum- Proper use of
.passthrough()and.nullable().optional()apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useKybAndOwnershipAssessmentPageLogic.tsx (3)
12-12: LGTM! Clean import addition.The import correctly adds the new
useAssessmentWarningFlagshook alongside the existinguseAssessmentChecks.
70-70: LGTM! Proper hook usage.The
useAssessmentWarningFlagshook is correctly invoked with the assessment parameter, following React hooks best practices.
90-90: LGTM! Backward-compatible extension.Adding
warningFlagsto the return object extends functionality without breaking existing consumers of this hook.apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/types.ts (1)
2-5: LGTM! Excellent refactoring to schema-driven types.This change eliminates type duplication by inferring the
Assessmenttype directly from the Zod schema, ensuring consistency between validation and TypeScript types. This follows best practices for maintaining a single source of truth.apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/KybAndOwnershipAssessmentPage.tsx (5)
3-3: LGTM! Appropriate icon imports.The new icon imports (
X,AlertTriangle,Info) align with the warning severity indicators used in theWarningBannercomponent.
12-13: LGTM! Clean imports for new functionality.The imports for
WarningFlagtype andctwutility are properly added to support the new warning banner functionality.
62-62: LGTM! Proper destructuring of warning flags.The
warningFlagsis correctly extracted from the page logic hook and will be passed to theWarningBannercomponent.
96-169: LGTM! Well-structured layout improvements.The layout restructuring with flex containers and overflow handling provides a clean foundation for the new warning banner while maintaining the existing functionality.
135-137: LGTM! Proper conditional rendering.The conditional rendering of the
WarningBannercorrectly checks for both the existence and length of warning flags before displaying.apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useAssessmentChecks.tsx (3)
39-39: LGTM: Clean data extraction.Good practice extracting the jurisdiction risk data upfront for better readability and consistent access pattern.
117-125: Excellent improvement: Dynamic risk assessment.Great refactor replacing hardcoded high-risk jurisdictions with dynamic risk level assessment. This makes the system more flexible and data-driven.
47-47: ```shell
#!/bin/bashDisplay the beginning of useAssessmentChecks.tsx to locate createCheck definition/import
sed -n '1,200p' apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/hooks/useAssessmentChecks.tsx
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| type WarningBannerProps = { | ||
| warningFlags: WarningFlag[]; | ||
| }; | ||
|
|
||
| const WarningBanner = ({ warningFlags }: WarningBannerProps) => { | ||
| if (!warningFlags.length) return null; | ||
|
|
||
| return ( | ||
| <div className="mb-6 space-y-2"> | ||
| {warningFlags.map(flag => ( | ||
| <div | ||
| key={flag.title} | ||
| className={ctw('flex items-center gap-3 rounded-md border p-2.5', { | ||
| 'border-red-200 bg-red-100': flag.severity === 'error', | ||
| 'border-warning bg-warning': flag.severity === 'warning', | ||
| 'border-gray-200 bg-gray-100': flag.severity === 'info', | ||
| })} | ||
| > | ||
| <div | ||
| className={ctw( | ||
| 'flex h-4 w-4 items-center justify-center rounded-full text-xs text-white', | ||
| { | ||
| 'bg-red-600': flag.severity === 'error', | ||
| 'bg-warning': flag.severity === 'warning', | ||
| 'bg-gray-400': flag.severity === 'info', | ||
| }, | ||
| )} | ||
| > | ||
| {flag.severity === 'info' ? '?' : 'X'} | ||
| </div> | ||
| <span className="text-sm text-gray-700">{flag.title}</span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); | ||
| }; |
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.
🛠️ Refactor suggestion
Improve accessibility and icon usage in WarningBanner.
The component implementation is good but has some areas for improvement:
- Accessibility: Missing proper ARIA attributes for screen readers
- Icon usage: Using text characters instead of the imported icons
- Semantic HTML: Could benefit from using semantic elements
Apply this diff to improve the component:
const WarningBanner = ({ warningFlags }: WarningBannerProps) => {
if (!warningFlags.length) return null;
return (
- <div className="mb-6 space-y-2">
+ <div className="mb-6 space-y-2" role="alert" aria-label="Assessment warnings">
{warningFlags.map(flag => (
<div
key={flag.title}
className={ctw('flex items-center gap-3 rounded-md border p-2.5', {
'border-red-200 bg-red-100': flag.severity === 'error',
'border-warning bg-warning': flag.severity === 'warning',
'border-gray-200 bg-gray-100': flag.severity === 'info',
})}
>
<div
className={ctw(
'flex h-4 w-4 items-center justify-center rounded-full text-xs text-white',
{
'bg-red-600': flag.severity === 'error',
'bg-warning': flag.severity === 'warning',
'bg-gray-400': flag.severity === 'info',
},
)}
+ aria-hidden="true"
>
- {flag.severity === 'info' ? '?' : 'X'}
+ {flag.severity === 'error' ? (
+ <X size={12} />
+ ) : flag.severity === 'warning' ? (
+ <AlertTriangle size={12} />
+ ) : (
+ <Info size={12} />
+ )}
</div>
- <span className="text-sm text-gray-700">{flag.title}</span>
+ <span className="text-sm text-gray-700" role="text">
+ {flag.title}
+ </span>
</div>
))}
</div>
);
};📝 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.
| type WarningBannerProps = { | |
| warningFlags: WarningFlag[]; | |
| }; | |
| const WarningBanner = ({ warningFlags }: WarningBannerProps) => { | |
| if (!warningFlags.length) return null; | |
| return ( | |
| <div className="mb-6 space-y-2"> | |
| {warningFlags.map(flag => ( | |
| <div | |
| key={flag.title} | |
| className={ctw('flex items-center gap-3 rounded-md border p-2.5', { | |
| 'border-red-200 bg-red-100': flag.severity === 'error', | |
| 'border-warning bg-warning': flag.severity === 'warning', | |
| 'border-gray-200 bg-gray-100': flag.severity === 'info', | |
| })} | |
| > | |
| <div | |
| className={ctw( | |
| 'flex h-4 w-4 items-center justify-center rounded-full text-xs text-white', | |
| { | |
| 'bg-red-600': flag.severity === 'error', | |
| 'bg-warning': flag.severity === 'warning', | |
| 'bg-gray-400': flag.severity === 'info', | |
| }, | |
| )} | |
| > | |
| {flag.severity === 'info' ? '?' : 'X'} | |
| </div> | |
| <span className="text-sm text-gray-700">{flag.title}</span> | |
| </div> | |
| ))} | |
| </div> | |
| ); | |
| }; | |
| type WarningBannerProps = { | |
| warningFlags: WarningFlag[]; | |
| }; | |
| const WarningBanner = ({ warningFlags }: WarningBannerProps) => { | |
| if (!warningFlags.length) return null; | |
| return ( | |
| <div className="mb-6 space-y-2" role="alert" aria-label="Assessment warnings"> | |
| {warningFlags.map(flag => ( | |
| <div | |
| key={flag.title} | |
| className={ctw('flex items-center gap-3 rounded-md border p-2.5', { | |
| 'border-red-200 bg-red-100': flag.severity === 'error', | |
| 'border-warning bg-warning': flag.severity === 'warning', | |
| 'border-gray-200 bg-gray-100': flag.severity === 'info', | |
| })} | |
| > | |
| <div | |
| className={ctw( | |
| 'flex h-4 w-4 items-center justify-center rounded-full text-xs text-white', | |
| { | |
| 'bg-red-600': flag.severity === 'error', | |
| 'bg-warning': flag.severity === 'warning', | |
| 'bg-gray-400': flag.severity === 'info', | |
| }, | |
| )} | |
| aria-hidden="true" | |
| > | |
| {flag.severity === 'error' ? ( | |
| <X size={12} /> | |
| ) : flag.severity === 'warning' ? ( | |
| <AlertTriangle size={12} /> | |
| ) : ( | |
| <Info size={12} /> | |
| )} | |
| </div> | |
| <span className="text-sm text-gray-700" role="text"> | |
| {flag.title} | |
| </span> | |
| </div> | |
| ))} | |
| </div> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In
apps/backoffice-v2/src/pages/KybAndOwnershipAssessmentPage/KybAndOwnershipAssessmentPage.tsx
lines 15 to 50, improve the WarningBanner component by replacing text characters
used as icons with the appropriate imported icon components, add ARIA attributes
such as role="alert" and aria-live="assertive" to the container div for better
screen reader support, and use semantic HTML elements like <section> or <aside>
instead of generic <div> to enhance meaning and accessibility.
|
Hello Guys , Can i pay someone to deploy the solution on my server , i tried my best and check a lot of developpers non was able to deploy it right . |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes