Accessing auth() and user() inside Models?

[ralphschindler]

https://github.com/alexeymezenin/laravel-best-practices#single-responsibility-principle

That example, while probably contrived, as the first example in the document seems to signal to the reader that calling auth() inside a model (they are model methods after all, they have get*Attribute()) is a good thing or widely accepted thing to do. Is this the case? I have not seen this in the wild much (auth() calls probably best exist in the controller or view layers).

If it's purely serving to demonstrate single responsibility, is there a better contrived use case to use? Are you open to this change?

[papalapa]

I totally agree with @ralphschindler . I will say more: try not to use facades or static calls anywhere. Use injections through method arguments. If this does not work out for you, then you are doing something wrong.

[alexeymezenin]

@papalapa I know most of the people who came from Java, Symfony, and similar techs don't like using Laravel helpers because those are hidden dependencies. I like using the helpers because, in a Laravel project, the framework is a dependency itself. We also use stuff like array_merge() in Models and Java people don't like PHP for things like that. But we still use those because it makes PHP code look readable, right? And many people don't like Java for its verbosity.

I love Laravel because it took the best from PHP and Ruby on Rails worlds. It allows us to write a super readable (maintainable) code. But most of the developers are trying to make a Laravel project look like a Symfony/Spring app instead of just using Symfony instead of Laravel.

By the way, I also like to use my own helpers like isAdmin() because they allow me to write readable code everywhere (controllers, middleware, views, etc) compared with Java-like code.

It is also very easy to migrate the code to any framework if you're using Laravel helpers, so I don't really see any issue with using the helpers.

[ralphschindler]

Hi! I think we're talking about 2 different things. I am not advocating to stop using helpers, not at all. I do in fact like helpers.

I am advocating that using the authentication specific (which implies HTTP) auth() and user() functionality should not exist inside a mutator/accessor inside a Model.

Take this code for example:

public function isVerifiedClient()
{
    return auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified();
}

If you were to do something like this: $user = User::find(123)->isVerifiedClient(); you would not actually be checking if user 123 is a "verified client" (as you'd expect by reading that line), you'd be checking if the logged in user is the verified client.

[alexeymezenin]

@ralphschindler thank you for the explanation.

I understand that it brakes MVC, but it's not an issue in my opinion. We have a lot of stuff like this in Laravel. I.e. ActiveRecord which many developers hate, PHP functions like array_sum() and global vars, helpers, a lot of magic, etc. I love Laravel for all these things that allow us to write short readable code even if the code doesn't look good for those guys who are used to Java and similar techs.

So, I'd still use the helper in the data layer (M in MVC) and views (V in MVC), because it's a part of the Framework and you can always override the original helper to change its functionality when needed.

My point is Laravel is very different from Spring and that's great. You don't want to use Laravel and try to make your app look like a Spring application. If you don't like PHP but you'd like to use ActiveRecord, you've got Ruby On Rails and Django. If you like PHP and you're looking for something like Spring, you can use Symfony. If you don't like all the mainstream languages and frameworks, you can use Go, Elixir, Rust, etc.

I'll probably change the code example in the future anyway because it looks like the example is confusing. I'm planning to do an update.

PS: I don't think your code example would work, how would you get 123 (user ID) in the accessor method without using auth()? Also, your code makes another DB call, and auth() doesn't do that.

[papalapa]

@alexeymezenin Ok, I understand and partially agree with you. Laravel encourages static calls and global functions. But I disagree that calling facades from anywhere is "Best Practices."

[JohnnyWalkerDigital]

@ralphschindler Totally agree with your example. (Not sure how the conversation got diverted into the pros and cons of helpers -- that really isn't what's being discussed here.) Using auth()->user() in a model is definitely bad practice. For example, any User model you instantiate should be for that particular user (whether it be the current one or not). It should never only relate to one user (the current one).

By hardcoding a model to only ever relate to the current authorised user you are completely undercutting the whole point of having models in the first place. (Imagine a Table class that only ever related to a specific wooden table.)

You should create an instance of a User and then run your method code on that instance. (I'm fairly certain everyone here is already doing this --- runs to check his own code!)

[JacekAndrzejewski]

@alexeymezenin Example posted by @ralphschindler could happen in unit test. Using auth() and user() in model is a bad idea, which was explained very well above by @JohnnyWalkerDesign.