Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
@joshcanhelp joshcanhelp force-pushed the fixed-client-create-links branch from e192050 to d0c3f64 Compare January 31, 2018 16:35
…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 joshcanhelp force-pushed the fixed-client-create-links branch from 0f95032 to 2ba3c50 Compare January 31, 2018 17:33
@joshcanhelp joshcanhelp force-pushed the fixed-client-create-links branch from 2ba3c50 to 66be3d0 Compare January 31, 2018 17:36
@joshcanhelp joshcanhelp requested a review from glena January 31, 2018 18:36
@joshcanhelp joshcanhelp added CH: Fixed and removed WIP labels Jan 31, 2018
@joshcanhelp
Copy link
Contributor Author

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

@joshcanhelp joshcanhelp requested a review from cocojoe January 31, 2018 21:18
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
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants