-
Notifications
You must be signed in to change notification settings - Fork 96
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
Deprecating wp-admin settings-related methods + classes #445
Conversation
joshcanhelp
commented
Apr 23, 2018
- WP_Auth0_Admin::cant_connect_to_auth0()
- WP_Auth0_Admin_Advanced::ADVANCED_DESCRIPTION
- WP_Auth0_Admin_Advanced::render_passwordless_enabled()
- WP_Auth0_Admin_Advanced::render_passwordless_method()
- WP_Auth0_Admin_Advanced::connections_validation()
- WP_Auth0_Admin_Appearance::APPEARANCE_DESCRIPTION
- WP_Auth0_Admin_Appearance::render_appearance_description()
- WP_Auth0_Admin_Basic::BASIC_DESCRIPTION
- WP_Auth0_Admin_Basic::render_allow_signup_regular_multisite()
- WP_Auth0_Admin_Basic::render_allow_signup_regular()
- WP_Auth0_Admin_Basic::render_basic_description()
- WP_Auth0_Admin_Dashboard class
- WP_Auth0_Admin_Features::FEATURES_DESCRIPTION
- WP_Auth0_Admin_Features::render_features_description()
- WP_Auth0_Admin_Generic::render_a0_switch()
- WP_Auth0_Dashboard_Plugins_Age class
- WP_Auth0_Dashboard_Plugins_Gender class
- WP_Auth0_Dashboard_Plugins_Generic class
- WP_Auth0_Dashboard_Plugins_IdP class
- WP_Auth0_Dashboard_Plugins_Income class
- WP_Auth0_Dashboard_Plugins_Location class
- WP_Auth0_Dashboard_Plugins_Signups class
- WP_Auth0_Dashboard_Widgets class
- WP_Auth0_InitialSetup::notify_setup()
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.
I think we have different concepts over "deprecation". A deprecated method/constant should still work as expected. If it is being deprecated, it means that a better alternative should be used instead. But that doesn't mean that the method will have it's logic removed or its value changed to an empty string. Let's discuss this internally Josh.
lib/admin/WP_Auth0_Admin.php
Outdated
@@ -248,9 +248,12 @@ public function admin_enqueue() { | |||
wp_enqueue_style( 'media' ); | |||
} | |||
|
|||
// TODO: Deprecate, not used | |||
/** | |||
* @deprecated 3.6.0 - Handled empty auth0_app_token notification, not needed |
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.
"not needed" sounds like an internal note. If this is user facing I'd use "This method is no longer required since..".
} | ||
|
||
// TODO: Deprecate | ||
/** | ||
* @deprecated 3.6.0 - Passwordless method is determined by tenant or Connections setting |
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.
Passwordless method should be first enabled on Connections and then enabled for each Application. No tenants involved AFAIK.
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.
Passwordless method should be first enabled on Connections
Passwordless methods are connections:
https://manage.auth0.com/#/connections/passwordless
Still, wording could be improved
@@ -618,50 +588,12 @@ public function link_accounts_validation( $old_options, $input ) { | |||
return $this->rule_validation( $old_options, $input, 'link_auth0_users', WP_Auth0_RulesLib::$link_accounts['name'] . '-' . get_auth0_curatedBlogName(), $link_script ); | |||
} | |||
|
|||
// TODO: Deprecate | |||
/** | |||
* @deprecated 3.6.0 - The setting this validated, passwordless_method, has been removed |
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.
The setting this validated
typo?
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.
@lbalmaceda -No but maybe poorly worded. This validated the passwordless_method
setting but the setting is being removed so the validation is going as well.
/** | ||
* Class WP_Auth0_Dashboard_Plugins_Age | ||
* | ||
* @deprecated 3.6.0 - Not supporting dashboard widgets |
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.
do you mean you no longer support dashboard widgets? In that case, I'd rephrase every mention of this to:
"Since x.y.z this library no longer supports dashboard widgets"
877ca6a
to
9efbddf
Compare
public function render_auth0_server_domain( $args = array() ) { | ||
$this->render_text_field( $args['label_for'], $args['opt_name'] ); | ||
$this->render_field_description( | ||
__( 'The Auth0 domain used by the setup wizard to fetch your account information', 'wp-auth0' ) | ||
); | ||
} | ||
|
||
public function render_metrics() { |
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.
Moved down, removed HTML, added deprecation docblock
public function render_jwt_auth_integration( $args = array() ) { | ||
$this->render_switch( $args['label_for'], $args['opt_name'] ); | ||
$this->render_field_description( __( 'This will enable the JWT Auth Users Repository override', 'wp-auth0' ) ); | ||
} | ||
|
||
// TODO: Deprecate, moved to WP_Auth0_Admin_features | ||
public function render_passwordless_enabled( $args = array() ) { |
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.
Moved down, added deprecation docblock
} | ||
|
||
// TODO: Deprecate | ||
public function render_passwordless_method() { |
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.
Moved down, removed HTML, added deprecation docblock
} | ||
|
||
// TODO: Deprecate | ||
public function render_advanced_description() { |
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.
Moved down, added deprecation docblock
@@ -618,53 +774,6 @@ public function link_accounts_validation( $old_options, $input ) { | |||
return $this->rule_validation( $old_options, $input, 'link_auth0_users', WP_Auth0_RulesLib::$link_accounts['name'] . '-' . get_auth0_curatedBlogName(), $link_script ); | |||
} | |||
|
|||
// TODO: Deprecate | |||
public function connections_validation( $old_options, $input ) { |
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.
Moved down, removed validation, added deprecation docblock
@@ -175,15 +267,6 @@ public function render_language_dictionary( $args = array() ) { | |||
); | |||
} | |||
|
|||
// TODO: Deprecate | |||
public function render_appearance_description() { |
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.
Moved down, added deprecation docblock
@@ -179,66 +268,6 @@ public function render_allow_signup() { | |||
); | |||
} | |||
|
|||
public function render_allow_wordpress_login( $args = array() ) { |
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.
Moved down, added regular docblock
} | ||
|
||
// TODO: Deprecate | ||
public function render_allow_signup_regular_multisite() { |
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.
Moved down, removed output, added deprecation docblock
} | ||
|
||
// TODO: Deprecate | ||
public function render_allow_signup_regular() { |
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.
Moved down, removed output, added deprecation docblock
public function render_override_wp_avatars( $args = array() ) { | ||
$this->render_switch( $args['label_for'], $args['opt_name'] ); | ||
$this->render_field_description( | ||
__( 'Overrides the WordPress avatar with the Auth0 profile avatar', 'wp-auth0' ) | ||
); | ||
} | ||
|
||
// TODO: Deprecate | ||
public function render_features_description() { |
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.
Moved down, added deprecation docblock
@@ -115,18 +115,6 @@ protected function rule_validation( $old_options, $input, $key, $rule_name, $rul | |||
return $input; | |||
} | |||
|
|||
// TODO: Deprecate | |||
protected function render_a0_switch( $id, $name, $value, $checked ) { |
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.
Moved down, added deprecation docblock
8271475
to
9292f4d
Compare
- WP_Auth0_Admin::cant_connect_to_auth0() - WP_Auth0_Admin_Advanced::ADVANCED_DESCRIPTION - WP_Auth0_Admin_Advanced::render_passwordless_enabled() - WP_Auth0_Admin_Advanced::render_passwordless_method() - WP_Auth0_Admin_Advanced::connections_validation() - WP_Auth0_Admin_Appearance::APPEARANCE_DESCRIPTION - WP_Auth0_Admin_Appearance::render_appearance_description() - WP_Auth0_Admin_Basic::BASIC_DESCRIPTION - WP_Auth0_Admin_Basic::render_allow_signup_regular_multisite() - WP_Auth0_Admin_Basic::render_allow_signup_regular() - WP_Auth0_Admin_Basic::render_basic_description() - WP_Auth0_Admin_Dashboard class - WP_Auth0_Admin_Features::FEATURES_DESCRIPTION - WP_Auth0_Admin_Features::render_features_description() - WP_Auth0_Admin_Generic::render_a0_switch() - WP_Auth0_Dashboard_Plugins_Age class - WP_Auth0_Dashboard_Plugins_Gender class - WP_Auth0_Dashboard_Plugins_Generic class - WP_Auth0_Dashboard_Plugins_IdP class - WP_Auth0_Dashboard_Plugins_Income class - WP_Auth0_Dashboard_Plugins_Location class - WP_Auth0_Dashboard_Plugins_Signups class - WP_Auth0_Dashboard_Widgets class - WP_Auth0_InitialSetup::notify_setup()
9292f4d
to
caa2d28
Compare
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.
LGTM 🎉