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

Improved setup wizard client create process #389

Merged
merged 3 commits into from
Feb 2, 2018

Conversation

joshcanhelp
Copy link
Contributor

@joshcanhelp joshcanhelp commented Jan 31, 2018

  • Refactored WP_Auth0_Api_Client::create_client() to improve the URLs being saved by default and combine create with update (web_origins can now be pushed during create)
  • Fix for blank site title causing client create to fail reported here
  • Helper functions on WP_Auth0_DBManager to grab correct links in the correct format

@joshcanhelp joshcanhelp added this to the v3-Next milestone Jan 31, 2018
…RL settings; fixing empty site title causing error in client creation; adding helper functions to WP_Auth0_Options for getting default client URL settings
@joshcanhelp
Copy link
Contributor Author

@glena - Main thing here is just making sure I'm using the right default URLs in create_client

Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general +1 for Doc Headers

$protocol = $this->_get_boolean( $this->wp_options->get( 'force_https_callback' ) ) ? 'https' : null;

return site_url( 'index.php?auth0=1', $protocol );
$protocol = $this->_get_boolean( $this->wp_options->get( 'force_https_callback' ) ) ? 'https' : '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why '' vs null?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is used later $site_url = empty( $protocol ) ? site_url( 'index.php' ) : site_url( 'index.php', $protocol );.

Honestly it is the same but would prefer null too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cocojoe @glena - I agree, change made!

@cocojoe
Copy link
Member

cocojoe commented Feb 1, 2018

Please test 😄 Might be worth adding some basic tests at least to ensure all the various URL generators/modifiers are valid.

@joshcanhelp joshcanhelp merged commit f10e2be into dev Feb 2, 2018
@joshcanhelp joshcanhelp deleted the fixed-client-create-links branch February 2, 2018 18:33
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants