[Website] GitHub export modal: Correctly compute the root path when exporting the entire site #2103
+6
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…xporting the entire site Defaults to `/` as a root path instead of `/playground` to fix an error in the full site exporting flow. One of the previous PRs for the GitHub export modal enforced using `/playground` as the exported path when the target path in repo was missing. This effectively broke the export feature as the initial REST API requests to list the existing files under the specified repository path failed with error 404. Furthermore, with all the data massaging done in the form, one of the steps first replaced `"/"` with `""` and then filtered it out as an empty value. This PR ensures that `"/"` remains a fallback for the relative repo path. Really, we could use some automated tests to confirm all the different use-cases for that modal continue to work. It's complicated with the OAuth procedure required. What we could do is either mock the responses or migrate to the isomorphic-git client and ditch the REST API dependency. Git protocol is so much faster than the REST API anyway. ## Testing instructions 1. Go to the GitHub export modal 2. Select "custom paths" and export the `/wordpress` directory to `/` in the repo 3. Give it a new, empty repo 4. Start the export procedure 5. Confirm a lot of network requests start. There should be no errors in red text in sight.
adamziel
added
[Type] Bug
Setup GitHub client locally
|
|
Really good client setup instructions @bgrgicak 😍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Defaults to
/as a root path instead of/playgroundto fix an error in the full site exporting flow.One of the previous PRs for the GitHub export modal enforced using
/playgroundas the exported path when the target path in repo was missing. This effectively broke the export feature as the initial REST API requests to list the existing files under the specified repository path failed with error 404.Furthermore, with all the data massaging done in the form, one of the steps first replaced
"/"with""and then filtered it out as an empty value. This PR ensures that"/"remains a fallback for the relative repo path.Really, we could use some automated tests to confirm all the different use-cases for that modal continue to work. It's complicated with the OAuth procedure required. What we could do is either mock the responses or migrate to the isomorphic-git client and ditch the REST API dependency. Git protocol is so much faster than the REST API anyway.
Testing instructions
/wordpressdirectory to/in the repoPossible closes #2084
cc @bgrgicak