Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE/QUESTION] Be able to have batches of activity logs/set causer for a duration #560

Closed
ragingdave opened this issue Jul 3, 2019 · 4 comments · Fixed by #866 or #787
Closed
Assignees

Comments

@ragingdave
Copy link
Contributor

Not sure if this would be related to #503, but I was wondering if there was any interest in being able to "batch" activity logs, like in automated changes, perhaps a user makes many changes that can be linked back to a single actitivity.
For example,
User deletes an author, then that cascades soft deletes to the books for that author.

Having a way to link those changes as being related would be very very useful IMHO.

Additionally, since something like this would be great to have in case of background actions, maybe have a way to set the causer for the duration of some subset of logging? That way you can link activity logs to a user, but then also link all related changes to the original that started it all, while also being able to set a causer like in my example, knowing that the user specifically didn't trigger the soft delete on books, but that was caused by the author model.

That's all sort of convoluted, but I wanted to see if anyone else had some need for something like this.

@Gummibeer
Copy link
Collaborator

This question consists of two parts.

  1. Setting the causer - is possible already and the easiest way to set the causer for multiple tasks is to login the user. If this creates creates unwanted behavior you can override the logger class and add some logic to get the user from any other source like a container binding or manager service.

  2. Allow to batch multiple activities - this is something pretty difficult for a package like this. The only way I see is to add methods like startBatch() and endBatch(). If you want to batch everything in a request it's easy - but it also could be that you do edit multiple models in a batch or delete multiple comments by a clear command and want to log the activities in a batch per comment.
    In console commands this is much more common. The start batch should also be able to get a activity batch ID passed in because it could be that a task like send an email is done in the queue but belongs to the activity batch created in the request.

Also something to cover would be conflicting batches.

  1. start batch#1
  2. delete product
  3. start batch#2 (because there is a call in model logic)
  4. delete product-variants

To which batch should the product-variants activities belong? Or should this throw an exception because of an already set batch? Or should the second batch create some kind of sub-batch like 1.1? If so the batch I'd would be limited to numbers and not UUIDs but the DB columns can't be numeric because of the dots. This could also be possible with UUIDs but would force a text column because of the length.

I'm open for this feature but it's implementation should cover a lot of cases and be customizable to mix it up with any custom needs.

My most favorite implementation would be something like the withoutEvents() method for models. In our case something like

ActivityLogger::useBatch(function () {
  activity()->log('my message');
  $product->delete();
});

This would start a new batch, assign all activities created during the callback to the batch and end the batch after the callback.
With this as the primary approach and a startBatch() (with corresponding end one) to start a batch in a middleware for a whole request or console command. I would like to throw an exception if batches will conflict.

Would these three methods cover your needs?

@ragingdave
Copy link
Contributor Author

The first of setting the causer from an extended Logger class....seems straightforward and I'm not really sure why I didn't just do that in the first place.

The second around handling batches, I like the idea of sticking to a familiar callback style like transactions do. If I were to submit this I would probably just simplify the handling of the batches and if there was a nested batch that started, it would be treated like the outer batch in the case of linking them.
I don't think it would make sense to have 2 batches at the same time, since that would kind of defeat the purpose of having a batch in the first place. Either that or somewhat borrow the same transaction style and allow for savepoints, that could loosely link batches, like a parent/child relationship. The only issue with having a nested sql lookup would be performance if the batches went deep level-wise which is why I would probably just opt for simplicity sake of just one batch at a time and anything that happens in that batch is one single batch

@Gummibeer
Copy link
Collaborator

Ok, and would you more like an exception if a second batch is started inside another one? Or should it quit silently and just run in the first batch?
Because I don't like to ignore what a developer does and handle things the expected way or exit. I would like the exception more.
With a isBatchOpen() or something like this method the developer could handle it himself if he can't prevent this conflict in general but check it to prevent the exception.

@Gummibeer Gummibeer added the hacktoberfest https://hacktoberfest.digitalocean.com label Sep 25, 2019
This was referenced Mar 13, 2020
@Gummibeer Gummibeer self-assigned this Mar 13, 2020
@github-actions github-actions bot added the stale label May 4, 2020
@Gummibeer Gummibeer removed the stale label May 4, 2020
@spatie spatie deleted a comment from github-actions bot May 4, 2020
@Gummibeer Gummibeer mentioned this issue Sep 16, 2020
Merged
9 tasks
nagi1 added a commit to nagi1/laravel-activitylog that referenced this issue Apr 2, 2021
nagi1 added a commit to nagi1/laravel-activitylog that referenced this issue Apr 2, 2021
@nagi1 nagi1 mentioned this issue Apr 3, 2021
10 tasks
This was linked to pull requests Apr 19, 2021
Merged
nagi1 referenced this issue in nagi1/laravel-activitylog Apr 24, 2021
@Gummibeer
Copy link
Collaborator

I will close this issue even if v4 isn't released yet. But the task itself is done and I want to check which tasks are really open. Please keep an eye on #787

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants