feat: ability to share a preconfigured diff scene #1158

patnir41 · 2022-08-18T17:35:52Z

Selecting comparison options in diff scenes will update an opts parameter in the URL, which is used to drive the state of the selected options in the scene.

…config-difs-8-15 Merging filter sharing PR changes into main

patnir41 · 2022-08-18T17:44:59Z

(For @josephaxisa) I created a new branch from the previous PR. After making some final changes to that PR and merging it into main, I merged main into this branch. I thought conflicts were resolved until I pushed and saw there were still some conflicts which I resolved in the GitHub editor. Let me know if there's anything I could/should have done differently.

github-actions · 2022-08-18T17:50:41Z

APIX Tests

    1 files ±0   86 suites +2 5m 36s ⏱️ +6s
400 tests +8 386 ✔️ +7 13 💤 ±0 1 ❌ +1
420 runs +8 406 ✔️ +7 13 💤 ±0 1 ❌ +1

For more details on these failures, see this check.

Results for commit 358c56c. ± Comparison against base commit 135dce3.

patnir41 · 2022-08-18T18:01:53Z

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

@@ -54,9 +54,9 @@ export const SideNavMethods = styled(
      const _isOpen = !isOpen
      setIsOpen(_isOpen)
      if (_isOpen) {
-        navigate(`/${specKey}/methods/${tag}`)
+        navigate(`/${specKey}/methods/${tag}`, { opts: null })


The scenario to consider here (and with the same change in SideNavTypes): we are in a diff scene with selected options, and opts param in the url. When we open the sideNav and open a method/tag scene, this opts parameter should no longer persist. The reason we don't use navigateWithGlobalParams to remove this parameter is because if we started at a tag scene with a filter selected and want to select another tag, this filter should still persist when we open the new tag scene.

So, technically, the purpose of this line in plain english is more like "navigate to the tag scene and ONLY keep the tag filter variable if we have one in the URL"

This makes me reconsider what we initially had proposed where we have a specific navigateTo methods. I think in the long term there will be 2 cases:

We navigate to a scene, remove all scene-specific params first, then push global params (currently the case with navigateWithGlobalParams, etc)

We navigate to a scene, keeping ONLY specific parameters, and removing anything else in the URL.

To better account for case 2, I think having custom navigate methods makes the most sense. Here we'd explicitly have a navigateToTagScene instead of having to explicitly nullify any extra parameters that could persist when navigating to a tag scene from the SideNav.

@josephaxisa

patnir41 · 2022-08-18T18:04:52Z

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

+      const diffOptionsParam = getDiffOptionsFromUrl(params.get('opts'))
+      if (diffOptionsParam) {
+        setDiffOptionsAction({ diffOptions: diffOptionsParam })
+        navigate(location.pathname, { opts: diffOptionsParam.join(',') })


The reason for this navigate here, which contrasts from tagStoreSync or globalStoreSync where we only dispatch to the store, is in the case where we have a list of options in the url, but one or more of those are not actually valid comparison options for the diff scene (e.g. opts=missing%2CINVALID%2Ctype)

getValidDiffOptions does the job of filtering out invalid comparison options from the URL and navigate does the job of updating the URL to only have the valid options out of those listed in the parameter.

github-actions · 2022-08-19T17:11:21Z

APIX Tests

0 files -     1 0 suites - 84 0s ⏱️ - 8m 16s
0 tests - 392 0 ✔️ - 379 0 💤 - 13 0 ❌ ±0
0 runs - 412 0 ✔️ - 399 0 💤 - 13 0 ❌ ±0

Results for commit a6a47ca. ± Comparison against base commit 180cf20.

This pull request removes 392 tests.

 DocTitle renders a heading with title prop content ‑  DocTitle renders a heading with title prop content
 MethodBadge renders with the provided verb ‑  MethodBadge renders with the provided verb
ApiSpecSelector it lists all available specs ‑ ApiSpecSelector it lists all available specs
ApiSpecSelector requests selected spec ‑ ApiSpecSelector requests selected spec
ApiSpecSelector the base spec is selected by default ‑ ApiSpecSelector the base spec is selected by default
Banner renders with button that opens api 4.0 dev portal link in new page ‑ Banner renders with button that opens api 4.0 dev portal link in new page
Banner sets local storage value and unrenders on close button click ‑ Banner sets local storage value and unrenders on close button click
BrowserAdaptor copies location href to clipboard ‑ BrowserAdaptor copies location href to clipboard
BrowserAdaptor returns correct font overrides ‑ BrowserAdaptor returns correct font overrides
CodeCopy displays code and clipboard UI ‑ CodeCopy displays code and clipboard UI
…

github-actions · 2022-08-19T20:58:39Z

APIX Tests

0 files -     1 0 suites - 84 0s ⏱️ - 8m 16s
0 tests - 392 0 ✔️ - 379 0 💤 - 13 0 ❌ ±0
0 runs - 412 0 ✔️ - 399 0 💤 - 13 0 ❌ ±0

Results for commit d92df04. ± Comparison against base commit 180cf20.

This pull request removes 392 tests.

 DocTitle renders a heading with title prop content ‑  DocTitle renders a heading with title prop content
 MethodBadge renders with the provided verb ‑  MethodBadge renders with the provided verb
ApiSpecSelector it lists all available specs ‑ ApiSpecSelector it lists all available specs
ApiSpecSelector requests selected spec ‑ ApiSpecSelector requests selected spec
ApiSpecSelector the base spec is selected by default ‑ ApiSpecSelector the base spec is selected by default
Banner renders with button that opens api 4.0 dev portal link in new page ‑ Banner renders with button that opens api 4.0 dev portal link in new page
Banner sets local storage value and unrenders on close button click ‑ Banner sets local storage value and unrenders on close button click
BrowserAdaptor copies location href to clipboard ‑ BrowserAdaptor copies location href to clipboard
BrowserAdaptor returns correct font overrides ‑ BrowserAdaptor returns correct font overrides
CodeCopy displays code and clipboard UI ‑ CodeCopy displays code and clipboard UI
…

github-actions · 2022-08-26T22:07:52Z

APIX Tests

0 files -     1 0 suites - 84 0s ⏱️ - 8m 16s
0 tests - 392 0 ✔️ - 379 0 💤 - 13 0 ❌ ±0
0 runs - 412 0 ✔️ - 399 0 💤 - 13 0 ❌ ±0

Results for commit 377dac8. ± Comparison against base commit 180cf20.

This pull request removes 392 tests.

 DocTitle renders a heading with title prop content ‑  DocTitle renders a heading with title prop content
 MethodBadge renders with the provided verb ‑  MethodBadge renders with the provided verb
ApiSpecSelector it lists all available specs ‑ ApiSpecSelector it lists all available specs
ApiSpecSelector requests selected spec ‑ ApiSpecSelector requests selected spec
ApiSpecSelector the base spec is selected by default ‑ ApiSpecSelector the base spec is selected by default
Banner renders with button that opens api 4.0 dev portal link in new page ‑ Banner renders with button that opens api 4.0 dev portal link in new page
Banner sets local storage value and unrenders on close button click ‑ Banner sets local storage value and unrenders on close button click
BrowserAdaptor copies location href to clipboard ‑ BrowserAdaptor copies location href to clipboard
BrowserAdaptor returns correct font overrides ‑ BrowserAdaptor returns correct font overrides
CodeCopy displays code and clipboard UI ‑ CodeCopy displays code and clipboard UI
…

github-actions · 2022-08-31T00:18:20Z

APIX Tests

0 files -     1 0 suites - 86 0s ⏱️ - 6m 34s
0 tests - 408 0 ✔️ - 395 0 💤 - 13 0 ❌ ±0
0 runs - 428 0 ✔️ - 415 0 💤 - 13 0 ❌ ±0

Results for commit 73ca49c. ± Comparison against base commit a4ac3bc.

This pull request removes 408 tests.

 DocTitle renders a heading with title prop content ‑  DocTitle renders a heading with title prop content
 MethodBadge renders with the provided verb ‑  MethodBadge renders with the provided verb
APIErrorDialog defaults an empty error message ‑ APIErrorDialog defaults an empty error message
APIErrorDialog is hidden if open is false ‑ APIErrorDialog is hidden if open is false
APIErrorDialog ok button click fires ‑ APIErrorDialog ok button click fires
APIErrorDialog shows detailed errors ‑ APIErrorDialog shows detailed errors
APIErrorDialog shows simple errors ‑ APIErrorDialog shows simple errors
APIErrorDialog skips empty documentation url ‑ APIErrorDialog skips empty documentation url
APIErrorDisplay defaults an empty error message ‑ APIErrorDisplay defaults an empty error message
APIErrorDisplay fetches error doc ‑ APIErrorDisplay fetches error doc
…

patnir41 · 2022-09-06T17:11:15Z

packages/api-explorer/e2e/diffScene.spec.ts

@@ -265,4 +270,70 @@ describe('Diff Scene', () => {
    expect(diff40to31Page1Methods).toHaveLength(15)
    expect(diff40to31Page1Methods).toContain('delete_board_item')
  })
+
+  it('updates when a comparison option is toggled', async () => {


Can remove this, but I thought it would be nice to have an integration test ensuring the DiffScene operates properly other than the unit tests which I couldn't make comprehensive due to the spec file bug.

github-actions · 2022-09-06T17:20:16Z

APIX Tests

    1 files   88 suites 6m 9s ⏱️
422 tests 409 ✔️ 13 💤 0 ❌
442 runs 429 ✔️ 13 💤 0 ❌

Results for commit a8965ea.

josephaxisa

thanks Niral. I noted some feedback below

josephaxisa · 2022-09-07T20:51:42Z

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

@@ -55,9 +54,9 @@ export const SideNavMethods = styled(
      const _isOpen = !isOpen
      setIsOpen(_isOpen)
      if (_isOpen) {
-        navigateWithGlobalParams(`/${specKey}/methods/${tag}`)
+        navigate(`/${specKey}/methods/${tag}`, { opts: null })


why not navigateWithGlobalParams?

josephaxisa · 2022-09-07T20:51:50Z

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

@@ -54,9 +54,9 @@ export const SideNavTypes = styled(
      const _isOpen = !isOpen
      setIsOpen(_isOpen)
      if (_isOpen) {
-        navigate(`/${specKey}/types/${tag}`)
+        navigate(`/${specKey}/types/${tag}`, { opts: null })


josephaxisa · 2022-09-07T20:51:54Z

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

      } else {
-        navigate(`/${specKey}/types`)
+        navigate(`/${specKey}/types`, { opts: null })


josephaxisa · 2022-09-07T20:51:58Z

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

      } else {
-        navigateWithGlobalParams(`/${specKey}/methods`)
+        navigate(`/${specKey}/methods`, { opts: null })


josephaxisa · 2022-09-07T21:23:06Z

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

-    expect(mockHistoryPush).toHaveBeenCalledWith(`/${specKey}/methods/${tag}`)
+    expect(mockHistoryPush).toHaveBeenCalledWith({
+      pathname: `/${specKey}/methods/${tag}`,
+      search: '',


I think this change won't be needed if navigateWithGlobalParams is used.

josephaxisa · 2022-09-07T21:23:14Z

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

+    expect(mockHistoryPush).toHaveBeenCalledWith({
+      pathname: `/${specKey}/methods`,
+      search: '',
+    })


packages/api-explorer/src/scenes/DiffScene/diffUtils.ts

josephaxisa · 2022-09-07T22:03:56Z

packages/api-explorer/src/scenes/DiffScene/diffUtils.ts

+ * Returns all valid diff options contained in list
+ * @param opts url diff options parameter value
+ */
+export const getValidDiffOptions = (opts: string | null) => {


can opts just be made an optional string arg here? is null explicitly needed?

I pass in searchParams.get('opts') in both DiffScene and diffStoreSync. When the param doesn't exist in the URL it searchParams.get() will return null, so this was just a way to be able to handle it without doing any explicit checks before passing it in as input.

github-actions · 2022-09-07T23:14:58Z

APIX Tests

    1 files ±  0   88 suites +2 8m 6s ⏱️ + 2m 14s
422 tests +14 409 ✔️ +14 13 💤 ±0 0 ❌ ±0
442 runs +14 429 ✔️ +14 13 💤 ±0 0 ❌ ±0

Results for commit c34fdee. ± Comparison against base commit 0db7157.

github-actions · 2022-09-13T19:56:13Z

APIX Tests

    1 files   88 suites 7m 50s ⏱️
422 tests 409 ✔️ 13 💤 0 ❌
442 runs 429 ✔️ 13 💤 0 ❌

Results for commit 09a0666.

github-actions · 2022-09-20T19:23:44Z

APIX Tests

    1 files   88 suites 6m 9s ⏱️
422 tests 409 ✔️ 13 💤 0 ❌
442 runs 429 ✔️ 13 💤 0 ❌

Results for commit c54f9b3.

patnir41 and others added 23 commits August 9, 2022 02:35

Clean branch off main with changes; WIP validating param

7fa7881

Added utils in path for verifying filter

1fe3e4a

Merge with main, sdk sharing with filter sharing

e3b7e3d

Refactoring and unit testing implemented

853531a

Fixed bug on state of TypeTagScene

2d28c7f

WIP creating custom reconciliation and navigation hooks

7c04e61

Fix to not using isSynced

863bf47

Refactor using isSync variable determining render

6e3f9ea

Custom sync hooks, new navigate functions in hook for params

6339b11

Code cleanup

0b09d4f

Code clean up

2d54f37

Fixed invalid verb param on tab switching, WIP hook testing

caa189c

WIP globalStoreSync tests

ae47fa0

globalSync and tagSync testing included, refactored files

70ae6e2

Clean up per Jax comments

376e21b

Fix navigation hook function naming

e1e4c0a

Merge branch 'patnir41/filter-sharing-custom-hooks' into patnir41/pre…

d8bfd8a

…config-difs-8-15 Merging filter sharing PR changes into main

Diff scene options in URL, testing WIP

93b959a

Added testing & functional, WIP DiffScene.spec

6d46d80

WIP DiffScene testing

4048143

Merging changes from main with tag filter PR

ffda1ca

WIP: finalizing tests

071e0b5

Merge branch 'main' into patnir41/preconfig-difs-8-15

358c56c

patnir41 commented Aug 18, 2022

View reviewed changes

patnir41 and others added 2 commits August 18, 2022 21:16

Changes to util files to fix imports

91917f5

Merge branch 'main' into patnir41/preconfig-difs-8-15

a6a47ca

WIP DiffScene.spec testing still, functionality bug fix

d92df04

DiffScene testing bug, no default state set immediately

377dac8

patnir41 and others added 2 commits August 30, 2022 18:06

WIP unable to init store with default settings in DiffScene spec

ea02ec2

Merge branch 'main' into patnir41/preconfig-difs-8-15

73ca49c

patnir41 added 7 commits August 31, 2022 20:52

Setting up diffscene spec for other tests

c0297ed

Removing testReduxUtils, making spec file without this test

ef95f97

Moving forward with finalizing document ignoring spec bug

90840d4

Merge branch 'main' into patnir41/preconfig-difs-8-15

f883a97

Refactoring file and tests

268ecd3

Wrap up DiffScene spec, WIP e2e

7e8e558

Added 1 e2e test for DiffScene

a8965ea

patnir41 commented Sep 6, 2022

View reviewed changes

patnir41 marked this pull request as ready for review September 6, 2022 17:13

patnir41 requested review from josephaxisa, bryans99 and jkaster September 6, 2022 17:13

josephaxisa requested changes Sep 7, 2022

View reviewed changes

Refactoring diffUtils function

c34fdee

Merge branch 'main' into patnir41/preconfig-difs-8-15

09a0666

Merge branch 'main' into patnir41/preconfig-difs-8-15

c54f9b3

patnir41 requested a review from a team as a code owner March 12, 2024 22:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ability to share a preconfigured diff scene #1158

feat: ability to share a preconfigured diff scene #1158

patnir41 commented Aug 18, 2022

patnir41 commented Aug 18, 2022 •

edited

Loading

github-actions bot commented Aug 18, 2022

patnir41 Aug 18, 2022 •

edited

Loading

patnir41 Sep 7, 2022

patnir41 Aug 18, 2022 •

edited

Loading

github-actions bot commented Aug 19, 2022

github-actions bot commented Aug 19, 2022

github-actions bot commented Aug 26, 2022

github-actions bot commented Aug 31, 2022

patnir41 Sep 6, 2022

github-actions bot commented Sep 6, 2022

josephaxisa left a comment

josephaxisa Sep 7, 2022

josephaxisa Sep 7, 2022

josephaxisa Sep 7, 2022

josephaxisa Sep 7, 2022

josephaxisa Sep 7, 2022

josephaxisa Sep 7, 2022

josephaxisa Sep 7, 2022

patnir41 Sep 7, 2022 •

edited

Loading

github-actions bot commented Sep 7, 2022

github-actions bot commented Sep 13, 2022

github-actions bot commented Sep 20, 2022

feat: ability to share a preconfigured diff scene #1158

Are you sure you want to change the base?

feat: ability to share a preconfigured diff scene #1158

Conversation

patnir41 commented Aug 18, 2022

patnir41 commented Aug 18, 2022 • edited Loading

github-actions bot commented Aug 18, 2022

APIX Tests

patnir41 Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patnir41 Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 19, 2022

APIX Tests

github-actions bot commented Aug 19, 2022

APIX Tests

github-actions bot commented Aug 26, 2022

APIX Tests

github-actions bot commented Aug 31, 2022

APIX Tests

Choose a reason for hiding this comment

github-actions bot commented Sep 6, 2022

APIX Tests

josephaxisa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patnir41 Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 7, 2022

APIX Tests

github-actions bot commented Sep 13, 2022

APIX Tests

github-actions bot commented Sep 20, 2022

APIX Tests

patnir41 commented Aug 18, 2022 •

edited

Loading

patnir41 Aug 18, 2022 •

edited

Loading

patnir41 Aug 18, 2022 •

edited

Loading

patnir41 Sep 7, 2022 •

edited

Loading