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

fix(core): remove slash suffix from uris [release] #628

Merged
merged 8 commits into from
Aug 10, 2021

Conversation

andersfylling
Copy link
Contributor

@andersfylling andersfylling commented Aug 3, 2021

Sometimes the requests urls ends with /. This becomes a problem when using GET parameters:
https://api.cognitedata.com/api/playground/projects/cognitesdk-js/documents/feedback/?status=CREATED

While /documents/feedback/?status=CREATED responds with a 404, /documents/feedback?status=CREATED responds with an expected 200 status code.

image
image

@andersfylling andersfylling changed the title fix(core): remove trailing suffix from urls fix(core): remove trailing suffix from urls [release] Aug 3, 2021
@andersfylling andersfylling marked this pull request as ready for review August 3, 2021 11:09
@andersfylling andersfylling requested review from a team and vegardok August 3, 2021 11:09
@codecov
Copy link

codecov bot commented Aug 3, 2021

Codecov Report

Merging #628 (bf6cfc4) into master (29b3a17) will decrease coverage by 1.51%.
The diff coverage is 69.38%.

@@            Coverage Diff             @@
##           master     #628      +/-   ##
==========================================
- Coverage   77.15%   75.63%   -1.52%     
==========================================
  Files          87       90       +3     
  Lines        2779     3185     +406     
  Branches      390      440      +50     
==========================================
+ Hits         2144     2409     +265     
- Misses        599      776     +177     
+ Partials       36        0      -36     
Impacted Files Coverage Δ
packages/stable/src/__tests__/testUtils.ts 100.00% <ø> (ø)
packages/stable/src/types.ts 100.00% <ø> (ø)
.../beta/src/api/templateGroups/templateGraphQlApi.ts 40.00% <14.28%> (-26.67%) ⬇️
packages/stable/src/api/3d/nodes3DApi.ts 81.81% <20.00%> (-18.19%) ⬇️
packages/stable/src/api/projects/projectsApi.ts 78.94% <20.00%> (-21.06%) ⬇️
packages/beta/src/types.ts 44.11% <33.33%> (-3.89%) ⬇️
packages/stable/src/api/3d/revisions3DApi.ts 95.65% <33.33%> (-4.35%) ⬇️
packages/core/src/baseCogniteClient.ts 74.54% <40.35%> (-17.46%) ⬇️
...eta/src/api/templateGroups/templateInstancesApi.ts 82.60% <50.00%> (-1.71%) ⬇️
packages/beta/src/cogniteClient.ts 96.96% <66.66%> (+0.09%) ⬆️
... and 83 more

@andersfylling andersfylling changed the title fix(core): remove trailing suffix from urls [release] fix(core): remove slash suffix from uris [release] Aug 3, 2021
@andersfylling
Copy link
Contributor Author

To make a more unbiased argument; The RFC states that uri/path segments are to be separated by slashes. Given that get parameters aren't a segment, there should not be a slash after the path, separating the path and get parameters.

@cognite-bulldozer cognite-bulldozer bot requested a review from a team August 10, 2021 09:36
@andersfylling andersfylling merged commit fe65d57 into master Aug 10, 2021
@andersfylling andersfylling deleted the fix/url-trailing-slash branch August 10, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants