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

[11.x] Fix ObservedBy Attribute that did not take custom observables in account #50540

Closed
wants to merge 1 commit into from

Conversation

comhon-project
Copy link

@comhon-project comhon-project commented Mar 13, 2024

Hello laravel team !
I made this pull request to fix the issue described here

Bug description

When using PHP Attribute ObservedBy to observe my model (instead of register the observer in the EventServiceProvider), "custom" observables added through the initialize{Trait} using addObservableEvents are not taken in account.

(when I use EventServiceProvider to register the observer, it works well).

my investigation :
When using ObservedBy, "custom" observables are not registered because when entering in HasEvents::observe for the first time, the Model is booting (and already tagged as booted in Model::$booted). And the instanciated model in HasEvents::observe line 67 ($instance = new static;) will not boot the model because it is already booting.

Conversely when we register the observer in EventServiceProvider we directly call HasEvents::observe and the previously mentioned $instance = new static; boot the model and can register "custom" observables.

Steps To Reproduce

create a model that use ObservedBy

#[ObservedBy([MyObserver::class])]
class MyModel extends Model
{
    use MyTrait;
    
    ...
}

create a trait that use addObservableEvents

trait MyTrait
{
    public function initializeMyTrait()
    {
        $this->addObservableEvents('foo');
    }

    /**
     * to easily fire the event for debugging
     */
    public function fireFoo()
    {
        $this->fireModelEvent('foo');
    }
}

create the observer MyObserver

class MyObserver
{
    public function foo(MyModel $myModel)
    {
        var_dump('triggered foo');
    }
}

finally call fireFoo

$myModel = new MyModel();

$myModel->fireFoo(); // doesn't call MyObserver::foo

Solution found

Do not make the trait HasEvents bootable by removing bootHasEvents.
And just call HasEvents::observe at the very end of bootTraits method.
This way all stuff that need to be initialized will be initialized before the model instantiation in the method HasEvents::observe

@driesvints
Copy link
Member

@comhon-project best if you add a thorough description to your PR and not just link to an issue.

@taylorotwell
Copy link
Member

This feels really edge case. We don't even document those Eloquent methods.

@comhon-project
Copy link
Author

@taylorotwell
as I said in the issue description, when we register observer from EventServiceProvider, it works fine!
The two registration method (from PHP Attribute and form EventServiceProvider) should behave exactly the same (even with undocumented stuffs).

The documentation says we can use both methods, implicitly that says it is the exact same behaviour.

IMHO either two methods must behave exactly the same, either the Php attribute method must be rollbacked.

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.

Php Attribute ObservedBy doesn't take in account observables added in Traits
3 participants