-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
feat: Disable Signups for new users #7254
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the work.
However it is a little bit more complicated than needed, please see my reviews.
Co-authored-by: Alexis Saettler <[email protected]>
Co-authored-by: Alexis Saettler <[email protected]>
Co-authored-by: Alexis Saettler <[email protected]>
Co-authored-by: Alexis Saettler <[email protected]>
@asbiin, can you give feedback on the current state of the code? Is there anything else that needs to be fixed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @davpsh I left some comments to fix the code, I've tested it but I got an issue running it ...
public function __construct( | ||
protected SignupHelper $signupHelper | ||
) { | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't working actually
public function __construct( | |
protected SignupHelper $signupHelper | |
) { | |
} |
@@ -40,6 +46,7 @@ public function __invoke(Request $request): Response | |||
} | |||
|
|||
return Inertia::render('Auth/Login', $data + [ | |||
'isSignupEnabled' => $this->signupHelper->isEnabled(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should fix it
'isSignupEnabled' => $this->signupHelper->isEnabled(), | |
'isSignupEnabled' => app(SignupHelper::class)->isEnabled(), |
public function __construct( | ||
protected SignupHelper $signupHelper, | ||
) { | ||
} | ||
|
||
public function handle(Request $request, Closure $next): Response | ||
{ | ||
abort_if(! $this->signupHelper->isEnabled(), 403, trans('Registration is currently disabled')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public function __construct( | |
protected SignupHelper $signupHelper, | |
) { | |
} | |
public function handle(Request $request, Closure $next): Response | |
{ | |
abort_if(! $this->signupHelper->isEnabled(), 403, trans('Registration is currently disabled')); | |
public function handle(Request $request, Closure $next): Response | |
{ | |
abort_if(! app(SignupHelper::class)->isEnabled(), 403, trans('Registration is currently disabled')); |
use Tests\TestCase; | ||
|
||
class RegistrationTest extends TestCase | ||
{ | ||
use DatabaseTransactions; | ||
|
||
#[Test] | ||
public function registration_screen_can_be_rendered() | ||
public function testAccessToRegistrationPage(): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep the new syntax
public function testAccessToRegistrationPage(): void | |
#[Test] | |
public function registration_screen_can_be_rendered(): void |
use Laravel\Jetstream\Jetstream; | ||
use PHPUnit\Framework\Attributes\Test; | ||
use Mockery; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Mockery; | |
use PHPUnit\Framework\Attributes\Test; | |
use Mockery; |
} | ||
|
||
#[Test] | ||
public function new_users_can_register() | ||
public function testRegistration(): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public function testRegistration(): void | |
#[Test] | |
public function new_users_can_register() |
These changes allow to enable/disable new user registration based on ENV setting. The feature was implemented in version 4.x and was not present in chandler. Fixes #6687
Changes:
APP_DISABLE_SIGNUP