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.

Sign up for GitHub

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

Jump to bottom

feat: closest path match for API Explorer #811

Merged
merged 12 commits into from
Sep 3, 2021
Merged

feat: closest path match for API Explorer #811

merged 12 commits into from
Sep 3, 2021

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Sep 3, 2021 •
edited
Loading

This has more than just closest path match, but everything else was part of the journey

  • OAuth routing is cleaned up
  • On-demand spec loading for Diffs is fixed (it started crashing after the last APIX update)

Closest path match

Closest path match means that changing spec versions in API Explorer (extension and stand-alone) will land the user on the "closest" path match when the spec key (API version) changes.

For example:

  1. Original path: /3.1/methods/Homepage/delete_homepage and the user select API 4.0 creating
  2. Tried path: /4.0/methods/Homepage/delete_homepage endpoint doesn't exist, so APIX ends up at
  3. Final path: /4.0/methods/Homepage because the Homepage category still exists

Otherwise, APIX may end up at /4.0/methods.

"Closest match" rules also apply for Types, but because types currently have no Category (a TODO item) APIX just ends up at <speckey>/types if the type in spec A doesn't exist in spec B

jkaster and others added 8 commits August 23, 2021 22:34
@jkaster
fix: fallback on spec fetch and spec key change
ffc761f
@jkaster
Getting better at the fallback
fc19c8a 
However, specs are being loaded before OAuth redirect occurs on return from OAuth handshake
@jkaster
Still losing method name URLs when reloading API Explorer
9c64496 
It's a "too much OAuth" issue
@jkaster
Don't have double OAuth calls but now the spec doesn't get loaded
53c776a
@jkaster
Merge remote-tracking branch 'origin/main' into jk/nice_redirect
9b5bed9 
# Conflicts:
#	packages/sdk/src/4.0/methodsInterface.ts
@jkaster
Still fighting OAuth calls
3993613
@bryans99
Fix OAUTH return.
a654024 
1. OauthScene hoisted to ApiExplorer from AppRouter.
2. AppRouter simplified now that OauthScene hoisted.
3. Simplified async functions in OauthScene. Fixed bug that lost
returnUrl.
4. Fixed NPE in SideNav
@jkaster
fix DiffScene for on-demand spec load
08375aa
@google-cla google-cla bot added the cla: yes label Sep 3, 2021
@jkaster jkaster marked this pull request as draft September 3, 2021 01:38
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

APIX Tests

    1 files    77 suites   2m 4s ⏱️
287 tests 274 ✔️ 13 💤 0 ❌
306 runs  293 ✔️ 13 💤 0 ❌

Results for commit 1a46454.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Codegen Tests

  1 files    6 suites   57s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 1a46454.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Codegen Tests

  1 files    6 suites   52s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 67e81e5.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

APIX Tests

    1 files    77 suites   2m 54s ⏱️
287 tests 272 ✔️ 13 💤 2 ❌
306 runs  291 ✔️ 13 💤 2 ❌

For more details on these failures, see this check.

Results for commit 67e81e5.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Typescript Tests

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

Results for commit 67e81e5.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Codegen Tests

  1 files    6 suites   57s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 2af9b33.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

APIX Tests

    1 files    77 suites   2m 42s ⏱️
287 tests 274 ✔️ 13 💤 0 ❌
306 runs  293 ✔️ 13 💤 0 ❌

Results for commit 2af9b33.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Typescript Tests

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

Results for commit 2af9b33.

@jkaster jkaster marked this pull request as ready for review September 3, 2021 16:07
@jkaster jkaster requested a review from jhardy September 3, 2021 16:36
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Codegen Tests

  1 files    6 suites   52s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit c68c9fd.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

APIX Tests

    1 files    77 suites   2m 43s ⏱️
287 tests 274 ✔️ 13 💤 0 ❌
306 runs  293 ✔️ 13 💤 0 ❌

Results for commit c68c9fd.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Typescript Tests

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

Results for commit c68c9fd.

bryans99
bryans99 approved these changes Sep 3, 2021
View reviewed changes
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

aside from issues I found. Let's merge this and fix issues in smaller PRs

packages/api-explorer/src/components/SelectorContainer/ApiSpecSelector.spec.tsx
@@ -34,6 +34,16 @@ import { ApiSpecSelector } from './ApiSpecSelector'

const specState = getLoadedSpecState()

jest.mock('react-router-dom', () => {
const ReactRouterDOM = jest.requireActual('react-router-dom')