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

feat: [#176299898] Amex warning (transaction >1000 EUR) #2671

Merged
merged 7 commits into from
Jan 5, 2021

Conversation

GiovanniMancini
Copy link
Contributor

@GiovanniMancini GiovanniMancini commented Jan 5, 2021

Short description

This PR adds an InfoBox in PickPaymentMethod Screen to warn AMEX users of limits affecting their transactions

List of changes proposed in this pull request

  • Added an InfoBox in PickPaymentMethodScreen

How to test

Visual Test: please see the attached images
Before delivery, a test in production with a real card and real payment is needed

  • Test 1: No AMEX, Amount > 1000 EUR

- Test 2: No AMEX, Amount < 1000 EUR

- Test 3: Si AMEX, Amount < 1000 EUR

- Test 4: Si AMEX, Amount > 1000 EUR

@pagopa-github-bot pagopa-github-bot changed the title [#176299898] Amex warning (transaction >1000 EUR) feat: [#176299898] Amex warning (transaction >1000 EUR) Jan 5, 2021
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Jan 5, 2021

Affected stories

  • 🌟 #176299898: Come CIT voglio capire perché non posso usare la mia carta AMEX per pagamenti superiori a €1000

Generated by 🚫 dangerJS against 8939a71

@codecov
Copy link

codecov bot commented Jan 5, 2021

Codecov Report

Merging #2671 (8939a71) into master (d0582ef) will increase coverage by 0.00%.
The diff coverage is 57.14%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #2671    +/-   ##
========================================
  Coverage   51.54%   51.54%            
========================================
  Files         730      730            
  Lines       20731    20738     +7     
  Branches     3967     3646   -321     
========================================
+ Hits        10686    10690     +4     
- Misses      10001    10004     +3     
  Partials       44       44            
Impacted Files Coverage Δ
...screens/wallet/payment/PickPaymentMethodScreen.tsx 49.09% <57.14%> (+1.17%) ⬆️

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 d0582ef...8939a71. Read the comment docs.

infoBoxContainer: { padding: 20, backgroundColor: IOColors.orange },
infoBoxMessage: {
color: IOColors.white,
backgroundColor: IOColors.orange
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
backgroundColor: IOColors.orange

Maybe we could avoid this background since it is already set in infoBoxContainer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing out, I'll fix it

verifica.importoSingoloVersamento >= 100000 && (
<View style={styles.infoBoxContainer}>
<InfoBox alignedCentral={true} iconColor={IOColors.white}>
<Text style={styles.infoBoxMessage}>
Copy link
Contributor

Choose a reason for hiding this comment

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

It's pretty the same but we should prefer IO typography components

Suggested change
<Text style={styles.infoBoxMessage}>
<Label weight={"Regular"} color={"white"}>
{I18n.t("wallet.alert.amex")}
</Label>

@@ -116,6 +124,16 @@ class PickPaymentMethodScreen extends React.Component<Props> {

<View spacer={true} />

{wallets.some(myWallet => myWallet.creditCard?.brand === "AMEX") &&
verifica.importoSingoloVersamento >= 100000 && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Could 10000 be a const declared outside the component?
In that way it should be easier to read/edit
https://en.wikipedia.org/wiki/Magic_number_(programming)

@Undermaken Undermaken merged commit 8e171d3 into master Jan 5, 2021
@Undermaken Undermaken deleted the 176299898-amex-warning branch January 5, 2021 14:23
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.

3 participants