[5.2] Cast incrementing IDs to Integer by default

[mattstauffer]

If a model is incrementing, set a default to cast the primary key to type integer.

This fixes an inconsistency where an instance created with Model::create would have an integer ID, but model instances fetched from the database would have a string ID.

Written while pairing with @adamwathan.

Also fixes another test which broke when implementing this--it was setting the primary key of an incrementing table to the value 'foo', which is invalid. Updated the test to use a non-incrementing model.

[GrahamCampbell]

I'd rather we used the more conventional int here.

[mattstauffer]

@GrahamCampbell Fixed--thanks!

[JosephSilber]

Do we really need this in every call to getCasts?

Why not do it once in the constructor?

[adamwathan]

@JosephSilber getDates works the same way, so I think either both should be done at construction or on the fly.

[JosephSilber]

@adamwathan now that we can do dates in the casts, we should be doing all of that in the constructor.

[GrahamCampbell]

getDates works the same way, so I think either both should be done at construction or on the fly.

Maybe we should change them both in 5.2 in the interest of efficiency?

[GrahamCampbell]

Maybe leave that to another PR though, to be discussed after this is either rejected or accepted.

[taylorotwell]

I agree we'll probably need to do this one time. How do we want to handle that?

[sherbrow]

This was unexpected.
Before: incrementing only meant that the insert would trigger a get to set the newly generated value in the model, and in that case where it was not a integer, the value would not change
After: incrementing means integer casting

It breaks previously non-integer primary keys when incrementing was let to the default true value.
Not the biggest change, but still worth mentioning (and incrementing seems not to be documented before that breaking change in 5.2)

Notes: Model (5.1) and Model@insertAndSetId (5.1)

Edit:
There was only a conditional cast before, in processor there
Shouldn't that test (is_numeric) be reproduced somehow instead of forcing the cast ?

[adamwathan]

@sherbrow Are there any cases where would you want $incrementing = true with a non-integer key? Or is it more a matter of not doing the cast if someone is using a non-integer key and forget to disable $incrementing?

[sherbrow]

@adamwathan It's definitely the case where one would forget to disable $incrementing, as stated, I don't think it was in the docs before 5.2.
It appears in the upgrade guide, so it's probably my fault not to take that into account in the first place.