From 274fa6f54608acb7207a51d37fd7f62aade12ddf Mon Sep 17 00:00:00 2001 From: Emanuele Dall'Ara <71103219+LeleDallas@users.noreply.github.com> Date: Thu, 5 Dec 2024 12:30:48 +0100 Subject: [PATCH] chore: [IOBP-1020] Using optimistic UI for remove payment method action (#6446) ## Short description This pull request introduces several changes to include an optimistic UI logic for the handling of deleted wallet cards in the wallet management system ## List of changes proposed in this pull request - Introduced a new `DeletedCard` type and updated the `WalletCardsState` type to include an optional `deletedCard` property. - Added logic to handle the `paymentsDeleteMethodAction` request, cancel, and failure actions, ensuring that deleted cards can be restored if necessary. - Modified the `selectWalletCards` selector to exclude the `deletedCard` from the list of wallet cards. - Removed the call to `walletRemoveCards` and added logic to dispatch a failure action with network error details if the deletion fails ## How to test - Try to remove a payment method with a backend error (_add a delay to the dev-server `addPaymentWalletHandler` delete function with 400 response_) - Check if the deleted method is coming back at its original position ## Preview https://github.com/user-attachments/assets/29120b17-5b40-440b-80d1-1ab0e42a5c97 --------- Co-authored-by: Alessandro --- .../details/saga/handleDeleteWalletDetails.ts | 25 ++---- .../__tests__/WalletCardsContainer.test.tsx | 19 ++-- .../wallet/store/__tests__/cards.test.ts | 90 +++++++++++++++++++ ts/features/wallet/store/reducers/cards.ts | 44 ++++++++- ts/features/wallet/store/selectors/index.ts | 7 +- 5 files changed, 157 insertions(+), 28 deletions(-) diff --git a/ts/features/payments/details/saga/handleDeleteWalletDetails.ts b/ts/features/payments/details/saga/handleDeleteWalletDetails.ts index dc8e4bc65f0..1160ced0e42 100644 --- a/ts/features/payments/details/saga/handleDeleteWalletDetails.ts +++ b/ts/features/payments/details/saga/handleDeleteWalletDetails.ts @@ -1,22 +1,15 @@ -import { put } from "typed-redux-saga/macro"; import * as E from "fp-ts/lib/Either"; +import { put } from "typed-redux-saga/macro"; import { ActionType } from "typesafe-actions"; +import { getGenericError, getNetworkError } from "../../../../utils/errors"; +import { readablePrivacyReport } from "../../../../utils/reporters"; +import { WalletClient } from "../../common/api/client"; +import { withPaymentsSessionToken } from "../../common/utils/withPaymentsSessionToken"; import { paymentsDeleteMethodAction, paymentsGetMethodDetailsAction } from "../store/actions"; -import { readablePrivacyReport } from "../../../../utils/reporters"; -import { getGenericError, getNetworkError } from "../../../../utils/errors"; -import { WalletClient } from "../../common/api/client"; -import { walletRemoveCards } from "../../../wallet/store/actions/cards"; -import { mapWalletIdToCardKey } from "../../common/utils"; -import { withPaymentsSessionToken } from "../../common/utils/withPaymentsSessionToken"; -/** - * Handle the remote call to start Wallet onboarding payment methods list - * @param getPaymentMethods - * @param action - */ export function* handleDeleteWalletDetails( deleteWalletById: WalletClient["deleteIOPaymentWalletById"], action: ActionType<(typeof paymentsDeleteMethodAction)["request"]> @@ -33,10 +26,6 @@ export function* handleDeleteWalletDetails( if (E.isRight(deleteWalletResult)) { if (deleteWalletResult.right.status === 204) { - yield* put( - walletRemoveCards([mapWalletIdToCardKey(action.payload.walletId)]) - ); - // handled success const successAction = paymentsDeleteMethodAction.success( action.payload.walletId @@ -66,6 +55,10 @@ export function* handleDeleteWalletDetails( action.payload.onFailure?.(); } } catch (e) { + const failureAction = paymentsDeleteMethodAction.failure({ + ...getNetworkError(e) + }); + yield* put(failureAction); yield* put( paymentsGetMethodDetailsAction.failure({ ...getNetworkError(e) }) ); diff --git a/ts/features/wallet/components/__tests__/WalletCardsContainer.test.tsx b/ts/features/wallet/components/__tests__/WalletCardsContainer.test.tsx index 4a9190accb2..9ecb08d55d9 100644 --- a/ts/features/wallet/components/__tests__/WalletCardsContainer.test.tsx +++ b/ts/features/wallet/components/__tests__/WalletCardsContainer.test.tsx @@ -68,14 +68,17 @@ const T_CARDS: WalletCardsState = { } }; -const T_PLACEHOLDERS: WalletCardsState = _.mapValues( - T_CARDS, - card => - ({ - type: "placeholder", - category: card.category, - key: card.key - } as WalletCard) +const T_PLACEHOLDERS: WalletCardsState = _.omit( + _.mapValues( + T_CARDS, + card => + ({ + type: "placeholder", + category: card.category, + key: card.key + } as WalletCard) + ), + "deletedCard" ); describe("WalletCardsContainer", () => { diff --git a/ts/features/wallet/store/__tests__/cards.test.ts b/ts/features/wallet/store/__tests__/cards.test.ts index 5e8bd6a8994..653edfbdc82 100644 --- a/ts/features/wallet/store/__tests__/cards.test.ts +++ b/ts/features/wallet/store/__tests__/cards.test.ts @@ -9,6 +9,9 @@ import { walletUpsertCard } from "../actions/cards"; import { selectWalletCards } from "../selectors"; +import { walletResetPlaceholders } from "../actions/placeholders"; +import { paymentsDeleteMethodAction } from "../../../payments/details/store/actions"; +import { getNetworkError } from "../../../../utils/errors"; const T_CARD_1: WalletCard = { category: "bonus", @@ -130,4 +133,91 @@ describe("Wallet cards reducer", () => { [T_CARD_1.key]: T_CARD_1 }); }); + + it("should remove placeholder cards from the store", () => { + const globalState = appReducer(undefined, applicationChangeState("active")); + const store = createStore(appReducer, globalState as any); + + const placeholderCard: WalletCard = { ...T_CARD_1, type: "placeholder" }; + + store.dispatch(walletAddCards([placeholderCard, T_CARD_2, T_CARD_3])); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + [placeholderCard.key]: placeholderCard, + [T_CARD_2.key]: T_CARD_2, + [T_CARD_3.key]: T_CARD_3 + }); + + store.dispatch(walletResetPlaceholders([placeholderCard])); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + [T_CARD_2.key]: T_CARD_2, + [T_CARD_3.key]: T_CARD_3 + }); + }); + + it("should handle paymentsDeleteMethodAction request", () => { + const globalState = appReducer(undefined, applicationChangeState("active")); + const store = createStore(appReducer, globalState as any); + + const cardKey = { + ...T_CARD_1, + key: "method_1234" + }; + + store.dispatch(walletAddCards([cardKey])); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + [cardKey.key]: cardKey + }); + + store.dispatch( + paymentsDeleteMethodAction.request({ + walletId: "1234" + }) + ); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + deletedCard: { ...cardKey, index: 0 } + }); + }); + + it("should handle paymentsDeleteMethodAction cancel and failure", () => { + const globalState = appReducer(undefined, applicationChangeState("active")); + const store = createStore(appReducer, globalState as any); + const networkError = getNetworkError(new Error("test")); + + const cardKey = { + ...T_CARD_1, + key: "method_1234" + }; + + store.dispatch(walletAddCards([cardKey, T_CARD_2, T_CARD_3])); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + [cardKey.key]: cardKey, + [T_CARD_2.key]: T_CARD_2, + [T_CARD_3.key]: T_CARD_3 + }); + + store.dispatch( + paymentsDeleteMethodAction.request({ + walletId: "1234" + }) + ); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + deletedCard: { ...cardKey, index: 2 }, + [T_CARD_2.key]: T_CARD_2, + [T_CARD_3.key]: T_CARD_3 + }); + + store.dispatch(paymentsDeleteMethodAction.failure(networkError)); + + expect(store.getState().features.wallet.cards).toStrictEqual({ + [cardKey.key]: { ...cardKey, index: 2 }, + [T_CARD_2.key]: T_CARD_2, + [T_CARD_3.key]: T_CARD_3 + }); + }); }); diff --git a/ts/features/wallet/store/reducers/cards.ts b/ts/features/wallet/store/reducers/cards.ts index 78dec7566aa..55e6ec78f8b 100644 --- a/ts/features/wallet/store/reducers/cards.ts +++ b/ts/features/wallet/store/reducers/cards.ts @@ -8,8 +8,16 @@ import { walletUpsertCard } from "../actions/cards"; import { walletResetPlaceholders } from "../actions/placeholders"; +import { paymentsDeleteMethodAction } from "../../../payments/details/store/actions"; +import { mapWalletIdToCardKey } from "../../../payments/common/utils"; -export type WalletCardsState = { [key: string]: WalletCard }; +type DeletedCard = WalletCard & { index: number }; + +export type WalletCardsState = { + [key: string]: WalletCard; +} & { + deletedCard?: DeletedCard; +}; const INITIAL_STATE: WalletCardsState = {}; @@ -47,6 +55,40 @@ const reducer = ( return Object.fromEntries( Object.entries(state).filter(([, { type }]) => type !== action.payload) ); + + case getType(paymentsDeleteMethodAction.request): { + const cardKey = mapWalletIdToCardKey(action.payload.walletId); + const deletedCard = { + ...state[cardKey], + index: Object.keys(state).indexOf(cardKey) + }; + + const newState = Object.fromEntries( + Object.entries(state).filter(([key]) => key !== cardKey) + ); + + return { + ...newState, + deletedCard + }; + } + + case getType(paymentsDeleteMethodAction.cancel): + case getType(paymentsDeleteMethodAction.failure): { + if (!state.deletedCard) { + return state; // No deletedCard to restore + } + + const { deletedCard, ...rest } = state; + // Reconstruct state with deletedCard in its original position + const restoredEntries = [ + ...Object.entries(rest).slice(0, deletedCard.index), + [deletedCard.key, deletedCard], + ...Object.entries(rest).slice(deletedCard.index) + ]; + + return Object.fromEntries(restoredEntries); + } } return state; }; diff --git a/ts/features/wallet/store/selectors/index.ts b/ts/features/wallet/store/selectors/index.ts index 0f2cbc42e2e..5a922b24b25 100644 --- a/ts/features/wallet/store/selectors/index.ts +++ b/ts/features/wallet/store/selectors/index.ts @@ -13,9 +13,10 @@ export const selectWalletPlaceholders = createSelector( ) ); -export const selectWalletCards = createSelector(selectWalletFeature, wallet => - Object.values(wallet.cards) -); +export const selectWalletCards = createSelector(selectWalletFeature, wallet => { + const { deletedCard, ...cards } = wallet.cards; + return Object.values(cards); +}); /** * Gets the cards sorted by their category order, specified in the {@see walletCardCategories} array