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: APP-396 two decimal digits for usd amount #2544

Merged
merged 10 commits into from
Dec 9, 2024

Conversation

r41ph
Copy link
Contributor

@r41ph r41ph commented Nov 13, 2024

Description

https://regennetwork.atlassian.net/browse/APP-396

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • provided a link to the relevant issue or specification
  • provided instructions on how to test
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

How to test

  1. https://deploy-preview-2544--regen-marketplace.netlify.app/project/mai-ndombe-4/buy
  2. Verify that the currency input when set to USD only accepts two decimals.
  3. Verify that an error is shown when the amount of USD is less than 0.50

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items
.

I have...

  • confirmed all author checklist items have been addressed
  • reviewed code correctness and readability
  • verified React components follow DRY principles
  • reviewed documentation is accurate
  • reviewed tests
  • manually tested (if applicable)

Copy link

netlify bot commented Nov 13, 2024

Deploy Preview for regen-website ready!

Name Link
🔨 Latest commit 052aff7
🔍 Latest deploy log https://app.netlify.com/sites/regen-website/deploys/67570f6de99e33000774bec1
😎 Deploy Preview https://deploy-preview-2544--regen-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

mergify bot commented Nov 13, 2024

⚠️ The sha of the head commit of this PR conflicts with #2538. Mergify cannot evaluate rules on this PR. ⚠️

Copy link

netlify bot commented Nov 13, 2024

Deploy Preview for terrasos ready!

Name Link
🔨 Latest commit 052aff7
🔍 Latest deploy log https://app.netlify.com/sites/terrasos/deploys/67570f6d2f88040008b85bcb
😎 Deploy Preview https://deploy-preview-2544--terrasos.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@r41ph r41ph requested a review from blushi November 13, 2024 13:37
@r41ph
Copy link
Contributor Author

r41ph commented Nov 13, 2024

@erikalogie one thing to consider is if we want to round up the currency amount when in USD because when we cut the decimals o two we may end up charging less than the actual credits price - let me know what you think

@blushi
Copy link
Member

blushi commented Nov 18, 2024

@erikalogie one thing to consider is if we want to round up the currency amount when in USD because when we cut the decimals o two we may end up charging less than the actual credits price - let me know what you think

@erikalogie @S4mmyb any thoughts on this?
Imagine the exact amount is $10.897, should we charge 10.89 or 10.90?

@S4mmyb
Copy link
Member

S4mmyb commented Nov 18, 2024

@erikalogie one thing to consider is if we want to round up the currency amount when in USD because when we cut the decimals o two we may end up charging less than the actual credits price - let me know what you think

@erikalogie @S4mmyb any thoughts on this? Imagine the exact amount is $10.897, should we charge 10.89 or 10.90?

I think we should round.

@erikalogie
Copy link
Collaborator

@erikalogie one thing to consider is if we want to round up the currency amount when in USD because when we cut the decimals o two we may end up charging less than the actual credits price - let me know what you think

@erikalogie @S4mmyb any thoughts on this? Imagine the exact amount is $10.897, should we charge 10.89 or 10.90?

I think we should round up to $10.90 in this case

@blushi
Copy link
Member

blushi commented Nov 20, 2024

@erikalogie one thing to consider is if we want to round up the currency amount when in USD because when we cut the decimals o two we may end up charging less than the actual credits price - let me know what you think

@erikalogie @S4mmyb any thoughts on this? Imagine the exact amount is $10.897, should we charge 10.89 or 10.90?

I think we should round.

I think we should round up to $10.90 in this case

Ok so if it were $10.894, we would charge 10.89.

@S4mmyb
Copy link
Member

S4mmyb commented Nov 20, 2024

@erikalogie one thing to consider is if we want to round up the currency amount when in USD because when we cut the decimals o two we may end up charging less than the actual credits price - let me know what you think

@erikalogie @S4mmyb any thoughts on this? Imagine the exact amount is $10.897, should we charge 10.89 or 10.90?

I think we should round.

I think we should round up to $10.90 in this case

Ok so if it were $10.894, we would charge 10.89.

Yep! <5 we round down, >=5 round up for that third decimal place.

@r41ph r41ph force-pushed the feat-APP-396-two-decimal-digits-for-usd-amount branch from c1c129a to f1eb325 Compare December 3, 2024 14:52
@r41ph
Copy link
Contributor Author

r41ph commented Dec 3, 2024

@blushi @erikalogie @S4mmyb have a look at this please

@erikalogie
Copy link
Collaborator

I think the translation string is missing for the error:
Screenshot 2024-12-03 at 11 41 28 AM

Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

Looks good overall. One thing: If I type leading zeros in the currency input, then it's only transformed into a correct value, eg 0.1 when I type the final 1, while for the credits amount input, as long as I have one leading zero, I can't get others
I think both inputs should have consistent behaviors

image

@r41ph r41ph requested a review from blushi December 4, 2024 15:46
@r41ph r41ph force-pushed the feat-APP-396-two-decimal-digits-for-usd-amount branch from 3b4fad9 to 2191b4f Compare December 5, 2024 13:11
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

Pre-approving
Pending @erikalogie to have another look and translation to be fixed:

image
this shows up when I enter an amount lower than < 50 cents

@erikalogie
Copy link
Collaborator

Pre-approving Pending @erikalogie to have another look and translation to be fixed:

image this shows up when I enter an amount lower than < 50 cents

Let me know when this is fixed and I'll test again @r41ph

@r41ph
Copy link
Contributor Author

r41ph commented Dec 9, 2024

@erikalogie please have look, the translation should be fixed now

@erikalogie
Copy link
Collaborator

LGTM

@blushi
Copy link
Member

blushi commented Dec 9, 2024

@erikalogie please have look, the translation should be fixed now

@r41ph could you run i18n:translate so it translates the missing es translations for "Must be at least"?

@r41ph r41ph merged commit 27e158b into dev Dec 9, 2024
18 checks passed
@r41ph r41ph deleted the feat-APP-396-two-decimal-digits-for-usd-amount branch December 9, 2024 15:56
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