spatie / laravel-activitylog

Log activity inside your Laravel app
https://docs.spatie.be/laravel-activitylog
MIT License
5.36k stars 714 forks source link

Translatable Model event #611

Closed tariksisbacak closed 5 years ago

tariksisbacak commented 5 years ago

Hi,

I using astrotomic/laravel-translatable models. When an event is triggered on the master model. It doesn't include translate model.

Therefore, when I create separate events for both models, they become independent of the relationship.

Page - Model PageTranslation - Model

I've searched the documentation and think I should do it manually.

5 @sebastiandedeyne

What do you think ?

Thanks.

Gummibeer commented 5 years ago

Hey,

I maintain both packages. The main model save event is also fired if only translation has changed. But you can't access the translations for the changes in the activity log. So to log everything your translation model should also use the LogsActivity trait. Do you have any further questions?

tariksisbacak commented 5 years ago

Hey,

I maintain both packages. The main model save event is also fired if only translation has changed. But you can't access the translations for the changes in the activity log. So to log everything your translation model should also use the LogsActivity trait. Do you have any further questions?

All I want is to create a single log, but the system creates a log for the main model and each language.

the result has not changed again.

<?php

namespace App\Models\Page;

use Illuminate\Database\Eloquent\Model;
use Astrotomic\Translatable\Translatable;
use Spatie\Activitylog\Traits\LogsActivity;
use Spatie\Activitylog\Traits\CausesActivity;
use Illuminate\Database\Eloquent\SoftDeletes;
use Astrotomic\Translatable\Contracts\Translatable as TranslatableContract;

class Page extends Model
{
    use LogsActivity,
        CausesActivity,
        SoftDeletes;

   protected $table = 'pages';
    protected $softDelete = true;

    public $translationModel = 'App\Models\Page\PageTranslations';
    public $translatedAttributes = ['name', 'excerpt', 'content', 'slug'];

    protected $fillable = [
        'sort', 'status'
    ];

protected static $logFillable = true;
    protected static $logOnlyDirty = true;
    protected static $logName = "page";
    public function getDescriptionForEvent(string $eventName): string
    {
        return 'page.' . $eventName;
    }

}

<?php

namespace App\Models\Page;

use Illuminate\Database\Eloquent\Model;
use Spatie\Activitylog\Traits\LogsActivity;

class PageTranslations extends Model
{
    use LogsActivity;

    protected $table = 'page_translations';

    protected $fillable = ['name', 'excerpt', 'content', 'slug'];

    public $timestamps = false;

}
Gummibeer commented 5 years ago

Okay, this is the general problem we have with many relationships. #560 could be kind of a solution. To have at least a batch identifier which connects all activitylog entries. But this is primary based in the Laravel core which doesn't fire any events for many relationship changes. Which also based in it's nature that there are different ways to save them. For this special combination there could be a way to override the translatable trait, put all translation changes into a temporary array and read from there during the after listener. We already do this for the updated event https://github.com/spatie/laravel-activitylog/blob/808a3e5c119d31d1115ff7831efa866aa3fe9692/src/Traits/DetectsChanges.php#L17-L24

A similar approach could be a solution. But it won't happen in one of the packages. But I would be happy to link a package that combines these packages if you want to do one?

tariksisbacak commented 5 years ago

Overall I understood the problem. Thank you.

But I didn't understand which two packages you were talking about combining?

Gummibeer commented 5 years ago

The two you refer to 😉

So you can create a new package which requires both packages, adds a trait which uses both package traits and overrides/adds needed logic. I you do just comment the resulted package/repo here and I will link it in at least the translatable docs.

tariksisbacak commented 5 years ago

How do we proceed?

Gummibeer commented 5 years ago

I won't proceed in any of both packages because for activitylog it has to be a solution for all many relationships which we haven't found yet. And for translatable it's a coupling to another package which I won't add. It could be that I create a package in future, if not already happened. But it's not on my current roadmap. So you are free to fix it in your own app, create a package for it. If you have any questions, want a review or anything you can ask me. 😊

tariksisbacak commented 5 years ago

Thank you for your support @Gummibeer :)