Laravel-activitylog: Getting relationships logs

[woliveirac]

Hello,

i'm not sure if i can do this already, i've read the docs and all the related issues for this ... but, i couldn't find a solution or at least, not a good one.

My problem is that i have a model Client and another model ClientStrategy which is a HasMany relationship and i need to get all the changes made to the Client which include the ClientStrategies.

The thing is, i want to do this:

Activities::where($client)->get(); // returns all the logs including the relationships changes (insert, delete, update)

ps: // just to give an idea, i know this code isn't the right way of doing this

In my activity_log table ... i have:

I need to list those changes exactly like that in the table, but, i can't figured out how can i do that using a where clause that is "client_id = $x", because, i can't query like:

Activities::where('client_id',$id)->get(); // just to give an idea, i know this code isn't the right way of doing this

For now, i'm using the properties and querying like this:

I could think of a few ways to get this going overwriting some things, but, i don't want to get to that point.

Is there any way of doing this with one query only or at least the least queries possible and avoiding loops?

Thank you.

[Gummibeer]

Ok, first I want to resume what I understood and hope that this is what you need to do!?

You have two models Client & ClientStrategy. They are related to each other like Client 1:n ClientStrategy.
Your goal is to query activities that have one client or related client strategies as subject.

Am I right so far?

If you don't have to do it in one query (even this should be possible) you can do:

$activities = Activity::forSubject($client)->orWhere(function($query) use ($client) {
    $query
            ->where('subject_type', (new ClientStrategy())->getMorphClass())
            ->where('subject_id', $client->strategies()->pluck('id'));
})->get();

[woliveirac]

@Gummibeer Yes, pretty much what i wanted to do, thanks.

To be honest, i kind of thought about this, on a different way, not so easy as that.

I still have a "problem", the "Client" model has at least 8 relationships, as ClientStrategy being one of them, i will need an "OR" for each of them ... right?

The "activity_log" table could have (i'm just thinking) 2 more fields, such as:

"subject_parent" and "subject_parent_id"

With those two fields, we could do all those "OR"s disappear, because our query would be something like this:

$activities = Activity::forSubject($client)->orWhere(function($query) use ($client) {
$query
->where('subject_parent', $client)
->where('subject_parent_id', $client->id);
})->get();

// with those two fields, i think i can get faster results instead of getting all the related ids and querying for them.

Or even go deeper and make a change in the "forSubject" method to the result already bring the "rows" that the current "Subject" is being served as a parent.

But, thank you very much anyway, your code works for me now, if i ever have any perfomance related issues because of that, i'll think on something else.

Let me know what you think, if you understood what i just said.

Happy new year :D

[Gummibeer]

Hey,

happy to help! 😊
Your two columns could solve your problem, they should be nullable morph columns (take a look in the laravel docs $table->nullableMorphs('taggable'); - https://laravel.com/docs/5.7/migrations#columns ). To fill them you can use an event listener - #462 & #466 as reference.

Related to your last idea: I wouldn't alter the current behavior. I would add a forSubjectParent() scope and if you want it in a single scope a second forSubjectAndParent(). Every method you override could lead to conflicts in future versions.

All these things would be possible without overriding any package logic. Just extend the model and change it in the config. Add an observer and introduce new methods in the new extending model.

And last: I wouldn't wait for a problem, these changes are simple now. Later you would have to fill the new columns via a migration and so on. So do it better now the "right" way instead of waiting for problems. 😉
Primary if you have 8 childs - logs will explode very fast and in this case my first solution will be slow as hell.

And for sure, also happy new year for you! 🍾

[woliveirac]

@Gummibeer Well, that's a good idea, i'll try to get this to work and i let you know the results.

For now, i think you could mark as solved this issue.

Thank you!

[woliveirac]

Hey there, it's me again.

I'm trying to do what we talked above.

The thing is ... i can't figure out how can i retrive a "parent" model of any model instance.

For example, i want to do something like this:

ClientStrategy::findOrFail(15)->getParent(); // returns the client model because ClientStrategy belongsTo Client model.

i could write a method which would give me that information, yes, but, the thing is that i need that in a generic form because i'm using it on my observer ... that is listening to the Activity::updating event.

The whole scenario right now is:

I have created the ActivityCustom model which Extends Activity

I have added the nullableMorphs fields that you have mentioned and now i have this table structure:

So now, in my Observer, i need to fill the nullableMorphs fields with the parent_id and parent_type ... but, i have no idea how can i do that ...

Client model has:

and the ClientStrategy model is:

How can i fill those two fields generically inside the Observer ... ?

I thought about doing something like this, but, doesn't work:

Any ideas? What am i doing wrong?

Thank you!

[Gummibeer]

Hey,

First I would like to point you to #472 - this will allow you to set these fields from the ClientStrategy::tapActivity() and not an external observer.

This will also prevent you from creating a dynamic getParent() but you can use the existing relationship method.

Now you have to add the parent relation to your custom activity model. This will allow you to do following:

ClientStrategy

$activity->parent()->associate($this->client);

Otherwise I would add a switch case in the observer:

switch(get_class($subject)) {
  case ClientStrategy::class:
    $activity->parent()->associate($subject->strategy);
    break;
}

You should prevent setting these attributes your own because there is a MorphMap in laravel.

[woliveirac]

Hello,

well, thank you very much @Gummibeer , got this working.

I got rid of the Observer and instead i used the TapActivity that you have pointed #472 .

I will post the steps just for the people curious about this issue.

Also, thank you pointing out the MorphMap, it'll be really useful in the future.

PS: I had to pull the "ft-activity-tap" branch because it isn't in the Master yet (this got me thinking for a second why isn't it working haha)

So, it's really easy to get this working:

Result:

Thank you again.

[Gummibeer]

@woliveirac yes, atm you have to use dev-ft-activity-tap as composer version.
Thanks for your feedback! :)
Can you please add an approve review to the PR #472 ? This will help me to keep track of the approves in one place.

And just a personal hint, possible too late, I would rename parent to something like main_subject. Parent is a reserved keyword in PHP which could lead to problems in some circumstances. And it's not this absurd that Laravel could introduce a method/property with this name, or any other package.

[woliveirac]

Well, thanks for the tip @Gummibeer , already changed its name.

About the review in the PR, is it just a comment? or there is something or a process to do?

Thanks!

[Gummibeer]

@woliveirac you can use the green button at the upper-right https://github.com/spatie/laravel-activitylog/pull/472/files - there is a radio-select with the approve option (could be that you don't see it).
But a simple comment with "I've tested it and it works" is also ok/enough. :)
Thanks in advance! And thanks for your time to test it.