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

Remove the facade aliases usage since they can be removed/changed fro… #215

Merged
merged 3 commits into from
Apr 30, 2021
Merged

Remove the facade aliases usage since they can be removed/changed fro… #215

merged 3 commits into from
Apr 30, 2021

Conversation

Rezouce
Copy link
Contributor

@Rezouce Rezouce commented Apr 27, 2021

Changes

Use the fully qualified facade class names instead of their aliases which allow using the plugin even when for some reason the default aliases have been removed or replaced (ie. legacy code usage).

Testing

[x] This change has been tested on the latest version Laravel

Checklist

[x] I have read the Auth0 general contribution guidelines

[x] I have read the Auth0 Code of Conduct

…m the Laravel configuration which will make impossible to install and use the plugin.
@Rezouce Rezouce requested a review from a team as a code owner April 27, 2021 13:36
@@ -37,7 +37,7 @@ public function getUserByUserInfo(array $userInfo) : Authenticatable
public function getUserByIdentifier($identifier) : ?Authenticatable
{
// Get the user info of the user logged in (probably in session)
$user = \App::make('auth0')->getUser();
$user = \Illuminate\Support\Facades\App::make('auth0')->getUser();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be throwing an error with our tests. Any ideas on how to work around this?

@@ -21,11 +21,11 @@ class LoginServiceProvider extends ServiceProvider
*/
public function boot()
{
\Auth::provider('auth0', function ($app, array $config) {
\Illuminate\Support\Facades\Auth::provider('auth0', function ($app, array $config) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to import these with use statements at the top of the file as opposed to going the inline route? Not sure if this was a technical requirement, just curious if that would be a cleaner approach otherwise.

@evansims
Copy link
Member

Hey, @Rezouce! 👋 Thanks for your PR — I think this is a reasonable suggestion. I've left a few comments.

Copy link
Member

@evansims evansims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks excellent, thanks for your contribution on this! I'll merge it in for the next release.

@evansims evansims added this to the 6.5.0 milestone Apr 30, 2021
@evansims evansims merged commit 48b5b0c into auth0:master Apr 30, 2021
@evansims evansims modified the milestones: 6.5.0, 6.4.1 Aug 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2022
@Rezouce Rezouce deleted the remove-facade-aliases-dependency branch January 20, 2023 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants