fix: paging parsing issues #728

annguy3n · 2021-07-01T06:53:54Z

This PR fixes 2 issues:

raw response from parse is a relative path, when used with BaselessSettings and credentials: same-origin directly from the TS sdk.
total count header appears lowercase x-total-count

github-actions · 2021-07-01T07:01:11Z

Typescript Tests

    3 files -   4   72 suites - 4 41s ⏱️ - 3m 0s
136 tests - 24 136 ✔️ - 21 0 💤 -   3 0 ❌ ±0
462 runs - 97 462 ✔️ - 85 0 💤 - 12 0 ❌ ±0

Results for commit 4379307. ± Comparison against base commit 1a835a8.

This pull request removes 25 and adds 1 tests. Note that renamed tests count towards both.

LookerNodeSDK Dashboard endpoints create and update dashboard ‑ LookerNodeSDK Dashboard endpoints create and update dashboard
LookerNodeSDK Dashboard endpoints search_dashboards ‑ LookerNodeSDK Dashboard endpoints search_dashboards
LookerNodeSDK Datagroups gets all datagroups ‑ LookerNodeSDK Datagroups gets all datagroups
LookerNodeSDK Node environment no INI ‑ LookerNodeSDK Node environment no INI
LookerNodeSDK PUT smoke test set default color collection ‑ LookerNodeSDK PUT smoke test set default color collection
LookerNodeSDK Query calls create and run query ‑ LookerNodeSDK Query calls create and run query
LookerNodeSDK Query calls run_inline_query ‑ LookerNodeSDK Query calls run_inline_query
LookerNodeSDK Types with enums CreateQueryTask serializes and deserializes ‑ LookerNodeSDK Types with enums CreateQueryTask serializes and deserializes
LookerNodeSDK User CRUD-it checks create, update, and delete user ‑ LookerNodeSDK User CRUD-it checks create, update, and delete user
LookerNodeSDK User searches bad search returns no results ‑ LookerNodeSDK User searches bad search returns no results
…

paging pager works on relative url ‑ paging pager works on relative url

github-actions · 2021-07-01T16:20:37Z

Typescript Tests

    7 files ±0   76 suites ±0 3m 50s ⏱️ +9s
161 tests +1 158 ✔️ +1   3 💤 ±0 0 ❌ ±0
562 runs +3 550 ✔️ +3 12 💤 ±0 0 ❌ ±0

Results for commit 0798b5c. ± Comparison against base commit 1a835a8.

joeldodge79 · 2021-07-01T16:24:06Z

packages/sdk-rtl/src/paging.ts

@@ -34,7 +34,7 @@ import { IAPIMethods } from './apiMethods'
 import { BaseTransport } from './baseTransport'

 export const LinkHeader = 'link'
-export const TotalCountHeader = 'X-Total-Count'


I feel like I see the capitalized form of http headers more often: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers

also, server/client should consider them case-insensitive: https://stackoverflow.com/a/5259004/13802304

Yeah, it's a problem with the headers handling in JS. I responded via chat that I think we need to support both the Title case and lowercase at least.

joeldodge79 · 2021-07-01T16:45:18Z

packages/sdk-rtl/src/paging.ts

@@ -363,7 +363,7 @@ export class Paging<TSuccess extends ILength, TError>
  }

  parse(raw: IRawResponse): IPager<TSuccess, TError> {
-    const req = new URL(raw.url)
+    const req = new URL(raw.url, 'https://localhost')


hm, do we have access to the settings base_url here?

Exactly my thought, as well. Sorry, I should have responded to the PR rather than via chat.

jkaster

LGTM

jkaster · 2021-07-01T19:34:53Z

packages/sdk-rtl/src/paging.ts

+      raw.headers[name] ||
+      raw.headers[name.toLowerCase()] ||
+      raw.headers[name.toUpperCase()]


Recognizes all three reasonable casing options for the header

jkaster · 2021-07-01T19:36:55Z

packages/sdk-rtl/src/paging.ts

  parse(raw: IRawResponse): IPager<TSuccess, TError> {
-    const req = new URL(raw.url)
-    const params = req.searchParams
+    const params = new URLSearchParams(raw.url)


Handles full or relative urls

github-actions · 2021-07-01T19:46:49Z

Typescript Tests

    7 files   76 suites 3m 50s ⏱️
161 tests 158 ✔️   3 💤 0 ❌
562 runs 550 ✔️ 12 💤 0 ❌

Results for commit 4286ec7.

🤖 I have created a release \*beep\* \*boop\* --- <details><summary>sdk-codegen-all: 1.9.1</summary> ### Bug Fixes * paging parsing issues ([#728](https://www.github.com/looker-open-source/sdk-codegen/issues/728)) ([f8eec43](https://www.github.com/looker-open-source/sdk-codegen/commit/f8eec43bdfbe337d41b1da02c127d690c8815ed3)) </details> <details><summary>@looker/sdk-codegen-scripts: 21.0.20</summary> ### Bug Fixes * noStreams skips tally headers in generated source ([#727](https://www.github.com/looker-open-source/sdk-codegen/issues/727)) ([113c2c5](https://www.github.com/looker-open-source/sdk-codegen/commit/113c2c50c07c621cf94841af75557704fc3f5df7)) ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19 * @looker/sdk-node bumped from ^21.8.0 to ^21.8.1 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/sdk-rtl: 21.0.15</summary> ### Bug Fixes * paging parsing issues ([#728](https://www.github.com/looker-open-source/sdk-codegen/issues/728)) ([f8eec43](https://www.github.com/looker-open-source/sdk-codegen/commit/f8eec43bdfbe337d41b1da02c127d690c8815ed3)) </details> <details><summary>@looker/sdk: 21.8.2</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/sdk-codegen: 21.0.19</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/extension-sdk: 21.8.2</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/sdk-node: 21.8.1</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/code-editor: 0.1.4</summary> ### Dependencies * The following workspace dependencies were updated * devDependencies * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19 </details> <details><summary>@looker/extension-sdk-react: 21.8.2</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/extension-sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/wholly-sheet: 0.5.10</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 * devDependencies * @looker/sdk-node bumped from ^21.8.0 to ^21.8.1 </details> <details><summary>@looker/run-it: 0.9.11</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19 * @looker/code-editor bumped from ^0.1.3 to ^0.1.4 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 </details> <details><summary>@looker/hackathon: 21.8.1</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/extension-sdk bumped from ^21.8.1 to ^21.8.2 * @looker/extension-sdk-react bumped from ^21.8.1 to ^21.8.2 * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 * @looker/wholly-sheet bumped from ^0.5.9 to ^0.5.10 </details> <details><summary>@looker/api-explorer: 0.9.11</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/code-editor bumped from ^0.1.3 to ^0.1.4 * @looker/run-it bumped from ^0.9.10 to ^0.9.11 * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19 * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15 * devDependencies * @looker/sdk-codegen-scripts bumped from ^21.0.19 to ^21.0.20 * @looker/sdk-node bumped from ^21.8.0 to ^21.8.1 </details> <details><summary>@looker/extension-api-explorer: 21.8.1</summary> ### Dependencies * The following workspace dependencies were updated * dependencies * @looker/api-explorer bumped from ^0.9.10 to ^0.9.11 * @looker/extension-sdk bumped from ^21.8.1 to ^21.8.2 * @looker/extension-sdk-react bumped from ^21.8.1 to ^21.8.2 * @looker/run-it bumped from ^0.9.10 to ^0.9.11 * @looker/sdk bumped from ^21.8.1 to ^21.8.2 * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19 </details> This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

annguy3n added 2 commits June 30, 2021 23:52

updates URL parse to works with relative path

9a5a992

update total count header to be lower case

4379307

google-cla bot added the cla: yes label Jul 1, 2021

annguy3n changed the title ~~Fix paging parsing issues~~ fix: paging parsing issues Jul 1, 2021

annguy3n requested a review from jkaster July 1, 2021 16:02

empty commit to retest

0798b5c

joeldodge79 reviewed Jul 1, 2021

View reviewed changes

jkaster added 4 commits July 1, 2021 19:13

Merge remote-tracking branch 'origin/main' into an/fix-parsing-error

59f6843

fix: detect any casing of header

fa7641b

added a comment for new URL()` base path

7b791b6

Simplify offset and limit value extraction

4286ec7

jkaster approved these changes Jul 1, 2021

View reviewed changes

jkaster merged commit f8eec43 into main Jul 1, 2021

jkaster deleted the an/fix-parsing-error branch July 1, 2021 20:05

This comment has been minimized.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: paging parsing issues #728

fix: paging parsing issues #728

annguy3n commented Jul 1, 2021 •

edited

Loading

github-actions bot commented Jul 1, 2021

github-actions bot commented Jul 1, 2021

joeldodge79 Jul 1, 2021

jkaster Jul 1, 2021

joeldodge79 Jul 1, 2021

jkaster Jul 1, 2021

jkaster left a comment

jkaster Jul 1, 2021

jkaster Jul 1, 2021

github-actions bot commented Jul 1, 2021

This comment has been minimized.

fix: paging parsing issues #728

fix: paging parsing issues #728

Conversation

annguy3n commented Jul 1, 2021 • edited Loading

github-actions bot commented Jul 1, 2021

Typescript Tests

github-actions bot commented Jul 1, 2021

Typescript Tests

joeldodge79 Jul 1, 2021

Choose a reason for hiding this comment

jkaster Jul 1, 2021

Choose a reason for hiding this comment

joeldodge79 Jul 1, 2021

Choose a reason for hiding this comment

jkaster Jul 1, 2021

Choose a reason for hiding this comment

jkaster left a comment

Choose a reason for hiding this comment

jkaster Jul 1, 2021

Choose a reason for hiding this comment

jkaster Jul 1, 2021

Choose a reason for hiding this comment

github-actions bot commented Jul 1, 2021

Typescript Tests

This comment has been minimized.

annguy3n commented Jul 1, 2021 •

edited

Loading