-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add Bin Lookup and Brand Selector. #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
977f610 to
55e1419
Compare
55e1419 to
2d5a60a
Compare
2d5a60a to
3db305d
Compare
3db305d to
417e20c
Compare
chore: remove console.log
| const hasCoBadgedSupport = (coBadgedSupport?.length ?? 0) > 0; | ||
|
|
||
| if (hasCoBadgedSupport && coBadgedSupport) { | ||
| const validValues = Object.values(CoBadgedSupport); | ||
| const invalidValues = coBadgedSupport.filter(value => !validValues.includes(value)); | ||
|
|
||
| if (invalidValues.length > 0) { | ||
| throw new Error( | ||
| `Invalid coBadgedSupport values: ${invalidValues.join(', ')}. ` + | ||
| `Valid values are: ${validValues.join(', ')}` | ||
| ); | ||
| } | ||
| } |
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.
Should this also be wrapped in a useEffect?
| let errors = validate(value); | ||
|
|
||
| // Check if selectedNetwork is required but not set | ||
| const requiresNetworkSelection = !isNilOrEmpty(coBadgedSupport) && (brandOptionsCount ?? 0) > 1; | ||
| const networkNotSelected = requiresNetworkSelection && !selectedNetwork; | ||
|
|
||
| // Add network selection error if required but not selected | ||
| if (networkNotSelected && !empty) { | ||
| const networkError = { | ||
| targetId: type, | ||
| type: 'network_not_selected' as const, | ||
| }; | ||
| errors = errors ? [...errors, networkError] : [networkError]; | ||
| } | ||
|
|
||
| const valid = !empty && !errors && !networkNotSelected; |
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.
Should we move this to a smaller function and test it in isolation?
| useEffect(() => { | ||
| const currentState = _elementMetadata[element.id]; | ||
| const newMetadata = { | ||
| binInfo: element.binInfo, | ||
| selectedNetwork: element.selectedNetwork, | ||
| }; | ||
|
|
||
| const hasChanged = | ||
| currentState?.binInfo !== newMetadata.binInfo || | ||
| currentState?.selectedNetwork !== newMetadata.selectedNetwork; | ||
|
|
||
| if (hasChanged && onChange) { | ||
| const event = createEvent(_elementRawValues[element.id]?.toString() || ''); | ||
| onChange(event); | ||
| } | ||
|
|
||
| _elementMetadata[element.id] = { | ||
| ...currentState, | ||
| ...newMetadata, | ||
| }; | ||
| }, [element.binInfo, element.selectedNetwork, onChange, createEvent, element.id]); | ||
|
|
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.
Wondering if we should also encapsulate in a hook and test it independently 🤔
Description
Testing required outside of automated testing?
Screenshots (if appropriate):
Rollback / Rollforward Procedure
Reviewer Checklist