CristalTeam / php-api-wrapper

:rainbow: Work with APIs like with Laravel Eloquent or Doctrine (no longer a dream)
MIT License
116 stars 32 forks source link

Setting belongsTo in Laravel to Eloquent Model #32

Open brucedevcom opened 2 years ago

brucedevcom commented 2 years ago

After updating an existing model in laravel to make use of ApiWrapper, I'm unable to load any belongsTo relationships from the model object that gets created. Trying to access them causes the following exception:

TypeError: Cristal\ApiWrapper\Relations\BelongsTo::__construct(): Argument #2 ($related) must be of type Cristal\ApiWrapper\Model, App\Models\User given

Is it possible to have a bridged model belong to an existing Eloquent model and I'm just missing something configuration-wise?

TZK- commented 2 years ago

Hello,

When you need to define relations between an API Model and a regular Eloquent model, you need to use the trait Cristal\ApiWrapper\Bridges\Laravel\HasApiRelations

This trait will override the relation methods (belongsTo, hasMany etc.), to support both API and Eloquent Models.

The documentation about relations is not ready yet (actually not really the time to do it), but I keep it in mind.

Let me know if you want more precision,

brucedevcom commented 2 years ago

Thanks for the response, I figured it involved the traits, but I had some trouble getting them going.

Since the error relates to the belongsTo method I'd assume I'd want to apply the HasApiRelations trait, though given the names of it and HasEloquentRelations I'm not too sure?

Either way I've tried various combinations of using those traits to both the Eloquent and API model classes but it doesn't even seem to be making much difference as trying to access the relationship on the API model still seems to be trying to call Cristal\ApiWrapper\Relations\BelongsTo.

I assume it's just me doing something wrong, but I'm not entirely sure what at this point!

TZK- commented 2 years ago

Can you provide the code of your Eloquent model and your API model and how your are calling the relation through your model just to be sure there are no understanding problems from my side ?

brucedevcom commented 2 years ago

Sure, they're pretty basic so far but my API Model is:

<?php

namespace App\Models;

use Cristal\ApiWrapper\Bridges\Laravel\Model;

class Site extends Model
{
    protected $primaryKey = 'uuid';
    protected $keyType = 'uuid';
    protected $entity = 'site';
    public $incrementing = false;

    public function owner()
    {
        return $this->belongsTo('App\Models\User');
    }
}

My Eloquent model is pretty much just the standard User model:

<?php

namespace App\Models;

use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Cristal\ApiWrapper\Bridges\Laravel\HasApiRelations;

class User extends Authenticatable
{
    use HasApiRelations, Notifiable;

    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        'email', 'password', 'api_token', 'username'
    ];

    public function sites()
    {
        return $this->hasMany('App\Models\Site', 'id', 'owner_id');
    }
}

As for the calling pretty much the standard approach, here's the output from testing in artisan tinker:

>>> $site = Site::find('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx');
[!] Aliasing 'Site' to 'App\Models\Site' for this Tinker session.
=> App\Models\Site {#3858
     +incrementing: false,
     +exists: true,
     +wasRecentlyCreated: false,
   }
>>> $site->owner
TypeError: Cristal\ApiWrapper\Relations\BelongsTo::__construct(): Argument #2 ($related) must be of type Cristal\ApiWrapper\Model, App\Models\User given

The reverse, trying to access the sites that belong to the user works fine. I can also see I've not mentioned, but this is with Laravel 8.83.4 and PHP 8.0.3.

TZK- commented 2 years ago

Ok, indeed we haven't implemented belongsTo relations from an API Model to an Eloquent Model.

I think you can turn into a hasOne relation instead. Can you confirm it works with that ?

I do not have enough time to dig into the issue at the moment, but I keep it in mind.

You can also try to open a pull request if you want :blush:

brucedevcom commented 2 years ago

Sure, I've just made a PR for a quick and dirty attempt. I'm going to put it through some more testing internally first

MiloDBD commented 9 months ago

Its been a while, I'm also looking for this functionality... can I help?