feat: Tag scene filter selection as URL search parameter #1140

patnir41 · 2022-08-09T02:36:47Z

Provides the ability to share a link to a method/type tag landing page with an applied filter. For example, being able to load a tag scene and have its methods already filtered on POST.

github-actions · 2022-08-09T02:49:19Z

APIX Tests

    1 files   81 suites 6m 14s ⏱️
333 tests 320 ✔️ 13 💤 0 ❌
349 runs 336 ✔️ 13 💤 0 ❌

Results for commit 7fa7881.

github-actions · 2022-08-09T18:22:23Z

APIX Tests

    1 files ±0   82 suites ±0 5m 22s ⏱️ -44s
358 tests +1 345 ✔️ +1 13 💤 ±0 0 ❌ ±0
374 runs +1 361 ✔️ +1 13 💤 ±0 0 ❌ ±0

Results for commit e3b7e3d. ± Comparison against base commit ae59b95.

This pull request removes 2 and adds 3 tests. Note that renamed tests count towards both.

MethodTagScene it filters methods by operation type ‑ MethodTagScene it filters methods by operation type
TypeTagScene it filters methods by operation type ‑ TypeTagScene it filters methods by operation type

MethodTagScene it pushes filter to URL on toggle ‑ MethodTagScene it pushes filter to URL on toggle
Settings selectors selectTagFilter selects ‑ Settings selectors selectTagFilter selects
TypeTagScene it pushes filter to URL on toggle ‑ TypeTagScene it pushes filter to URL on toggle

github-actions · 2022-08-09T19:38:04Z

APIX Tests

    1 files ±  0   82 suites ±0 6m 2s ⏱️ -4s
371 tests +14 358 ✔️ +14 13 💤 ±0 0 ❌ ±0
387 runs +14 374 ✔️ +14 13 💤 ±0 0 ❌ ±0

Results for commit 853531a. ± Comparison against base commit ae59b95.

This pull request removes 2 and adds 16 tests. Note that renamed tests count towards both.

MethodTagScene it filters methods by operation type ‑ MethodTagScene it filters methods by operation type
TypeTagScene it filters methods by operation type ‑ TypeTagScene it filters methods by operation type

MethodTagScene it pushes filter to URL on toggle ‑ MethodTagScene it pushes filter to URL on toggle
Settings selectors selectTagFilter selects ‑ Settings selectors selectTagFilter selects
TypeTagScene it pushes filter to URL on toggle ‑ TypeTagScene it pushes filter to URL on toggle
path utils getSceneType returns correct scene type given location with pathname ‑ path utils getSceneType returns correct scene type given location with pathname
path utils getSceneType returns empty string if there is no scene type ‑ path utils getSceneType returns empty string if there is no scene type
path utils isValidFilter invalidates wrong parameter for methods and types ‑ path utils isValidFilter invalidates wrong parameter for methods and types
path utils isValidFilter validates &#x27;all&#x27; as a valid filter for methods and types ‑ path utils isValidFilter validates &#x27;all&#x27; as a valid filter for methods and types
path utils isValidFilter validates DELETE as a valid method filter ‑ path utils isValidFilter validates DELETE as a valid method filter
path utils isValidFilter validates ENUMERATED as a valid type filter ‑ path utils isValidFilter validates ENUMERATED as a valid type filter
path utils isValidFilter validates GET as a valid method filter ‑ path utils isValidFilter validates GET as a valid method filter
…

patnir41 · 2022-08-09T20:03:55Z

packages/api-explorer/src/components/SelectorContainer/SelectorContainer.tsx

@@ -51,6 +51,9 @@ export const SelectorContainer: FC<SelectorContainerProps> = ({
  ...spaceProps
 }) => {
  const searchParams = new URLSearchParams(location.search)
+  // TODO: noticing that there are certain pages where we must delete extra params
+  //       before pushing its link, what's a way we can handle this?


If this was not included here, navigating to the diffScene after selecting a filter will persist that parameter in the URL. I worry that there will be a growing number of scenes to render where certain parameters are no longer necessary and must be explicitly removed/nullified to avoid bloating the URL.

What if we include an optional toRemove parameter to the buildPath functions, which is a list of strings corresponding to search params which we will remove from the URL? This could clean up my current changes in SideNavMethods.tsx and SideNavTypes.tsx.

patnir41 · 2022-08-09T20:05:11Z

packages/api-explorer/src/components/SideNav/SideNavMethods.tsx

-                  searchParams.toString()
-                )}`}
+                to={() => {
+                  searchParams.delete('v')


Relates to prior comment about removing parameters from the URL in a cleaner fashion, pointing out for reviewers

patnir41 · 2022-08-09T21:13:59Z

packages/api-explorer/src/ApiExplorer.tsx

@@ -141,6 +157,14 @@ export const ApiExplorer: FC<ApiExplorerProps> = ({
          sdk: alias === allAlias ? null : alias,
        })
      }
+


This gives code smell, as I can see it continually expanding as new parameters are added. I'm wondering if there is any insight on how to go about syncing multiple parameters at once in this initialization effect.

I think a good way to do this will be to augment initSettings so that url parameters can be passed in and override persisted settings when applicable. As a result, this effect will then become a one liner that syncs the url with the store using navigate.

github-actions · 2022-08-09T21:17:35Z

APIX Tests

    1 files ±  0   82 suites ±0 6m 2s ⏱️ -4s
371 tests +14 358 ✔️ +14 13 💤 ±0 0 ❌ ±0
387 runs +14 374 ✔️ +14 13 💤 ±0 0 ❌ ±0

Results for commit 2d28c7f. ± Comparison against base commit ae59b95.

This pull request removes 2 and adds 16 tests. Note that renamed tests count towards both.

MethodTagScene it filters methods by operation type ‑ MethodTagScene it filters methods by operation type
TypeTagScene it filters methods by operation type ‑ TypeTagScene it filters methods by operation type

MethodTagScene it pushes filter to URL on toggle ‑ MethodTagScene it pushes filter to URL on toggle
Settings selectors selectTagFilter selects ‑ Settings selectors selectTagFilter selects
TypeTagScene it pushes filter to URL on toggle ‑ TypeTagScene it pushes filter to URL on toggle
path utils getSceneType returns correct scene type given location with pathname ‑ path utils getSceneType returns correct scene type given location with pathname
path utils getSceneType returns empty string if there is no scene type ‑ path utils getSceneType returns empty string if there is no scene type
path utils isValidFilter invalidates wrong parameter for methods and types ‑ path utils isValidFilter invalidates wrong parameter for methods and types
path utils isValidFilter validates &#x27;all&#x27; as a valid filter for methods and types ‑ path utils isValidFilter validates &#x27;all&#x27; as a valid filter for methods and types
path utils isValidFilter validates DELETE as a valid method filter ‑ path utils isValidFilter validates DELETE as a valid method filter
path utils isValidFilter validates ENUMERATED as a valid type filter ‑ path utils isValidFilter validates ENUMERATED as a valid type filter
path utils isValidFilter validates GET as a valid method filter ‑ path utils isValidFilter validates GET as a valid method filter
…

josephaxisa · 2022-08-09T22:12:05Z

packages/api-explorer/src/components/SelectorContainer/SelectorContainer.tsx

@@ -51,6 +51,9 @@ export const SelectorContainer: FC<SelectorContainerProps> = ({
  ...spaceProps
 }) => {
  const searchParams = new URLSearchParams(location.search)
+  // TODO: noticing that there are certain pages where we must delete extra params
+  //       before pushing its link, what's a way we can handle this?
+  searchParams.delete('v')


could we replace the link component with say a Span that behaves like a link and calls navigate on click?

packages/api-explorer/src/scenes/TypeTagScene/TypeTagScene.tsx

josephaxisa · 2022-08-09T22:16:38Z

packages/api-explorer/src/scenes/TypeTagScene/TypeTagScene.tsx

-            value === type.metaType.toString().toUpperCase()) && (
-            <Link key={index} to={buildTypePath(specKey, tag.name, type.name)}>
+          (selectedTagFilter === 'ALL' ||
+            selectedTagFilter === type.metaType.toString().toUpperCase()) && (


I'm leaning towards not having this check. If user passes in an invalid value for a tag filter

Did you mean to finish this comment? If a user passes in an invalid value for a tag filter, the expectation is that we handle this and default the user to have selected 'ALL', thus all of the options would show.

packages/api-explorer/src/utils/hooks.ts

packages/api-explorer/src/utils/path.ts

packages/api-explorer/src/test-data/tagFilters.ts

josephaxisa · 2022-08-09T22:27:04Z

packages/api-explorer/src/ApiExplorer.tsx

@@ -141,6 +157,14 @@ export const ApiExplorer: FC<ApiExplorerProps> = ({
          sdk: alias === allAlias ? null : alias,
        })
      }
+


I think a good way to do this will be to augment initSettings so that url parameters can be passed in and override persisted settings when applicable. As a result, this effect will then become a one liner that syncs the url with the store using navigate.

patnir41 · 2022-08-10T15:49:37Z

packages/api-explorer/src/utils/path.ts

+ * @param filter filter tag for page
+ */
+export const isValidFilter = (
+  location: HLocation | Location,


@josephaxisa Given you preferred having the string pathname for the method getSceneType, would you prefer changing this from a location object to a string?

Clean branch off main with changes; WIP validating param

7fa7881

patnir41 requested a review from josephaxisa August 9, 2022 15:15

patnir41 added 2 commits August 9, 2022 17:59

Added utils in path for verifying filter

1fe3e4a

Merge with main, sdk sharing with filter sharing

e3b7e3d

Refactoring and unit testing implemented

853531a

patnir41 commented Aug 9, 2022

View reviewed changes

Fixed bug on state of TypeTagScene

2d28c7f

patnir41 marked this pull request as ready for review August 9, 2022 21:07

patnir41 commented Aug 9, 2022

View reviewed changes

josephaxisa reviewed Aug 9, 2022

View reviewed changes

patnir41 commented Aug 10, 2022

View reviewed changes

patnir41 closed this Aug 15, 2022

patnir41 mentioned this pull request Aug 18, 2022

feat: ability to share a preconfigured diff scene #1158

Open

feat: Tag scene filter selection as URL search parameter #1140

feat: Tag scene filter selection as URL search parameter #1140

Uh oh!

Conversation

patnir41 commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 9, 2022

APIX Tests

Uh oh!

github-actions bot commented Aug 9, 2022

APIX Tests

Uh oh!

github-actions bot commented Aug 9, 2022

APIX Tests

Uh oh!

patnir41 Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patnir41 Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

patnir41 Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

josephaxisa Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 9, 2022

APIX Tests

Uh oh!

josephaxisa Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

josephaxisa Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

patnir41 Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josephaxisa Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

patnir41 Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patnir41 commented Aug 9, 2022 •

edited

Loading

patnir41 Aug 9, 2022 •

edited

Loading

patnir41 Aug 10, 2022 •

edited

Loading