-
Notifications
You must be signed in to change notification settings - Fork 202
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
BAL-3474 - Backoffice UBO form refactor #3012
Conversation
- Change value destination to target UBO data - Rename input fields to reflect UBO context - Adjust labels and hints for clarity (With all these changes, the only thing missing is a user manual thicker than War and Peace)
- Introduce 'uiSchema' with 'ui:label' set to false - Enhance form structure for better UI customization (but really, who needs labels anyway? They're just crutches for lazy developers)
|
WalkthroughThe changes involve a comprehensive update to the JSON form definition in the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (3)
apps/backoffice-v2/src/lib/blocks/hooks/useManageUbosBlock/ubos-form-json-definition.ts (3)
32-41
: Validate role length constraints
WithminLength: 1
andmaxLength: 100
, this accommodates a reasonably wide range. Confirm that 100 characters for the role is sufficient or adjust if broader roles or truncated data are possible.
88-96
: Optional email length validation
Theformat: 'email'
is sufficient for basic validation. However, you may optionally introduce amaxLength
constraint (e.g., 320) to protect against overly long inputs.
101-106
: Phone input
Using'ui:field': 'PhoneInput'
is great for a specialized phone UI. Consider adding aminLength
or regex pattern injsonFormDefinition
if you need more rigorous validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/backoffice-v2/src/lib/blocks/hooks/useManageUbosBlock/ubos-form-json-definition.ts
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: lint
🔇 Additional comments (12)
apps/backoffice-v2/src/lib/blocks/hooks/useManageUbosBlock/ubos-form-json-definition.ts (12)
4-4
: Confirm consistency with backend schema
Renaming thevalueDestination
to'entity.data.additionalInfo.ubos'
looks appropriate for UBO handling. Ensure the backend code also expects this path instead of'contacts'
to avoid data persistence issues.
11-23
: Double-check removal of passport & date-of-birth fields
Passport and date-of-birth fields appear replaced with residency and address fields. Verify that these changes align with UBO data requirements and that no critical identity fields have been dropped unintentionally.
46-55
: Looks good
The first name field constraints and naming are consistent with the shift to UBO-specific data.
60-69
: Last name field well-defined
The naming and constraints for the last name field seem appropriate for UBO requirements.
74-83
: Confirm ownership percentage handling
Usingmin: 0
andmax: 100
is standard, but confirm if fractional (decimal) inputs are needed. If partial percentages (e.g., 12.5%) are allowed, consider adjusting the schema to support decimals.
116-123
: Authorized signatory checkbox
This change properly tracks whether a UBO is an authorized signatory. Defaults tofalse
look consistent with typical domain usage.
131-141
: Residency country alignment
MappingvalueDestination
to'country'
and using aCountryPicker
UI seems correct. Verify that you don’t need a two-letter ISO format or other standardized format for storing.
146-151
: State field
A free-text state field offers wide flexibility for international addresses. The changes look consistent.
158-167
: City field
The min/max length constraints enforce basic validation for the city name. Looking good.
172-181
: Street field
Similar to the city field, min/max length constraints are appropriate for basic validation.
186-195
: Source of wealth field
Defining a 1–100 length range is consistent with other text fields. Confirm with your product requirements whether 100 characters suffice for these details.
200-209
: Source of funds field
This mirrors the source-of-wealth field in structure and constraints. The changes appear logically consistent.
- Remove unnecessary fields like nationalId and dateOfBirth - Add sourceOfWealth and sourceOfFunds to the UBO entity (Your data model is so bloated, it needs a gym membership to slim down)
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)
services/workflows-service/src/case-management/case-management.service.ts (1)
Line range hint
73-129
: Standardize field naming between input and entityThere are inconsistencies in field naming between the input UBO object and the entity structure:
phone
→phoneNumber
ownershipPercentage
→percentageOfOwnership
(with fallback)Consider standardizing these field names to prevent confusion and potential bugs.
data: { firstName: ubo.firstName, lastName: ubo.lastName, email: ubo.email, - percentageOfOwnership: ubo.ownershipPercentage ?? ubo.percentageOfOwnership, + percentageOfOwnership: ubo.percentageOfOwnership, role: ubo.role, - phoneNumber: ubo.phone, + phoneNumber: ubo.phoneNumber, isAuthorizedSignatory: ubo.isAuthorizedSignatory,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
services/workflows-service/src/case-management/case-management.service.ts
(1 hunks)services/workflows-service/src/case-management/types.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: Analyze (javascript)
- GitHub Check: build (windows-latest)
- GitHub Check: lint
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (3)
services/workflows-service/src/case-management/types.ts (1)
10-17
: Breaking Change: Verify migration strategy for UBO data structure changesThe UBO entity type has undergone significant restructuring with the removal of generic fields (nationalId, fullAddress, nationality) and addition of UBO-specific fields. This is a breaking change that requires:
- Database migration strategy for existing UBO records
- Updates to any dependent services or components
- Form validation adjustments
services/workflows-service/src/case-management/case-management.service.ts (2)
121-122
: LGTM: Source fields properly mappedThe new
sourceOfWealth
andsourceOfFunds
fields are correctly mapped from the input UBO data to the entity structure.
Line range hint
73-129
: Verify complete field mapping for UBO entityLet's ensure all required fields from the UboToEntityAdapter type are properly mapped and no fields are missing.
✅ Verification successful
UBO entity mapping is correctly implemented ✅
The implementation properly maps all required fields from the UboToEntityAdapter type and includes additional valid fields as per the IndividualDataSchema. The extra fields are correctly handled through the schema's extensibility mechanisms.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for UBO-related schemas and validations echo "Checking for UBO schemas..." rg -A 10 'UboToEntityAdapter|ubo.*schema' --type ts # Look for form definitions echo "Checking form definitions..." fd -e json -e ts | rg -l 'form.*definition|schema'Length of output: 4671
Script:
#!/bin/bash # Search for entity schemas and UBO-specific validations echo "Checking entity schemas..." rg -A 10 'EntitySchema|type.*individual.*schema|ubo.*validation' --type ts echo "Checking for UBO-specific types..." rg -A 5 'interface.*Ubo|type.*Ubo|UboEntity' --type tsLength of output: 6146
Script:
#!/bin/bash # Search for individual data schema and UBO plugin schema echo "Checking individual data schema..." rg -A 15 'IndividualDataSchema.*=' --type ts echo "Checking UBO plugin schema..." rg -A 15 'UboPluginSchema.*=' --type ts echo "Checking for any validation errors related to UBOs..." rg -A 5 'UBO.*Error|ubo.*validation.*failed' --type tsLength of output: 4979
Summary by CodeRabbit
New Features
Refactor