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

[2.1.0] Document integrating spatie/laravel-activitylog #164

Closed
jny986 opened this issue Oct 9, 2019 · 13 comments
Closed

[2.1.0] Document integrating spatie/laravel-activitylog #164

jny986 opened this issue Oct 9, 2019 · 13 comments
Assignees
Labels
Milestone

Comments

@jny986
Copy link
Contributor

jny986 commented Oct 9, 2019

I am trying to integrate the spatie/laravel-activitylog , however, it is not picking up and saving to the tenant.

Is there a way to get this working to save to the DB for system if not a tenant domain and to the tenant if for their domains.

Any help would be appreciated!

@drbyte
Copy link
Contributor

drbyte commented Oct 9, 2019

I was talking with @stancl about this earlier today. I haven't had a chance to test it yet, but we were considering keeping the default connection (in the activitylog config file) set to central or mysql or whatever your central connection is, and using the bootstrapping event to set that config key to $tenantManager->getTenant()->getConnectionName()

@drbyte
Copy link
Contributor

drbyte commented Oct 9, 2019

I wonder if that default connection parameter could fallback to the Laravel default connection instead of the other way around.

Pinging @Gummibeer for input.

@jny986
Copy link
Contributor Author

jny986 commented Oct 10, 2019

I have ended up finding a way to do it.

I discovered that the Activity model sets the connectionName and the tableName in the Model class in the construct is it is not already set.

I simply created the following class to work with the Activity Model

use Spatie\Activitylog\Models\Activity as SpatieActivity;

class Activity extends SpatieActivity
{
    public function __construct(array $attributes = [])
    {
        if (! isset($this->connection)) {
            if (Domain::exists(request()->getHost())) {
                $this->setConnection('');
            } else {
                $this->setConnection(config('activitylog.database_connection'));
            }
        }

        if (! isset($this->table)) {
            $this->setTable(config('activitylog.table_name'));
        }

        parent::__construct($attributes);
    }
}

Domain::exists() is just a model extending the DomainModel to create an exists static method for checking for the domain.

It is a little bit of a hack but if you can find a way to use the events to overwrite the connection for packages like this it would be worth doing.

@stancl
Copy link
Member

stancl commented Oct 10, 2019

I'd suggest keeping the DB in the activity log config central and using this in AppServiceProvider:

Tenancy::hook('bootstrapped', function ($tenantManager) {
    config(['activitylog.connection', $tenantManager->getTenant()->getDatabaseConnection()]);
});

And on the ended event you'd set it back to your central connection.

Hope that makes sense, I'm writing this on mobile.

@Gummibeer I was going to open an issue. I think activitylog should fallback to the default DB connection, because:

  • changing your default DB connection would be enough to change your app's DB
  • some apps, like the ones using this package, use dynamic connections. Falling back to default is really useful here. I can imagine some sharding logic working similarly.

@Gummibeer
Copy link

During config loading we fall back to the default Laravel env. But I think you mean something like null in config and use database.default in the model construct? This would make sense and I don't see a point to don't do it. Would this solve your issue here? 🤔

@stancl
Copy link
Member

stancl commented Oct 10, 2019

Yeah, that's what I meant.

The connection is set each time the model is constructed.
https://github.com/spatie/laravel-activitylog/blob/808a3e5c119d31d1115ff7831efa866aa3fe9692/src/Models/Activity.php#L23

Since models aren't singletons, I think it would solve it.

I'll take a better look at this in the afternoon when I'm on my computer.

@stancl stancl added this to the 2.1.0 milestone Oct 10, 2019
@stancl stancl changed the title Intergrating 3rd Party Packages [2.1.0] Document integrating spatie/laravel-activitylog Oct 10, 2019
@stancl
Copy link
Member

stancl commented Oct 10, 2019

You can also set the connection to null and it should work. No need for hooks.

@jny986
Copy link
Contributor Author

jny986 commented Oct 15, 2019

@stancl I tried your suggestion of using the booted hook however it did not work even after I corrected a slight issue with your code.

Setting the Activity Log Connection to null does work however.

Thanks for the suggestion.

If I get a moment I will create a pull request for documentation of Spatie Activity Log

@stancl
Copy link
Member

stancl commented Oct 15, 2019

@jny986 Glad to hear that. No need to make a PR, I will be writing a documentation page specifically about integrating Spatie packages.

@Gummibeer
Copy link

https://github.com/spatie/laravel-activitylog/releases/tag/3.9.1

@stancl
Copy link
Member

stancl commented Oct 15, 2019

stancl/tenancy-docs@99655ea

https://tenancy.samuelstancl.me/docs/v2/spatie/

@stancl stancl closed this as completed Oct 15, 2019
@sawirricardo
Copy link

Hi @stancl , love this package. From what I read from the docs, it seems that we can only use 1 option rather than both? Say if I would like to log in the central, AND in the tenant scope, is that possible just by setting db_connection to null? Thank you

@lvillacin
Copy link

Bumping the previous question, @stancl, thanks!

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

No branches or pull requests

6 participants