ShouldDispatchAfterCommit does not work after child transactions are committed

[hansnn]

Laravel Version

10.32.1

PHP Version

8.2

Description

When a child transaction is committed inside of a parent transaction, any events with the ShouldDispatchAfterCommit interface that are dispatched after the child transaction has committed, but before the parent transaction is committed, will be dispatched immediately.

The problem is related to that the dispatcher adds the event listeners using DatabaseTransactionManager@addCallback, and that this method only adds the callback if it has any pendingTransactions - which are flushed whenever a transaction is committed.

Steps To Reproduce

I wrote a failing test case in ShouldDispatchAfterCommitEventTest.php that illustrates the problem:

public function testItOnlyDispatchesEventsAfterTheRootTransactionIsCommitted()
{
    Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class);

    DB::transaction(function () {
        // Begin and commit a child transaction. This line causes the test to fail.
        DB::transaction(function () {});

        Event::dispatch(new ShouldDispatchAfterCommitTestEvent);

        $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran);
    });

    $this->assertTrue(ShouldDispatchAfterCommitTestEvent::$ran);
}

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[cosmastech]

@hansnn do you have the failing test case in a repo? Or the body of those two classes at the very least?

[hansnn]

@cosmastech The test can be copy-pasted as-is into the tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php file of this repository.

I should've made that more clear in the description. I guess I assumed familiarity with the PR where this feature, and the test, were added.