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

feat: http response explorer for RunIt #800

Merged
merged 14 commits into from
Aug 18, 2021
Merged

feat: http response explorer for RunIt #800

merged 14 commits into from
Aug 18, 2021

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Aug 13, 2021

ResponseExplorer adds to the RunIt Response tab:

  • collapsible body section
  • collapsible header table
  • copy to clipboard for SDK calls view via new CodeCopy component
  • also fixes the spinner behavior for the default Response tab
  • Login and Configure options are only needed for OAuth
  • ConfigForm also shows OAuth client configuration information
  • When Save is clicked on the config form, API specifications are reloaded from the configured server
  • If values are filled out for RunIt before Login is clicked, when OAuth returns, RunIt is re-opened and the entered values are restored in the Request form
  • added yarn scripts
    • yarn dev:apix
    • yarn dev:xapix

New PR because old one was blocked by CLA requirements

@github-actions

This comment has been minimized.

@jkaster jkaster changed the title Response explorer implementation feat: http response explorer for RunIt Aug 13, 2021
@github-actions

This comment has been minimized.

Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move a little bit of code

@jkaster jkaster requested a review from bryans99 August 13, 2021 22:24
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Contributor

@josephaxisa josephaxisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is looking great! 👏

specs={specs}
envAdaptor={extensionEnvAdaptor}
setVersionsUrl={runItNoSet}
// TODO We need expand/collapse side nav for the headless extension before we enabled this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a ticket for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet. Wanted to discuss with you and @bryans99

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and @jhardy

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of the expand/collapse sidebar hamburger alternative. Created a ticket yet?


export type RunItHttpMethod = 'GET' | 'PUT' | 'POST' | 'PATCH' | 'DELETE'

/**
* Generic collection
*/
export type RunItValues = { [key: string]: any }
export type RunItValues = Record<string, any>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much cleaner :)

Comment on lines +127 to +140
// TODO make a badge for the verb.
// Once we are satisfied with the badge in the api-explorer package it should be moved here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a ticket for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an existing comment, and I did try method badge but it looked wonky. Want to give @jhardy a chance to look at it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont forget the ticket

Comment on lines 46 to 71
const getHeaders = (response: ResponseContent): HeaderTable => {
if (!response?.headers) return []
const result: HeaderTable = []
Object.entries(response.headers).forEach(([key, val]) =>
result.push([key, val])
)
return result
}

const getBodySize = (response: ResponseContent): string => {
const size =
!response || !response.body
? 0
: response?.body instanceof Blob
? response?.body.size
: response?.body.toString().length

return `${size} bytes`
}

export const NoWrap = styled(Span)`
display: inline-block;
direction: rtl;
white-space: nowrap;
overflow: hidden;
`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you consider putting these into a separate utils file? Are there any related unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a need for them outside of this component so I left them in this file.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Aug 17, 2021
@google-cla
Copy link

google-cla bot commented Aug 17, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

2 similar comments
@google-cla
Copy link

google-cla bot commented Aug 17, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@josephaxisa
Copy link
Contributor

@googlebot I fixed it.

- added yarn scripts to the root `package.json`:

  - `yarn dev:apix` for stand-alone API Explorer

  - `yarn dev:apixx` for extension API Explorer

- added `CodeCopy` component which is a wrapper for `CodeDisplay` and the clipboard UI

- copy to clipboard works both stand-alone and in extension

- ConfigForm.tsx `Verify` button no longer clears stored config. Only remove or save change the stored config.

- WIP on CopyClipboardIconButton.tsx but it's not working yet
- `yarn:xapix`
- code cleanup, but `CopyClipboardIconButton`  doesn't work yet
- Login returns and reopens the RunIt form with whatever values are set ... if values *have* been set

- Removed CopyClipboardIconButton.tsx because it does not pass A11y and is not mobile friendly
@google-cla
Copy link

google-cla bot commented Aug 17, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   2m 15s ⏱️
288 tests 275 ✔️ 13 💤 0 ❌
307 runs  294 ✔️ 13 💤 0 ❌

Results for commit 68716e4.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   2m 22s ⏱️
288 tests 275 ✔️ 13 💤 0 ❌
307 runs  294 ✔️ 13 💤 0 ❌

Results for commit 73bbd66.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   2m 22s ⏱️
288 tests 275 ✔️ 13 💤 0 ❌
307 runs  294 ✔️ 13 💤 0 ❌

Results for commit 6d9001b.

@github-actions
Copy link
Contributor

Typescript Tests

    6 files    75 suites   2m 51s ⏱️
164 tests 160 ✔️   4 💤 0 ❌
546 runs  534 ✔️ 12 💤 0 ❌

Results for commit 6d9001b.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 17, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   3m 1s ⏱️
288 tests 275 ✔️ 13 💤 0 ❌
307 runs  294 ✔️ 13 💤 0 ❌

Results for commit c723df5.

@github-actions
Copy link
Contributor

Typescript Tests

    6 files    75 suites   2m 52s ⏱️
164 tests 160 ✔️   4 💤 0 ❌
546 runs  534 ✔️ 12 💤 0 ❌

Results for commit c723df5.

bryans99
bryans99 previously approved these changes Aug 18, 2021
Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - there are a couple of items that could be cleaned up (they bug me :D ) but I'm okay without them.

There are a couple of buganizer tickets to write as well.

packages/api-explorer/src/ApiExplorer.tsx Outdated Show resolved Hide resolved
packages/api-explorer/src/ApiExplorer.tsx Outdated Show resolved Hide resolved
specs={specs}
envAdaptor={extensionEnvAdaptor}
setVersionsUrl={runItNoSet}
// TODO We need expand/collapse side nav for the headless extension before we enabled this
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of the expand/collapse sidebar hamburger alternative. Created a ticket yet?

Comment on lines +127 to +140
// TODO make a badge for the verb.
// Once we are satisfied with the badge in the api-explorer package it should be moved here
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont forget the ticket

@google-cla
Copy link

google-cla bot commented Aug 18, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   2m 57s ⏱️
288 tests 275 ✔️ 13 💤 0 ❌
307 runs  294 ✔️ 13 💤 0 ❌

Results for commit 5991eac.

@github-actions
Copy link
Contributor

Typescript Tests

    6 files    75 suites   2m 40s ⏱️
164 tests 160 ✔️   4 💤 0 ❌
546 runs  534 ✔️ 12 💤 0 ❌

Results for commit 5991eac.

Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkaster jkaster merged commit 3f273ea into main Aug 18, 2021
@jkaster jkaster deleted the jk/resp_exp_2 branch August 18, 2021 03:33
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants