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

AuthController implements user config #59

Closed
wants to merge 1 commit into from

Conversation

MGatner
Copy link
Collaborator

@MGatner MGatner commented Jun 27, 2019

There are a few places that loaded the Myth config file directly. Probably need some better way of handling this centrally (like perhaps having them all call the service, which would need a getConfig() command?). For now this fixes AuthController to check for a user config file in order to use the $allowRegistration property properly.

@lonnieezell
Copy link
Owner

Ok - that's something I didn't think about with your other PR, but the config() helper in CI4 does handle caching those for performance reasons, so this wouldn't be needed if we instead fix the order in CI4.

@MGatner
Copy link
Collaborator Author

MGatner commented Jun 28, 2019

Okay, I think I figured this one out. First, we were calling config(Auth::class) but since the namespace of Services.php is Myth\Auth\Config it was using Myth\Auth\Config\Auth so that wasn't going to work. Passing 'Auth' as a string in theory would do it, but config()(with $getShared) passes the un-namespaced string to $locator->locateFile which causes it to use legacyLocate() - which is App and System only.

I'll work on a PR of how I think this should be handled, then we can revisit Myth:Auth and (hopefully) just use config('Auth') everywhere.

@MGatner
Copy link
Collaborator Author

MGatner commented Jun 28, 2019

FYI: codeigniter4/CodeIgniter4#2079

@lonnieezell
Copy link
Owner

That PR has been merged. Thanks!

@MGatner
Copy link
Collaborator Author

MGatner commented Jun 29, 2019

Redoing this to take advantage of the updated config() functionality.

@MGatner MGatner closed this Jun 29, 2019
@MGatner MGatner deleted the controller-user-config branch June 29, 2019 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants