Can't get identifiableName of foreign key relationship

[eaverdeja]

Hey Chris,

I have two models, Task and Status:

class Tarefa extends Model
{
    use SoftDeletes;
    use \Venturecraft\Revisionable\RevisionableTrait;

   (...)
   
    public function status() {
        return $this->belongsTo('App\Status', 'status_id');
    }
}

class Status extends Model
{
    use \Venturecraft\Revisionable\RevisionableTrait;

    (...)

    public function tasks() {
        return $this->hasMany('App\Task', 'status_id');
    }

    public function identifiableName() {
        return $this->description;
    }
}

And i'm looping through the history just like in the docs:

@foreach($tarefa->revisionHistory as $history )
    <li>{{ $history->userResponsible()->name }} changed {{ $history->fieldName() }} from {{ $history->oldValue() }} to {{ $history->newValue() }}</li>
@endforeach

However, i'm still getting the foreign key values in $history->oldValue() and $history->newValue().
For ex. 'John changed status_id from 1 to 2'.

Where have i gone wrong? Thanks in advance!

ps. cool package

[stierler]

Just to be sure, are you saying the revision history for Status is showing status_id? Are you expecting the status field to show the actual status name/a field from the relationship?

If so, status_id showing up is the correct behavior (as that is the actual record in the database that is changing). I have extended the Revisionable class inside of some of my projects to include pre + post save hooks that will create/modify the records, and considered putting this into a newer version.. however I need to go back through the code and fix it up/test for older versions of Laravel.

In the not too distant future, I expect to push a v2 release on revisionable with some of the pending merge requests + feature changes - v2 will be Laravel 5.3+ most likely.

If you want more info, I could paste some examples if possible.. which Laravel version are you on?

[eaverdeja]

Yes, i was expecting $history->newValue() and $history->oldValue() to trigger the identifiableName() method overwritten in the Status model, given that the actual value thats updated (status_id) is a foreign key. However, identifiableName() is never triggered :(

At least thats what i got of the docs...

Im using Laravel 5.2!

note: i am NOT following laravels naming conventions for foreign keys! The code snippet i posted has simplified names for models and fields.

[Machecek]

Any progress on this issue? I'm also curious

[cstns]

Same here!

[lasselehtinen]

I have similar case with a model that has relationship to another model twice using different foreign keys. And indeed, the identifiableName method on the product is not working in either case, it just returns the id.

class Project extends Model
{
    use RevisionableTrait, Filterable;

    public function productRelationOne()
    {
        return $this->belongsTo('App\Product', 'foobar_product_id');
    }

    public function productRelationTwo()
    {
        return $this->belongsTo('App\Product', 'foobaz_product_id');
    }
}

[S-Kis]

I got a (bad) workaround for the issue with multiple foreign keys. Maybe it helps others, maybe i got helpful feedback for a better solution.

I've written a history method in my DepartmentsController to display all revisions grouped by the created_at timestamp. The parent_id in this case is the wanted foreign key.

/**
  * Show a list of revisions of a specific department
  * 
  * @return void
  */

   public function history(Department $department)
   {
       $histories = $department->revisionHistory()->orderBy('created_at', 'desc')->get();
       $history = $histories->groupBy(function($item) {
           if ($item->key == "parent_id") {
               $item->old_value = Department::find($item->old_value)->name;
               $item->new_value = Department::find($item->new_value)->name;
           }
       }
       return $item->created_at->toDateTimeString();
   });
        
       return view('departments.history', compact('department', 'history'));
   }

It's possible to add more if ($item->key == "foreign_key_id") statements or work with a switch/case.

[robindirksen1]

@astierler any progress with this issue on how to use relationships between models?

[andrechalom]

Ran into the same problem. I'm using non-standard names for the foreign keys as well, and Revisionable does not seem to have a way around this.

To clarify, the problem arises from around here:

revisionable/src/Venturecraft/Revisionable/Revision.php

Lines 137 to 146 in 1b7c48b

	$related_model = $this->getRelatedModel();
	// Now we can find out the namespace of of related model
	if (!method_exists($main_model, $related_model)) {
	$related_model = camel_case($related_model); // for cases like published_status_id
	if (!method_exists($main_model, $related_model)) {
	throw new \Exception('Relation ' . $related_model . ' does not exist for ' . $main_model);
	}
	}
	$related_class = $main_model->$related_model()->getRelated();

This works great if your database table has, say, a "product_id" column and your model has a $model->product() relation. However, if the "product_id" is related to a $model->mainProduct() relationship, the lines mentioned above are not able to correctly assign it. And I see no way to "hint" to Revisionable what the related model should be. I'll probably use a workaround similar to what's provided by @S-Kis, but this essentially means abandoning the "magic" around identifiableName...

[andrechalom]

OK, I've ended up forking Revisionable and changing the getRelatedModel function to

    private function getRelatedModel()
    {   
        $main_model = $this->revisionable_type;
        $main_model = new $main_model();
        // searches the declared relatedModels
        if (is_array($main_model->relatedModels)) {
            // this should work with array_keys, but somehow it's not working
            foreach ($main_model->relatedModels as $k => $v) {
                if ($k == $this->key) {
                    return $v; 
                }
            }
        }
        // if none found, try to guess it

        $idSuffix = '_id';

        return substr($this->key, 0, strlen($this->key) - strlen($idSuffix));
    }   

Now I can declared public $relatedModels = ['revisionable_relation_id' => 'relationName']; in my model, and this is picked up by Revisionable. I hope someone finds this helpful too ;)

[dani2191]

@andrechalom
Thanks, that helped me. Just for future reference, because I didn't get t at first:

public $relatedModels = ['custom_foreign_key_name' => 'eloquent_relationship_method'];

So in my case I had this relationship between an Establishment and an EstablishmentType:

class Establishment extends Model
{
    protected $table = 'establishments';
    public $relatedModels = ['establishment_type_id' => 'type'];
    public function type()
    {
        return $this->belongsTo(EstablishmentType::class, 'establishment_type_id');
    }
}

class EstablishmentType extends Model
{
    protected $table = 'establishments_types';
    public function identifiableName()
    {
         return $this->typeName;
    }
}

[mellester]

@andrechalom your fork seem like a good fix