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

Change database default connection if needed #59

Closed
wants to merge 1 commit into from
Closed

Change database default connection if needed #59

wants to merge 1 commit into from

Conversation

masterix21
Copy link
Collaborator

Using packages like Laravel Passport or Laravel Sanctum (and many others) could be useful to switch the default database connection to "tenant" instead of "landlord".

Using the config file, now it is merely possible:

/*
 * Using `\Spatie\Multitenancy\Tasks\SwitchTenantDatabaseTask`, could be useful to
 * set up the tenant database connection as default connection.
 */
'tenant_database_connection_as_default' => true,

@freekmurze
Copy link
Member

Couldn't you just set a tenant database as the default in the. databases.php config file if you'd like that?

@masterix21
Copy link
Collaborator Author

Your reasoning is not wrong, but there seem to be problems. Following the past issues, appears to be a solution to the questions:

I tried and seemed necessary. Furthermore, could be a way to skip the "--database=" argument with migrations, or other commands where the "--database=" argument is unimplemented (and there are a lot of packages).

@freekmurze
Copy link
Member

Your reasoning is not wrong, but there seem to be problems.

I don't fully understand the problem or how this PR solves the problem, but adding an extra configuration option for something that can already be done in configuration seems off.

I'd prefer a solution where no config option is added. Maybe the current implementation can be changed in a non-breaking way that solves the problem?

@masterix21
Copy link
Collaborator Author

See the attached video where I tried to explain the problem using Tinkerwell. I don't know if there are other solutions, but changing the default database connection works.

I will inspect better in the problem, but if you have an idea or understand what should be, please keep me updated.

example

@masterix21
Copy link
Collaborator Author

Another example.

It happens when you use a model without UsesTenantConnection (or UsesLandlordConnection).

example

@masterix21
Copy link
Collaborator Author

masterix21 commented Jun 22, 2020

A partial solution (already tested) could be a trait (UsesMixedConnection) with:

public function getConnectionName()
{
        return Tenant::checkCurrent()
            ? 'tenant'
            : 'landlord';
}

But it means that you must extend all original models with new ones that use the trait.

@masterix21 masterix21 mentioned this pull request Jul 7, 2020
@masterix21 masterix21 closed this Jul 27, 2020
@masterix21 masterix21 deleted the feature-switch-default-connection branch July 27, 2020 21:54
@sanketr43 sanketr43 mentioned this pull request May 3, 2023
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

Successfully merging this pull request may close these issues.

2 participants