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

Return log with causer and subject nested relations #281

Closed
saqueib opened this issue Oct 3, 2017 · 13 comments
Closed

Return log with causer and subject nested relations #281

saqueib opened this issue Oct 3, 2017 · 13 comments

Comments

@saqueib
Copy link

saqueib commented Oct 3, 2017

Is there any way I can load nested relation for causer and subject.
For example, I have Post and Attachment

Activity::with('subject.attachments', 'causer')->latest()

This works but I also have other Topics model which doesn't have this attachments relation so it blows

Call to undefined relationship [attachments] on model [App\Topic]

How I can solve this, I am login many other models as well.

@AlexVanderbist
Copy link
Member

Hi @saqueib

Use $activity->load('subject.attachments') to conditionally load the relationship when you're sure that the subject does have the attachments relationship.

(I think) another option would be to use protected $with = ['attachments'] on your subject model to always include the attachments relationship.

@skcin7
Copy link

skcin7 commented Mar 23, 2018

I'm having this exact same issue at the moment. When attempting to eager load for models that don't have the relationship, it causes an error. @saqueib, have you been able to find a solution to this yet?

@fernandocoronatomf
Copy link

Is there anyway to get the associated model name from the attributes?
Like

properties: "{"attributes":{"name":"Project Name","project_status_id_text":"72f76a5e-63cf-11e8-83e0-080027e27fee"},"old":{"name":"Project Name","project_stat us_id_text":"b48608b4-63ce-11e8-b55c-080027e27fee"}}",

It does not make sense to show on the UI: changed from ID to ID
I need the actual project status name...

Any thoughts?

@Gummibeer
Copy link
Collaborator

$activity->subject->name - the subject is just a simple polymorphic relation in the activity model.
If you want to get the name the model had you can log the whole changed model and not just the dirty columns or put it into the extra values. You could also edit the logged description to contain the name.

@fernandocoronatomf
Copy link

fernandocoronatomf commented May 30, 2018

@Gummibeer Sorry, I think maybe I did not make myself clear enough.
I don't want to get the $subject->name... I would like a way to inside the properties json, when I log project_status_id, instead of saving the ID, it could save the actual name of that entity.

properties: "{"attributes":{"name":"Project Name","project_status_id_text":"72f76a5e-63cf-11e8-83e0-080027e27fee"},"old":{"name":"Project Name","project_stat us_id_text":"b48608b4-63ce-11e8-b55c-080027e27fee"}}",

This could be changed to

properties: "{"attributes":{"name":"Project Name","project_status_id_text":"New Status"},"old":{"name":"Project Name","project_stat us_id_text":"Old Status"}}"

@Gummibeer
Copy link
Collaborator

@skcin7 just check if subject is instanceof or method_exists().

@AlexVanderbist even if the $with property would solve it I don't recommend to use this property if you don't 100% know what you do.
If you load two models each other you will get a memory limit exception cause they nest each other again and again. Also if you want to get the project one time without the attachments you can't so you will produce useless queries. And if you use toArray() for API or other things you will have to exclude the relationship before to don't return unrequested data.

@saqueib
Copy link
Author

saqueib commented May 30, 2018

@fernandocoronatomf add a fillable attribute with nested relation, something like this

// relation to get the status text from model
$logAttributes = ['projectStatus.name'];

public function projectStatus()
{
   return $this->belongsTo(ProjectStatus::class);
}

then you will be able to get the $subject->ProjectStatus->name

@fernandocoronatomf
Copy link

@saqueib I can see what you are saying, but that is not very helpful or just help me out understanding what you meant...

If I have the project_status_id in my properties new values json, how exactly getting it from $subject->projectStatus->name would help?

I would like to loop through the json properties and just print them on my VUE component

@robjbrain
Copy link

I've found two work arounds to this.

Solution One

Firstly you could modify replacePlaceholders method here:

$attributeValue = $attributeValue->toArray();
return array_get($attributeValue, $propertyName, $match);

And utilise array_reduce() instead:

$nestedProperty = explode('.', $propertyName);

return array_reduce($nestedProperty, function($attributeValue, $propertyName) {
    return $attributeValue->$propertyName;
}, $attributeValue);

//$attributeValue = $attributeValue->toArray();

//return array_get($attributeValue, $propertyName, $match);

Note you shouldn't just copy and paste this as it will need some additional checks to see if the property exists and is an object, but you get the idea.

Solution Two

The second solution is to use a regular expression to match all the relationships and preload them.

Simply append the following to the top of replacePlaceholders()

if (preg_match_all('/:([a-z0-9._-]+)\.[a-z0-9_-]+/i', $description, $matches)) {
    $activity->load($matches[1]);
}

What this will do is match all the properties minus the final attribute. So in the case of:

':causer.name added :subject.user.name to the :subject.group.name Research Group'

It will match the following:

$matches = array:3 [
  0 => "causer"
  1 => "subject.user"
  2 => "subject.group"
]

And then load those relationships on the model.

Then when toArray() is called the relations will exist within the array.

Would a PR for one of these be accepted?

@robjbrain
Copy link

It's actually incredibly easy to overwride the activitylogger class and implement this yourself.

Simply create a new class:

<?php

namespace App;

use Spatie\Activitylog\ActivityLogger;
use Spatie\Activitylog\Contracts\Activity;

class CustomActivityLogger extends ActivityLogger
{
    /**
     * @param string $description
     * @param \Spatie\Activitylog\Contracts\Activity $activity
     * @return string
     */
    protected function replacePlaceholders(string $description, Activity $activity): string
    {
        if (preg_match_all('/:([a-z0-9._-]+)\.[a-z0-9_-]+/i', $description, $matches)) {
            $activity->load($matches[1]);
        }

        return parent::replacePlaceholders($description, $activity);
    }
}

Then in AppServiceProvider.php (or wherever you'd like) simply bind your new logger to the service container:

$this->app->bind(\Spatie\Activitylog\ActivityLogger::class, \App\CustomActivityLogger::class);

And tada it works!

@Gummibeer
Copy link
Collaborator

@robjbrain I would change the array_get() to a data_get() call - this function can handle arrays and objects and works perfect to get for notated keys.

@fernandocoronatomf after a long time we get a new feature that could help you #472 .
This PR introduces tapActivity() method on the subject model. In this method you can manipulate the activity model like you want. Also load data from relations and put the in the properties collection.

@robjbrain
Copy link

Hey @Gummibeer I didn't have array_get() in my code that is what's in the current spatie implementation, you're right though that works fine. Perhaps the main package should be updated to include it?

Example:

return data_get($attributeValue, $propertyName, $match);            
//$attributeValue = $attributeValue->toArray();

//return array_get($attributeValue, $propertyName, $match);

@Gummibeer
Copy link
Collaborator

@robjbrain that was my idea behind 😉
I'm in holiday atm - so if you want to create a PR I would be happy to accept it. 😊
In best case with a new unit test that tests the mixed getter.

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

No branches or pull requests

6 participants