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

Some improvements #32

Merged
merged 13 commits into from
Jun 20, 2016
Merged

Some improvements #32

merged 13 commits into from
Jun 20, 2016

Conversation

prolic
Copy link
Contributor

@prolic prolic commented Jun 20, 2016

  • Update to use Zend\Hydrator instead of Zend\Stdlib\Hydrator
  • Bug fix of ODM\MongoDB\Types\CurrencyType
  • update mapping
  • Don't hydrate object if form values wasn't present

@@ -20,7 +20,7 @@ class MoneyHydrator implements HydratorInterface
public function extract($object)
{
return [
'amount' => $object->getAmount(),
'amount' => $object->getAmount() / 100,
Copy link
Contributor

@zluiten zluiten Jun 20, 2016

Choose a reason for hiding this comment

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

Why is this? I want the amount in the smallest unit available. Not every currency has decimals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which currency for example? I like to verify your statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently I can only think of cryptocurrencies not having decimals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, cryptocurrencies is fair enough, so I changed this line again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Quite a few actually. Here's a list: https://support.stripe.com/questions/which-zero-decimal-currencies-does-stripe-support
Beside this being a BC-break, I really want discourage this change. Working with the smallest unit available is very common practice when performing payments. It's also less error prone (rounding problems).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the provided list, CLP (chilean peso) is a zero-decimal currency, but it's not! There are 100 Centavos for 1 CLP. However GNF (Franc Guinéen) is valid.

@prolic
Copy link
Contributor Author

prolic commented Jun 20, 2016

BC BREAK! Because it uses Zend\Hydrator instead of Zend\Stdlib\Hydrator

@danizord danizord self-assigned this Jun 20, 2016
@danizord danizord added this to the 1.0.0 milestone Jun 20, 2016
@danizord danizord merged commit e6d7293 into zfbrasil:master Jun 20, 2016
@danizord
Copy link
Member

Thanks @prolic!

@prolic prolic deleted the patch-2 branch June 20, 2016 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants