Laravel-auditing: Not logging Affected Relations?

Created on 1 Jun 2016  路  95Comments  路  Source: owen-it/laravel-auditing

Is it correct that any updated relations are not being logged in the database?
I'm using eloquent's sync() function a lot, for updating many-to-many relations...

Any ideas how to do that?

enhancement help wanted

Most helpful comment

I strongly vote for this feature. I'm currently working on a custom auditor that would record (to a seperate table) the foreign key relationships relations to a separate table, then I extend $Model->getAuditArr() to pull it all together the way we want. Lots of DB hits and in general, not a perfect solution. Will switch off this when this feature is implemented.

All 95 comments

The auditing currently uses events, but in case of pivots, there are no events being triggered. There are many issues reported regarding this. See for example here: https://github.com/laravel/framework/issues/8452 .
I also need this and currently did not find a very nice way to handle it.

Can you not just override those methods like sync, attach and detach? Or even better would be a chain on the relationship in some way, will explain more if I come up with a way of doing this. Trying out this package and I think we will need it to work for this and we can work around it easy enough, if a workaround is good enough we will make a PR.

Sorry for the delay.

Hello @robclancy , let's think of something. I'll keep this open to give continuity to this improvement.

yes, 'sync attach detach' donot log,
4.x support it?

@fly-studio, as soon as version 4.0 is out the door, we'll try to work on many-to-many audit support for 4.1.

When will release 4 version? 馃槂

@rorc, when it's done (i.e. when I finish adding tests)

Meanwhile, you already can use "owen-it/laravel-auditing": "4.0.x-dev" in your composer.json.
The v4 documentation is available here.

@quetzyg Ok, thanks 馃槃

I strongly vote for this feature. I'm currently working on a custom auditor that would record (to a seperate table) the foreign key relationships relations to a separate table, then I extend $Model->getAuditArr() to pull it all together the way we want. Lots of DB hits and in general, not a perfect solution. Will switch off this when this feature is implemented.

How progress here? 馃榾

+1 still waiting for the best solution

considering using this package but I would need pivot auditing

tenho o mesmo problema

Experimental package for events in many to many relations:

https://github.com/neylsongularte/eloquent-extra-events

My temporary solution:

// Add on your AppServiceProvider
$saveManyToManyAudit = function ($event, $eventData) {
            $audit = new Audit;
            $audit->user_id = Auth::check() ? Auth::user()->getAuthIdentifier() : null;
            $audit->event = $event;
            $audit->auditable_id = $eventData['parent_id'];
            $audit->auditable_type = $eventData['parent_model'];
            $audit->old_values = [];
            unset($eventData['parent_id']);
            unset($eventData['parent_model']);
            $audit->new_values = $eventData;
            $audit->url = App::runningInConsole() ? 'console' : Request::fullUrl();
            $audit->ip_address = Request::ip();
            $audit->user_agent = Request::header('User-Agent');
            $audit->created_at = Carbon::now();
            $audit->save();
        };

        $this->app['events']->listen('eloquent.synced*', function ($eventData) use ($saveManyToManyAudit) {
            if($eventData['changes']['attached'] || $eventData['changes']['detached'] || $eventData['changes']['updated']) {
                $saveManyToManyAudit('synced', $eventData);
            }
        });

        $this->app['events']->listen('eloquent.attached*', function ($eventData) use ($saveManyToManyAudit) {
            $saveManyToManyAudit('attached', $eventData);
        });

        $this->app['events']->listen('eloquent.detached*', function ($eventData) use ($saveManyToManyAudit) {
            $saveManyToManyAudit('detached', $eventData);
        });

@anteriovieira

Any update concerning this problem?

Hi everyone,

I've been thinking in a few ways to achieve this, and I'm inclided to leave it up to the developer to implement part of the functionality.

The package can already handle other events besides created, updated,deleted and restored, and like Taylor suggests, the attach/detach/sync logic should be handled in a repository, in which the proper event is fired after the DB operation happens.

So, the idea is to add three new AuditableObserver methods: attached(), detached() and synced()

The developer would be responsible for firing/dispatching said events and pass the data they wish to audit/log.

From that point on, the appropriate AuditableObserver method would kick in and handle things.

I think this brings a good balance between new feature and code complexity.

Thoughts?

@quetzyg Sounds good to me! Because there won't be an official support concerning the events, I think that's a very good way, especially because the developers are now "unrestricted".

But please provide an example for the implementation in the docs 馃槢

@manniL, yeah, the idea is to keep it flexible, because I have no idea how developers want to store changes, specially when we're talking about sync events, which may affect more than one row in a pivot table. Do they want to create an audit with all the row changes or do they want to audit each row individually? There's no one-size-fits-all in this case.

And don't worry, the docs will be updated once this gets implemented.

@quetzyg That's a good plan!

Well, in my case I'd simply store the changes (e.g. User A Was linked to posts 1,2,3 and is now linked to Post 4,6,7, I'd just change the stored numbers). If the relation models itself were changed, it'd be fine if there would be an extra audit model for that.

For a simple case like the one you described it's fine to use just one audit record, but when you have extra data in the pivot or when you detach hundreds of relations in one go, things would get out of hand very quickly and stuffing all that data in just one record would make it hard to track changes.

But again, I'll leave that decision to the developer.

@quetzyg Yes, this can become very hairy in situations like the one you described. But as the developers are flexible, everyone will adapt it to their needs :)

I think I have a solution for issue. Are you accepting pull requests??? It has, I admin, some of the data issues stated above. I can have it done in acceptable form in a couple of Saturdays

@jschlies, sure. Feel free to send a PR and we'll review it.

I'm very curious! Good luck @jschlies and hit me up when u need help :)

:+1:

Hey guys, any progress so far?

I have done some work in this regard, but there's no ETA. You'll have to be patient 馃槃

I'm actively needing auditing for detach(), save() and sync() too at the moment, so I'm really keen to see what you are working on @quetzyg :)

See PR https://github.com/owen-it/laravel-auditing/pull/289
I'm going to try to give this some of my time. I hope I have a 'merge-able' branch in a few months. This PR is my work so far.

Please let me know if I'm on the right track.

Going forward, I want to:

  • create a separate table (audit_related_relations) which will (on eloquent.synced, eloquent.attached and eloquent.detached*) record the One To One, One To Many and Many To Many. Has Many Through, Polymorphic Relations and Many To Many Polymorphic Relations are going to be phase II. IN my PR, this is currently in audits.json_data
  • Develop a system/convention/config that limits which objects get audit_related_relations, do not get audit_related_relations or get audit_related_relations via a queued Job, and for the object in question, which relationships get audit_related_relations, do not get audit_related_relations or get audit_related_relations via a queued Job. This limited is important since it is easy to see how this can explode into huge amounts of processing.
  • Develop a system/convention/config that limits the size/depth of the resulting $Object->getaudit() object. This limited is important since it is easy to see how this can explode into huge amounts of processing and extremely large objects.
  • -

@quetzyg

I'm not a fan of using a second audit table. You also talk about a convention to limit what gets audited and what doesn't. I also don't like that idea. Read my comment about letting the developer decide which data should be audited.

Regarding "convention to limit what gets audited ", I plan on making , per object, either an entry in the config file OR a method call to that object. Completely configurable by developer.

I think I can do without a second table. The second table was easier when I was just coding for our own needs. I just need to fight through the key constraints and such. After some progress, I'll get back to you.

FYI, I think I'm not half way.

See latest:

  • Develop a system/convention/config that maps which objects get audit_related_relations, how those related objects are retrieved and the meth0d within the related obj that accesses the relatER, do not get audit_related_relations or get audit_related_relations via a queued Job, and for the object in question, which relationships get audit_related_relations, do not get audit_related_relations or get audit_related_relations via a queued Job. This limited is important since it is easy to see how this can explode into huge amounts of processing.
  • Develop a system/convention/config that limits the size/depth of the resulting $Object->getaudit() object. This limited is important since it is easy to see how this can explode into huge amounts of processing and extremely large objects.
  • -

I've added a migration to add related_id. I notice that there is no migration for updated_at. I have added this to my migration for now. Suggestions as to how to handle this?????

@jschlies, please keep the conversation about your PR in the actual PR comment section.

:+1:

So, the idea is to add three new AuditableObserver methods: attached(), detached() and synced()

How we can now extend AuditableObserver for add this methods?

But, if i dont want add new audit record for relation sync?
Before save my model, i try to add relation, but plugin ignoring new key (relation_name):

$model->new_values = collect($model->new_values)->put('relation_name', [1, 2, 3]);
$model->save();

@GrahamCampbell, can you give us guidance on this? :pensive:

Hey everyone,

so after reading the comments and ideas in here, I set out to get this working for me.

What I did was, I got a generic piece of code (call it a repository if you will) which handles attaching, detaching and updating pivots for any many-to-many relationship (ie. for any model pair that has a many-to-many relationship between them). In there, I start by storing the related models before any operation, do the operation and then get the updated related models. Finally, I fire up an event named eloquent.many2many.attached, eloquent.many2many.detached or eloquent.many2many.updatedPivots with these 2 arrays (the initial and updated related models) as data.

I defined an Event listener for eloquent.many2many.* (which allows me to catch only "my" events) where I store the initial and updated related models in the model being audited itself (ie. the "parent" mode whose relationships are being updated), call Auditor::execute() and in the auditAttachedAttributes, auditDetachedAttributes and auditUpdatedPivotsAttributes methods I simply do:

$old = $this->initialRelated;
$new = $this->updatedRelated;

Which allows me to get the wanted audit rows as I want them, ie. one per attach/detach/pivot update with the before and after related models. So far so good.

My problem is that when looking at a certain auditable model's audits (for "my" events) I have no way of knowing what related models those audits refer to. I have the before and after related models, but I don't know the type of those models.

I understand I could get this piece of information by having different events for different related models (eg. attachedModelA and attachedModelB), where the event name itself would tell me the related model's type, but that would force my generic many-to-many audit handling not to be generic anymore, since I would have to implement the audit*Attributes methods for all the possible events, ie. auditAttachedModelAAttributes, auditAttachedModelBAttributes, etc. This would indeed defeat the purpose of a generic approach as the one I'm hoping to achieve.

For a proposed solution... Maybe the $auditableEvents parameter could accept * like Laravel's Event listeners themselves accept? This, together with my suggestion in #320 would solve my problem, as I could define attached*, detached* and updatedPivots* as auditable events and make them all be handled by a generic method such as the one I have now (while still allowing different event names).

I did my 2 suggestions in https://github.com/owen-it/laravel-auditing/pull/324 - they do solve my problem!

@daniel-gomes-sociomantic ,
I think I get the above approach and I suggest it has merit. My comments are:

  • You seem to be limited to Eloquent relations. Our need included 'relations' that were outside the standard Eloquent relation.
  • I'm don't see how you limit which relations to ausit. I can foresee circumstances where you're not interested in auditing ALL many-to-many. Or is this logic found in auditAttachedModelBAttributes????
  • Would you logic work if a particular model was attached - detached - RE-attached???

You seem to be limited to Eloquent relations. Our need included 'relations' that were outside the standard Eloquent relation.

Technically, since I'm firing the events manually, it would be possibly to do so for non-eloquent relations, but not sure how the auditing package itself would handle them..

I'm don't see how you limit which relations to ausit. I can foresee circumstances where you're not interested in auditing ALL many-to-many. Or is this logic found in auditAttachedModelBAttributes????

As I said, I made all attaching/detaching/pivot updating be handled by a generic piece of code (a trait, to be specific) which is then used to update the many-to-many relationships I want audited. It would be simple enough to adapt the trait to allow handling the many-to-many relationship without firing the audit events.

Would you logic work if a particular model was attached - detached - RE-attached???

The way I implemented it, that would cause 3 audits, one for the attaching, one for the detaching and one for the re-attaching, each with their initial and updated states.

Thanks @daniel-gomes-sociomantic , I'm not sure I'm following 100% percent but it sounds like you've thought through the pitfalls we experienced.
I still believe in the efficacy of https://github.com/owen-it/laravel-auditing/pull/321 and I leave things to the group.

Would definitely like to be able to integrate auditing of attach/detach/sync events for related models into a project I'm currently taking over. I'm relatively new to laravel (and modern PHP, in general) and so far, laravel-auditing is one of my favorite libraries. This one additional (albeit non-trivial) functionality would elevate it to epic.

any news on this ?

Not yet.

Any news or release date?

Hi @marcelogarbin, thanks for your interest on the project!

Unfortunately, I cannot commit to a release date for this feature. There were a couple of PR's made, but to be honest, I didn't like the logic or the code.

I also have an internal branch I'm keeping up to date with my own implementation, but it still isn't exactly the way I like.

In case someone wants to sponsor this feature, I'll work exclusively on it, instead of doing it on my spare time, because Family > Work > Open Source.

We'll keep this open.

Thank you all!

Thanks for the quick feedback.
I started using the package today and I just noticed it for now.
I still have no deep knowledge of Laravel and the PHP testing part to be more consistent in helping you.
However, I'm at your disposal.

@quetzyg Since you are working on one behind the senes, what would you say your completion rate is thus far and how much more time would be needed?

@johnpaulmedina, the development of this feature is currently stalled and I have no idea when I'm gonna pick it up. If working exclusively, it could probably take me a month to have it finished (with planning + experimenting, development, documentation and tests).

But like I said a few comments back, priorities :)

@quetzyg As your development on this feature is private, is there something we could do to support you? Maybe you could share your progress and some instructions on what needs to be done?

I'm currently not working on this, but I'll probably revisit this soon. I am however, finishing up a new feature based on suggestions made in #395.

This might be of interest: https://github.com/fico7489/laravel-pivot

I wrote package, that covers all types of eloquent relationships, it covers previous link and adds much more.
Laravel Relationship Events

@chelout could you explain how to use your package to make sure this package created audit entries?

@OwenMelbz didn't get you, do you mean, how to use my package with laravel auditing?

@chelout yes,

For example, I've installed the package, added the several traits to the parent model e.g User

What else? - As when I update the relationships e.g $user->books()->attach(1); it does not create an audit entry.

Are there extra steps required to make it work together to make sure the audit log contains the changes to the relations?

@OwenMelbz for now this packages doesn't work together. My package is just a proposal to solve current issue. It is necessary to implement functionality in laravel auditing in order to use may package.

For my needs i wrote some traits that helps me to store audits for needed events.

@OwenMelbz fancy seeing you here. The major issue if it hasn't been discussed is Laravel doesn't fire events on pivot changes. I didn't find @chelout's package in my searching but found (https://github.com/fico7489/laravel-pivot) and created a trait that extended Laravel Auditing basically. Even when I added the 'events' that the Laravel Pivot package registers it wouldn't work so i choose a custom trait.

https://gist.github.com/AbbyJanke/9a1615651534a4c7b94f0d3b03144ce7

When i tried using the standard Auditor saving it failed for whatever reason so i created a second table (audits_pivot) and joined them, still working on making the join work so i excluded some code. But you get the idea of it.

For those interested and @quetzyg if you want to add it to the package here is my full code which is working and tracks the ID of the related model that is being attached/detached.

https://gist.github.com/AbbyJanke/4d245b22dbcec277c207f033f37dae3b

here is another version based on @AbbyJanke gist https://gist.github.com/ctf0/fa929221fb801e337ad0c03ac16b27c7

now the remaining part is to add the data from the events to the audit record b4 its created

Update

to get around the issue of saving the relation data along the original audit, you can instead use a timestamp check b4 rending the diff ex.

// revision item date
// foreach($revisions as $rev)
$revision_date = $rev->created_at->toDateTimeString();

// relation item date
// foreach($rev->relations as $rel)
$relation_date = $rel->created_at;

if($relation_date === $revision_date) {
    // render your data
}

but you still have the problem of not able to restore the relation changes because the original audit have no track of them

Update 2

check https://github.com/ctf0/Odin

is compatible for events relationships? chelout/laravel-relationship-events

Not using this package yet, as I'm going to run into this issue and doing research on it. Would like to see this implementation 馃憤

I find this package of great help and sorted out the triggering of attach, detach and sync events for pivot tables through some research and workarounds;

My current issue is that when I am calling $model->audits I am getting all the entries related to created, updated, etc events but not the entries from event DB with values different than this package's defaults.
Any ideas on how I could work around this?

@thitami if you've hit a snag, please create a new issue and fill in the template with all the info to reproduce said problem.

Thanks, @quetzyg.
Will do shortly.

@AbbyJanke @ctf0
Are the attributes saved? How I get this?

@eumanito yes, check odin

I think the missing feature to make it work (model events on pivot tables) is now in Laravel 5.8

https://laravel.com/docs/5.8/releases

Intermediate Table / Pivot Model Events
In previous versions of Laravel, Eloquent model events were not dispatched when attaching, detaching, or syncing custom intermediate table / "pivot" models of a many-to-many relationship. When using custom intermediate table models in Laravel 5.8, these events will now be dispatched.

Here is the PR:
https://github.com/laravel/framework/pull/27571

@milhouse1337 Thank you for notifying. I will test in my application.

@eumanito did you make it work with 5.8 Im lookin to implement this

@LeonAlvarez not yet. When I do, I will post it here.

Is the pivot observer already implemented in 5.8?

Yes, @milhouse1337 shared here, take a look.

I think the missing feature to make it work (model events on pivot tables) is now in Laravel 5.8

https://laravel.com/docs/5.8/releases

Intermediate Table / Pivot Model Events
In previous versions of Laravel, Eloquent model events were not dispatched when attaching, detaching, or syncing custom intermediate table / "pivot" models of a many-to-many relationship. When using custom intermediate table models in Laravel 5.8, these events will now be dispatched.

Here is the PR:
laravel/framework#27571

Can you please explain more how to use with audit?

Hi.
As I understand, it is not yet implemented auditing many to many relations, right?

Helo I also need this function for a projekt, is there news?

I found a beter pakage https://altek.gitlab.io/accountant it works out of box for pivot relations super easy and no hacks

@waska14 I have not implemented it yet

@PizzaTibe Thanks for contributing, I will test this package.

Laravel 5.8 now fires eloquent events if a custom Pivot model exists. Therefore, it should be possible to get pivot audits by

1) Creating a custom pivot model
2) Adding Auditable Trait
3) Append the pivot model audit to the main model(s) audits manually in the model for display purposes.

Maybe this should be added somewhere in the docs because this seems like it is a common question.

I can confirm it works in Laravel 5.8.
As far as I can tell, your pivot table needs to have its own incrementing id and you need to set public $incrementing = true; on the pivot model. Otherwise you'll run into sql integrity constraint violations, since no auditable_id is passed.

(https://laravel.com/docs/5.8/eloquent-relationships#defining-custom-intermediate-table-models)

I'm surprised there has been no follow-up here since August, as I'm unable to get this working. It seems like, even if you have $incrementing = true on your pivot model, Laravel doesn't include the model ID, and thus the Auditing package is unable to use it. Has anybody been successful in getting this package to work with custom pivot models?

I gave up. To work I'm using this package: https://github.com/fico7489/laravel-pivot

I gave up. To work I'm using this package: https://github.com/fico7489/laravel-pivot

It looks like it _was_ working in version 9.3.1 but then regressed in the latest release. Put an issue up about it, hopefully we can get it working again.

I can confirm it works in Laravel 5.8.
As far as I can tell, your pivot table needs to have its own incrementing id and you need to set public $incrementing = true; on the pivot model. Otherwise you'll run into sql integrity constraint violations, since no auditable_id is passed.

(https://laravel.com/docs/5.8/eloquent-relationships#defining-custom-intermediate-table-models)

Can you provide a gist ?

@quetzyg do you have any plans on bringing the pivot auditing capability from your package Accountant into this one?

@quetzyg do you have any plans on bringing the pivot auditing capability from your package Accountant into this one?

I do not, but you can always use Accountant, instead.

@quetzyg saving everything for one field change in a table with 30+ columns is overkill for my use-case, especially when 1 or 2 fields change frequently, so I prefer Auditing's flexibility.

@quetzyg How to fire a relation?
I have a translatable model and the $translatedAttributes doesn't affect when making changes in this model.

up

could do optional functionality with Laravel Pivot (https://github.com/fico7489/laravel-pivot)

new Pivot Model Events from laravel not the best style

Any update on this idea @quetzyg

@tmishutin like I said a few times before, I'm no longer a maintainer of this project. Read the entire thread for suggested alternatives if you require such feature.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

WDC picture WDC  路  5Comments

watercayman picture watercayman  路  5Comments

PrafullaKumarSahu picture PrafullaKumarSahu  路  8Comments

donaldtbd picture donaldtbd  路  3Comments

franciscojun picture franciscojun  路  8Comments