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

[IOCOM-1557] New payment flow from a message's details #5899

Merged
merged 9 commits into from
Jun 25, 2024

Conversation

Vangaorth
Copy link
Contributor

@Vangaorth Vangaorth commented Jun 25, 2024

Short description

This PR adds the new payment flow when starting from a message (standard or SEND). It is triggered only if the related feature flag is active.

The payment code that handles new payment flow does not support SEND messages or new DS ones (it is compatible only with the legacy standard message). In order to meet the deadline, such code had to be duplicated and changed to support both SEND and new standard messages. It is also uncovered by automatic testing.

List of changes proposed in this pull request

  • ts/features/payments/checkout/tempWorkaround/pagoPaPaymentWorkaround.ts: duplicated code to support SEND and new standard messages (the main difference is that it does not rely upon being callend inside a React Component, so not having a direct dependency upon useDispatch and useNavigation hooks)
  • ts/features/messages/utils/index.ts: entry point for the new flow for SEND and new standard messages
  • ts/features/messages/components/MessageDetail/PaymentButton.tsx: entry point for the old standard messages
  • New entry point callers (SEND messages and new standard ones):
    • ts/features/messages/components/MessageDetail/MessageDetailsPaymentButton.tsx
    • ts/features/messages/components/MessageDetail/MessagePaymentItem.tsx
    • ts/features/pn/components/MessageFooter.tsx

How to test

Using the io-dev-api-server on this branch IOBP-698-mock-redirect-method, generate messages with payment and SEND messages with payment(s). For each combination of the new DS and the new Wallet, check that the flow work as intended. Exhaustive list of use-cases is:

  • DS off, Wallet off, start payment from the CTA in a standard message;
  • DS off, Wallet off, start payment from the payment list in a SEND message;
  • DS off, Wallet off, start payment from the payment button in a SEND message;
  • DS on, Wallet off, start payment from the CTA in a standard message;
  • DS on, Wallet off, start payment from the payment list in a standard message;
  • DS on, Wallet off, start payment from the payment list in a SEND message;
  • DS on, Wallet off, start payment from the payment button in a SEND message;
  • DS off, Wallet on, start payment from the CTA in a standard message;
  • DS off, Wallet on, start payment from the payment list in a SEND message;
  • DS off, Wallet on, start payment from the payment button in a SEND message;
  • DS on, Wallet on, start payment from the CTA in a standard message;
  • DS on, Wallet on, start payment from the payment list in a standard message;
  • DS on, Wallet on, start payment from the payment list in a SEND message;
  • DS on, Wallet on, start payment from the payment button in a SEND message;

@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Jun 25, 2024

Warnings
⚠️ An error occurred during the retrieval of a ticket, the title of the pull request is updated taking into account only the recovered tickets
⚠️

There was an error retrieving a ticket: value ["Incident"] at [root.fields.0.issuetype.name] is not a valid ["Epic" | "Story" | "Task" | "Sottotask" | "Sub-task" | "Subtask" | "Bug"]

Generated by 🚫 dangerJS against 9750d0a

Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 51.85185% with 13 lines in your changes missing coverage. Please review.

Project coverage is 49.57%. Comparing base (4f204b4) to head (1d804b0).
Report is 208 commits behind head on master.

Current head 1d804b0 differs from pull request most recent head 9750d0a

Please upload reports for the commit 9750d0a to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5899      +/-   ##
==========================================
+ Coverage   48.42%   49.57%   +1.15%     
==========================================
  Files        1488     1703     +215     
  Lines       31617    33748    +2131     
  Branches     7669     8209     +540     
==========================================
+ Hits        15311    16732    +1421     
- Misses      16238    16955     +717     
+ Partials       68       61       -7     
Files Coverage Δ
...ents/MessageDetail/MessageDetailsPaymentButton.tsx 75.00% <100.00%> (+3.57%) ⬆️
...es/components/MessageDetail/MessagePaymentItem.tsx 93.33% <100.00%> (+0.11%) ⬆️
ts/features/pn/components/MessageFooter.tsx 66.66% <100.00%> (ø)
ts/navigation/NavigationService.ts 80.00% <ø> (ø)
ts/features/messages/utils/index.ts 97.77% <83.33%> (-2.23%) ⬇️
...essages/components/MessageDetail/PaymentButton.tsx 82.60% <50.00%> (-11.84%) ⬇️
...checkout/tempWorkaround/pagoPaPaymentWorkaround.ts 25.00% <25.00%> (ø)

... and 792 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

Copy link
Contributor

@Hantex9 Hantex9 left a comment

Choose a reason for hiding this comment

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

LGTM.

Considering the Jira ticket instructions to refactor the utility function by replacing it with a custom hook and removing the workaround, the flow is working as expected.

@Vangaorth Vangaorth merged commit 48abc55 into master Jun 25, 2024
11 checks passed
@Vangaorth Vangaorth deleted the IOCOM-1557_newPayFromMes branch June 25, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants