-
Notifications
You must be signed in to change notification settings - Fork 92
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
Hide the signup tab if registrations are turned off #460
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,7 @@ public static function Instance() { | |
} | ||
|
||
public function is_wp_registration_enabled() { | ||
if ( is_multisite() ) { | ||
return users_can_register_signup_filter(); | ||
} | ||
return get_site_option( 'users_can_register', 0 ) == 1; | ||
return is_multisite() ? users_can_register_signup_filter() : get_site_option( 'users_can_register' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would that second value be the same to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Common practice. If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for that link ⚡️ |
||
} | ||
|
||
public function set_connection( $key, $value ) { | ||
|
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.
what if
is_wp_registration_enabled
is false so that the first condition passes; but then theallowSignup
is true. You would keepallowSignUp=true
instead of turning it off (sinceis_wp_registration_enabled
is false)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.
Intended. That would mean someone has
{"allowSignUp":true}
in their extra Lock settings and we want to respect that.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.
but won't that fail due to wp_reg being 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.
Using the main pathway, yes, but some of that is hook-able so it can be overridden. These extra settings JSON fields say they will override all other settings and I want them to be able to do that, even if it's edge case