Skip to content

Adds retry button for project baseUrl warning #5325

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

Merged
merged 15 commits into from
Feb 27, 2020
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
2 changes: 1 addition & 1 deletion packages/desktop-gui/cypress/integration/project_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ describe('Project', function () {
cy.stub(this.ipc, 'getSpecs').yields(null, this.specs)
cy.stub(this.ipc, 'closeProject').resolves()
cy.stub(this.ipc, 'onConfigChanged')
cy.stub(this.ipc, 'onProjectWarning')

this.getProjectStatus = this.util.deferred()
cy.stub(this.ipc, 'getProjectStatus').returns(this.getProjectStatus.promise)

this.updaterCheck = this.util.deferred()

cy.stub(this.ipc, 'updaterCheck').resolves(this.updaterCheck.promise)
})
})
Expand Down
71 changes: 66 additions & 5 deletions packages/desktop-gui/cypress/integration/warning_message_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ describe('Warning Message', function () {
cy.stub(this.ipc, 'onProjectWarning')
cy.stub(this.ipc, 'externalOpen')

this.pingBaseUrl = this.util.deferred()
cy.stub(this.ipc, 'pingBaseUrl').returns(this.pingBaseUrl.promise)

this.start()
})
})
Expand All @@ -38,7 +41,7 @@ describe('Warning Message', function () {
this.ipc.onProjectWarning.yield(null, this.warningObj)
})

cy.get('.alert-warning button').click()
cy.get('.alert-warning .close').click()

cy.get('.alert-warning').should('not.exist')
})
Expand All @@ -48,7 +51,7 @@ describe('Warning Message', function () {
this.ipc.onProjectWarning.yield(null, this.warningObj)
})

cy.get('.alert-warning button').click()
cy.get('.alert-warning .close').click()
cy.get('.alert-warning').should('not.exist').then(() => {
this.ipc.onProjectWarning.yield(null, this.warningObj)
})
Expand All @@ -61,7 +64,7 @@ describe('Warning Message', function () {
this.ipc.onProjectWarning.yield(null, this.warningObj)
})

cy.get('.alert-warning button').click()
cy.get('.alert-warning .close').click()
cy.get('.alert-warning').should('not.exist').then(() => {
this.ipc.onProjectWarning.yield(null, {
type: 'PRETTY_BAD_WARNING',
Expand Down Expand Up @@ -132,6 +135,64 @@ describe('Warning Message', function () {
cy.get('.specs').invoke('position').its('top').should('gt', 100)
})

it('does not show retry button for non-baseUrl warnings', function () {
cy.shouldBeOnProjectSpecs().then(() => {
this.ipc.onProjectWarning.yield(null, this.warningObj)
})

cy.contains('Try Again').should('not.exist')
})

context('baseUrl warnings', function () {
beforeEach(function () {
this.warningObj.type = 'CANNOT_CONNECT_BASE_URL_WARNING'

cy.shouldBeOnProjectSpecs().then(() => {
this.ipc.onProjectWarning.yield(null, this.warningObj)
})
})

it('shows retry button', function () {
cy.contains('Try Again')
})

it('pings baseUrl and disables retry button when clicked', function () {
cy.contains('Try Again').click()
.should('be.disabled')
.should('have.text', 'Retrying...')
.find('i').should('have.class', 'fa-spin')

cy.wrap(this.ipc.pingBaseUrl).should('be.called')
})

it('shows warning and enables retry button if baseUrl is still unreachable', function () {
cy.contains('Try Again').click().then(function () {
this.pingBaseUrl.reject(this.warningObj)
})

cy.get('.alert-warning').should('be.visible')
cy.contains('Try Again').should('not.be.disabled')
cy.contains('Try Again').find('i').should('not.have.class', 'fa-spin')
})

it('does not show warning if baseUrl is reachable', function () {
cy.contains('Try Again').click().then(function () {
this.pingBaseUrl.resolve()
})

cy.get('.alert-warning').should('not.be.visible')
})

it('shows real error if one results from pinging baseUrl', function () {
cy.contains('Try Again').click().then(function () {
this.pingBaseUrl.reject(new Error('Some other error'))
})

cy.contains('An unexpected error occurred')
cy.contains('Some other error')
})
})

context('with multiple warnings', function () {
beforeEach(function () {
this.warningObj2 = {
Expand Down Expand Up @@ -166,12 +227,12 @@ describe('Warning Message', function () {
.should('contain', 'Some warning')
.should('contain', 'Other message')

cy.get('.alert-warning button').first().click()
cy.get('.alert-warning .close').first().click()
cy.get('.alert-warning')
.should('not.contain', 'Some warning')
.should('contain', 'Other message')

cy.get('.alert-warning button').click()
cy.get('.alert-warning .close').click()
cy.get('.alert-warning')
.should('not.contain', 'Some warning')
.should('not.contain', 'Other message')
Expand Down
1 change: 1 addition & 0 deletions packages/desktop-gui/src/lib/ipc.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ register('on:spec:changed', false)
register('on:project:error', false)
register('on:project:warning', false)
register('ping:api:server')
register('ping:baseUrl')
register('remove:project')
register('request:access')
register('setup:dashboard:project')
Expand Down
34 changes: 21 additions & 13 deletions packages/desktop-gui/src/project/project-model.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import _ from 'lodash'
import { computed, observable, action } from 'mobx'
import { action, computed, observable, toJS } from 'mobx'

import Browser from '../lib/browser-model'
import Warning from './warning-model'

const cacheProps = [
'id',
Expand Down Expand Up @@ -57,7 +59,7 @@ export default class Project {
@observable resolvedConfig
@observable error
/** @type {{[key: string] : {warning:Error & {dismissed: boolean}}}} */
@observable warnings = {}
@observable _warnings = {}
@observable apiError
@observable parentTestsFolderDisplay
@observable integrationExampleName
Expand Down Expand Up @@ -123,6 +125,10 @@ export default class Project {
return this.browsers[0]
}

@computed get warnings () {
return _.reject(this._warnings, { isDismissed: true })
}

@action update (props) {
if (!props) return

Expand Down Expand Up @@ -220,28 +226,24 @@ export default class Project {
}

@action addWarning (warning) {
const id = warning.type
const type = warning.type

if (id && this.warnings[id] && this.warnings[id].dismissed) {
if (type && this._warnings[type] && this._warnings[type].isDismissed) {
return
}

this.warnings[id] = { ...warning }
this._warnings[type] = new Warning(warning)
}

@action clearWarning (warning) {
@action dismissWarning (warning) {
if (!warning) {
// calling with no warning clears all warnings
return _.each(this.warnings, ((warning) => {
return this.clearWarning(warning)
return _.each(this._warnings, ((warning) => {
return this.dismissWarning(warning)
}))
}

warning.dismissed = true
}

_serializeWarning (warning) {
return `${warning.type}:${warning.name}:${warning.message}`
warning.setDismissed(true)
}

@action setApiError = (err = {}) => {
Expand All @@ -258,6 +260,12 @@ export default class Project {
return _.pick(this, 'id', 'path')
}

getConfigValue (key) {
if (!this.resolvedConfig) return

return toJS(this.resolvedConfig[key]).value
}

serialize () {
return _.pick(this, cacheProps)
}
Expand Down
30 changes: 28 additions & 2 deletions packages/desktop-gui/src/project/project.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,42 @@ class Project extends Component {
_renderWarnings = () => {
const { warnings } = this.props.project

return _.map(warnings, (warning, i) => (!warning.dismissed && <WarningMessage key={i} warning={warning} onClearWarning={() => this._removeWarning(warning)}/>))
return _.map(warnings, (warning, i) => (
<WarningMessage
key={i}
warning={warning}
onRetry={() => this._retryPingingBaseUrl(warning)}
onDismissWarning={() => this._removeWarning(warning)}
/>
))
}

_removeWarning = (warning) => {
this.props.project.clearWarning(warning)
this.props.project.dismissWarning(warning)
}

_reopenProject = () => {
projectsApi.reopenProject(this.props.project)
}

_retryPingingBaseUrl = (warning) => {
const { project } = this.props

warning.setRetrying(true)

projectsApi.pingBaseUrl(project.getConfigValue('baseUrl'))
.then(() => {
project.dismissWarning(warning)
})
.catch((err) => {
if (err && err.type === warning.type) return

project.setError(err)
})
.finally(() => {
warning.setRetrying(false)
})
}
}

export default Project
16 changes: 14 additions & 2 deletions packages/desktop-gui/src/project/warning-message.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import cs from 'classnames'
import React, { Component } from 'react'
import { observer } from 'mobx-react'
import MarkdownRenderer from '../lib/markdown-renderer'

@observer
class WarningMessage extends Component {
render () {
const warningText = this.props.warning.message.split('\n').join('<br />')
const { warning } = this.props
const warningText = warning.message.split('\n').join('<br />')

return (
<div className='alert alert-warning'>
Expand All @@ -15,8 +17,18 @@ class WarningMessage extends Component {
</p>
<div>
<MarkdownRenderer markdown={warningText}/>
{warning.isRetryable &&
<button
className='retry-button btn btn-default btn-sm'
disabled={warning.isRetrying}
onClick={this.props.onRetry}
>
<i className={cs('fas fa-sync', { 'fa-spin': warning.isRetrying })} />
{warning.isRetrying ? 'Retrying...' : 'Try Again'}
</button>
}
</div>
<button className='btn btn-link close' onClick={this.props.onClearWarning}>
<button className='btn btn-link close' onClick={this.props.onDismissWarning}>
<i className='fas fa-times' />
</button>
</div>
Expand Down
27 changes: 27 additions & 0 deletions packages/desktop-gui/src/project/warning-model.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { action, computed, observable } from 'mobx'

class Warning {
@observable type
@observable message
@observable isDismissed = false
@observable isRetrying = false

constructor (props) {
this.type = props.type
this.message = props.message
}

@computed get isRetryable () {
return this.type === 'CANNOT_CONNECT_BASE_URL_WARNING'
}

@action setDismissed (isDismissed) {
this.isDismissed = isDismissed
}

@action setRetrying (isRetrying) {
this.isRetrying = isRetrying
}
}

export default Warning
7 changes: 6 additions & 1 deletion packages/desktop-gui/src/projects/projects-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,18 @@ const openProject = (project) => {

const reopenProject = (project) => {
project.clearError()
project.clearWarning()
project.dismissWarning()

return closeProject(project)
.then(() => {
return openProject(project)
})
}

const pingBaseUrl = (url) => {
return ipc.pingBaseUrl(url)
}

const removeProject = (project) => {
ipc.removeProject(project.path)
projectsStore.removeProject(project)
Expand Down Expand Up @@ -234,4 +238,5 @@ export default {
runSpec,
closeBrowser,
getRecordKeys,
pingBaseUrl,
}
11 changes: 11 additions & 0 deletions packages/desktop-gui/src/styles/components/_alerts.scss
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@
flex-shrink: 0;
position: relative;

.retry-button {
font-size: 12px;
position: unset;
top: unset;
right: unset;

i {
margin-right: 0.5em;
}
}

code, pre {
border: none;
color: #8a6d3b;
Expand Down
8 changes: 8 additions & 0 deletions packages/server/lib/gui/events.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,14 @@ handleEvent = (options, bus, event, id, type, arg) ->
err.apiUrl = apiUrl
sendErr(err)

when "ping:baseUrl"
baseUrl = arg
ensureUrl.isListening(baseUrl)
.then(send)
.catch (err) ->
warning = errors.get("CANNOT_CONNECT_BASE_URL_WARNING", baseUrl)
sendErr(warning)

when "set:clipboard:text"
clipboard.writeText(arg)
sendNull()
Expand Down