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: SDK language selection as URL search parameter #1123

Merged
merged 31 commits into from
Aug 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
730271f
SDK parameter in URL and new queryParams type in navigate
patnir41 Jul 22, 2022
b1c0af9
Added sdk parameter to object passed into navigate
patnir41 Jul 26, 2022
78ca5f5
Fixed/removed case for empty object, to be disucssed
patnir41 Jul 26, 2022
af4a710
Refactor commit wasn
patnir41 Jul 26, 2022
7897da3
Quick comment refactor
patnir41 Jul 26, 2022
93d5c93
Refactor to have URL drive state in APIExplorer, test additions
patnir41 Jul 28, 2022
8a9f8b4
Minor refactor
patnir41 Jul 28, 2022
92ab597
Comment per Bryn request
patnir41 Jul 29, 2022
00633a8
Refactor, enforce flow to URL and URL driving state, removed store re…
patnir41 Jul 29, 2022
7273536
Refactor to pass tests
patnir41 Jul 29, 2022
9c29e96
Fix to All option parameter showing
patnir41 Jul 30, 2022
16e3883
Fix and unit test for choosing SDK = All
patnir41 Aug 1, 2022
842926c
Pushing extension abbreviation to URL, fix state issues, added tests/…
patnir41 Aug 2, 2022
d7436d8
URL drives state over initActions, show sdk=all in URL due to state i…
patnir41 Aug 3, 2022
abc3555
Set initial URL and Explorer state based on initialized state in loca…
patnir41 Aug 4, 2022
8e96e15
Add back removed line
patnir41 Aug 4, 2022
6a25ce8
TypeScript error fix
patnir41 Aug 4, 2022
3170b7a
Removes sdk parameter on selecting All
patnir41 Aug 4, 2022
a7f0748
Significant optimizations and change to sdkLanguage storage
patnir41 Aug 5, 2022
d399430
Working e2e tests
patnir41 Aug 5, 2022
1898cc2
WIP comments from Joseph on e2e and sdkAlias selector
patnir41 Aug 5, 2022
d643f68
Passing e2e tests
patnir41 Aug 7, 2022
391a90b
Remove need for catching legacy store value
patnir41 Aug 7, 2022
141ee1b
Refactoring & cleanup
patnir41 Aug 8, 2022
047412b
Selector refactor, remove redundant logic
patnir41 Aug 8, 2022
7f9f872
Documentation for allSdkLanguages
patnir41 Aug 8, 2022
bf28b9b
Revert test checking default language selected
patnir41 Aug 8, 2022
3727b65
Remove unpredictability with e2e tests
patnir41 Aug 8, 2022
24ded0a
Refactoring and modifying e2e tests
patnir41 Aug 8, 2022
d8c3f51
allAlias instead of hardcode 'all'
patnir41 Aug 8, 2022
88c99a1
Merge branch 'main' into patnir41/sdk-sharing-url
patnir41 Aug 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions packages/api-explorer/e2e/diffScene.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ describe('Diff Scene', () => {
page.evaluate((el) => el.innerText.match(/^[a-z_]*/)[0], resultCard)
)
)
expect(page1Methods).toHaveLength(15)
expect(page1Methods).toHaveLength(16)
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 failing in main, fixed so that it is up to date and accurately reflecting new additional method.

expect(page1Methods).toContain('delete_board_item')
}

Expand All @@ -176,13 +176,15 @@ describe('Diff Scene', () => {
const methodLink = await page.$(`${resultCardsSelector} a[role=link]`)
expect(methodLink).not.toBeNull()
const methodText = await page.evaluate((e) => e.innerText, methodLink)
expect(methodText).toMatch(`delete_board_item for 4.0`)
expect(methodText).toMatch(`delete_alert for 4.0`)

// Click and validate destination
await methodLink.click()
await page.waitForSelector(`div[class*=MethodBadge]`, { timeout: 5000 })
const compUrl = page.url()
expect(compUrl).toEqual(`${BASE_URL}/4.0/methods/Board/delete_board_item`)
expect(compUrl).toEqual(
`${BASE_URL}/4.0/methods/Alert/delete_alert?sdk=py`
)
}
})

Expand Down Expand Up @@ -217,7 +219,7 @@ describe('Diff Scene', () => {
// Check the URL
// Would like to do this earlier, but not sure what to wait on
const compUrl = page.url()
expect(compUrl).toEqual(`${BASE_URL}/diff/3.1/4.0`)
expect(compUrl).toEqual(`${BASE_URL}/diff/3.1/4.0?sdk=py`)

// Check the results
const diffResultCards = await page.$$(resultCardsSelector)
Expand All @@ -228,7 +230,7 @@ describe('Diff Scene', () => {
)
)

expect(diff31to40Page1Methods).toHaveLength(15)
expect(diff31to40Page1Methods).toHaveLength(16)
expect(diff31to40Page1Methods).toContain('delete_board_item')

// Click the switch button
Expand All @@ -245,7 +247,7 @@ describe('Diff Scene', () => {
await page.waitForTimeout(150)

const switchUrl = page.url()
expect(switchUrl).toEqual(`${BASE_URL}/diff/4.0/3.1`)
expect(switchUrl).toEqual(`${BASE_URL}/diff/4.0/3.1?sdk=py`)

// Check the results again, even though they should be the same
const diff40to31Page1Methods = await Promise.all(
Expand All @@ -254,7 +256,7 @@ describe('Diff Scene', () => {
)
)

expect(diff40to31Page1Methods).toHaveLength(15)
expect(diff40to31Page1Methods).toHaveLength(16)
expect(diff40to31Page1Methods).toContain('delete_board_item')
})
})
22 changes: 19 additions & 3 deletions packages/api-explorer/e2e/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ describe('API Explorer', () => {
await goToPage(v40)
})

afterEach(async () => {
await page.evaluate(() => {
localStorage.clear()
})
})

it('renders a method page', async () => {
await Promise.all([
page.waitForNavigation(),
Expand All @@ -55,7 +61,7 @@ describe('API Explorer', () => {
expect(page).toClick('h3', { text: 'Get All Dashboards' }),
])
await expect(page.url()).toEqual(
`${v40}/methods/Dashboard/all_dashboards`
`${v40}/methods/Dashboard/all_dashboards?sdk=py`
)

// title
Expand Down Expand Up @@ -245,6 +251,12 @@ describe('API Explorer', () => {
await goToPage(v40)
})

afterEach(async () => {
await page.evaluate(() => {
localStorage.clear()
})
})

it('searches methods', async () => {
await expect(page).toFill('input[aria-label="Search"]', 'get workspace')
// TODO: find a better way to avoid the scenario where L215 executes before search returns
Expand All @@ -255,7 +267,9 @@ describe('API Explorer', () => {
await expect(page).toMatchElement('button', { text: 'Types (0)' })
await expect(page).toClick('a', { text: 'Get Workspace' })
await expect(page).toMatchElement('h2', { text: 'Get Workspace' })
await expect(page.url()).toEqual(`${v40}/methods/Workspace/workspace`)
await expect(page.url()).toEqual(
`${v40}/methods/Workspace/workspace?sdk=py&s=get+workspace`
)
})

it('searches types', async () => {
Expand All @@ -267,7 +281,9 @@ describe('API Explorer', () => {
await expect(page).toClick('button', { text: 'Types (1)' })
await expect(page).toClick('a', { text: 'WriteTheme' })
await expect(page).toMatchElement('h2', { text: 'WriteTheme' })
await expect(page.url()).toEqual(`${v40}/types/Theme/WriteTheme`)
await expect(page.url()).toEqual(
`${v40}/types/Theme/WriteTheme?sdk=py&s=writetheme`
)
})
})
})
41 changes: 36 additions & 5 deletions packages/api-explorer/src/ApiExplorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ import {
useSpecStoreState,
selectSpecs,
selectCurrentSpec,
selectSdkLanguage,
} from './state'
import { getSpecKey, diffPath } from './utils'
import { getSpecKey, diffPath, useNavigation, findSdk, allAlias } from './utils'

export interface ApiExplorerProps {
adaptor: IApixAdaptor
Expand All @@ -85,19 +86,23 @@ export const ApiExplorer: FC<ApiExplorerProps> = ({
declarationsLodeUrl = `${apixFilesHost}/declarationsIndex.json`,
headless = false,
}) => {
useSettingStoreState()
const { initialized } = useSettingStoreState()
useLodesStoreState()
const { working, description } = useSpecStoreState()
const specs = useSelector(selectSpecs)
const spec = useSelector(selectCurrentSpec)
const selectedSdkLanguage = useSelector(selectSdkLanguage)
const { initLodesAction } = useLodeActions()
const { initSettingsAction, setSearchPatternAction } = useSettingActions()
const { initSettingsAction, setSearchPatternAction, setSdkLanguageAction } =
useSettingActions()
const { initSpecsAction, setCurrentSpecAction } = useSpecActions()

const location = useLocation()
const navigate = useNavigation()
const [hasNavigation, setHasNavigation] = useState(true)
const toggleNavigation = (target?: boolean) =>
setHasNavigation(target || !hasNavigation)
const searchParams = new URLSearchParams(location.search)

const hasNavigationToggle = useCallback((e: MessageEvent<any>) => {
if (e.origin === window.origin && e.data.action === 'toggle_sidebar') {
Expand All @@ -116,6 +121,29 @@ export const ApiExplorer: FC<ApiExplorerProps> = ({
return () => unregisterEnvAdaptor()
}, [])

useEffect(() => {
// reconcile local storage state with URL or vice versa
patnir41 marked this conversation as resolved.
Show resolved Hide resolved
if (initialized) {
const sdkParam = searchParams.get('sdk') || ''
const sdk = findSdk(sdkParam)
const validSdkParam =
!sdkParam.localeCompare(sdk.alias, 'en', { sensitivity: 'base' }) ||
!sdkParam.localeCompare(sdk.language, 'en', { sensitivity: 'base' })
if (validSdkParam) {
// sync store with URL
setSdkLanguageAction({
sdkLanguage: sdk.language,
})
} else {
// sync URL with store
const { alias } = findSdk(selectedSdkLanguage)
navigate(location.pathname, {
sdk: alias === allAlias ? null : alias,
})
}
}
}, [initialized])

useEffect(() => {
const maybeSpec = location.pathname?.split('/')[1]
if (spec && maybeSpec && maybeSpec !== diffPath && maybeSpec !== spec.key) {
Expand All @@ -124,9 +152,12 @@ export const ApiExplorer: FC<ApiExplorerProps> = ({
}, [location.pathname, spec])

useEffect(() => {
const searchParams = new URLSearchParams(location.search)
if (!initialized) return
const searchPattern = searchParams.get('s') || ''
setSearchPatternAction({ searchPattern: searchPattern! })
const sdkParam = searchParams.get('sdk') || 'all'
const { language: sdkLanguage } = findSdk(sdkParam)
setSearchPatternAction({ searchPattern })
setSdkLanguageAction({ sdkLanguage })
}, [location.search])

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ describe('DocSDKs', () => {
'it can render a %s method declaration',
(sdkLanguage) => {
store = createTestStore({
settings: { initialized: false, sdkLanguage },
settings: {
initialized: false,
sdkLanguage: sdkLanguage,
},
})
renderWithReduxProvider(
<DocSDKs api={api} method={api.methods.run_look} />,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,61 +24,88 @@

*/
import React from 'react'
import { act, screen, waitFor } from '@testing-library/react'
import { screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { codeGenerators } from '@looker/sdk-codegen'
import * as reactRedux from 'react-redux'

import { registerTestEnvAdaptor } from '@looker/extension-utils'
import { defaultSettingsState, settingsSlice } from '../../state'
import { renderWithReduxProvider } from '../../test-utils'
import {
createTestStore,
renderWithRouterAndReduxProvider,
} from '../../test-utils'
import { findSdk } from '../../utils'
import { languages } from '../../test-data'
import { defaultSettingsState } from '../../state'
import { SdkLanguageSelector } from './SdkLanguageSelector'

const mockHistoryPush = jest.fn()
jest.mock('react-router-dom', () => {
const ReactRouterDOM = jest.requireActual('react-router-dom')
return {
...ReactRouterDOM,
useHistory: () => ({
push: mockHistoryPush,
location,
}),
}
})
describe('SdkLanguageSelector', () => {
window.HTMLElement.prototype.scrollIntoView = jest.fn()
const store = createTestStore({ settings: { initialized: true } })

beforeEach(() => {
localStorage.clear()
})

test('it has the correct default language selected', () => {
renderWithReduxProvider(<SdkLanguageSelector />)
test('it has the correct default language selected', async () => {
renderWithRouterAndReduxProvider(<SdkLanguageSelector />, undefined, store)
expect(screen.getByRole('textbox')).toHaveValue(
defaultSettingsState.sdkLanguage
)
})

test('it lists all available languages and "All" as options', async () => {
renderWithReduxProvider(<SdkLanguageSelector />)
await act(async () => {
await userEvent.click(screen.getByRole('textbox'))
await waitFor(() => {
expect(screen.getAllByRole('option')).toHaveLength(
codeGenerators.length + 1
)
renderWithRouterAndReduxProvider(<SdkLanguageSelector />, undefined, store)
userEvent.click(screen.getByRole('textbox'))
await waitFor(() => {
expect(screen.getAllByRole('option')).toHaveLength(
codeGenerators.length + 1
)
languages.forEach((language) => {
expect(
screen.getByRole('option', { name: language })
).toBeInTheDocument()
})
})
})

test('it stores the selected language in localStorage', async () => {
patnir41 marked this conversation as resolved.
Show resolved Hide resolved
registerTestEnvAdaptor()
const mockDispatch = jest.fn()
jest.spyOn(reactRedux, 'useDispatch').mockReturnValue(mockDispatch)
renderWithReduxProvider(<SdkLanguageSelector />)

const selector = screen.getByRole('textbox')
expect(defaultSettingsState.sdkLanguage).toEqual('Python')
expect(selector).toHaveValue('Python')

userEvent.click(selector)
await act(async () => {
await userEvent.click(screen.getByRole('option', { name: 'TypeScript' }))
test.each(languages.filter((l) => l !== 'All'))(
'choosing `%s` pushes its alias to url',
async (language) => {
renderWithRouterAndReduxProvider(
<SdkLanguageSelector />,
undefined,
store
)
const selector = screen.getByRole('textbox')
userEvent.click(selector)
await waitFor(async () => {
expect(mockDispatch).toHaveBeenLastCalledWith(
settingsSlice.actions.setSdkLanguageAction({
sdkLanguage: 'TypeScript',
})
)
await userEvent.click(screen.getByRole('option', { name: language }))
const sdk = findSdk(language)
expect(mockHistoryPush).toHaveBeenLastCalledWith({
pathname: location.pathname,
search: `sdk=${sdk.alias}`,
})
})
}
)

test("choosing 'All' removes sdk parameter from the url", async () => {
renderWithRouterAndReduxProvider(<SdkLanguageSelector />, undefined, store)
userEvent.click(screen.getByRole('textbox'))
await waitFor(async () => {
await userEvent.click(screen.getByRole('option', { name: 'All' }))
expect(mockHistoryPush).toHaveBeenLastCalledWith({
pathname: location.pathname,
search: '',
})
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,43 +24,36 @@

*/
import type { FC } from 'react'
import React from 'react'
import { codeGenerators } from '@looker/sdk-codegen'
import React, { useEffect, useState } from 'react'
import { Select } from '@looker/components'
import { useSelector } from 'react-redux'
import type { SelectOptionProps } from '@looker/components'

import { useSettingActions, selectSdkLanguage } from '../../state'
import { selectSdkLanguage } from '../../state'
import { allAlias, useNavigation } from '../../utils'
import { allSdkLanguageOptions } from './utils'

/**
* Allows the user to select their preferred SDK language
*/
export const SdkLanguageSelector: FC = () => {
const { setSdkLanguageAction } = useSettingActions()
const navigate = useNavigation()
const selectedSdkLanguage = useSelector(selectSdkLanguage)
const [language, setLanguage] = useState(selectedSdkLanguage)
const options = allSdkLanguageOptions()

const allSdkLanguages: SelectOptionProps[] = codeGenerators.map((gen) => ({
value: gen.language,
}))

allSdkLanguages.push({
options: [
{
value: 'All',
},
],
})

const handleChange = (language: string) => {
setSdkLanguageAction({ sdkLanguage: language })
const handleChange = (alias: string) => {
navigate(location.pathname, { sdk: alias === allAlias ? null : alias })
}

useEffect(() => {
setLanguage(selectedSdkLanguage)
}, [selectedSdkLanguage])
patnir41 marked this conversation as resolved.
Show resolved Hide resolved

return (
<Select
aria-label="sdk language selector"
value={selectedSdkLanguage}
value={language}
onChange={handleChange}
options={allSdkLanguages}
options={options}
/>
)
}
Loading