Tap no longer works since 4.4.3

[seabasss]

Describe the bug
Since 4.4.3 team_id is no longer being tapped for my activities. See code below.

To Reproduce

activity()
->tap(function (Activity $activity) {
    $activity->team_id = $this->team_id;
})
->by($this->contact)
->on($this->post)
->event('clicked')
->log('clicked on the link');

Edit // I found out the issue. It's this addition to ActivityLogger.php:

if (isset($activity->subject) && method_exists($activity->subject, 'tapActivity')) {
        $this->tap([$activity->subject, 'tapActivity'], $activity->event ?? '');
}

That made my activity()->tap() be overridden by the tapActivity method on the subject model (which I'm also using independently). I made it work anyway, but shouldn't you be able to tap whatever you want with activity()->tap() without something else overriding it? Thanks!

Expected behavior
team_id should be set

Versions (please complete the following information)

• PHP: 8.1
• Database: 5.5.5
• Laravel: 9.7
• Package: 4.4.3 and newer

[FrancisMawn]

Hi @seabasss I'm the one who pushed that PR.

I won't pretend like I know all the ins and outs of this package but my understanding is that both methods should be called? This addition should not override anything, it adds an optional callback before the activity gets saved.

What does your tapActivity method looks like on your post model?

[Gummibeer]

I'm also pretty sure that both methods are called - as the tap() doesn't store the callbacks or anything but simply call it the moment tap() is called. So the only thing that could override the team_id is your tapActivity() method in case it also defines a team_id but with a different logic.

PS: just from the name team_id it sounds more like it could be filed from an observer which will just define it always independendently of where the log comes from.

[FrancisMawn]

I did some additional testing and both methods are called. Like @Gummibeer said, I suspect you are overriding the team_id attribute in both of your callbacks, in which case the tapActivity on the model will be prioritised. Can you confirm?

[seabasss]

Thanks! Yes, i have different logic and it’s not only team_id I want to tap, but in my case I would prefer if tap() is prioritized, since tapActivity() is always run and it renders tap() useless. And in the case you want tapActivity(), just don’t use tap().

If there are other aspects of why this is not a good idea I’ll rest my case. I got it to work without tap(), I was just a little puzzled when my activities stopped showing up in my feed since my filter key stopped being logged.

[Gummibeer]

I would recommend you to use something like that $activity->team_id ??= $this->team_id in your taps. This way the first that adds one is the ruling one. Independent how many other taps follow.

[seabasss]

I would recommend you to use something like that $activity->team_id ??= $this->team_id in your taps. This way the first that adds one is the ruling one. Independent how many other taps follow.

Ah, that would work. Thanks!

[afagard]

I too consider this a "breaking" change. I haven't dove in to the previous versions code before updating, but I was using tapActivity in the model and tap for "random" activity() events with a ->tap() ... before tap wasn't getting overridden, and now it is. indeed i define the same sort of thing as team_id in both. I think this should be called out in the docs.

Note: I personally preferred it the previous way, but I suppose I might have been using this package incorrectly.

[Gummibeer]

Tap isn't overridden but both are executed now.