Adds retry button for project baseUrl warning (#5325)

* Adds retry button for project baseUrl warning

* Refactors project retry button

* Refactors to improve testability and adds tests

* Adds back reopenProject method

* Fixes warning message test errors

* Makes requested changes

* improve selectors

* fix dismissing warnings

* rename clear warning to dismiss warning

* refactor / improve

- only add retry button if it's the base url warning
- keep warning around while waiting for retry
- move tests to warning message spec

* disable retry button and show retrying message while retrying base url

Co-authored-by: ngunnarson <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
Co-authored-by: Chris Breiding <[email protected]>

packages/desktop-gui/cypress/integration/project_spec.js

@@ -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)
     })
   })

packages/desktop-gui/cypress/integration/warning_message_spec.js

@@ -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()
     })
   })
@@ -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')
   })
@@ -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)
     })
@@ -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',
@@ -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 = {
@@ -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')

packages/desktop-gui/src/lib/ipc.js

@@ -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')

packages/desktop-gui/src/project/project-model.js

@@ -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',
@@ -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
@@ -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
 
@@ -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 = {}) => {
@@ -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)
   }

packages/desktop-gui/src/project/project.jsx

@@ -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

packages/desktop-gui/src/project/warning-message.jsx

@@ -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'>
@@ -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>

packages/desktop-gui/src/project/warning-model.js

@@ -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

packages/desktop-gui/src/projects/projects-api.js

@@ -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)
@@ -234,4 +238,5 @@ export default {
   runSpec,
   closeBrowser,
   getRecordKeys,
+  pingBaseUrl,
 }

packages/desktop-gui/src/styles/components/_alerts.scss

@@ -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;

packages/server/lib/gui/events.coffee

@@ -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()