Update AuthController.php

[mjamilasfihani]

Fix #560 Would you mine to take some review @MGatner ? Since I little bit confused which one we must use in redirect function

[MGatner]

There's actually a problem with Routes.php that will prevent us from doing this correctly (probably my fault, missed it during hasty review). The $reservedRoutes Config property is in the format $routeName => $routePath so when Routes loads then it should be using the key as the route name, not the value:

-    $routes->get($reservedRoutes['login'], 'AuthController::login', ['as' => $reservedRoutes['login']]);
+    $routes->get($reservedRoutes['login'], 'AuthController::login', ['as' => 'login']);
// etc

If you want to make those changes in this PR then you can just use the route name directly in your controller changes:

-            return redirect()->to(route_to($this->config->reservedRoutes['reset-password']) . '?token=' . $this->auth->user()->reset_hash)->withCookies();
+            $url = url_to('reset-password') . '?token=' . $this->auth->user()->reset_hash);
+            return redirect()->to($url)->withCookies();

[mjamilasfihani]

thank you for the review. it's fine, nobody is perfect :100: okay, I'll check it up.

[mjamilasfihani]

it's seem the cs-fixer has newer version (maybe), but I would like to re-styling it in different PR.

[mjamilasfihani]

PHPUnit has worked as well, would you like to start reviewing @MGatner with me?

[mjamilasfihani]

Thanks, do I have to change all the uri action in views too? I think, it still relevant with this PR.

[MGatner]

I think most of the views use the named route, so are fine. If you see any that use the Config value as a route name then that is a problem.

[mjamilasfihani]

Well noted.

[MGatner]

@manageruz Any input on this? Feel free to merge. Someone will need to update the coding styles and static analysis for the pipeline to start working again.

[manageruz]

Thanks, do I have to change all the uri action in views too? I think, it still relevant with this PR.

If you mean form tag's action url, they are all uses url_to helper with named routes. So no need to change them.

[manageruz]

@manageruz Any input on this? Feel free to merge. Someone will need to update the coding styles and static analysis for the pipeline to start working again.

It looks good as you mentioned above. We already have two PR's to fix pipeline issues (rector and csfixer), but it would be great if you can help with PHPStan errors. I can't make my mind for proper solution with it.

[MGatner]

@mjamilasfihani Can you rebase? We had to fix some of the static analysis in order to get the pipelines passing and created a small merge conflict.

[mjamilasfihani]

Ready to go @MGatner :)

[MGatner]

Thanks for rebasing! You will need to use the shorthand class names in docblock comments with an import statement to pass Rector. See here for desired changes: https://github.com/lonnieezell/myth-auth/actions/runs/3326512578/jobs/5500293576#step:7:1

Or you can run the following:

./vendor/bin/rector process
composer style

[mjamilasfihani]

Apologize, I was not so care with composer scripts @MGatner . Well, @manageruz you can do your job now.

[manageruz]

Thank you @mjamilasfihani for PR. And big thanks to the @MGatner for the review and help.

[MGatner]

🥳