[Form] Add model_type option to MoneyType

[mdoutreluingne]

Fixes #19353

[mdoutreluingne]

It seems that the names of the options are arranged in alphabetical order, but I'm not sure, can you confirm this?

[OskarStark]

can you confirm this?

Yes

[mdoutreluingne]

Ok, I'll do that in another PR targeting branch 5.4

[yceruto]

The reason looks confusing to me. How is it possible to lose decimals when the divisor is 1?

Looking at the implementation, it seems more related to the fact that the integer value doesn't have any effect when the divisor is 1.

[mdoutreluingne]

I misspoke, it is in fact linked to the integer with a divisor is 1.

I find that the caution message is no longer useful. What do you think?

[javiereguiluz]

@mdoutreluingne I still don't understand this caution. But it's not because of your PR. I've checked the code and I don't understand it either. I also read this discussion and I still don't understand it: symfony/symfony#51884 (comment)

How would you explain this in a way that anybody can understand it? Thanks.

[mdoutreluingne]

@javiereguiluz

From what I've understood from looking at the implementation, an exception is thrown if the model_type is integer and the divisor is 1, because according to this condition the casting is only done if the divisor is not equal to 1, otherwise it only returns the value entered.

Me too, I don't understand why an exception is thrown if the divisor equals 1. Casting can be done perfectly well if the divisor is equal to 1.

I've added this caution message to warn the reader that if he changes the model_type for integer, he must also change the divisor (because the default divisor value is 1), otherwise an exception will be thrown.

But maybe I've added a message that is ultimately of no interest to the reader and the message of the exception is enough.

What do you think?

[javiereguiluz]

Yes, I think it's better to not add this to the docs. Developers will get a explict exception message if they do "wrong".

But, maybe we should revisit this or refactor the code to make it easier to understand. It seems that nobody truly understands this code 😐

[mdoutreluingne]

Removing caution message done.

I agree, I think it should be refactor the code to make it easier to understand.

[javiereguiluz]

I've opened symfony/symfony#53488 to see if we can finally understand the reason of this code. Thanks.

[javiereguiluz]

Thanks Maxime! I've merged this because your contribution is complete and we don't need to wait to fix symfony/symfony#53488. Thanks.