bestmomo / laravel-email-confirmation

Add email confimation to Laravel project
89 stars 28 forks source link

when confirmation, always use 'auth.providers.users.model' #19

Closed emile-yamaji closed 6 years ago

emile-yamaji commented 6 years ago

vendor/bestmomo/laravel-email-confirmation/src/Traits/RegistersUsers.php

 44     public function confirm($id, $confirmation_code)
 45     {
 46         $model = config('auth.providers.users.model');
 47 
 48         $user = $model::whereId($id)->whereConfirmationCode($confirmation_code)->firstOrFail();
 49         $user->confirmation_code = null;
 50         $user->confirmed = true;
 51         $user->save();
 52 
 53         return redirect(route('login'))->with('confirmation-success', trans('confirmation::confirmation.success'));
 54     }

In Line 46, uses a fixed provider name. The name of the provider can be freely set in config, and there are cases where multiple providers are used. Therefore, I think that it would be better to allow you to select the provider you want to use.

How about the changes like the following?

vendor/bestmomo/laravel-email-confirmation/src/Traits/RegistersUsers.php

    public function confirm($id, $confirmation_code)
    {   
+        $providers = config('auth.providers');
+        if(empty($providers[$this->provider]) {
+                $model = $providers[$this->provider]['model'];
+        } else {
+                $model = $providers['users']['model'];
+        }        
-        $model = config('auth.providers.users.model');

        $user = $model::whereId($id)->whereConfirmationCode($confirmation_code)->firstOrFail();
        $user->confirmation_code = null;
        $user->confirmed = true;
        $user->save();

        return redirect(route('login'))->with('confirmation-success', trans('confirmation::confirmation.success'));
    }   

app/Http/Controllers/Admin/Auth/RegisterController.php

  use Bestmomo\LaravelEmailConfirmation\Traits\RegistersUsers;

  class RegisterController extends Controller
  {  
      use RegistersUsers;
+      protected $provider = 'admins';

In the above example the admins provider is used. If provider is not selected, default users will be used. I'm not sure if this modification policy is suitable. Please give me your opinion.

bestmomo commented 6 years ago

Hello,

Thanks for suggestion, I think there must be a more general approch. I must dig deeper...

I think about something like that:

public function confirm($id, $confirmation_code)
{
    $model = $this->guard ()->getProvider()->createModel();
    $user = $model->whereId($id)->whereConfirmationCode($confirmation_code)->firstOrFail();
    $user->confirmation_code = null;
    $user->confirmed = true;
    $user->save();

    return redirect(route('login'))->with('confirmation-success', trans('confirmation::confirmation.success'));
}

What do you think ?

emile-yamaji commented 6 years ago

@bestmomo Thank you for answering. I think your approach is better than my approach.

emile-yamaji commented 6 years ago

I try to pull request with your approach !

bestmomo commented 6 years ago

ok ;)