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

Doesn't work with Laravel 9.35 Model::preventsAccessingMissingAttributes() #1164

Closed
nikosid opened this issue Oct 20, 2022 · 7 comments · Fixed by #1168 or #1213
Closed

Doesn't work with Laravel 9.35 Model::preventsAccessingMissingAttributes() #1164

nikosid opened this issue Oct 20, 2022 · 7 comments · Fixed by #1168 or #1213

Comments

@nikosid
Copy link

nikosid commented Oct 20, 2022

  • Jetstream Version: 2.x-dev
  • Jetstream Stack: Livewire
  • Uses Teams: yes
  • Laravel Version: 9.36.3
  • PHP Version: 8.1.10
  • Database Driver & Version: MySQL

Description:

Errors regarding teams when running tests. For example:

The attribute [current_team_id] either does not exist or was not retrieved for model [App\Models\User].

or

The attribute [profile_photo_path] either does not exist or was not retrieved for model [App\Models\User].

Steps To Reproduce:

sail bin pest --filter "test_confirm_password_screen_can_be_rendered"

@markjaquith
Copy link
Contributor

Confirmed with Laravel 9.36.4 and Jetstream 2.12.3 on a brand new install.

I haven't had time to fully investigate, but this seems to be related to Auth::user() returning an object without all the columns.

@flick36
Copy link

flick36 commented Oct 21, 2022

I don't think it's a bug, you have to add what you need to the UserFactory,

'current_team_id' => null,
'profile_photo_path' => null,
'two_factor_secret' => null,

#1162 (comment)

@markjaquith
Copy link
Contributor

@flick36 But... the UserFactory is created by Jetstream. Shouldn't the user factory it creates work with the tests it creates, while using this new mode that people are (rightly) being encouraged to use? Really asking... maybe the answer is "no", in which case this can be solved by documentation. But making the user factory definition work with these tests in strict mode seems like a kindness that people would appreciate.

@flick36
Copy link

flick36 commented Oct 21, 2022

It adds everything it needs by default, it's up to you to update it based on your needs, not everyone needs teams, twofactor auth, or profile photos, that's why i think it isn't included, and you must activate thos features manually, by uncommenting the corresponding feature on config/jetstream.php, so if you need any of those, and YOU want to activate the strict mode, you have to add it to a service provider, you have to then add that to the UserFactory

@markjaquith
Copy link
Contributor

markjaquith commented Oct 21, 2022

Adding these to the user factory definition allows the tests to pass:

'current_team_id' => null,
'profile_photo_path' => null,
'two_factor_secret' => null,
'two_factor_confirmed_at' => null,
'two_factor_recovery_codes' => null,

@markjaquith
Copy link
Contributor

not everyone needs teams, twofactor auth, or profile photos, that's why i think it isn't included, and you must activate thos features manually

I haven't activated these features. The only thing I did after a bog-standard Laravel and Jetstream (Livewire) install was add the one line to activate strict mode.

Further, the migrations to create the columns for these features are created by Jetstream regardless, so it seems reasonable to have the default user factory include defaults for these columns, even if you're not using the underlying features that they power.

If someone not using these features calls User::first(), they're going to get an object with these columns included. I think the factory should do the same. What's the downside?

@citricguy
Copy link

I believe this could also be fixed in the tests themselves.

Adding $user->refresh(); after the user model is created will refresh the user model with the necessary fields and allow the tests to pass. I think this ensures Model::preventAccessingMissingAttributes(); is doing what it is supposed to.

@markjaquith Your factory additions solve this issue as well, but could allow Model::preventAccessingMissingAttributes(); issues to sneak through tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants