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

Fix querying allTenants when trait on model was not booted yet #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mbaeuerle
Copy link

  • Although a new model is created before, the static::$landlord instance is still null at the moment when it should call newQueryWithoutTenants. I don’t know if this is a PHP bug but it works when moving the creation of the model to the previous line
  • This issue only arises when no model was created or queried before calling allTenants.
  • Therefore the test only works as the first test in the class because the trait is only booted once per test session and the static::$landlord stays always initialized.

@rikvdlooi
Copy link
Contributor

I just experienced this bug as well. It is not a php bug, this is the intended behaviour of Laravel.

  1. If the model is not instantiated the boot() method of the Model will not be called
  2. If the boot() method of the Model has not been called the bootBelongsToTenants() method in BelongsToTenants.php will also not be called

Instantiating the model before calling the newQueryWithoutTenants does indeed fix this, because this will also cause the boot method to be called. 😄

@mbaeuerle
Copy link
Author

mbaeuerle commented Sep 26, 2017

Yes it's up to Laravel to boot the trait after an instance of a model was created for the first time but the problem can also arise without Laravel:

<?php
  class A {
    public static $instance;
    
    public function __construct() {
      static::$instance = new B();
    }
    
    public static function do() {
      static::$instance->print(new static());
    }
  }
  
  class B {
    public function print($instance) {
      print("hello");
    }
  }
  
  A::do(); // Fails with "Fatal error: Uncaught Error: Call to a member function print() on null"
?>

which is working when using the same fix:

$i = new static();
static::$instance->print($i);

@rikvdlooi
Copy link
Contributor

Yep true. Do you think it is beneficial to write an if statement around it. Like first testing if static::$landlord is defined, and otherwise instatializing it?

Also one test is wining about a static call to faker. Should this be fixed or is this fine like it is now?

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.

2 participants