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

Add self-hosted url info in Studio setup section #5089

Merged
merged 9 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions extension/src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ export class Setup
pythonBinPath: getBinDisplayText(pythonBinPath),
remoteList,
sectionCollapsed: collectSectionCollapsed(this.focusedSection),
selfHostedStudioUrl: this.studio.getSelfHostedStudioUrl(),
shareLiveToStudio: !!this.studio.getShareLiveToStudio()
})
this.focusedSection = undefined
Expand Down
5 changes: 5 additions & 0 deletions extension/src/setup/studio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export class Studio extends Disposable {
return this.studioUrl
}

public getSelfHostedStudioUrl() {
const url = this.getStudioUrl()
return url === DEFAULT_STUDIO_URL ? null : url
}

public removeStudioAccessToken(dvcRoots: string[]) {
return this.removeKeyFromConfig(dvcRoots, ConfigKey.STUDIO_TOKEN)
}
Expand Down
1 change: 1 addition & 0 deletions extension/src/setup/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type SetupData = {
pythonBinPath: string | undefined
remoteList: RemoteList
sectionCollapsed: typeof DEFAULT_SECTION_COLLAPSED | undefined
selfHostedStudioUrl: string | null
shareLiveToStudio: boolean
isAboveLatestTestedVersion: boolean | undefined
}
Expand Down
4 changes: 4 additions & 0 deletions extension/src/setup/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,14 @@ export class WebviewMessages {
return commands.executeCommand(
RegisteredCommands.ADD_STUDIO_ACCESS_TOKEN
)
case MessageFromWebviewType.SAVE_STUDIO_URL:
return commands.executeCommand(RegisteredCommands.UPDATE_STUDIO_URL)
case MessageFromWebviewType.REMOVE_STUDIO_TOKEN:
return commands.executeCommand(
RegisteredCommands.REMOVE_STUDIO_ACCESS_TOKEN
)
case MessageFromWebviewType.REMOVE_STUDIO_URL:
return commands.executeCommand(RegisteredCommands.REMOVE_STUDIO_URL)
case MessageFromWebviewType.SET_STUDIO_SHARE_EXPERIMENTS_LIVE:
return this.updateStudioOffline(message.payload)
case MessageFromWebviewType.REQUEST_STUDIO_TOKEN:
Expand Down
66 changes: 66 additions & 0 deletions extension/src/test/suite/setup/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ suite('Setup Test Suite', () => {
pythonBinPath: undefined,
remoteList: undefined,
sectionCollapsed: undefined,
selfHostedStudioUrl: null,
shareLiveToStudio: false
})
}).timeout(WEBVIEW_TEST_TIMEOUT)
Expand Down Expand Up @@ -441,6 +442,7 @@ suite('Setup Test Suite', () => {
pythonBinPath: undefined,
remoteList: undefined,
sectionCollapsed: undefined,
selfHostedStudioUrl: null,
shareLiveToStudio: true
})
}).timeout(WEBVIEW_TEST_TIMEOUT)
Expand Down Expand Up @@ -495,6 +497,7 @@ suite('Setup Test Suite', () => {
pythonBinPath: undefined,
remoteList: undefined,
sectionCollapsed: undefined,
selfHostedStudioUrl: null,
shareLiveToStudio: true
})
}).timeout(WEBVIEW_TEST_TIMEOUT)
Expand Down Expand Up @@ -549,6 +552,7 @@ suite('Setup Test Suite', () => {
pythonBinPath: undefined,
remoteList: { [dvcDemoPath]: undefined },
sectionCollapsed: undefined,
selfHostedStudioUrl: null,
shareLiveToStudio: true
})
}).timeout(WEBVIEW_TEST_TIMEOUT)
Expand Down Expand Up @@ -1332,6 +1336,68 @@ suite('Setup Test Suite', () => {
)
})

it("should handle a message from the webview to update the user's self hosted url", async () => {
const { setup, mockExecuteCommand } = buildSetup({
disposer: disposable
})

// eslint-disable-next-line @typescript-eslint/no-explicit-any
stub(Setup.prototype as any, 'getCliCompatible').returns(true)
Copy link
Member

Choose a reason for hiding this comment

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

[C] Maybe this should be moved into buildSetup

Copy link
Member

Choose a reason for hiding this comment

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

and should be stubbed on the instance (if possible)

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 was thinking about that as well since we are running this line in multiple tests. I'll look into moving it in a followup.


const webview = await setup.showWebview()
await webview.isReady()

const mockMessageReceived = getMessageReceivedEmitter(webview)

const commandCalled = new Promise(resolve =>
mockExecuteCommand.callsFake(() => {
resolve(undefined)
return Promise.resolve(undefined)
})
)

mockMessageReceived.fire({
type: MessageFromWebviewType.SAVE_STUDIO_URL
})

await commandCalled

expect(mockExecuteCommand).to.be.calledWithExactly(
RegisteredCommands.UPDATE_STUDIO_URL
)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it("should handle a message from the webview to remove the user's self hosted url", async () => {
const { setup, mockExecuteCommand } = buildSetup({
disposer: disposable
})

// eslint-disable-next-line @typescript-eslint/no-explicit-any
stub(Setup.prototype as any, 'getCliCompatible').returns(true)

const webview = await setup.showWebview()
await webview.isReady()

const commandCalled = new Promise(resolve =>
mockExecuteCommand.callsFake(() => {
resolve(undefined)
return Promise.resolve(undefined)
})
)

const mockMessageReceived = getMessageReceivedEmitter(webview)

mockMessageReceived.fire({
type: MessageFromWebviewType.SAVE_STUDIO_URL
})

await commandCalled

expect(mockExecuteCommand).to.be.calledWithExactly(
RegisteredCommands.UPDATE_STUDIO_URL
)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should check if experiments and dvc are setup', async () => {
const { setup } = buildSetup({
disposer: disposable,
Expand Down
4 changes: 4 additions & 0 deletions extension/src/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export enum MessageFromWebviewType {
RESIZE_PLOTS = 'resize-plots',
REQUEST_STUDIO_TOKEN = 'request-studio-token',
SAVE_STUDIO_TOKEN = 'save-studio-token',
SAVE_STUDIO_URL = 'save-studio-url',
SET_COMPARISON_MULTI_PLOT_VALUE = 'update-comparison-multi-plot-value',
SET_SMOOTH_PLOT_VALUE = 'update-smooth-plot-value',
SHOW_EXPERIMENT_LOGS = 'show-experiment-logs',
Expand All @@ -71,6 +72,7 @@ export enum MessageFromWebviewType {
REMOTE_REMOVE = 'remote-remove',
REMOVE_CUSTOM_PLOTS = 'remove-custom-plots',
REMOVE_STUDIO_TOKEN = 'remove-studio-token',
REMOVE_STUDIO_URL = 'remove-studio-url',
MODIFY_WORKSPACE_PARAMS_AND_QUEUE = 'modify-workspace-params-and-queue',
MODIFY_WORKSPACE_PARAMS_AND_RUN = 'modify-workspace-params-and-run',
MODIFY_WORKSPACE_PARAMS_RESET_AND_RUN = 'modify-workspace-params-reset-and-run',
Expand Down Expand Up @@ -224,6 +226,7 @@ export type MessageFromWebview =
type: MessageFromWebviewType.REMOVE_CUSTOM_PLOTS
}
| { type: MessageFromWebviewType.REMOVE_STUDIO_TOKEN }
| { type: MessageFromWebviewType.REMOVE_STUDIO_URL }
| {
type: MessageFromWebviewType.REORDER_PLOTS_COMPARISON
payload: string[]
Expand Down Expand Up @@ -291,6 +294,7 @@ export type MessageFromWebview =
| { type: MessageFromWebviewType.UPGRADE_DVC }
| { type: MessageFromWebviewType.SETUP_WORKSPACE }
| { type: MessageFromWebviewType.SAVE_STUDIO_TOKEN }
| { type: MessageFromWebviewType.SAVE_STUDIO_URL }
| { type: MessageFromWebviewType.REQUEST_STUDIO_TOKEN }
| { type: MessageFromWebviewType.ADD_CONFIGURATION }
| { type: MessageFromWebviewType.ZOOM_PLOT; payload?: string }
Expand Down
78 changes: 78 additions & 0 deletions webview/src/setup/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const DEFAULT_DATA = {
pythonBinPath: undefined,
remoteList: undefined,
sectionCollapsed: undefined,
selfHostedStudioUrl: null,
shareLiveToStudio: false
}

Expand Down Expand Up @@ -675,6 +676,65 @@ describe('App', () => {
within(iconWrapper).getByTestId(TooltipIconType.WARNING)
).toBeInTheDocument()
})

it('should show the self hosted url with actions to change it if the user has set one', () => {
const selfHostedUrl = 'https://studio.example.com'
renderApp({ selfHostedStudioUrl: selfHostedUrl })

const urlDetails = screen.getByTestId('studio-url-details')

expect(
within(urlDetails).getByText('Self-Hosted Url:')
).toBeInTheDocument()
expect(within(urlDetails).getByText(selfHostedUrl)).toBeInTheDocument()

const updateUrlBtn = within(urlDetails).getByText('Update')
const removeUrlBtn = within(urlDetails).getByText('Remove')

expect(updateUrlBtn).toBeInTheDocument()
expect(removeUrlBtn).toBeInTheDocument()

mockPostMessage.mockClear()
fireEvent.click(updateUrlBtn)

expect(mockPostMessage).toHaveBeenCalledTimes(1)
expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SAVE_STUDIO_URL
})

mockPostMessage.mockClear()
fireEvent.click(removeUrlBtn)

expect(mockPostMessage).toHaveBeenCalledTimes(1)
expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.REMOVE_STUDIO_URL
})
})

it('should show the self hosted url with "Not found" with an action to add one if the user has not set one', () => {
renderApp()

const urlDetails = screen.getByTestId('studio-url-details')

expect(
within(urlDetails).getByText('Self-Hosted Url:')
).toBeInTheDocument()
expect(within(urlDetails).getByText('Not found')).toBeInTheDocument()

const addUrlBtn = within(urlDetails).getByText('Add Url')

expect(addUrlBtn).toBeInTheDocument()

mockPostMessage.mockClear()
fireEvent.click(addUrlBtn)

expect(mockPostMessage).toHaveBeenCalledTimes(1)
expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SAVE_STUDIO_URL
})

mockPostMessage.mockClear()
})
})

describe('Studio connected', () => {
Expand Down Expand Up @@ -718,6 +778,24 @@ describe('App', () => {
within(iconWrapper).getByTestId(TooltipIconType.PASSED)
).toBeInTheDocument()
})

it('should show the self hosted url with actions to change it if the user has set one', () => {
const selfHostedUrl = 'https://studio.example.com'
renderApp({ isStudioConnected: true, selfHostedStudioUrl: selfHostedUrl })

const urlDetails = screen.getByTestId('studio-url-details')

expect(
within(urlDetails).getByText('Self-Hosted Url:')
).toBeInTheDocument()
expect(within(urlDetails).getByText(selfHostedUrl)).toBeInTheDocument()

const updateUrlBtn = within(urlDetails).getByText('Update')
const removeUrlBtn = within(urlDetails).getByText('Remove')

expect(updateUrlBtn).toBeInTheDocument()
expect(removeUrlBtn).toBeInTheDocument()
})
})

describe('focused section', () => {
Expand Down
4 changes: 4 additions & 0 deletions webview/src/setup/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
import { updateRemoteList } from '../state/remoteSlice'
import {
updateIsStudioConnected,
updateSelfHostedStudioUrl,
updateShareLiveToStudio
} from '../state/studioSlice'
import { setStudioShareExperimentsLive } from '../util/messages'
Expand Down Expand Up @@ -119,6 +120,9 @@ export const feedStore = (
case 'shareLiveToStudio':
dispatch(updateShareLiveToStudio(data.data.shareLiveToStudio))
continue
case 'selfHostedStudioUrl':
dispatch(updateSelfHostedStudioUrl(data.data.selfHostedStudioUrl))
continue
case 'remoteList':
dispatch(updateRemoteList(data.data.remoteList))
continue
Expand Down
42 changes: 0 additions & 42 deletions webview/src/setup/components/dvc/DvcEnvCommandRow.tsx

This file was deleted.

Loading
Loading