From 339c68533499a902f083a06ec08d8e2ce2f8f535 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 14:43:54 -0500 Subject: [PATCH 01/16] fix source maps for local debugging --- devtools/runjson/localdev.json | 2 +- web/src/webpack.config.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/devtools/runjson/localdev.json b/devtools/runjson/localdev.json index 10586d9c49..1830c70cf7 100644 --- 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/web/src/webpack.config.js b/web/src/webpack.config.js index 6b3ab3595d..38945c6e56 100644 --- a/web/src/webpack.config.js +++ b/web/src/webpack.config.js @@ -97,7 +97,7 @@ module.exports = { }, // Source maps used for debugging information - devtool: 'eval', + devtool: 'inline-source-map', // webpack-dev-server configuration devServer: { disableHostCheck: true, From 23d3d708c799a0c1b2f1628e2b151ef52d3c8e39 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 14:44:14 -0500 Subject: [PATCH 02/16] add error boundary to dialogs --- web/src/app/dialogs/FormDialog.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/web/src/app/dialogs/FormDialog.js b/web/src/app/dialogs/FormDialog.js index 9af63ca9c9..dec2277ca6 100644 --- 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()} + + {this.renderForm()} + {this.renderCaption()} + {this.renderErrors()} + {this.renderActions()} + ) From 87cfbfafcb34ef62f23832220907a8501164d0b0 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 14:45:38 -0500 Subject: [PATCH 03/16] fix fmt script --- web/src/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/package.json b/web/src/package.json index b7cd6242f1..04051379b2 100644 --- a/web/src/package.json +++ b/web/src/package.json @@ -7,7 +7,7 @@ "slint": "stylelint ./app/**/*.css ./app/**/*.scss --syntax scss", "lint": "eslint .", "precommit-msg": "echo 'Pre-commit checks...' && exit 0", - "fmt": "prettier -l --write '**/*.{js,yml,yaml,json,css,scss,ts,html}' || eslint --fix .", + "fmt": "prettier -l --write '**/*.{js,yml,yaml,json,css,scss,ts,html}' && eslint --fix .", "build-deps": "webpack --config ./webpack.dll.config.js --progress" }, "jest": { From 0680409fe9f6c1ea401df64c5bb8f54d91495530 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 14:50:23 -0500 Subject: [PATCH 04/16] convert QuerySelect components to hooks --- .../app/selection/EscalationPolicySelect.js | 14 +- web/src/app/selection/LabelKeySelect.js | 25 +- web/src/app/selection/QuerySelect.js | 432 +++++++----------- web/src/app/selection/RotationSelect.js | 23 +- web/src/app/selection/ScheduleSelect.js | 24 +- web/src/app/selection/ServiceSelect.js | 23 +- web/src/app/selection/SlackChannelSelect.js | 33 +- web/src/app/selection/TimeZoneSelect.js | 19 +- web/src/app/selection/UserSelect.js | 14 +- 9 files changed, 227 insertions(+), 380 deletions(-) diff --git a/web/src/app/selection/EscalationPolicySelect.js b/web/src/app/selection/EscalationPolicySelect.js index 1f81a04f37..988c0540d7 100644 --- 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 - } -} + +export const EscalationPolicySelect = makeQuerySelect( + 'EscalationPolicySelect', + { query, valueQuery }, +) diff --git a/web/src/app/selection/LabelKeySelect.js b/web/src/app/selection/LabelKeySelect.js index 3457d51ca1..d1de17d1b6 100644 --- 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 ( - ({ - 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 }), +}) diff --git a/web/src/app/selection/QuerySelect.js b/web/src/app/selection/QuerySelect.js index 22c2e57917..118f0bdf15 100644 --- a/web/src/app/selection/QuerySelect.js +++ b/web/src/app/selection/QuerySelect.js @@ -1,12 +1,11 @@ -import React from 'react' +import React, { useState, useEffect } from 'react' import p from 'prop-types' -import Query from '../util/Query' -import { debounce } from 'lodash-es' +import { memoize, omit } from 'lodash-es' import MaterialSelect from './MaterialSelect' import { mergeFields, fieldAlias, mapInputVars } from '../util/graphql' import FavoriteIcon from '@material-ui/icons/Star' import { DEBOUNCE_DELAY } from '../config' -import { POLL_INTERVAL } from '../util/poll_intervals' +import { useQuery } from 'react-apollo' // valueCheck ensures the type is `arrayOf(p.string)` if `multiple` is set // and `p.string` otherwise. @@ -15,292 +14,213 @@ function valueCheck(props, ...args) { return p.string(props, ...args) } -/* - * This component is to be used when we want to create a select based on a query from the database. - * (i.e. users, services, schedules, etc) - */ -export default class QuerySelect extends React.PureComponent { - static propTypes = { - // The provided query must map to a Search/Connection type - // field. (like `escalationPolicies` or `users`) - query: p.object.isRequired, - - // valueQuery should return the same data as `query` but for - // a single element given an `id` input variable. - valueQuery: p.object, - - // mapDataNode should take a node from the query response and return - // a structure in the format of: `{label, value, isFavorite}` - mapDataNode: p.func, - - // variables can be used to add extra parameters to the query - // such as: `{ input: { favoritesFirst: true } }` - // - // Provided variables will be merged with normal pagination/search. - variables: p.object, - - // If defaultQueryVariables is set, then when there is no search - // (user has focused the select, but not typed anything) it will be - // used to fetch a list of default results. - // - // This can be used to display favorites by default by setting - // `defaultQueryVariables={ input: { favoritesOnly: true } }` - defaultQueryVariables: p.object, - - // If specified, a "Create" option will be provided for the users - // provided text. It will be called instead of `onChange` if the user - // selects the generated option. - // - // Example: if the user types `foobar` and there is not a `foobar` option, - // then `Create "foobar"` will be displayed in the dropdown. - onCreate: p.func, - - error: p.bool, - onChange: p.func.isRequired, - value: valueCheck, - - multiple: p.bool, - name: p.string, - placeholder: p.string, - } +const defaultMapNode = ({ name: label, id: value, isFavorite }) => ({ + label, + value, + isFavorite, +}) - static defaultProps = { - error: false, - value: null, - multiple: false, - mapDataNode: node => ({ - label: node.name, - value: node.id, - isFavorite: Boolean(node.isFavorite), - }), - variables: {}, - } +const asArray = value => { + if (!value) return [] - state = { - outdated: false, - search: '', - skip: true, - } - - componentDidMount() { - this._refresh = setInterval(() => this.forceUpdate(), POLL_INTERVAL) - } - componentWillUnmount() { - clearInterval(this._refresh) - this.onInputChange.cancel() + return Array.isArray(value) ? value : [value] +} +const mapValueQuery = (query, index) => + mapInputVars(fieldAlias(query, 'data' + index), { id: 'id' + index }) +// useValues will return a set of {id, label} +// for the provided value. +// +// If value is not an array, a single object (instead of array) +// is returned. +function makeUseValues(query, mapNode) { + if (!query) { + // no value query, so always use the map function + return function useValuesNoQuery(_value) { + const value = asArray(_value).map(v => ({ value: v, label: v })) + return [Array.isArray(_value) ? value : value[0] || null, null] + } } - onInputChange = debounce(search => { - this.setState({ search, skip: !search, outdated: false }) - }, DEBOUNCE_DELAY) + const getQueryBySize = memoize(size => { + let q = mapValueQuery(query, 0) - isEmpty() { - return this.props.multiple - ? this.props.value.length === 0 - : !this.props.value - } - - render() { - return this.renderValueQuery() - } - - renderValueQuery() { - if (!this.props.valueQuery) { - // if no query provided, display the value itself - const mapVal = v => ({ value: v, label: v }) - let value = [] - if (this.props.value && this.props.multiple) { - value = this.props.value.map(mapVal) - } else if (this.props.value && !this.props.multiple) { - value = [mapVal(this.props.value)] - } - - return this.renderOptionsQuery(value) + for (let i = 1; i < size; i++) { + q = mergeFields(q, mapValueQuery(query, i)) } - let query, variables, getOptions - if (this.props.multiple) { - query = fieldAlias(this.props.valueQuery, 'data') - variables = { id: this.props.value[0] } - this.props.value.slice(1).forEach((val, idx) => { - const varName = 'id' + idx - - query = mergeFields( - query, - mapInputVars(fieldAlias(this.props.valueQuery, 'data' + idx), { - id: varName, - }), - ) - - variables[varName] = val - }) - getOptions = data => - data && data.data ? Object.values(data).map(this.props.mapDataNode) : [] - } else { - query = fieldAlias(this.props.valueQuery, 'data') - variables = { id: this.props.value } - getOptions = data => - data && data.data ? [this.props.mapDataNode(data.data)] : [] + return q + }) + return function useValuesQuery(_value) { + const value = asArray(_value) + const variables = {} + value.forEach((v, i) => { + variables['id' + i] = v + }) + + const { data, error } = useQuery(getQueryBySize(value.length), { + skip: !value.length, + variables, + returnPartialData: true, + fetchPolicy: 'cache-first', + pollInterval: 0, + }) + + if (!value.length) { + return [null, error] } - return ( - this.renderOptionsQuery(getOptions(data))} - /> - ) - } + const result = value.map((v, i) => { + const name = 'data' + i + if (!data || !data[name]) return { value: v, label: 'Loading...' } - renderOptionsQuery(valueOptions) { - const getOptions = data => - data && data.data && data.data.nodes - ? data.data.nodes.map(this.props.mapDataNode) - : [] + return mapNode(data[name]) + }) - const skip = Boolean( - (this.state.skip && this.state.search) || - (!this.state.search && !this.props.defaultQueryVariables), - ) - const omitOpts = {} - if (this.props.multiple) { - omitOpts.omit = this.props.value + if (Array.isArray(_value)) { + return [result, error] } + return [result[0], error] + } +} - let variables - if (!this.state.search && this.props.defaultQueryVariables) { - variables = this.props.defaultQueryVariables - } else { - variables = { - ...this.props.variables, - input: { - first: 5, - search: this.state.search, - ...omitOpts, - ...this.props.variables.input, - }, - } +// makeUseOptions will provide the available options for the given query. +function makeUseOptions(query, mapNode, vars, defaultVars) { + const q = fieldAlias(query, 'data') + return function useOptions(value, search) { + const params = { first: 5, omit: asArray(value) } + const input = search + ? { ...vars, ...params, search } + : { ...defaultVars, ...params } + + const { data, loading, error } = useQuery(q, { + skip: !search && !defaultVars, + variables: { input }, + fetchPolicy: 'network-only', + pollInterval: 0, + }) + + let result = [] + + if (!loading && data && data.data) { + result = data.data.nodes.map(mapNode) } - return ( - - this.renderSelect(valueOptions, { - options: getOptions(data), - loading, - error, - }) - } - /> - ) + return [result, { loading, error }] } +} + +export const querySelectPropTypes = { + // If specified, a "Create" option will be provided for the users + // provided text. It will be called instead of `onChange` if the user + // selects the generated option. + // + // Example: if the user types `foobar` and there is not a `foobar` option, + // then `Create "foobar"` will be displayed in the dropdown. + onCreate: p.func, + + error: p.bool, + onChange: p.func, + value: valueCheck, + + multiple: p.bool, + name: p.string, + placeholder: p.string, +} - renderSelect(valueOptions, { options, loading, error }) { - // keep unused variables so they are not used in rest spread +export function makeQuerySelect(displayName, options) { + const { + mapDataNode = defaultMapNode, + variables = {}, + query, + valueQuery, + defaultQueryVariables, + } = options + + const useValues = makeUseValues(valueQuery, mapDataNode) + const useOptions = makeUseOptions( + query, + mapDataNode, + variables, + defaultQueryVariables, + ) + + function QuerySelect(props) { const { - onCreate, - query, - valueQuery, - mapDataNode, - onChange, - value, - search, - variables, - defaultQueryVariables, + value = props.multiple ? [] : null, + multiple = false, + placeholder, - ...rest - } = this.props - let noOptionsMessage = 'No options' - if (this.state.skip) noOptionsMessage = 'Start typing...' - else if (error) noOptionsMessage = 'Error: ' + error - - const selectOptions = - this.state.search || defaultQueryVariables - ? options.concat(valueOptions) - : valueOptions - let selectValue, changeCallback - - const mapVal = val => - selectOptions.find(opt => opt.value === val) || { - value: val, - label: 'Loading...', - } - - if (this.props.multiple) { - changeCallback = val => { - const created = val.find(v => v.isCreate) - if (created) { - onCreate(created.value) - return - } - onChange(val ? val.map(v => v.value) : []) - } - selectValue = value.map(mapVal) - } else { - changeCallback = val => { - if (val && val.isCreate) { - onCreate(val.value) - return - } - onChange((val && val.value) || '') - } - if (value) selectValue = mapVal(value) + onCreate = () => {}, + onChange = () => {}, + ...otherProps + } = props + + const [search, setSearch] = useState('') + const [searchInput, setSearchInput] = useState('') + const [renderCheck, setRenderCheck] = useState(0) + const [optionCache] = useState({}) + const [selectValue] = useValues(value) + const [ + selectOptions, + { loading: optionsLoading, error: optionsError }, + ] = useOptions(value, search) + + useEffect(() => { + const t = setTimeout(() => setRenderCheck(renderCheck + 1), 1000) + return () => clearTimeout(t) + }) + useEffect(() => { + const t = setTimeout(() => setSearch(searchInput), DEBOUNCE_DELAY) + + return () => clearTimeout(t) + }, [searchInput]) + + const cachify = option => { + const key = JSON.stringify(omit(option, 'icon')) + if (!optionCache[key]) optionCache[key] = option + return optionCache[key] } - if ( - onCreate && - this.state.search && - !selectOptions.find(o => o.value === this.state.search) - ) { - selectOptions.push({ - isCreate: true, - value: this.state.search, - label: `Create "${this.state.search}"`, - }) + const handleChange = newVal => { + setSearch('') + setSearchInput('') + const created = asArray(newVal).find(v => v.isCreate) + if (created) onCreate(created.value) + else if (multiple) onChange(asArray(newVal).map(v => v.value)) + else onChange(newVal.value) } + let noOptionsMessage = 'No options' + if (!searchInput && !selectOptions.length) + noOptionsMessage = 'Start typing...' + else if (optionsError) noOptionsMessage = 'Error: ' + optionsError + return ( noOptionsMessage} - onInputChange={val => { - this.setState({ outdated: true }) - this.onInputChange(val) - }} - value={selectValue} - options={ - (this.state.outdated && []) || - selectOptions.map(opt => ({ + onInputChange={val => setSearchInput(val)} + value={multiple ? asArray(selectValue) : selectValue} + multiple={multiple} + options={selectOptions + .map(opt => ({ ...opt, icon: opt.isFavorite ? : null, })) - } + .map(cachify)} placeholder={ placeholder || - (defaultQueryVariables && !search ? 'Start typing...' : null) + (defaultQueryVariables && !searchInput ? 'Start typing...' : null) } - onChange={val => { - this.setState({ search: '', outdated: false }) - - // ignore any pending search updates - this.onInputChange.cancel() - changeCallback(val) - }} - {...rest} + onChange={val => handleChange(val)} + {...otherProps} /> ) } + + QuerySelect.displayName = displayName + QuerySelect.propTypes = querySelectPropTypes + + return QuerySelect } diff --git a/web/src/app/selection/RotationSelect.js b/web/src/app/selection/RotationSelect.js index e716af3751..aee1439594 100644 --- a/web/src/app/selection/RotationSelect.js +++ b/web/src/app/selection/RotationSelect.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: RotationSearchOptions) { @@ -24,16 +22,9 @@ const valueQuery = gql` } } ` -export class RotationSelect extends React.PureComponent { - render() { - return ( - - ) - } -} +export const RotationSelect = makeQuerySelect('RotationSelect', { + variables: { favoritesFirst: true }, + defaultQueryVariables: { favoritesOnly: true }, + query, + valueQuery, +}) diff --git a/web/src/app/selection/ScheduleSelect.js b/web/src/app/selection/ScheduleSelect.js index 7939cd564d..5f0abf9c4e 100644 --- a/web/src/app/selection/ScheduleSelect.js +++ b/web/src/app/selection/ScheduleSelect.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: ScheduleSearchOptions) { @@ -24,16 +22,10 @@ const valueQuery = gql` } } ` -export class ScheduleSelect extends React.PureComponent { - render() { - return ( - - ) - } -} + +export const ScheduleSelect = makeQuerySelect('ScheduleSelect', { + variables: { favoritesFirst: true }, + defaultQueryVariables: { favoritesOnly: true }, + query, + valueQuery, +}) diff --git a/web/src/app/selection/ServiceSelect.js b/web/src/app/selection/ServiceSelect.js index 9ee9275fdf..8981cc6347 100644 --- a/web/src/app/selection/ServiceSelect.js +++ b/web/src/app/selection/ServiceSelect.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: ServiceSearchOptions) { @@ -24,16 +22,9 @@ const valueQuery = gql` } } ` -export class ServiceSelect extends React.PureComponent { - render() { - return ( - - ) - } -} +export const ServiceSelect = makeQuerySelect('ServiceSelect', { + variables: { favoritesFirst: true }, + defaultQueryVariables: { favoritesOnly: true }, + query, + valueQuery, +}) diff --git a/web/src/app/selection/SlackChannelSelect.js b/web/src/app/selection/SlackChannelSelect.js index de2aa22d21..6d05b8ee38 100644 --- a/web/src/app/selection/SlackChannelSelect.js +++ b/web/src/app/selection/SlackChannelSelect.js @@ -1,8 +1,5 @@ -import React from 'react' import gql from 'graphql-tag' -import QuerySelect from './QuerySelect' -import withStyles from '@material-ui/core/styles/withStyles' -import except from 'except' +import { makeQuerySelect } from './QuerySelect' const query = gql` query($input: SlackChannelSearchOptions) { @@ -24,27 +21,7 @@ const valueQuery = gql` } ` -const styles = { - slackButton: { - textTransform: 'none', - backgroundColor: 'white', - border: '1px solid', - borderColor: 'lightgrey', - width: 'fit-content', - }, - slackIcon: { - marginRight: '0.5em', - }, -} - -@withStyles(styles) -export class SlackChannelSelect extends React.PureComponent { - render = () => ( - - ) -} +export const SlackChannelSelect = makeQuerySelect('SlackChannelSelect', { + query, + valueQuery, +}) diff --git a/web/src/app/selection/TimeZoneSelect.js b/web/src/app/selection/TimeZoneSelect.js index 6e3dd2dace..e6186b9619 100644 --- a/web/src/app/selection/TimeZoneSelect.js +++ b/web/src/app/selection/TimeZoneSelect.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: TimeZoneSearchOptions) { @@ -13,14 +11,7 @@ const query = gql` } ` -export class TimeZoneSelect extends React.PureComponent { - render() { - return ( - ({ label: n.id, value: n.id })} - /> - ) - } -} +export const TimeZoneSelect = makeQuerySelect('TimeZoneSelect', { + query, + mapDataNode: ({ id }) => ({ label: id, value: id }), +}) diff --git a/web/src/app/selection/UserSelect.js b/web/src/app/selection/UserSelect.js index 5cbcf61d5f..bd6fa1fbe6 100644 --- a/web/src/app/selection/UserSelect.js +++ b/web/src/app/selection/UserSelect.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: UserSearchOptions) { @@ -22,8 +20,8 @@ const valueQuery = gql` } } ` -export class UserSelect extends React.PureComponent { - render() { - return - } -} + +export const UserSelect = makeQuerySelect('UserSelect', { + query, + valueQuery, +}) From b4bba6ac5a525e74fa702333c9fd9fa2dbb1f6dd Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 14:58:12 -0500 Subject: [PATCH 05/16] fix alerts list lint errors - jsx-a11y/click-events-have-key-events - jsx-a11y/no-static-element-interactions --- .../components/AlertsListDataWrapper.js | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/web/src/app/alerts/components/AlertsListDataWrapper.js b/web/src/app/alerts/components/AlertsListDataWrapper.js index ca3dc83194..5d046044a8 100644 --- 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 = ( this.toggleChecked(alert.number)} + onClick={e => e.stopPropagation()} /> ) @@ -131,34 +131,35 @@ export default class AlertsListDataWrapper extends Component { } return ( - + {checkbox} -
history.push(`/alerts/${alert.number}`)} - > - - - {alert.number}: - {alert.status.toUpperCase()} - - {onServicePage ? null : ( - {alert.service.name} - )} - - {alert.summary} + + + + {alert.number}: + {alert.status.toUpperCase()} + + {onServicePage ? null : ( + {alert.service.name} + )} + + {alert.summary} + + + + + + {moment(alert.created_at) + .local() + .fromNow()} - - - - {moment(alert.created_at) - .local() - .fromNow()} - - - -
+ ) } From 996bde8577ace0938417c68c75b1213c159633f7 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 15:14:00 -0500 Subject: [PATCH 06/16] fix alerts tests --- web/src/cypress/integration/alerts.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/web/src/cypress/integration/alerts.ts b/web/src/cypress/integration/alerts.ts index f692606fb6..49a1c31cc7 100644 --- a/web/src/cypress/integration/alerts.ts +++ b/web/src/cypress/integration/alerts.ts @@ -23,10 +23,7 @@ function testAlerts(screen: ScreenFormat) { .should('contain', alert.summary) .should('contain', alert.number) .should('contain', alert.service.name) - cy.get('ul[data-cy=alerts-list] div[role=button]').should( - 'have.length', - 1, - ) + cy.get('ul[data-cy=alerts-list] li').should('have.length', 1) }) it('should handle searching by summary', () => { // by summary @@ -35,10 +32,7 @@ function testAlerts(screen: ScreenFormat) { .should('contain', alert.summary) .should('contain', alert.number) .should('contain', alert.service.name) - cy.get('ul[data-cy=alerts-list] div[role=button]').should( - 'have.length', - 1, - ) + cy.get('ul[data-cy=alerts-list] li').should('have.length', 1) }) it('should handle searching by service name', () => { @@ -48,10 +42,7 @@ function testAlerts(screen: ScreenFormat) { .should('contain', alert.summary) .should('contain', alert.number) .should('contain', alert.service.name) - cy.get('ul[data-cy=alerts-list] div[role=button]').should( - 'have.length', - 1, - ) + cy.get('ul[data-cy=alerts-list] li').should('have.length', 1) }) it('should handle toggling show by favorites filter', () => { From 26ff0689d47e420d83ed1c32e81400a26782f6a4 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 15:25:52 -0500 Subject: [PATCH 07/16] add create option if onCreate is set --- web/src/app/selection/QuerySelect.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/web/src/app/selection/QuerySelect.js b/web/src/app/selection/QuerySelect.js index 118f0bdf15..35bd6f59da 100644 --- a/web/src/app/selection/QuerySelect.js +++ b/web/src/app/selection/QuerySelect.js @@ -151,11 +151,13 @@ export function makeQuerySelect(displayName, options) { placeholder, - onCreate = () => {}, + onCreate: _onCreate, onChange = () => {}, ...otherProps } = props + const onCreate = _onCreate || (() => {}) + const [search, setSearch] = useState('') const [searchInput, setSearchInput] = useState('') const [renderCheck, setRenderCheck] = useState(0) @@ -166,6 +168,18 @@ export function makeQuerySelect(displayName, options) { { loading: optionsLoading, error: optionsError }, ] = useOptions(value, search) + if ( + _onCreate && + searchInput && + !selectOptions.some(o => o.value === searchInput) + ) { + selectOptions.push({ + isCreate: true, + value: searchInput, + label: `Create "${searchInput}"`, + }) + } + useEffect(() => { const t = setTimeout(() => setRenderCheck(renderCheck + 1), 1000) return () => clearTimeout(t) From eaa4f2cddeb974f0b065085ea0f0858805196006 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 30 Aug 2019 15:36:04 -0500 Subject: [PATCH 08/16] add code comments --- web/src/app/selection/QuerySelect.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/web/src/app/selection/QuerySelect.js b/web/src/app/selection/QuerySelect.js index 35bd6f59da..d79412632d 100644 --- a/web/src/app/selection/QuerySelect.js +++ b/web/src/app/selection/QuerySelect.js @@ -25,13 +25,12 @@ const asArray = value => { return Array.isArray(value) ? value : [value] } + const mapValueQuery = (query, index) => mapInputVars(fieldAlias(query, 'data' + index), { id: 'id' + index }) -// useValues will return a set of {id, label} -// for the provided value. -// -// If value is not an array, a single object (instead of array) -// is returned. + +// makeUseValues will return a hook that will fetch select values for the +// given ids. function makeUseValues(query, mapNode) { if (!query) { // no value query, so always use the map function @@ -83,7 +82,8 @@ function makeUseValues(query, mapNode) { } } -// makeUseOptions will provide the available options for the given query. +// makeUseOptions will return a hook that will fetch available options +// for a given search query. function makeUseOptions(query, mapNode, vars, defaultVars) { const q = fieldAlias(query, 'data') return function useOptions(value, search) { @@ -127,12 +127,26 @@ export const querySelectPropTypes = { placeholder: p.string, } +// makeQuerySelect will return a new React component that can be used +// as a select input with connected search. export function makeQuerySelect(displayName, options) { const { + // mapDataNode can be passed to change how server-returned values are mapped to select options mapDataNode = defaultMapNode, + + // variables will be mixed into the query parameters variables = {}, + + // query is used to fetch available options. It should define + // an `input` variable that takes `omit`, `first`, and `search` properties. query, + + // valueQuery is optional and used to fetch labels for selected ids. valueQuery, + + // defaultQueryVariables, if specified, will be used when the component is active + // but has no search parameter. It is useful for showing favorites before the user + // enters a search term. defaultQueryVariables, } = options From 7abcf039038515ad81fc4551b2a2910e7f63684e Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 14:22:26 -0500 Subject: [PATCH 09/16] allow clearing single selects --- web/src/app/selection/QuerySelect.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/web/src/app/selection/QuerySelect.js b/web/src/app/selection/QuerySelect.js index d79412632d..71f62a7c07 100644 --- a/web/src/app/selection/QuerySelect.js +++ b/web/src/app/selection/QuerySelect.js @@ -174,7 +174,6 @@ export function makeQuerySelect(displayName, options) { const [search, setSearch] = useState('') const [searchInput, setSearchInput] = useState('') - const [renderCheck, setRenderCheck] = useState(0) const [optionCache] = useState({}) const [selectValue] = useValues(value) const [ @@ -194,10 +193,6 @@ export function makeQuerySelect(displayName, options) { }) } - useEffect(() => { - const t = setTimeout(() => setRenderCheck(renderCheck + 1), 1000) - return () => clearTimeout(t) - }) useEffect(() => { const t = setTimeout(() => setSearch(searchInput), DEBOUNCE_DELAY) @@ -216,7 +211,7 @@ export function makeQuerySelect(displayName, options) { const created = asArray(newVal).find(v => v.isCreate) if (created) onCreate(created.value) else if (multiple) onChange(asArray(newVal).map(v => v.value)) - else onChange(newVal.value) + else onChange((newVal && newVal.value) || null) } let noOptionsMessage = 'No options' From 2f412122066262ae5dee9d34f9a68fc3106e12d8 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 14:56:18 -0500 Subject: [PATCH 10/16] fix service search query --- service/search.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/service/search.go b/service/search.go index eca9ae3d36..d31e86df7a 100644 --- 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 ( @@ -182,6 +183,7 @@ func (db *DB) Search(ctx context.Context, opts *SearchOptions) ([]Service, error if opts == nil { opts = &SearchOptions{} } + return nil, errors.New("who knew?") userCheck := permission.User if opts.FavoritesUserID != "" { From 3d1bbe4c04bf8f7d01004f5308eafbcba006a485 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 14:56:29 -0500 Subject: [PATCH 11/16] set default error policy --- web/src/app/apollo.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/src/app/apollo.js b/web/src/app/apollo.js index 9573a84b3b..caf245be28 100644 --- 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 } From 910c124489a3d32eca6bb01ff0963f9a7ecfad99 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 14:56:59 -0500 Subject: [PATCH 12/16] display error in search select if things break --- web/src/app/selection/QuerySelect.js | 36 +++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/web/src/app/selection/QuerySelect.js b/web/src/app/selection/QuerySelect.js index 71f62a7c07..7cbb33b108 100644 --- a/web/src/app/selection/QuerySelect.js +++ b/web/src/app/selection/QuerySelect.js @@ -6,6 +6,32 @@ import { mergeFields, fieldAlias, mapInputVars } from '../util/graphql' import FavoriteIcon from '@material-ui/icons/Star' import { DEBOUNCE_DELAY } from '../config' import { useQuery } from 'react-apollo' +import { Typography, makeStyles } from '@material-ui/core' +import { Error } from '@material-ui/icons' +import { styles } from '../styles/materialStyles' + +const useStyles = makeStyles(theme => { + return { + error: styles(theme).error, + } +}) + +function ErrorMessage({ value }) { + const classes = useStyles() + return ( + + + +   + {value} + + + ) +} // valueCheck ensures the type is `arrayOf(p.string)` if `multiple` is set // and `p.string` otherwise. @@ -97,10 +123,14 @@ function makeUseOptions(query, mapNode, vars, defaultVars) { variables: { input }, fetchPolicy: 'network-only', pollInterval: 0, + errorPolicy: 'all', // needs to be set explicitly for some reason }) let result = [] + if (error) { + return [[], { error }] + } if (!loading && data && data.data) { result = data.data.nodes.map(mapNode) } @@ -215,7 +245,11 @@ export function makeQuerySelect(displayName, options) { } let noOptionsMessage = 'No options' - if (!searchInput && !selectOptions.length) + if (optionsError) { + noOptionsMessage = ( + + ) + } else if (!searchInput && !selectOptions.length) noOptionsMessage = 'Start typing...' else if (optionsError) noOptionsMessage = 'Error: ' + optionsError From 450c9c382df627dc44599e42517bd7f247421bee Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 14:57:47 -0500 Subject: [PATCH 13/16] remove debug code --- service/search.go | 1 - 1 file changed, 1 deletion(-) diff --git a/service/search.go b/service/search.go index d31e86df7a..38aa7d79bb 100644 --- a/service/search.go +++ b/service/search.go @@ -183,7 +183,6 @@ func (db *DB) Search(ctx context.Context, opts *SearchOptions) ([]Service, error if opts == nil { opts = &SearchOptions{} } - return nil, errors.New("who knew?") userCheck := permission.User if opts.FavoritesUserID != "" { From f0358eb63e92f014047b2d2cda68dbc7c53f622b Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 15:00:47 -0500 Subject: [PATCH 14/16] fix timezone search --- timezone/search.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/timezone/search.go b/timezone/search.go index 4334df7580..0ef0f47a86 100644 --- 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)), } } From a891e1ef8ca6771aa7771ae9243b63625b53735b Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 15:09:40 -0500 Subject: [PATCH 15/16] don't omit options in single-select mode --- web/src/app/selection/QuerySelect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/app/selection/QuerySelect.js b/web/src/app/selection/QuerySelect.js index 7cbb33b108..d1f994b390 100644 --- a/web/src/app/selection/QuerySelect.js +++ b/web/src/app/selection/QuerySelect.js @@ -113,7 +113,7 @@ function makeUseValues(query, mapNode) { function makeUseOptions(query, mapNode, vars, defaultVars) { const q = fieldAlias(query, 'data') return function useOptions(value, search) { - const params = { first: 5, omit: asArray(value) } + const params = { first: 5, omit: Array.isArray(value) ? value : [] } // only omit in multi-select mode const input = search ? { ...vars, ...params, search } : { ...defaultVars, ...params } From f891e7dce54487b9fb36204d7d8a432180d0bf69 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 12 Sep 2019 16:07:22 -0500 Subject: [PATCH 16/16] convert ScheduleTZFilter to hooks (fixes jump) --- web/src/app/schedules/ScheduleTZFilter.js | 91 +++++++++++------------ 1 file changed, 44 insertions(+), 47 deletions(-) diff --git a/web/src/app/schedules/ScheduleTZFilter.js b/web/src/app/schedules/ScheduleTZFilter.js index ac64419bb6..62e1639ab9 100644 --- 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 ( - this.renderControl(data.schedule.timeZone)} - /> - ) - } + return ( + 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 ( - 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, }