kirkbushell / eloquence

A drop-in library for certain database functionality in Laravel, that allows for extra features that may never make it into the main project.
MIT License
548 stars 58 forks source link

Eager loading not working #91

Closed eempey closed 3 years ago

eempey commented 3 years ago

I'm not sure if I'm missing a step somewhere but my eager loaded belongsTo relationship breaks when I use this package. This code works without the CamelCasing trait but when I add it get an error: ErrorException Undefined property: App\Models\User::$status. My versions of PHP, Laravel and Eloquence are all > 8.



namespace App\Models;

use Eloquence\Behaviours\CamelCasing;

class User extends Authenticatable
{
    use CamelCasing;

    protected $fillable = [
        'status',
    ];

    protected $with = ['status'];

    public function status()
    {
        return $this->belongsTo(UserStatus::class, 'status', 'status');
    }
}```
kirkbushell commented 3 years ago

You have a clash of concerns here - you have a status attribute and a status relationship - you can't have both.

eempey commented 3 years ago

Thank you for the quick response! The problem persists even if I remove 'status' from the $fillable array

kirkbushell commented 3 years ago

Are you sure you're using an Eloquent model? The PHP error message suggests not.

You need to provide more code, but at this point time, I do not believe it's an Eloquence issue.

eempey commented 3 years ago

It's definitely an eloquent model (this is from a template created via Laravel Jetstream) though I suppose there could be some code in Authenticable that is not playing nice with Eloquence. This is the complete file:

<?php

namespace App\Models;

use Eloquence\Behaviours\CamelCasing;
use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Laravel\Fortify\TwoFactorAuthenticatable;
use Laravel\Jetstream\HasProfilePhoto;
use Laravel\Sanctum\HasApiTokens;

class User extends Authenticatable
{
    use HasApiTokens;
    use HasFactory;
    use HasProfilePhoto;
    use Notifiable;
    use TwoFactorAuthenticatable;
    use CamelCasing;

    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        'firstName',
        'lastName',
        'email',
        'password',
        'phoneNumber',
        'position',
        'department',
    ];

    /**
     * The attributes that should be hidden for arrays.
     *
     * @var array
     */
    protected $hidden = [
        'password',
        'rememberToken',
        'twoFactorRecoveryCodes',
        'twoFactorSecret',
    ];

    /**
     * The attributes that should be cast to native types.
     *
     * @var array
     */
    protected $casts = [
        'emailVerifiedAt' => 'datetime',
    ];

    /**
     * The accessors to append to the model's array form.
     *
     * @var array
     */
    protected $appends = [
        'profilePhotoUrl',
    ];

    /**
     * The relationships that should always be loaded.
     *
     * @var array
     */
    protected $with = ['status'];

    public function status()
    {
        return $this->belongsTo(UserStatus::class, 'status', 'status');
    }
}

And this is authenticable:

<?php

namespace Illuminate\Foundation\Auth;

use Illuminate\Auth\Authenticatable;
use Illuminate\Auth\MustVerifyEmail;
use Illuminate\Auth\Passwords\CanResetPassword;
use Illuminate\Contracts\Auth\Access\Authorizable as AuthorizableContract;
use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract;
use Illuminate\Contracts\Auth\CanResetPassword as CanResetPasswordContract;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Foundation\Auth\Access\Authorizable;

class User extends Model implements
    AuthenticatableContract,
    AuthorizableContract,
    CanResetPasswordContract
{
    use Authenticatable, Authorizable, CanResetPassword, MustVerifyEmail;
}
eempey commented 3 years ago

The other attributes are being converted properly by eloquence

kirkbushell commented 3 years ago

Can you show me how you're accessing the attribute?

kirkbushell commented 3 years ago

And the full stack trace.

eempey commented 3 years ago

stack trace: https://flareapp.io/share/4m4000Ym#F57

eempey commented 3 years ago

Accessing it like this:

class StaffController extends Controller
{
    /**
     * Display a listing of the resource.
     */
    public function index(): Response
    {
        $staffList = User::all();

        return Inertia::render('Staff',
            [
                'staff' => $staffList
            ]
        );
    }

Or I could also remove the $with from the model and do $staffList = User::with('status')->get() in the Controller and get the same result. Both approaches work without eloquence

kirkbushell commented 3 years ago

Accessing it like this:

class StaffController extends Controller
{
    /**
     * Display a listing of the resource.
     */
    public function index(): Response
    {
        $staffList = User::all();

        return Inertia::render('Staff',
            [
                'staff' => $staffList
            ]
        );
    }

Or I could also remove the $with from the model and do $staffList = User::with('status')->get() in the Controller and get the same result. Both approaches work without eloquence

This doesn't tell me much. I suspect it's the User::all() ? Yet stack trace doesn't support that, appears to be done via the auth guard. Eloquence isn't doing anything other than deferring the call to getRelationValue.

Is there a status column on users?

eempey commented 3 years ago

Yes. There is also a user_statuses table with columns status(primary key) and description

kirkbushell commented 3 years ago

So I think part of the issue is the relationship name. It seems to me that it's trying to fetch an attribute called status, but you have a relationship called status. If you rename the relationship name, I think it'll work.

eempey commented 3 years ago

Good call! I renamed the status column on both tables to status_id and it's working. Thank you for taking the time!

kirkbushell commented 3 years ago

That is probably a better standard to follow anyway ;)

Glad to help.