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

[BUG]: Attributes are not automatically cast to their column types #74

Open
maxrice opened this issue Sep 8, 2024 · 4 comments
Open
Assignees
Labels
development help wanted Extra attention is needed v4
Milestone

Comments

@maxrice
Copy link

maxrice commented Sep 8, 2024

Describe the bug

AFAIK, Eloquent will cast attributes to the column type as defined in the table. When adding a custom model with say, an integer column, the attribute type when accessing it like $instance->attribute will always be a string instead of integer.

Steps to reproduce the issue

Create a custom model:

class Test extends AbstractModel {
	
	protected $table = 'test';
}

and a matching table:

CREATE TABLE test (
  PRIMARY KEY  (id),
  id bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT,
  test_count int(20) UNSIGNED DEFAULT 0 NOT NULL,
)

Insert a record into the table:

INSERT INTO test (test_count) VALUES (1234);

Then, load the model and dump the integer column:

$test = Test::find( 1 );
var_dump( $test->test_count );

And you'll get:

string(4) "1234"

Interestingly, if you add the column to the $casts property, like:

protected $casts = [
	'test_count' => 'integer',
];

then you get the expected behavior:

int(1234)

IMO that's an acceptable workaround if there's no way to have Eloquent automatically cast the column values, due to a conflict with $wpdb or something.

Your setup

  • WordPress Version: 6.6.1
  • Module version: 3.2.0
  • Are you using framework ?: no
@maxrice maxrice added the bug Something isn't working label Sep 8, 2024
@dimitriBouteille dimitriBouteille added this to the 4.x.x milestone Sep 9, 2024
@dimitriBouteille
Copy link
Owner

Hey @maxrice

Thanks for opening this issue. I did not know this feature of Eloquent, I have always used the $cast property.

Since this is a correction that may impact the functioning of the library, I prefer to fix this bug in the next major release: 4.0.0. I hope to release the 4.x.x in november.

As stated at the beginning of the issue, the easiest way is to go through the $casts property.

Best,

@maxrice
Copy link
Author

maxrice commented Sep 9, 2024

@dimitriBouteille yeah it seems like it depends on the driver being used, based on laravel/framework#11780, anyhow. So it might be worth some additional investigation when you start working on the next major release, but not a big deal to use the $casts property either. thanks!

@dimitriBouteille
Copy link
Owner

Hi @maxrice

I’m looking to fix this problem. However, I don’t see any reference in Laravel or Eloquent that automatically cast an attribute according to the type of column.

I tried to install a Laravel project to see if there is a difference, and laravel also uses the manual cast system.

When installing Laravel, the User class is automatically created and uses the casts function :

<?php

namespace App\Models;

// use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;

class User extends Authenticatable
{
    use HasFactory, Notifiable;

    /**
     * The attributes that are mass assignable.
     *
     * @var array<int, string>
     */
    protected $fillable = [
        'name',
        'email',
        'password',
    ];

    /**
     * The attributes that should be hidden for serialization.
     *
     * @var array<int, string>
     */
    protected $hidden = [
        'password',
        'remember_token',
    ];

    /**
     * Get the attributes that should be cast.
     *
     * @return array<string, string>
     */
    protected function casts(): array
    {
        return [
            'email_verified_at' => 'datetime',
            'password' => 'hashed',
        ];
    }
}

When I disable the cast of the email_verified_at property, it is a string type that is returned :

/** @var \App\Models\User $user */
$user = \App\Models\User::find(1);
dd($user, $user->email_verified_at, $user->getAttribute('email_verified_at'));

Capture d’écran du 2024-09-28 15-46-40

Do you have a link to documentation that talks about this automatic cast?

@dimitriBouteille dimitriBouteille added help wanted Extra attention is needed and removed bug Something isn't working labels Sep 28, 2024
@maxrice
Copy link
Author

maxrice commented Sep 28, 2024

@dimitriBouteille It doesn't seem to be mentioned explicitly in the docs, though I did find a few mentions in blog posts about how this is done "automatically". From re-reading laravel/framework#11780, I think this happens in the database driver itself, not Laravel, so it probably can't be "fixed" in WP ORM; could consider adding a note to the docs about using the $casts property?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development help wanted Extra attention is needed v4
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants