Validation error not displayed for Incorrect eth address in "Sent To" page #19599 #19862
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmilad75
commented
May 1, 2024
| (rf/reg-event-fx :wallet/validate-address | ||
| (fn [{:keys [db]} [address]] |
There was a problem hiding this comment.
These events were implemented for mock purposes. Now, they're not needed anymore. So I deleted them
Jenkins BuildsClick to see older builds (27)
|
mmilad75
requested review from
vkjr,
briansztamfater,
ulisesmac and
OmarBasem
and removed request for
vkjr and
briansztamfater
vkjr
reviewed
May 2, 2024
| (rf/reg-event-fx :wallet/address-validation-failed | ||
| (fn [{:keys [db]}] | ||
| {:db (assoc-in db [:wallet :ui :search-address :valid-ens-or-address?] false)})) | ||
|
|
||
| (rf/reg-event-fx :wallet/validate-address |
There was a problem hiding this comment.
Here the event is used as a container for validation logic which is not ideal. I'd suggest moving logic to a function instead and use it in place of event, wdyt?
OmarBasem
reviewed
May 2, 2024
OmarBasem
approved these changes
May 2, 2024
vkjr
approved these changes
May 2, 2024
vkjr
reviewed
May 2, 2024
| @@ -32,7 +33,15 @@ | |||
| recipient (rf/sub [:wallet/wallet-send-recipient]) | |||
| recipient-plain-address? (= send-address recipient) | |||
| valid-ens-or-address? (rf/sub [:wallet/valid-ens-or-address?]) | |||
| contacts (rf/sub [:contacts/active])] | |||
| contacts (rf/sub [:contacts/active]) | |||
| validate-address (fn [address] | |||
There was a problem hiding this comment.
Maybe it can be taken out of component for better readability, wdyt?
83% of end-end tests have passedFailed tests (7)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (43)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
|
29% of end-end tests have passedFailed tests (5)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Passed tests (2)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
|
|
hi @mmilad75 Thank you for PR. No issues from my side. Ready to be merged |
0% of end-end tests have passedFailed tests (2)Click to expandClass TestWalletMultipleDevice:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #19599
RPReplay_Final1714654918.MP4