archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.65k stars 429 forks source link

Synced resources between tenants throws error when inserting new model on tenant #658

Closed kr-apps closed 2 years ago

kr-apps commented 3 years ago

Describe the bug

When using Synced resources between tenants, the method updateResourceInCentralDatabaseAndGetTenants on the listener UpdateSyncedResource trows the following error:

  SQLSTATE[42S22]: 
Column not found: 1054 Unknown column 'code' in 'field list' (SQL: insert into 
`users` (`name`, `email`,  `password`, `remember_token`, `code`, `global_id`) 
values (Hiram Hermann, yschumm@example.com, secret, fd9ca932-23c3-4e2f-abb6-38bc82cd92c4))

Steps to reproduce

Assuming we want to sync users:

  1. Set up Synced resources between tenants following the documentation
  2. Add some extra columns on the tenant's users migration, for example code
  3. Try to add a NEW user on the tenant that doesn't exist on the central database

Expected behavior

Since the user doesn't exists on the central database the updateResourceInCentralDatabaseAndGetTenants method will try to insert the new user on the central database using all the attributes from the model on the tenant's database, but like the columns from the tenants are not the same the insert can't happen.

Your setup

stancl commented 3 years ago

Does getSyncedAttributeNames() not work?

kr-apps commented 3 years ago

Does getSyncedAttributeNames() not work?

To make it work I override the method updateResourceInCentralDatabaseAndGetTenants in this specific part:

BEFORE

// If the resource doesn't exist at all in the central DB, we create
// the record with all attributes, not just the synced ones.
$centralModel = $event->model->getCentralModelName()::create($event->model->getAttributes());
event(new SyncedResourceChangedInForeignDatabase($event->model, null));

NOW

// If the resource doesn't exist at all in the central DB, we create
// the record with all attributes, not just the synced ones.
$centralModel = $event->model->getCentralModelName()::create($syncedAttributes); <----CHANGED THIS LINE
event(new SyncedResourceChangedInForeignDatabase($event->model, null));

Also I have to add global_id in the fields to be synced.

PiotrekPKP commented 3 years ago

I have same error :c

stancl commented 3 years ago

I guess there should be more configuration for how synced resources should be created, i.e. whether it should use all attributes, just the synced one, or something else. I'll add this in v4.

amjeed-ay commented 2 years ago

@stancl This is how I solve it, just exclude the id when creating to the central model

$attr = collect($event->model->getAttributes())->except(['id'])->toArray()

$centralModel = $event->model->getCentralModelName()::create($attr);

event(new SyncedResourceChangedInForeignDatabase($event->model, null));

amjeed-ay commented 2 years ago

Then for temporary solution before the official release, I need to override UpdateSyncedResource.php in my composer.json

"autoload": {
        "exclude-from-classmap": [
            "vendor/stancl/tenancy/src/Listeners/UpdateSyncedResource.php"
        ],
        "psr-4": {
            "App\\": "app/",
            "Database\\Factories\\": "database/factories/",
            "Database\\Seeders\\": "database/seeders/"
        },
        "files": [
            "app/Overides/UpdateSyncedResource.php"
        ]
    },
mohamedsabil83 commented 2 years ago

@amjeed-ay you can also make a new listener which extends the original one and override the method on it. then, replace it in the TenanceServiceProvider