FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(http): keep slashes in queryParams in redirects by jsjoeio · Pull Request #4928 · coder/code-server · GitHub

Repository navigation

feat(http): keep slashes in queryParams in redirects - #4928

Merged
jsjoeio merged 3 commits into
mainfrom
jsjoeio/test-redirect
Mar 1, 2022
Merged

jsjoeio merged 3 commits into
mainfrom
jsjoeio/test-redirect

Conversation

jsjoeio commented Mar 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

When a user opens code-server at a url like localhost:8080 without the -e (ignore last opened flag), we redirect to that workspace and append the folder as a query param.

Before, it would encode the url to something like: localhost:8080/?folder=%2FUsers%2Fjp%2Fdev%2Fcoder

Now, it maintains the slashes and redirects to: localhost:8080/?folder=/Users/p/Fdev/coder

Fixes this issue coder/vscode#45 (comment) identified in the VS Code 1.64 update.

This allows us to easily test our redirect path construction logic where we get
the relative path, the query string and construct a redirect path.

By extracting this from `redirect`, we can easily test this logic in a unit
test.

I did this so we could test some logic where slashes in query strings should be
made human-friendly for users.
jsjoeio added the testing Anything related to testing label Mar 1, 2022
jsjoeio added this to the March 2022 milestone Mar 1, 2022
jsjoeio self-assigned this Mar 1, 2022
jsjoeio temporarily deployed to CI March 1, 2022 16:50 Inactive

github-actions Bot commented Mar 1, 2022 •
edited
Loading

Copy link
Copy Markdown

✨ Coder.com for PR #4928 deployed! It will be updated on every commit.

jsjoeio force-pushed the jsjoeio/test-redirect branch from 5b78e3c to 31e131b Compare March 1, 2022 17:35
jsjoeio temporarily deployed to CI March 1, 2022 17:35 Inactive
jsjoeio changed the title Jsjoeio/test redirect feat(http): keep slashes in queryParams in redirects Mar 1, 2022

codecov Bot commented Mar 1, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #4928 (8bc7645) into main (1465d8d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4928      +/-   ##
==========================================
+ Coverage   69.96%   70.02%   +0.05%     
==========================================
  Files          29       29              
  Lines        1655     1658       +3     
  Branches      364      364              
==========================================
+ Hits         1158     1161       +3     
  Misses        423      423              
  Partials       74       74              
Impacted Files Coverage Δ
src/node/http.ts 69.38% <100.00%> (+0.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1465d8d...8bc7645. Read the comment docs.

jsjoeio temporarily deployed to npm March 1, 2022 17:44 Inactive
jsjoeio marked this pull request as ready for review March 1, 2022 17:45
jsjoeio requested a review from a team March 1, 2022 17:45
jsjoeio mentioned this pull request Mar 1, 2022
28 tasks done
code-asher temporarily deployed to CI March 1, 2022 18:44 Inactive
code-asher temporarily deployed to npm March 1, 2022 18:55 Inactive
jsjoeio merged commit 506d3f4 into main Mar 1, 2022
jsjoeio deleted the jsjoeio/test-redirect branch March 1, 2022 19:11
jsjoeio modified the milestones: March 2022, 4.0.3 Mar 1, 2022
TinLe pushed a commit to TinLe/code-server that referenced this pull request Apr 23, 2022
* refactor(http): extract logic into constructRedirectPath

This allows us to easily test our redirect path construction logic where we get
the relative path, the query string and construct a redirect path.

By extracting this from `redirect`, we can easily test this logic in a unit
test.

I did this so we could test some logic where slashes in query strings should be
made human-friendly for users.

* feat(testing): add tests for constructRedirectPath

Co-authored-by: Asher <ash@coder.com>

This branch was previously deployed

2 inactive deployments
npm — 8bc76459 Deployed Mar 1, 2022 by code-asher
CI — 8bc76459 Deployed Mar 1, 2022 by code-asher
This file contains hidden or 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Anything related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL