owen-it / laravel-auditing

Record the change log from models in Laravel
https://laravel-auditing.com
MIT License
3.01k stars 387 forks source link

Possible bug in auditSync: Unexpected old/new values #931

Closed tricki closed 4 months ago

tricki commented 4 months ago
Q A
Bug? yes
New Feature? yes
Framework Laravel
Framework version 10
Package version 13
PHP version 8

Actual Behaviour

The "old" and "new" values for auditSync don't make much sense to me, but because the documentation doesn't list an example I don't know if this is actually a bug or expected behavior.

  1. Object has relation to A, B and C
  2. Sync, removing A and adding D
  3. In the audit: Old = A and New = D
  4. Sync, removing B
  5. In the audit: Old = B and New = []

Expected Behaviour

At step 4, I'd expect Old to be "A, B, C", and New to be "B, C, D". In step 5, I'd expect Old = "B, C, D" and New = "C, D"

Steps to Reproduce

I wrote a simple failing test to illustrate: https://github.com/owen-it/laravel-auditing/pull/930

erikn69 commented 4 months ago

https://github.com/owen-it/laravel-auditing/issues/919#issuecomment-2034836663

willpower232 commented 4 months ago
  1. Sync, removing A and adding D

I think the problem is here, using sync to append relations is tricky because as pointed out sync will reset the whole relationship.

You would have to do something like

$categories = $model->categories;

// remove D from categories

$categories->push($A);

$model->auditSync('categories', $categories);

See also

tricki commented 4 months ago

OK, then I misunderstood how it's supposed to work (basically old/new becomes removed/added). I think what confused me was this line in the docs:

The audited value is the array version of the relationship return!

Could be rephrased or extended.

Thanks for the quick response. Closing this issue. I've also changed the title hoping it will be easier to discover by anyone having the same problem.

parallels999 commented 4 months ago

Could be rephrased or extended.

I think so, you can open a PR to make it more understandable, English is not my language