Skip to content
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

fix: [#173031364] The request to abort a payment is never sent #2933

Merged
merged 13 commits into from
Apr 1, 2021

Conversation

Undermaken
Copy link
Contributor

@Undermaken Undermaken commented Mar 23, 2021

Short description

This PR fixes a bug that avoids to send the delete request to abort a payment.

Here why this bug happens:

  • user asks to abort the payment
  • the component dispatches these actions
    • backToEntrypointPayment
    • runDeleteActivePaymentSaga
    • paymentInitializeState
  • backToEntrypointPayment remove from store the id of the running payment
  • runDeleteActivePaymentSaga can't find any valid payment id to delete

how the fix works
Now there is a dedicated saga that does all above actions in the right order

  • runDeleteActivePaymentSaga
  • backToEntrypointPayment
  • paymentInitializeState
Registrazione.schermo.2021-03-23.alle.15.11.37.mov

@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Mar 23, 2021

Affected stories

  • 🐞 #173031364: [minor] Quando annullo un pagamento non viene eseguito correttamente l'annulla

Generated by 🚫 dangerJS against 4b0d02a

@pagopa-github-bot pagopa-github-bot changed the title [#173031364] Request to abort a payment is never sent fix: [#173031364] Request to abort a payment is never sent Mar 23, 2021
@codecov
Copy link

codecov bot commented Mar 23, 2021

Codecov Report

Merging #2933 (4b0d02a) into master (83da2b0) will increase coverage by 0.00%.
The diff coverage is 30.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2933   +/-   ##
=======================================
  Coverage   58.77%   58.78%           
=======================================
  Files         905      905           
  Lines       24880    24882    +2     
  Branches     4584     4585    +1     
=======================================
+ Hits        14623    14626    +3     
+ Misses      10160    10159    -1     
  Partials       97       97           
Impacted Files Coverage Δ
...eens/wallet/payment/ConfirmPaymentMethodScreen.tsx 63.06% <0.00%> (+1.11%) ⬆️
...creens/wallet/payment/TransactionSummaryScreen.tsx 18.86% <0.00%> (+0.23%) ⬆️
ts/sagas/wallet.ts 37.50% <16.66%> (-0.23%) ⬇️
ts/store/actions/wallet/payment.ts 100.00% <100.00%> (ø)
ts/store/reducers/wallet/payment.ts 27.02% <100.00%> (+0.99%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83da2b0...4b0d02a. Read the comment docs.

@Undermaken Undermaken marked this pull request as ready for review March 23, 2021 14:19
@Undermaken Undermaken changed the title fix: [#173031364] Request to abort a payment is never sent fix: [#173031364] The request to abort a payment is never sent Mar 23, 2021
Copy link
Contributor

@debiff debiff left a comment

Choose a reason for hiding this comment

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

The same sequence of dispatched actions of ConfirmPaymentMethodScreen is in TransactionSummaryScreen:

const onCancel = () => {
// on cancel:
// navigate to entrypoint of payment or wallet home
dispatch(backToEntrypointPayment());
// delete the active payment from pagoPA
dispatch(runDeleteActivePaymentSaga());
// reset the payment state
dispatch(paymentInitializeState());
};

this.props.backToEntrypointPayment();
this.props.resetPayment();

Can be in the scope of this PR?

@debiff
Copy link
Contributor

debiff commented Mar 26, 2021

There is another behaviour that I'm not sure is correct:

Screen.Recording.2021-03-26.at.18.04.19.mov

The action PAYMENT_INITIALIZE_STATE is dispatched two times.

I can't find where is the unexpected dispatch but I also found a saga that maybe will be obsolete after your PR:

yield fork(paymentsDeleteUncompletedSaga);

What do you think?

@Undermaken
Copy link
Contributor Author

ConfirmPaymentMethodScreen

sure! Nice catch!
changed in 2a3c93c

@Undermaken
Copy link
Contributor Author

Undermaken commented Mar 29, 2021

There is another behaviour that I'm not sure is correct:

Screen.Recording.2021-03-26.at.18.04.19.mov
The action PAYMENT_INITIALIZE_STATE is dispatched two times.

I can't find where is the unexpected dispatch but I also found a saga that maybe will be obsolete after your PR:

yield fork(paymentsDeleteUncompletedSaga);

What do you think?

🔎
you are right
This is because the action is dispatched twice

  1. from abortRunningPaymentSaga -> yield put(backToEntrypointPayment());
  2. from watchBackToEntrypointPaymentSaga that is started from abortRunningPaymentSaga by dispatching yield put(backToEntrypointPayment());

changed in b387aab

@Undermaken Undermaken requested a review from debiff March 29, 2021 09:21
@debiff debiff merged commit 9b6a940 into master Apr 1, 2021
@debiff debiff deleted the 173031364-fix-abort-payment branch April 1, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants