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

tapactivity description does not save properly #562

Closed
rxng opened this issue Jul 11, 2019 · 13 comments · Fixed by #563
Closed

tapactivity description does not save properly #562

rxng opened this issue Jul 11, 2019 · 13 comments · Fixed by #563
Assignees
Labels

Comments

@rxng
Copy link

rxng commented Jul 11, 2019

Hi,

Not sure if this is a bug, but I am trying to get tapActivity() to save meaningful descriptions besides just "Update".

For example, when a user logs in, I need it to save a log with description "Logged in"

public function tapActivity(Activity $activity, string $eventName)
{
    if($activity->properties["attributes"]["last_login"]) {
        $activity->description = "Logged in";
    }
}

However, this code doesn't work and description keeps defaulting back to "Update". It's really frustrating and wonder if anyone has a solution?

@Gummibeer
Copy link
Collaborator

Hey,
do you have started to debug it? So dump if it goes into the if, what is the value of $activity->description after the if and so on?

@Gummibeer Gummibeer added bug and removed question labels Jul 11, 2019
@Gummibeer Gummibeer self-assigned this Jul 11, 2019
@Gummibeer
Copy link
Collaborator

Ok, I've checked the code and found the problem:

$logger = app(ActivityLogger::class)
->useLog($logName)
->performedOn($model)
->withProperties($attrs);
if (method_exists($model, 'tapActivity')) {
$logger->tap([$model, 'tapActivity'], $eventName);
}
$logger->log($description);

$activity->description = $this->replacePlaceholders($description, $activity);

The description is the only attribute set after the tapActivity() call. I will try to find a way to fix it.

@rxng
Copy link
Author

rxng commented Jul 11, 2019

Confirming that

dd($activity->description) at the end of the function yields correct new description, just not saving to database correctly.

@Gummibeer
Copy link
Collaborator

@rxng could you please test and verify dev-fix-issue-562? The unittest passes but a realworld test would be great.

@rxng
Copy link
Author

rxng commented Jul 11, 2019

@Gummibeer sure thing, how do I get this? I ran

composer require spatie/laravel-activitylog

but got

Using version ^3.2 for spatie/laravel-activitylog
Nothing to install or update

@Gummibeer
Copy link
Collaborator

You have to run it with the version, or just edit the version in composer.json

composer require spatie/laravel-activitylog:dev-fix-issue-562

@rxng
Copy link
Author

rxng commented Jul 11, 2019

@Gummibeer

"Package spatie/laravel-activitylog at version dev-fix-issue-562 has a PHP requirement incompatible with your PHP version (7.1.25) "

@Gummibeer
Copy link
Collaborator

That's right - the latest versions require PHP7.2.

@Gummibeer
Copy link
Collaborator

@rxng
Copy link
Author

rxng commented Aug 12, 2019

I'm having trouble installing this into my Laravel 5.5 after upgrading to php 7.2

Noticed, you released into 3.6.2, so I tried to get that version but am getting this

spatie/laravel-activitylog 3.6.2 requires illuminate/database ~5.8.0

Any chance I can try and get the fix in? Sorry for the inconvenience but I am stuck!

Thanks :)

@Gummibeer
Copy link
Collaborator

As first solution I can only recommend you to upgrade to the latest laravel version. => LTS is a lie

As second solution you could override the ActivityLogger service and paste the fix from the PR in your overridden service.

Because it's not a security fix I won't add it to an older version.

@rxng
Copy link
Author

rxng commented Aug 12, 2019

Thank you, Tom. I will try to work around that until I can safely upgrade all my systems.

Sorry to be a pain, could you advise on how we might achieve this override?

Hopefully someone else can benefit from this too!

@Gummibeer
Copy link
Collaborator

Hey,

You just have to create a new class which extends the package ActivityLogger and copy the method updated in the PR in the new class.
After this you just have to bind your new class to the old one in a service provider.

$this->app->bind(
    ActivityLogger::class,
    YourNewActivityLogger::class
);

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

Successfully merging a pull request may close this issue.

2 participants