Forget current when making new tenant current...

[telkins]

I'm not 100% sure that this change is a good idea. It makes sense to me for what I think would be good, in general. I'm not sure, however, because this is my first jaunt down Multi-Tenancy Lane. ;-)

Anyway, this small change does two things on calls to makeCurrent():

• First, it will return early if the tenant is already the current.
• Second, it will call forgetCurrent() for the "old" tenant.

Please let me know what's good about this and, more importantly, what's bad with this and why. :-)

Thx.

[masterix21]

Thank you

[telkins]

@masterix21 Thx. Appreciated. I'm guessing this change, especially the forget-current-tenant-before-making-another-tenant-current sounded good...? I wasn't sure, tbh....

[masterix21]

@telkins, I've found a bug introduced by this PR.

Using a job to execute an Artisan command (Artisan::call(...), the Tenant is supplied, but Laravel resets the configuration applied by the tasks. To solve the problem, I removed the lines from 17 to 20 from Tenant model.

I'm releasing the fix.

[telkins]

@telkins, I've found a bug introduced by this PR.

Using a job to execute an Artisan command (Artisan::call(...), the Tenant is supplied, but Laravel resets the configuration applied by the tasks. To solve the problem, I removed the lines from 17 to 20 from Tenant model.

I'm releasing the fix.

Not sure I understand the problem. Which configuration is reset?

FYI...this new change breaks my tests. :-(

I'll start looking into it to see what can be done....I do know that I had experienced problems before with moving between tenants (and back) in individual tests.....

[telkins]

@masterix21 OK. It looks like it's failing when a job is executed during a test. There may be other causes, but this is what seems to be happening for (a subset of...?) my tests.

I'll continue to dig...

[telkins]

I should correct my previous post: I fire an event that a listener (implementing ShouldQueue) "handles....

I've not specified that it's tenant aware or anything. I'll read the docs really quick to see what it says about listeners (vs jobs)....

[telkins]

OK. I'm not sure, but I think that the problem is related to the transactions during testing.

If/when the current tenant is switched, the transaction context is also lost. This presents two problems, I believe:

• during test tear down, those "active" transactions cannot be rolled back
• during an individual test, another transaction context is created which is unaware of the previous transaction context, which hasn't been committed....so transaction Add CurrentTenant class that can be resolved from container #2 can't see database changes that were made in transaction Remove duplicate docs content #1

I've drilled down to this conclusion a couple of times during the last couple weeks as I've migrated a project to use this package. The tests are breaking and I'm not sure what a "perfect" solution is. In fact, I'm not even sure what a good solution is for some of this stuff.

I really hope that someone can help out, because i'll have to stay on 1.6.4 until I can find a way forward. Either the package needs to be updated -- somehow...? -- to allow for testing or I've got to figure out a way to fix my tests. I'm open to suggestions...! :-)