search-select: refactor to use React hooks (#124)

* fix source maps for local debugging * add error boundary to dialogs * fix fmt script * convert QuerySelect components to hooks * fix alerts list lint errors - jsx-a11y/click-events-have-key-events - jsx-a11y/no-static-element-interactions * fix alerts tests * fix service search query * set default error policy * display error in search select if things break * fix timezone search * don't omit options in single-select mode * convert ScheduleTZFilter to hooks (fixes jump)
target · Sep 13, 2019 · 936e0d5 · 936e0d5
1 parent 551a828
commit 936e0d5
Show file tree

Hide file tree

Showing 19 changed files with 375 additions and 476 deletions.
diff --git a/devtools/runjson/localdev.json b/devtools/runjson/localdev.json
@@ -28,7 +28,7 @@
     "Command": [
       "./node_modules/.bin/webpack-dev-server",
       "--inline",
-      "--devtool=cheap-module-source-map",
+      "--devtool=inline-source-map",
       "--allowed-hosts=docker.for.mac.host.internal",
       "--port=3035",
       "--progress=false",

diff --git a/service/search.go b/service/search.go
@@ -3,12 +3,13 @@ package service
 import (
 	"context"
 	"database/sql"
+	"strings"
+	"text/template"
+
 	"github.com/target/goalert/permission"
 	"github.com/target/goalert/search"
 	"github.com/target/goalert/util/sqlutil"
 	"github.com/target/goalert/validation/validate"
-	"strings"
-	"text/template"
 
 	"github.com/pkg/errors"
 )
@@ -58,7 +59,7 @@ var searchTemplate = template.Must(template.New("search").Parse(`
 	{{end}}
 	WHERE true
 	{{if .Omit}}
-		AND not id = any(:omit)
+		AND not svc.id = any(:omit)
 	{{end}}
 	{{- if and .LabelKey .LabelNegate}}
 		AND svc.id NOT IN (

diff --git a/timezone/search.go b/timezone/search.go
@@ -3,12 +3,13 @@ package timezone
 import (
 	"context"
 	"database/sql"
+	"strconv"
+	"text/template"
+
 	"github.com/target/goalert/permission"
 	"github.com/target/goalert/search"
 	"github.com/target/goalert/util/sqlutil"
 	"github.com/target/goalert/validation/validate"
-	"strconv"
-	"text/template"
 
 	"github.com/pkg/errors"
 )
@@ -86,7 +87,7 @@ func (opts renderData) QueryArgs() []sql.NamedArg {
 	return []sql.NamedArg{
 		sql.Named("search", opts.SearchStr()),
 		sql.Named("afterName", opts.After.Name),
-		sql.Named("omit", sqlutil.UUIDArray(opts.Omit)),
+		sql.Named("omit", sqlutil.StringArray(opts.Omit)),
 	}
 }
 

diff --git a/web/src/app/alerts/components/AlertsListDataWrapper.js b/web/src/app/alerts/components/AlertsListDataWrapper.js
@@ -8,7 +8,7 @@ import Typography from '@material-ui/core/Typography'
 import moment from 'moment'
 import withStyles from '@material-ui/core/styles/withStyles'
 import { connect } from 'react-redux'
-import { withRouter } from 'react-router-dom'
+import { Link } from 'react-router-dom'
 import { setCheckedAlerts } from '../../actions'
 import { bindActionCreators } from 'redux'
 import statusStyles from '../../util/statusStyles'
@@ -47,7 +47,6 @@ const mapDispatchToProps = dispatch =>
   mapStateToProps,
   mapDispatchToProps,
 )
-@withRouter
 export default class AlertsListDataWrapper extends Component {
   static propTypes = {
     alert: p.object.isRequired,
@@ -98,7 +97,7 @@ export default class AlertsListDataWrapper extends Component {
   }
 
   render() {
-    const { alert, checkedAlerts, classes, history, onServicePage } = this.props
+    const { alert, checkedAlerts, classes, onServicePage } = this.props
 
     const checkbox = (
       <Checkbox
@@ -114,6 +113,7 @@ export default class AlertsListDataWrapper extends Component {
         disableRipple
         tabIndex={-1}
         onChange={() => this.toggleChecked(alert.number)}
+        onClick={e => e.stopPropagation()}
       />
     )
 
@@ -131,34 +131,35 @@ export default class AlertsListDataWrapper extends Component {
     }
 
     return (
-      <ListItem button className={statusClass}>
+      <ListItem
+        button
+        className={statusClass}
+        component={Link}
+        to={`/alerts/${alert.number}`}
+      >
         {checkbox}
-        <div
-          className={classes.listItem}
-          onClick={() => history.push(`/alerts/${alert.number}`)}
-        >
-          <ListItemText disableTypography style={{ paddingRight: '2.75em' }}>
-            <Typography>
-              <b>{alert.number}: </b>
-              {alert.status.toUpperCase()}
-            </Typography>
-            {onServicePage ? null : (
-              <Typography variant='caption'>{alert.service.name}</Typography>
-            )}
-            <Typography variant='caption' noWrap>
-              {alert.summary}
+
+        <ListItemText disableTypography style={{ paddingRight: '2.75em' }}>
+          <Typography>
+            <b>{alert.number}: </b>
+            {alert.status.toUpperCase()}
+          </Typography>
+          {onServicePage ? null : (
+            <Typography variant='caption'>{alert.service.name}</Typography>
+          )}
+          <Typography variant='caption' noWrap>
+            {alert.summary}
+          </Typography>
+        </ListItemText>
+        <ListItemSecondaryAction>
+          <ListItemText disableTypography>
+            <Typography variant='caption'>
+              {moment(alert.created_at)
+                .local()
+                .fromNow()}
             </Typography>
           </ListItemText>
-          <ListItemSecondaryAction>
-            <ListItemText disableTypography>
-              <Typography variant='caption'>
-                {moment(alert.created_at)
-                  .local()
-                  .fromNow()}
-              </Typography>
-            </ListItemText>
-          </ListItemSecondaryAction>
-        </div>
+        </ListItemSecondaryAction>
       </ListItem>
     )
   }

diff --git a/web/src/app/apollo.js b/web/src/app/apollo.js
@@ -88,6 +88,7 @@ const defaultLink = ApolloLink.from([
 export const LegacyGraphQLClient = new ApolloClient({
   link: defaultLink,
   cache: new InMemoryCache(),
+  defaultOptions: { errorPolicy: 'all' },
 })
 
 const graphql2HttpLink = createHttpLink({
@@ -130,7 +131,7 @@ cache = new InMemoryCache({
   },
 })
 
-const queryOpts = { fetchPolicy: 'cache-and-network' }
+const queryOpts = { fetchPolicy: 'cache-and-network', errorPolicy: 'all' }
 if (new URLSearchParams(location.search).get('poll') !== '0') {
   queryOpts.pollInterval = POLL_INTERVAL
 }

diff --git a/web/src/app/dialogs/FormDialog.js b/web/src/app/dialogs/FormDialog.js
@@ -15,6 +15,7 @@ import DialogContentError from './components/DialogContentError'
 import { styles as globalStyles } from '../styles/materialStyles'
 import gracefulUnmount from '../util/gracefulUnmount'
 import { Form } from '../forms'
+import ErrorBoundary from '../main/ErrorBoundary'
 
 const styles = theme => {
   const { cancelButton, dialogWidth } = globalStyles(theme)
@@ -131,10 +132,12 @@ export default class FormDialog extends React.PureComponent {
             if (valid) onSubmit()
           }}
         >
-          {this.renderForm()}
-          {this.renderCaption()}
-          {this.renderErrors()}
-          {this.renderActions()}
+          <ErrorBoundary>
+            {this.renderForm()}
+            {this.renderCaption()}
+            {this.renderErrors()}
+            {this.renderActions()}
+          </ErrorBoundary>
         </Form>
       </Dialog>
     )

diff --git a/web/src/app/schedules/ScheduleTZFilter.js b/web/src/app/schedules/ScheduleTZFilter.js
@@ -1,12 +1,12 @@
 import React from 'react'
 import p from 'prop-types'
-import { connect } from 'react-redux'
 import { urlParamSelector } from '../selectors'
 import { setURLParam } from '../actions'
-import Query from '../util/Query'
 import gql from 'graphql-tag'
 import { FormControlLabel, Switch } from '@material-ui/core'
 import { oneOfShape } from '../util/propTypes'
+import { useQuery } from 'react-apollo'
+import { useSelector, useDispatch } from 'react-redux'
 
 const tzQuery = gql`
   query($id: ID!) {
@@ -17,53 +17,50 @@ const tzQuery = gql`
   }
 `
 
-@connect(
-  state => ({ zone: urlParamSelector(state)('tz', 'local') }),
-  dispatch => ({
-    setZone: value => dispatch(setURLParam('tz', value, 'local')),
-  }),
-)
-export class ScheduleTZFilter extends React.PureComponent {
-  static propTypes = {
-    label: p.func,
-
-    // one of scheduleID or scheduleTimeZone must be specified
-    _tz: oneOfShape({
-      scheduleID: p.string,
-      scheduleTimeZone: p.string,
-    }),
+export function ScheduleTZFilter(props) {
+  const params = useSelector(urlParamSelector)
+  const zone = params('tz', 'local')
+  const dispatch = useDispatch()
+  const setZone = value => dispatch(setURLParam('tz', value, 'local'))
+  const { data, loading, error } = useQuery(tzQuery, {
+    pollInterval: 0,
+    variables: { id: props.scheduleID },
+  })
 
-    // provided by connect
-    zone: p.string,
-    setZone: p.func,
+  let label, tz
+  if (error) {
+    label = 'Error: ' + (error.message || error)
+  } else if (loading) {
+    label = 'Fetching timezone information...'
+  } else {
+    tz = data.schedule.timeZone
+    label = props.label ? props.label(tz) : `Show times in ${tz}`
   }
-  render() {
-    const { scheduleID, scheduleTimeZone } = this.props
-    if (scheduleTimeZone) return this.renderControl(scheduleTimeZone)
 
-    return (
-      <Query
-        variables={{ id: scheduleID }}
-        query={tzQuery}
-        noPoll
-        render={({ data }) => this.renderControl(data.schedule.timeZone)}
-      />
-    )
-  }
+  return (
+    <FormControlLabel
+      control={
+        <Switch
+          checked={zone !== 'local'}
+          onChange={e => setZone(e.target.checked ? tz : 'local')}
+          value={tz}
+          disabled={Boolean(loading || error)}
+        />
+      }
+      label={label}
+    />
+  )
+}
+ScheduleTZFilter.propTypes = {
+  label: p.func,
 
-  renderControl(tz) {
-    const { zone, label, setZone } = this.props
-    return (
-      <FormControlLabel
-        control={
-          <Switch
-            checked={zone !== 'local'}
-            onChange={e => setZone(e.target.checked ? tz : 'local')}
-            value={tz}
-          />
-        }
-        label={label ? label(tz) : `Show times in ${tz}`}
-      />
-    )
-  }
+  // one of scheduleID or scheduleTimeZone must be specified
+  _tz: oneOfShape({
+    scheduleID: p.string,
+    scheduleTimeZone: p.string,
+  }),
+
+  // provided by connect
+  zone: p.string,
+  setZone: p.func,
 }
diff --git a/web/src/app/selection/EscalationPolicySelect.js b/web/src/app/selection/EscalationPolicySelect.js
@@ -1,7 +1,5 @@
-import React from 'react'
-
 import gql from 'graphql-tag'
-import QuerySelect from './QuerySelect'
+import { makeQuerySelect } from './QuerySelect'
 
 const query = gql`
   query($input: EscalationPolicySearchOptions) {
@@ -22,8 +20,8 @@ const valueQuery = gql`
     }
   }
 `
-export class EscalationPolicySelect extends React.PureComponent {
-  render() {
-    return <QuerySelect {...this.props} query={query} valueQuery={valueQuery} />
-  }
-}
+
+export const EscalationPolicySelect = makeQuerySelect(
+  'EscalationPolicySelect',
+  { query, valueQuery },
+)
diff --git a/web/src/app/selection/LabelKeySelect.js b/web/src/app/selection/LabelKeySelect.js
@@ -1,5 +1,4 @@
-import React from 'react'
-import QuerySelect from './QuerySelect'
+import { makeQuerySelect } from './QuerySelect'
 import gql from 'graphql-tag'
 
 const query = gql`
@@ -12,19 +11,9 @@ const query = gql`
   }
 `
 
-export class LabelKeySelect extends React.PureComponent {
-  render() {
-    return (
-      <QuerySelect
-        {...this.props}
-        variables={{ input: { uniqueKeys: true } }}
-        defaultQueryVariables={{ input: { uniqueKeys: true } }}
-        mapDataNode={node => ({
-          label: node.key,
-          value: node.key,
-        })}
-        query={query}
-      />
-    )
-  }
-}
+export const LabelKeySelect = makeQuerySelect('LabelKeySelect', {
+  variables: { uniqueKeys: true },
+  defaultQueryVariables: { uniqueKeys: true },
+  query,
+  mapDataNode: ({ key }) => ({ label: key, value: key }),
+})