Model events not called in tests that are wrapped in a transaction

[mnightingale]

• Scout Version: 8.5.1
• Laravel Version: 8.20.1
• PHP Version: 7.4.13
• Database Driver & Version: MySQL 5.7.32

Description:

From my testing and investigation #436 seems to have broken tests when using the RefreshDatabase or DatabaseTransactions traits. I think it's because they both operate the test within a transaction so the Scout events are never fired. If it matters during my tests I use a mysql connection and the teamtnt/laravel-scout-tntsearch-driver driver.

I'm not sure how it could be resolved, ideally you would want the changes #436 made but if it was set to false for the tests then production/testing would be different and I'm not sure if that would be desirable.

Steps To Reproduce:

Create a test that has the DatabaseTransactions trait, use a model factory to create a model instance. Laravel\Scout\ModelObserver@saved is never called hence model is never made searchable.

[driesvints]

I'm confirming with @themsaid what the best course of action here would be.

[themsaid]

As indicated in the issue, it's a side effect of https://github.com/laravel/scout/pull/436. I can't think of a way to stop counting tests transactions so that the code runs as expected. The only think I can think of is that you don't use the DatabaseTransactions trait if you have $afterCommit=true

[driesvints]

I've sent in a PR to revert this: https://github.com/laravel/scout/pull/438

[adamthehutt]

@driesvints Is there any way this could be configurable? That way when running tests we could set afterCommit to false but keep it as true under normal circumstances. There would still be BC implications so it would probably still need to wait for v9, but something to think about.

I don't know about about observer invocation to know what the best implementation would be. But maybe it's as simple as adding a constructor to the observer class along the lines of:

class ModelObserver
{
    public function __construct()
    {
        $this->afterCommit = config("scout.after_commit", true);
    }

Or something like that?

[bakerkretzmar]

Personally I think this behaviour is correct, if a bit annoying. Scout shouldn't sync model data to a search index until that data has been persisted into the database.

This might require some adjustments to tests, but that's a much smaller issue than the original bug itself, which breaks real app code.

[driesvints]

I closed my PR because it technically doesn't breaks real app code.

@adamthehutt a config option seems overkill. The current behavior definitely should be the default. But we need to find a way to let people still run their database migrations while using scout.