From 458ea11fce2d496fd7a0fb90c448b16b21e6473f Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 4 Apr 2024 09:37:33 -0500 Subject: [PATCH 1/4] switch to urql --- .../escalation-policies/PolicyStepsCard.tsx | 136 ++++++------------ .../escalation-policies/PolicyStepsQuery.tsx | 16 +-- web/src/app/escalation-policies/stepUtil.tsx | 10 +- web/src/app/rotations/util.tsx | 6 +- 4 files changed, 57 insertions(+), 111 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepsCard.tsx b/web/src/app/escalation-policies/PolicyStepsCard.tsx index 32c33c6f7f..f93c34d877 100644 --- a/web/src/app/escalation-policies/PolicyStepsCard.tsx +++ b/web/src/app/escalation-policies/PolicyStepsCard.tsx @@ -1,11 +1,11 @@ -import React, { Suspense, useRef, useState } from 'react' +import React, { Suspense, useEffect, useState } from 'react' import Button from '@mui/material/Button' import Card from '@mui/material/Card' import CardHeader from '@mui/material/CardHeader' import Dialog from '@mui/material/Dialog' import Typography from '@mui/material/Typography' import { Add } from '@mui/icons-material' -import { gql, useMutation } from '@apollo/client' +import { gql, useMutation } from 'urql' import FlatList from '../lists/FlatList' import CreateFAB from '../lists/CreateFAB' import PolicyStepCreateDialog from './PolicyStepCreateDialog' @@ -13,21 +13,15 @@ import PolicyStepCreateDialogDest from './PolicyStepCreateDialogDest' import { useResetURLParams, useURLParam } from '../actions' import DialogTitleWrapper from '../dialogs/components/DialogTitleWrapper' import DialogContentError from '../dialogs/components/DialogContentError' -import { policyStepsQuery } from './PolicyStepsQuery' import { useIsWidthDown } from '../util/useWidth' import { reorderList } from '../rotations/util' import PolicyStepEditDialog from './PolicyStepEditDialog' import PolicyStepDeleteDialog from './PolicyStepDeleteDialog' import PolicyStepEditDialogDest from './PolicyStepEditDialogDest' import OtherActions from '../util/OtherActions' -import { - getStepNumber, - renderChips, - renderChipsDest, - renderDelayMessage, -} from './stepUtil' +import { renderChips, renderChipsDest, renderDelayMessage } from './stepUtil' import { useExpFlag } from '../util/useExpFlag' -import { Destination, EscalationPolicy, Target } from '../../schema' +import { Destination, Target } from '../../schema' const mutation = gql` mutation UpdateEscalationPolicyMutation( @@ -54,104 +48,61 @@ export default function PolicyStepsCard( ): React.ReactNode { const hasDestTypesFlag = useExpFlag('dest-types') - const { escalationPolicyID, repeat, steps = [] } = props - const isMobile = useIsWidthDown('md') const stepNumParam = 'createStep' const [createStep, setCreateStep] = useURLParam(stepNumParam, false) const resetCreateStep = useResetURLParams(stepNumParam) - const oldID = useRef(null) - const oldIdx = useRef(null) - const newIdx = useRef(null) + const [stepIDs, setStepIDs] = useState(props.steps.map((s) => s.id)) - type Swap = { oldIndex: number; newIndex: number } - const [lastSwap, setLastSwap] = useState>([]) + useEffect(() => { + setStepIDs(props.steps.map((s) => s.id)) + }, [props.steps.map((s) => s.id).join(',')]) // update steps when order changes - const [error, setError] = useState(null) + const orderedSteps = stepIDs.map((id) => { + const step = props.steps.find((s) => s.id === id) + if (!step) throw new Error('Step not found') // should be impossible + return step + }) const [editStepID, setEditStepID] = useURLParam('editStep', '') - const editStep = steps.find((step) => step.id === editStepID) + const editStep = props.steps.find((step) => step.id === editStepID) const resetEditStep = useResetURLParams('editStep') const [deleteStep, setDeleteStep] = useState('') - const [updateEscalationPolicy] = useMutation(mutation, { - onCompleted: () => { - oldID.current = null - oldIdx.current = null - newIdx.current = null - }, - onError: (err) => setError(err), - }) + const [updateError, setUpdateError] = useState(null) + const [, commit] = useMutation(mutation) async function onReorder( oldIndex: number, newIndex: number, ): Promise { - setLastSwap(lastSwap.concat({ oldIndex, newIndex })) + const newStepIDs = reorderList(stepIDs, oldIndex, newIndex) + setStepIDs(newStepIDs) - const updatedStepIDs = reorderList( - steps.map((step) => step.id), - oldIndex, - newIndex, - ) - - return updateEscalationPolicy({ - variables: { + return commit( + { input: { - id: escalationPolicyID, - stepIDs: updatedStepIDs, + id: props.escalationPolicyID, + stepIDs: newStepIDs, }, }, - update: (cache, { data }) => { - // mutation returns true on a success - if (!data.updateEscalationPolicy) { - return - } - - // get the current state of the steps in the cache - const cacheData = cache.readQuery<{ - escalationPolicy: EscalationPolicy - }>({ - query: policyStepsQuery, - variables: { id: escalationPolicyID }, - }) - if (!cacheData) throw new Error('Cache data not found') - const escalationPolicy = cacheData.escalationPolicy - const steps = escalationPolicy.steps.slice() - - if (steps.length > 0) { - const newSteps = reorderList(steps, oldIndex, newIndex) - - // write new steps order to cache - cache.writeQuery({ - query: policyStepsQuery, - variables: { id: escalationPolicyID }, - data: { - escalationPolicy: { - ...escalationPolicy, - steps: newSteps, - }, - }, - }) - } - }, - optimisticResponse: { - __typename: 'Mutation', - updateEscalationPolicy: true, - }, + { additionalTypenames: ['EscalationPolicy'] }, + ).catch((err) => { + setUpdateError(err) + setStepIDs(props.steps.map((s) => s.id)) }) } function renderRepeatText(): React.ReactNode { - if (!steps.length) { + if (!stepIDs.length) { return null } let text = '' - if (repeat === 0) text = 'Do not repeat' - else if (repeat === 1) text = 'Repeat once' - else text = `Repeat ${repeat} times` + if (props.repeat === 0) text = 'Do not repeat' + else if (props.repeat === 1) text = 'Repeat once' + else text = `Repeat ${props.repeat} times` return ( @@ -160,8 +111,6 @@ export default function PolicyStepsCard( ) } - const { message: errMsg } = error || {} - return ( {isMobile && ( @@ -171,12 +120,12 @@ export default function PolicyStepsCard( {hasDestTypesFlag ? ( ) : ( )} @@ -203,12 +152,12 @@ export default function PolicyStepsCard( data-cy='steps-list' emptyMessage='No steps currently on this Escalation Policy' headerNote='Notify the following:' - items={steps.map((step) => ({ + items={orderedSteps.map((step, idx) => ({ id: step.id, disableTypography: true, title: ( - Step #{getStepNumber(step.id, steps)}: + Step #{idx + 1}: ) as unknown as string, // needed to work around MUI incorrect types subText: ( @@ -216,7 +165,12 @@ export default function PolicyStepsCard( {step.actions ? renderChipsDest(step.actions) : renderChips(step)} - {renderDelayMessage(steps, step, repeat)} + {renderDelayMessage( + step, + idx, + props.repeat, + idx === orderedSteps.length - 1, + )} ), secondaryAction: ( @@ -238,25 +192,25 @@ export default function PolicyStepsCard( /> {renderRepeatText()} - setError(null)}> + setUpdateError(null)}> - + {editStep && ( {hasDestTypesFlag ? ( ) : ( @@ -265,7 +219,7 @@ export default function PolicyStepsCard( )} {deleteStep && ( setDeleteStep('')} stepID={deleteStep} /> diff --git a/web/src/app/escalation-policies/PolicyStepsQuery.tsx b/web/src/app/escalation-policies/PolicyStepsQuery.tsx index 02c483c5ad..a6f47ab978 100644 --- a/web/src/app/escalation-policies/PolicyStepsQuery.tsx +++ b/web/src/app/escalation-policies/PolicyStepsQuery.tsx @@ -1,7 +1,6 @@ import React from 'react' -import { gql, useQuery } from '@apollo/client' +import { gql, useQuery } from 'urql' import PolicyStepsCard from './PolicyStepsCard' -import Spinner from '../loading/components/Spinner' import { GenericError, ObjectNotFound } from '../error-pages' import { useExpFlag } from '../util/useExpFlag' @@ -60,16 +59,11 @@ export const policyStepsQueryDest = gql` function PolicyStepsQuery(props: { escalationPolicyID: string }): JSX.Element { const hasDestTypesFlag = useExpFlag('dest-types') - const { data, loading, error } = useQuery( - hasDestTypesFlag ? policyStepsQueryDest : policyStepsQuery, - { - variables: { id: props.escalationPolicyID }, - }, - ) + const [{ data, error }] = useQuery({ + query: hasDestTypesFlag ? policyStepsQueryDest : policyStepsQuery, + variables: { id: props.escalationPolicyID }, + }) - if (!data && loading) { - return - } if (error) { return } diff --git a/web/src/app/escalation-policies/stepUtil.tsx b/web/src/app/escalation-policies/stepUtil.tsx index c17f82f92f..8cfa3dca04 100644 --- a/web/src/app/escalation-policies/stepUtil.tsx +++ b/web/src/app/escalation-policies/stepUtil.tsx @@ -106,13 +106,11 @@ export function renderChips({ targets: _t }: Step): ReactElement { * repeats, and if the message is rendering on the last step */ export function renderDelayMessage( - steps: Step[], step: Step, + idx: number, repeat: number, + isLastStep: boolean, ): ReactNode { - const len = steps.length - const isLastStep = getStepNumber(step.id, steps) === len - // if it's the last step and should not repeat, do not render end text if (isLastStep && repeat === 0) { return null @@ -121,10 +119,10 @@ export function renderDelayMessage( const pluralizer = (x: number): string => (x === 1 ? '' : 's') let repeatText = `Move on to step #${ - getStepNumber(step.id, steps) + 1 + idx + 1 } after ${step.delayMinutes} minute${pluralizer(step.delayMinutes)}` - if (isLastStep && getStepNumber(step.id, steps) === 1) { + if (isLastStep && idx === 0) { repeatText = `Repeat after ${step.delayMinutes} minutes` } diff --git a/web/src/app/rotations/util.tsx b/web/src/app/rotations/util.tsx index 00056c47bf..cce40b20a6 100644 --- a/web/src/app/rotations/util.tsx +++ b/web/src/app/rotations/util.tsx @@ -24,11 +24,11 @@ export function calcNewActiveIndex( // reorderList will move an item from the oldIndex to the newIndex, preserving order // returning the result as a new array. -export function reorderList( - _items: unknown[], +export function reorderList( + _items: T[], oldIndex: number, newIndex: number, -): unknown[] { +): T[] { const items = _items.slice() items.splice(oldIndex, 1) // remove 1 element from oldIndex position items.splice(newIndex, 0, _items[oldIndex]) // add dest to newIndex position From bc7973595455c7cce3806c4221e86160138ee4da Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 4 Apr 2024 09:48:18 -0500 Subject: [PATCH 2/4] fix err reporting --- .../app/escalation-policies/PolicyStepsCard.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepsCard.tsx b/web/src/app/escalation-policies/PolicyStepsCard.tsx index f93c34d877..1e3c9d76c0 100644 --- a/web/src/app/escalation-policies/PolicyStepsCard.tsx +++ b/web/src/app/escalation-policies/PolicyStepsCard.tsx @@ -71,7 +71,14 @@ export default function PolicyStepsCard( const [deleteStep, setDeleteStep] = useState('') const [updateError, setUpdateError] = useState(null) - const [, commit] = useMutation(mutation) + const [status, commit] = useMutation(mutation) + + useEffect(() => { + if (status.error) { + setUpdateError(status.error) + setStepIDs(props.steps.map((s) => s.id)) + } + }, [status.error]) async function onReorder( oldIndex: number, @@ -88,10 +95,7 @@ export default function PolicyStepsCard( }, }, { additionalTypenames: ['EscalationPolicy'] }, - ).catch((err) => { - setUpdateError(err) - setStepIDs(props.steps.map((s) => s.id)) - }) + ) } function renderRepeatText(): React.ReactNode { From 9bac64f6d872018385c65fce03ad6fcf9669ba61 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 4 Apr 2024 10:23:00 -0500 Subject: [PATCH 3/4] set additional typename --- .../PolicyStepCreateDialog.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepCreateDialog.tsx b/web/src/app/escalation-policies/PolicyStepCreateDialog.tsx index f45581d1ab..2d1732e281 100644 --- a/web/src/app/escalation-policies/PolicyStepCreateDialog.tsx +++ b/web/src/app/escalation-policies/PolicyStepCreateDialog.tsx @@ -50,15 +50,18 @@ function PolicyStepCreateDialog(props: { maxWidth='sm' onClose={props.onClose} onSubmit={() => - createStep({ - input: { - escalationPolicyID: props.escalationPolicyID, - delayMinutes: parseInt( - (value && value.delayMinutes) || defaultValue.delayMinutes, - ), - targets: (value && value.targets) || defaultValue.targets, + createStep( + { + input: { + escalationPolicyID: props.escalationPolicyID, + delayMinutes: parseInt( + (value && value.delayMinutes) || defaultValue.delayMinutes, + ), + targets: (value && value.targets) || defaultValue.targets, + }, }, - }).then((result) => { + { additionalTypenames: ['EscalationPolicy'] }, + ).then((result) => { if (!result.error) { props.onClose() } From 2288f07eeba009a8028c19521b110f98c24ccd81 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 4 Apr 2024 10:26:58 -0500 Subject: [PATCH 4/4] handle deleted step --- .../escalation-policies/PolicyStepsCard.tsx | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepsCard.tsx b/web/src/app/escalation-policies/PolicyStepsCard.tsx index 1e3c9d76c0..d3a27d806b 100644 --- a/web/src/app/escalation-policies/PolicyStepsCard.tsx +++ b/web/src/app/escalation-policies/PolicyStepsCard.tsx @@ -31,16 +31,18 @@ const mutation = gql` } ` +type StepInfo = { + id: string + delayMinutes: number + stepNumber: number + actions?: Destination[] + targets: Target[] +} + export type PolicyStepsCardProps = { escalationPolicyID: string repeat: number - steps: Array<{ - id: string - delayMinutes: number - stepNumber: number - actions?: Destination[] - targets: Target[] - }> + steps: Array } export default function PolicyStepsCard( @@ -59,11 +61,9 @@ export default function PolicyStepsCard( setStepIDs(props.steps.map((s) => s.id)) }, [props.steps.map((s) => s.id).join(',')]) // update steps when order changes - const orderedSteps = stepIDs.map((id) => { - const step = props.steps.find((s) => s.id === id) - if (!step) throw new Error('Step not found') // should be impossible - return step - }) + const orderedSteps = stepIDs + .map((id) => props.steps.find((s) => s.id === id)) + .filter((s) => s) as StepInfo[] const [editStepID, setEditStepID] = useURLParam('editStep', '') const editStep = props.steps.find((step) => step.id === editStepID)