Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libs/actions/Wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ function acceptWalletTerms(parameters: AcceptWalletTermsParams) {
value: {
isPendingOnfidoResult: null,
shouldShowFailedKYC: true,
hasFailedOnfido: true,
Comment thread
allgandalf marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep hasFailedOnfido in a server-synced update path

This makes the failed-KYC marker local to the client that submitted AcceptWalletTerms. request.failureData is only applied in applyHTTPSOnyxUpdates() for the originating request (src/libs/actions/OnyxUpdates.ts), but both src/pages/EnablePayments/EnablePaymentsPage.tsx and src/pages/settings/Wallet/WalletPage/index.tsx use userWallet.hasFailedOnfido to block/relabel the wallet flow. Because this change is replacing the removed backend push, another logged-in device/session for the same account will never receive the failure state and can still show the normal “Enable wallet” path after KYC has already failed.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm @Valforte , I think this does make sense , should we push the update in Auth ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it should have a push in the backend so all devices are updated. Can you work on it as a follow up?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I see it should be done in Auth instead of this PR. What's your thought?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YEahh ,i agree the update should be sent in auth it makes more sense !, I'll raise a follow up on monday

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets put this PR on hold, i don't think we would need this PR really after we merge the auth one

},
},
{
Expand Down
Loading