[Laravel] Fix virtual fields metadata

[GromNaN]

Q	A
Branch?	4.0
Tickets	-
License	MIT
Doc PR	-

[GromNaN]

The primary attribute is accessed for every fields here:

core/src/Laravel/Eloquent/Metadata/Factory/Property/EloquentPropertyNameCollectionMetadataFactory.php

Lines 55 to 56 in 80fc2d2

	foreach ($this->modelMetadata->getAttributes($model) as $property) {
	if (!$property['primary'] && $property['hidden']) {

In MongoDB, "id" ends as a "virtual field" because we have an accessor for it: https://github.com/mongodb/laravel-mongodb/blob/5.0.2/src/Eloquent/DocumentModel.php#L78-L94
Maybe the metadata should be merged when a field is defined both in database schema and as a virtual field.

[toitzi]

You might also want to do this on line 83 in the same class and remove the isColumnPrimaryKey method all along, imho your solution is cleaner.

[GromNaN]

The nullable field must be a boolean, as it is given to the new Type() here:

core/src/Laravel/Eloquent/Metadata/Factory/Property/EloquentPropertyMetadataFactory.php

Lines 73 to 81 in 80fc2d2

	$type = match ($builtinType) {
	'double', 'real' => new Type(Type::BUILTIN_TYPE_FLOAT, $p['nullable']),
	'datetime', 'date', 'timestamp' => new Type(Type::BUILTIN_TYPE_OBJECT, $p['nullable'], \DateTime::class),
	'immutable_datetime', 'immutable_date' => new Type(Type::BUILTIN_TYPE_OBJECT, $p['nullable'], \DateTimeImmutable::class),
	'collection', 'encrypted:collection' => new Type(Type::BUILTIN_TYPE_ITERABLE, $p['nullable'], Collection::class, true),
	'encrypted:array' => new Type(Type::BUILTIN_TYPE_ARRAY, $p['nullable']),
	'encrypted:object' => new Type(Type::BUILTIN_TYPE_OBJECT, $p['nullable']),
	default => new Type(\in_array($builtinType, Type::$builtinTypes, true) ? $builtinType : Type::BUILTIN_TYPE_STRING, $p['nullable']),
	};

[GromNaN]

WIP: Trying to add a test on virtual field.

[soyuka]

can you run cs-fixer (inside the root of the project)? I'd like to merge this to fix virtual props

[GromNaN]

I think #6668 does the trick, but I'll leave it to you.

[domthomas-dev]

I think #6668 does the trick, but I'll leave it to you.

No, with 4.0.2, same error.

[toitzi]

@domthomas-dev #6668 was not released with 4.0.2, in order to test it you have to apply the patch yourself, i can confirm it is working

[domthomas-dev]

@domthomas-dev #6668 was not released with 4.0.2, in order to test it you have to apply the patch yourself, i can confirm it is working

Ok, but ...how ? :/ (I'm sorry, certainely it's a stupid question )

[soyuka]

I'll tag a release Friday :), @domthomas-dev easiest is to install a dev version that points on the 4.0 branch using composer repositories.

Btw, @GromNaN what's left to merge this?