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

Ensure there isn't a trailing slash on the REST URL before we break t… #586

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Jun 2, 2020

Description of the Change

Ensure there isn't a trailing slash on the REST URL before we break that URL into it's various pieces, in our get_site_url_from_rest_url function.

Fixes #585

Benefits

View links for external connections will be correct if that external connection is set up with a trailing slash

Possible Drawbacks

None

Verification Process

  1. Created a new external connection with a trailing slash
  2. Pushed a post to this connection
  3. Verified the View link pointed to the REST URL (i.e. example.com/wp-json/?p=10)
  4. Added the fixes here and tested again
  5. Verified the View link pointed to the FE URL (i.e. example.com/?p=10)

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

#585

@jeffpaul jeffpaul requested a review from dinhtungdu June 3, 2020 03:47
@jeffpaul jeffpaul added this to the 1.6.0 milestone Jun 3, 2020
@jeffpaul jeffpaul added the type:bug Something isn't working. label Jun 3, 2020
@jeffpaul
Copy link
Member

jeffpaul commented Jun 3, 2020

I'll hold off on merging this until we get the auth wizard PR merged in so that we don't create further conflicts on that merge.

@jeffpaul
Copy link
Member

jeffpaul commented Jun 8, 2020

The auth wizard PR is merged and it doesn't appear that we've got any merge conflicts here. So @dinhtungdu @dkotter you two 👍 to merge this one in?

@dkotter
Copy link
Collaborator Author

dkotter commented Jun 8, 2020

Good on my end

@jeffpaul jeffpaul merged commit d419951 into develop Jun 8, 2020
@jeffpaul jeffpaul deleted the fix/external-view-link branch July 2, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

View links for pushed content sometime use REST URL
3 participants