From 1bdff41273fd5632e7108200d16530b1c27a207f Mon Sep 17 00:00:00 2001 From: AllenDing Date: Mon, 4 Mar 2024 12:23:18 -0600 Subject: [PATCH 1/8] add backend support for dest types --- graphql2/generated.go | 15 +++++++- graphql2/graph/escalationpolicy.graphqls | 4 ++ graphql2/graphqlapp/contactmethod.go | 7 +--- graphql2/graphqlapp/destinationtypes.go | 48 ++++++++++++++++++++++++ graphql2/graphqlapp/escalationpolicy.go | 30 +++++++++++++++ graphql2/models_gen.go | 1 + 6 files changed, 98 insertions(+), 7 deletions(-) diff --git a/graphql2/generated.go b/graphql2/generated.go index 74078c5207..d952dc6df5 100644 --- a/graphql2/generated.go +++ b/graphql2/generated.go @@ -91,6 +91,7 @@ type ResolverRoot interface { UserOverride() UserOverrideResolver CreateEscalationPolicyStepInput() CreateEscalationPolicyStepInputResolver OnCallNotificationRuleInput() OnCallNotificationRuleInputResolver + UpdateEscalationPolicyStepInput() UpdateEscalationPolicyStepInputResolver } type DirectiveRoot struct { @@ -1016,6 +1017,9 @@ type CreateEscalationPolicyStepInputResolver interface { type OnCallNotificationRuleInputResolver interface { Dest(ctx context.Context, obj *OnCallNotificationRuleInput, data *DestinationInput) error } +type UpdateEscalationPolicyStepInputResolver interface { + Actions(ctx context.Context, obj *UpdateEscalationPolicyStepInput, data []DestinationInput) error +} type executableSchema struct { schema *ast.Schema @@ -33781,7 +33785,7 @@ func (ec *executionContext) unmarshalInputUpdateEscalationPolicyStepInput(ctx co asMap[k] = v } - fieldsInOrder := [...]string{"id", "delayMinutes", "targets"} + fieldsInOrder := [...]string{"id", "delayMinutes", "targets", "actions"} for _, k := range fieldsInOrder { v, ok := asMap[k] if !ok { @@ -33809,6 +33813,15 @@ func (ec *executionContext) unmarshalInputUpdateEscalationPolicyStepInput(ctx co return it, err } it.Targets = data + case "actions": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("actions")) + data, err := ec.unmarshalODestinationInput2ᚕgithubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐDestinationInputᚄ(ctx, v) + if err != nil { + return it, err + } + if err = ec.resolvers.UpdateEscalationPolicyStepInput().Actions(ctx, &it, data); err != nil { + return it, err + } } } diff --git a/graphql2/graph/escalationpolicy.graphqls b/graphql2/graph/escalationpolicy.graphqls index d5ca648b9c..561330fc1e 100644 --- a/graphql2/graph/escalationpolicy.graphqls +++ b/graphql2/graph/escalationpolicy.graphqls @@ -5,3 +5,7 @@ extend type EscalationPolicyStep { extend input CreateEscalationPolicyStepInput { actions: [DestinationInput!] @goField(forceResolver: true) # force resolver for initial compat } + +extend input UpdateEscalationPolicyStepInput { + actions: [DestinationInput!] @goField(forceResolver: true) # force resolver for initial compat +} \ No newline at end of file diff --git a/graphql2/graphqlapp/contactmethod.go b/graphql2/graphqlapp/contactmethod.go index 2ddc81141a..caa5bc30e3 100644 --- a/graphql2/graphqlapp/contactmethod.go +++ b/graphql2/graphqlapp/contactmethod.go @@ -139,13 +139,8 @@ func (m *Mutation) CreateUserContactMethod(ctx context.Context, input graphql2.C cfg := config.FromContext(ctx) if input.Dest != nil { - err := validate.IDName("input.name", input.Name) - if err != nil { - addInputError(ctx, err) - return nil, errAlreadySet - } if err := (*App)(m).ValidateDestination(ctx, "input.dest", input.Dest); err != nil { - return nil, err + return nil, errAlreadySet } t, v := CompatDestToCMTypeVal(*input.Dest) input.Type = &t diff --git a/graphql2/graphqlapp/destinationtypes.go b/graphql2/graphqlapp/destinationtypes.go index 9dc1da1ead..02c5eed64f 100644 --- a/graphql2/graphqlapp/destinationtypes.go +++ b/graphql2/graphqlapp/destinationtypes.go @@ -451,6 +451,54 @@ func (q *Query) DestinationTypes(ctx context.Context) ([]graphql2.DestinationTyp SupportsSearch: true, }}, }, + { + Type: destRotation, + Name: "Rotation", + Enabled: true, + IsContactMethod: false, + IsEPTarget: true, + IsSchedOnCallNotify: true, + SupportsStatusUpdates: false, + StatusUpdatesRequired: false, + RequiredFields: []graphql2.DestinationFieldConfig{{ + FieldID: fieldRotationID, + Label: "Rotation", + InputType: "text", + SupportsSearch: true, + }}, + }, + { + Type: destSchedule, + Name: "Schedule", + Enabled: true, + IsContactMethod: false, + IsEPTarget: true, + IsSchedOnCallNotify: true, + SupportsStatusUpdates: false, + StatusUpdatesRequired: false, + RequiredFields: []graphql2.DestinationFieldConfig{{ + FieldID: fieldScheduleID, + Label: "Schedule", + InputType: "text", + SupportsSearch: true, + }}, + }, + { + Type: destUser, + Name: "User", + Enabled: true, + IsContactMethod: false, + IsEPTarget: true, + IsSchedOnCallNotify: true, + SupportsStatusUpdates: false, + StatusUpdatesRequired: false, + RequiredFields: []graphql2.DestinationFieldConfig{{ + FieldID: fieldUserID, + Label: "User", + InputType: "text", + SupportsSearch: true, + }}, + }, } slices.SortStableFunc(types, func(a, b graphql2.DestinationTypeInfo) int { diff --git a/graphql2/graphqlapp/escalationpolicy.go b/graphql2/graphqlapp/escalationpolicy.go index 8ac07f8221..fcaf7b59a2 100644 --- a/graphql2/graphqlapp/escalationpolicy.go +++ b/graphql2/graphqlapp/escalationpolicy.go @@ -20,6 +20,7 @@ import ( type EscalationPolicy App type EscalationPolicyStep App type CreateEscalationPolicyStepInput App +type UpdateEscalationPolicyStepInput App func (a *App) EscalationPolicy() graphql2.EscalationPolicyResolver { return (*EscalationPolicy)(a) } func (a *App) EscalationPolicyStep() graphql2.EscalationPolicyStepResolver { @@ -47,6 +48,27 @@ func (a *CreateEscalationPolicyStepInput) Actions(ctx context.Context, input *gr return nil } +func (a *App) UpdateEscalationPolicyStepInput() graphql2.UpdateEscalationPolicyStepInputResolver { + return (*UpdateEscalationPolicyStepInput)(a) +} + +func (a *UpdateEscalationPolicyStepInput) Actions(ctx context.Context, input *graphql2.UpdateEscalationPolicyStepInput, actions []graphql2.DestinationInput) error { + tgts := make([]assignment.RawTarget, len(actions)) + var err error + for i, action := range actions { + if err := (*App)(a).ValidateDestination(ctx, fmt.Sprintf("%d.dest", i), &action); err != nil { + return err + } + tgts[i], err = CompatDestToTarget(action) + if err != nil { + return validation.NewFieldError("actions", "invalid DestInput") + } + } + input.Targets = tgts + input.Actions = actions + return nil +} + func contains(ids []string, id string) bool { for _, x := range ids { if x == id { @@ -333,6 +355,14 @@ func (m *Mutation) UpdateEscalationPolicyStep(ctx context.Context, input graphql } } + if input.Actions != nil { + for _, action := range input.Actions { + if err := (*App)(m).ValidateDestination(ctx, "input.actions", &action); err != nil { + return err + } + } + } + return err }) diff --git a/graphql2/models_gen.go b/graphql2/models_gen.go index 4db168d83e..6c2177988f 100644 --- a/graphql2/models_gen.go +++ b/graphql2/models_gen.go @@ -759,6 +759,7 @@ type UpdateEscalationPolicyStepInput struct { ID string `json:"id"` DelayMinutes *int `json:"delayMinutes,omitempty"` Targets []assignment.RawTarget `json:"targets,omitempty"` + Actions []DestinationInput `json:"actions,omitempty"` } type UpdateGQLAPIKeyInput struct { From e3cc96c68e20bf58de97966a9a4d6971a21dcdff Mon Sep 17 00:00:00 2001 From: AllenDing Date: Mon, 25 Mar 2024 11:25:09 -0500 Subject: [PATCH 2/8] add dest support for ep edit dialog and storybook --- graphql2/graph/escalationpolicy.graphqls | 4 +- .../PolicyStepEditDialogDest.stories.tsx | 143 ++++++++++++++++++ .../PolicyStepEditDialogDest.tsx | 132 ++++++++++++++++ .../escalation-policies/PolicyStepsCard.jsx | 21 ++- web/src/app/util/RequireConfig.tsx | 16 +- web/src/schema.d.ts | 1 + 6 files changed, 305 insertions(+), 12 deletions(-) create mode 100644 web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx create mode 100644 web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx diff --git a/graphql2/graph/escalationpolicy.graphqls b/graphql2/graph/escalationpolicy.graphqls index 561330fc1e..2e13ae83f6 100644 --- a/graphql2/graph/escalationpolicy.graphqls +++ b/graphql2/graph/escalationpolicy.graphqls @@ -7,5 +7,5 @@ extend input CreateEscalationPolicyStepInput { } extend input UpdateEscalationPolicyStepInput { - actions: [DestinationInput!] @goField(forceResolver: true) # force resolver for initial compat -} \ No newline at end of file + actions: [DestinationInput!] @goField(forceResolver: true) # force resolver for initial compat +} diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx new file mode 100644 index 0000000000..a9b2285895 --- /dev/null +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx @@ -0,0 +1,143 @@ +import React from 'react' +import type { Meta, StoryObj } from '@storybook/react' +import PolicyStepEditDialogDest from './PolicyStepEditDialogDest' +import { expect, userEvent, screen, waitFor, fn } from '@storybook/test' +import { handleDefaultConfig, handleExpFlags } from '../storybook/graphql' +import { HttpResponse, graphql } from 'msw' +import { InputFieldError } from '../util/errtypes' +import { DestinationInput, UpdateEscalationPolicyStepInput } from '../../schema' + +const meta = { + title: 'escalation-policies/PolicyStepEditDialogDest', + component: PolicyStepEditDialogDest, + render: function Component(args) { + return + }, + tags: ['autodocs'], + parameters: { + docs: { + story: { + inline: false, + iframeHeight: 400, + }, + }, + msw: { + handlers: [ + handleDefaultConfig, + handleExpFlags('dest-types'), + graphql.query('ValidateDestination', ({ variables: vars }) => { + return HttpResponse.json({ + data: { + destinationFieldValidate: vars.input.value === '+12225558989', + }, + }) + }), + graphql.query('DestDisplayInfo', ({ variables: vars }) => { + return HttpResponse.json({ + data: { + destinationDisplayInfo: { + text: vars.input.values[0].value, + iconURL: 'builtin://phone-voice', + iconAltText: 'Voice Call', + }, + }, + }) + }), + + graphql.mutation( + 'updateEscalationPolicyStep', + ({ variables: vars }) => { + console.log(vars) + if (vars.input.escalationPolicyID === '1') { + return HttpResponse.json({ + data: { + updateEscalationPolicyStep: true, + }, + }) + } + return HttpResponse.json({ + data: null, + errors: [ + { + message: 'This is a generic input field error', + path: ['editEscalationPolicyStep', 'input', 'name'], + extensions: { + code: 'INVALID_INPUT_VALUE', + }, + } satisfies InputFieldError, + ], + }) + }, + ), + ], + }, + }, +} satisfies Meta + +const action: DestinationInput[] = [ + { + type: 'builtin-twilio-sms', + values: [{ fieldID: 'phone-number', value: '11235550123' }], + }, +] + +const stepInput: UpdateEscalationPolicyStepInput = { + actions: action, + delayMinutes: 10, + id: '1', +} + +export default meta +type Story = StoryObj + +export const EditPolicyStep: Story = { + argTypes: { + onClose: { action: 'onClose' }, + }, + args: { + escalationPolicyID: '1', + step: stepInput, + onClose: fn(), + }, + play: async ({ args }) => { + await userEvent.type( + await screen.findByPlaceholderText('11235550123'), + '12225558989', + ) + + await userEvent.click(await screen.findByText('Add Action')) + + await waitFor(async function Icon() { + await userEvent.click(await screen.findByTestId('destination-chip')) + }) + await userEvent.click(await screen.findByText('Submit')) + + await waitFor(async function Close() { + expect(args.onClose).toHaveBeenCalled() + }) + }, +} + +// export const GenericError: Story = { +// args: { +// escalationPolicyID: '2', +// step: stepInput, +// onClose: fn(), +// }, +// play: async () => { +// await userEvent.click(await screen.findByText('Dest Type Error EP Step')) +// await userEvent.click(await screen.findByText('Generic Error EP Step')) +// await userEvent.type( +// await screen.findByPlaceholderText('11235550123'), +// '12225558989', +// ) + +// await userEvent.click(await screen.findByText('Add Action')) +// await userEvent.click(await screen.findByText('Submit')) + +// // expect error message +// await expect( +// await screen.findByText('This is a generic input field error'), +// ).toBeVisible() +// }, +// } diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx new file mode 100644 index 0000000000..b23b5411f0 --- /dev/null +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx @@ -0,0 +1,132 @@ +import React, { useEffect, useState } from 'react' +import { CombinedError, gql, useClient, useMutation } from 'urql' +import { splitErrorsByPath } from '../util/errutil' +import FormDialog from '../dialogs/FormDialog' +import { UpdateEscalationPolicyStepInput } from '../../schema' +import PolicyStepFormDest, { FormValue } from './PolicyStepFormDest' +import { errorPaths } from '../users/UserContactMethodFormDest' + +interface PolicyStepEditDialogDestProps { + escalationPolicyID: string + onClose: () => void + step: UpdateEscalationPolicyStepInput +} + +const mutation = gql` + mutation ($input: UpdateEscalationPolicyStepInput!) { + updateEscalationPolicyStep(input: $input) + } +` + +const query = gql` + query GetEscalationPolicyStep($input: ID!) { + escalationPolicy(id: $input) { + id + steps { + id + delayMinutes + actions { + type + values { + fieldID + value + } + } + } + } + } +` + +interface EscalationPolicyStep { + actions: Destination[] + id: string + delayMinutes: number +} + +interface Destination { + type: string + values: FieldValuePair[] +} + +interface FieldValuePair { + fieldID: string + value: string +} + +function PolicyStepEditDialogDest( + props: PolicyStepEditDialogDestProps, +): JSX.Element { + const defaultValue: FormValue = { + actions: props.step.actions ?? [], + delayMinutes: props.step.delayMinutes ?? 0, + } + + const [value, setValue] = useState(null) + + const [editStepStatus, editStep] = useMutation(mutation) + const [stepErr, setStepErr] = useState(null) + + const [formErrors, otherErrs] = splitErrorsByPath( + stepErr, + errorPaths('destinationDisplayInfo.input'), + ) + + useEffect(() => { + setStepErr(null) + }, [value]) + + useEffect(() => { + setStepErr(editStepStatus.error || null) + }, [editStepStatus.error]) + + const validationClient = useClient() + + useEffect(() => { + validationClient + .query(query, { + input: props.escalationPolicyID, + }) + .toPromise() + .then((res) => { + const matchedStep = res.data.escalationPolicy.steps.find( + (step: EscalationPolicyStep) => step.id === props.step.id, + ) + setValue(matchedStep) + }) + }, [editStepStatus.stale]) + + return ( + + editStep( + { + input: { + id: props.step.id, + delayMinutes: + (value && value.delayMinutes) || defaultValue.delayMinutes, + actions: (value && value.actions) || defaultValue.actions, + }, + }, + { additionalTypenames: ['EscalationPolicy'] }, + ).then((result) => { + if (!result.error) props.onClose() + }) + } + form={ + setValue(value)} + /> + } + /> + ) +} + +export default PolicyStepEditDialogDest diff --git a/web/src/app/escalation-policies/PolicyStepsCard.jsx b/web/src/app/escalation-policies/PolicyStepsCard.jsx index 8b6cc99f9b..a8ec903e18 100644 --- a/web/src/app/escalation-policies/PolicyStepsCard.jsx +++ b/web/src/app/escalation-policies/PolicyStepsCard.jsx @@ -19,6 +19,7 @@ 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, @@ -221,11 +222,21 @@ export default function PolicyStepsCard(props) { {editStepID && ( - step.id === editStepID)[0]} - /> + + {hasDestTypesFlag ? ( + step.id === editStepID)[0]} + /> + ) : ( + step.id === editStepID)[0]} + /> + )} + )} {deleteStep && ( t.isContactMethod) } -export function useEPTargetTypes(): DestinationTypeInfo[] { - const cfg = React.useContext(ConfigContext) - return cfg.destTypes.filter((t) => t.isEPTarget) -} - /** useSchedOnCallNotifyTypes returns a list of schedule on-call notification destination types. */ export function useSchedOnCallNotifyTypes(): DestinationTypeInfo[] { const cfg = React.useContext(ConfigContext) return cfg.destTypes.filter((t) => t.isSchedOnCallNotify) } +// useEPTargetTypes returns a list of escalation policy target types, removing duplicate entries. +export function useEPTargetTypes(): DestinationTypeInfo[] { + const cfg = React.useContext(ConfigContext) + return cfg.destTypes + .sort((a, b) => a.name.localeCompare(b.name)) + .filter( + (t, idx: number, array: DestinationTypeInfo[]) => + t.isEPTarget && (idx === 0 || t.name !== array[idx - 1].name), + ) +} + // useDestinationType returns information about the given destination type. export function useDestinationType(type: DestinationType): DestinationTypeInfo { const ctx = React.useContext(ConfigContext) diff --git a/web/src/schema.d.ts b/web/src/schema.d.ts index e1f36ea2a8..e298802063 100644 --- a/web/src/schema.d.ts +++ b/web/src/schema.d.ts @@ -1120,6 +1120,7 @@ export interface UpdateEscalationPolicyInput { } export interface UpdateEscalationPolicyStepInput { + actions?: null | DestinationInput[] delayMinutes?: null | number id: string targets?: null | TargetInput[] From 754dd1f419fec469caa10c8efbcd87a1d354f7f5 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 25 Mar 2024 11:48:00 -0500 Subject: [PATCH 3/8] revert unecessary changes --- graphql2/graphqlapp/contactmethod.go | 7 +++- graphql2/graphqlapp/destinationtypes.go | 48 ------------------------- web/src/app/util/RequireConfig.tsx | 16 +++------ 3 files changed, 11 insertions(+), 60 deletions(-) diff --git a/graphql2/graphqlapp/contactmethod.go b/graphql2/graphqlapp/contactmethod.go index 0e841f877b..b7984dd547 100644 --- a/graphql2/graphqlapp/contactmethod.go +++ b/graphql2/graphqlapp/contactmethod.go @@ -140,9 +140,14 @@ func (m *Mutation) CreateUserContactMethod(ctx context.Context, input graphql2.C cfg := config.FromContext(ctx) if input.Dest != nil { - if err := (*App)(m).ValidateDestination(ctx, "input.dest", input.Dest); err != nil { + err := validate.IDName("input.name", input.Name) + if err != nil { + addInputError(ctx, err) return nil, errAlreadySet } + if err := (*App)(m).ValidateDestination(ctx, "input.dest", input.Dest); err != nil { + return nil, err + } t, v := CompatDestToCMTypeVal(*input.Dest) input.Type = &t input.Value = &v diff --git a/graphql2/graphqlapp/destinationtypes.go b/graphql2/graphqlapp/destinationtypes.go index 02c5eed64f..9dc1da1ead 100644 --- a/graphql2/graphqlapp/destinationtypes.go +++ b/graphql2/graphqlapp/destinationtypes.go @@ -451,54 +451,6 @@ func (q *Query) DestinationTypes(ctx context.Context) ([]graphql2.DestinationTyp SupportsSearch: true, }}, }, - { - Type: destRotation, - Name: "Rotation", - Enabled: true, - IsContactMethod: false, - IsEPTarget: true, - IsSchedOnCallNotify: true, - SupportsStatusUpdates: false, - StatusUpdatesRequired: false, - RequiredFields: []graphql2.DestinationFieldConfig{{ - FieldID: fieldRotationID, - Label: "Rotation", - InputType: "text", - SupportsSearch: true, - }}, - }, - { - Type: destSchedule, - Name: "Schedule", - Enabled: true, - IsContactMethod: false, - IsEPTarget: true, - IsSchedOnCallNotify: true, - SupportsStatusUpdates: false, - StatusUpdatesRequired: false, - RequiredFields: []graphql2.DestinationFieldConfig{{ - FieldID: fieldScheduleID, - Label: "Schedule", - InputType: "text", - SupportsSearch: true, - }}, - }, - { - Type: destUser, - Name: "User", - Enabled: true, - IsContactMethod: false, - IsEPTarget: true, - IsSchedOnCallNotify: true, - SupportsStatusUpdates: false, - StatusUpdatesRequired: false, - RequiredFields: []graphql2.DestinationFieldConfig{{ - FieldID: fieldUserID, - Label: "User", - InputType: "text", - SupportsSearch: true, - }}, - }, } slices.SortStableFunc(types, func(a, b graphql2.DestinationTypeInfo) int { diff --git a/web/src/app/util/RequireConfig.tsx b/web/src/app/util/RequireConfig.tsx index b9167ae67c..5dfed3bb6b 100644 --- a/web/src/app/util/RequireConfig.tsx +++ b/web/src/app/util/RequireConfig.tsx @@ -216,21 +216,15 @@ export function useContactMethodTypes(): DestinationTypeInfo[] { return cfg.destTypes.filter((t) => t.isContactMethod) } -/** useSchedOnCallNotifyTypes returns a list of schedule on-call notification destination types. */ -export function useSchedOnCallNotifyTypes(): DestinationTypeInfo[] { +export function useEPTargetTypes(): DestinationTypeInfo[] { const cfg = React.useContext(ConfigContext) - return cfg.destTypes.filter((t) => t.isSchedOnCallNotify) + return cfg.destTypes.filter((t) => t.isEPTarget) } -// useEPTargetTypes returns a list of escalation policy target types, removing duplicate entries. -export function useEPTargetTypes(): DestinationTypeInfo[] { +/** useSchedOnCallNotifyTypes returns a list of schedule on-call notification destination types. */ +export function useSchedOnCallNotifyTypes(): DestinationTypeInfo[] { const cfg = React.useContext(ConfigContext) - return cfg.destTypes - .sort((a, b) => a.name.localeCompare(b.name)) - .filter( - (t, idx: number, array: DestinationTypeInfo[]) => - t.isEPTarget && (idx === 0 || t.name !== array[idx - 1].name), - ) + return cfg.destTypes.filter((t) => t.isSchedOnCallNotify) } // useDestinationType returns information about the given destination type. From 6b758ff9caa1338d2e7417270db502d89a9004c5 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 26 Mar 2024 10:02:21 -0500 Subject: [PATCH 4/8] remove redundant validations --- graphql2/graphqlapp/escalationpolicy.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/graphql2/graphqlapp/escalationpolicy.go b/graphql2/graphqlapp/escalationpolicy.go index fcaf7b59a2..ee7b662ed1 100644 --- a/graphql2/graphqlapp/escalationpolicy.go +++ b/graphql2/graphqlapp/escalationpolicy.go @@ -65,7 +65,6 @@ func (a *UpdateEscalationPolicyStepInput) Actions(ctx context.Context, input *gr } } input.Targets = tgts - input.Actions = actions return nil } @@ -355,14 +354,6 @@ func (m *Mutation) UpdateEscalationPolicyStep(ctx context.Context, input graphql } } - if input.Actions != nil { - for _, action := range input.Actions { - if err := (*App)(m).ValidateDestination(ctx, "input.actions", &action); err != nil { - return err - } - } - } - return err }) From a0d2ae7f5b6ab4f816d425eb7e6ae16af3239b36 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 26 Mar 2024 13:33:41 -0500 Subject: [PATCH 5/8] cleanup data flow --- .../PolicyStepEditDialogDest.tsx | 85 ++++++++----------- .../escalation-policies/PolicyStepsCard.jsx | 4 +- 2 files changed, 38 insertions(+), 51 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx index b23b5411f0..0dc25a029a 100644 --- a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx @@ -1,15 +1,20 @@ import React, { useEffect, useState } from 'react' -import { CombinedError, gql, useClient, useMutation } from 'urql' +import { CombinedError, gql, useMutation, useQuery } from 'urql' import { splitErrorsByPath } from '../util/errutil' import FormDialog from '../dialogs/FormDialog' -import { UpdateEscalationPolicyStepInput } from '../../schema' import PolicyStepFormDest, { FormValue } from './PolicyStepFormDest' import { errorPaths } from '../users/UserContactMethodFormDest' +import { + Destination, + EscalationPolicy, + FieldValuePair, + UpdateEscalationPolicyStepInput, +} from '../../schema' interface PolicyStepEditDialogDestProps { escalationPolicyID: string onClose: () => void - step: UpdateEscalationPolicyStepInput + stepID: string } const mutation = gql` @@ -19,8 +24,8 @@ const mutation = gql` ` const query = gql` - query GetEscalationPolicyStep($input: ID!) { - escalationPolicy(id: $input) { + query GetEscalationPolicyStep($id: ID!) { + escalationPolicy(id: $id) { id steps { id @@ -37,31 +42,30 @@ const query = gql` } ` -interface EscalationPolicyStep { - actions: Destination[] - id: string - delayMinutes: number -} - -interface Destination { - type: string - values: FieldValuePair[] -} - -interface FieldValuePair { - fieldID: string - value: string -} - function PolicyStepEditDialogDest( props: PolicyStepEditDialogDestProps, -): JSX.Element { - const defaultValue: FormValue = { - actions: props.step.actions ?? [], - delayMinutes: props.step.delayMinutes ?? 0, - } +): React.ReactNode { + const [stepQ] = useQuery<{ escalationPolicy: EscalationPolicy }>({ + query, + variables: { id: props.escalationPolicyID }, + }) + const step = stepQ.data?.escalationPolicy.steps.find( + (s) => s.id === props.stepID, + ) + + if (!step) throw new Error('Step not found') - const [value, setValue] = useState(null) + const [value, setValue] = useState({ + actions: (step.actions || []).map((a: Destination) => ({ + // remove extraneous fields + type: a.type, + values: a.values.map((v: FieldValuePair) => ({ + fieldID: v.fieldID, + value: v.value, + })), + })), + delayMinutes: step.delayMinutes, + }) const [editStepStatus, editStep] = useMutation(mutation) const [stepErr, setStepErr] = useState(null) @@ -79,22 +83,6 @@ function PolicyStepEditDialogDest( setStepErr(editStepStatus.error || null) }, [editStepStatus.error]) - const validationClient = useClient() - - useEffect(() => { - validationClient - .query(query, { - input: props.escalationPolicyID, - }) - .toPromise() - .then((res) => { - const matchedStep = res.data.escalationPolicy.steps.find( - (step: EscalationPolicyStep) => step.id === props.step.id, - ) - setValue(matchedStep) - }) - }, [editStepStatus.stale]) - return ( { @@ -121,7 +108,7 @@ function PolicyStepEditDialogDest( setValue(value)} /> } diff --git a/web/src/app/escalation-policies/PolicyStepsCard.jsx b/web/src/app/escalation-policies/PolicyStepsCard.jsx index a8ec903e18..170a3c4180 100644 --- a/web/src/app/escalation-policies/PolicyStepsCard.jsx +++ b/web/src/app/escalation-policies/PolicyStepsCard.jsx @@ -227,13 +227,13 @@ export default function PolicyStepsCard(props) { step.id === editStepID)[0]} + stepID={editStepID} /> ) : ( step.id === editStepID)[0]} + step={steps.find((step) => step.id === editStepID)} /> )} From e9cc7b038980027f24eee710636d7ad05a454107 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 26 Mar 2024 13:35:06 -0500 Subject: [PATCH 6/8] add disable portal --- web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx index 0dc25a029a..363191a562 100644 --- a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx @@ -15,6 +15,7 @@ interface PolicyStepEditDialogDestProps { escalationPolicyID: string onClose: () => void stepID: string + disablePortal?: boolean } const mutation = gql` @@ -88,6 +89,7 @@ function PolicyStepEditDialogDest( title='Edit Step' loading={editStepStatus.fetching} errors={otherErrs} + disablePortal={props.disablePortal} maxWidth='sm' onClose={props.onClose} onSubmit={() => From d8296de7f4b5ac02943690a4fa74aa8bed074f0a Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 26 Mar 2024 13:48:28 -0500 Subject: [PATCH 7/8] update query name and mock --- .../PolicyStepEditDialogDest.stories.tsx | 182 ++++++++++-------- .../PolicyStepEditDialogDest.tsx | 4 +- 2 files changed, 105 insertions(+), 81 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx index a9b2285895..3db1e626e6 100644 --- a/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.stories.tsx @@ -1,24 +1,29 @@ import React from 'react' import type { Meta, StoryObj } from '@storybook/react' import PolicyStepEditDialogDest from './PolicyStepEditDialogDest' -import { expect, userEvent, screen, waitFor, fn } from '@storybook/test' +import { expect, fn, userEvent, waitFor, within } from '@storybook/test' import { handleDefaultConfig, handleExpFlags } from '../storybook/graphql' import { HttpResponse, graphql } from 'msw' -import { InputFieldError } from '../util/errtypes' -import { DestinationInput, UpdateEscalationPolicyStepInput } from '../../schema' +import { DestFieldValueError } from '../util/errtypes' +import { EscalationPolicyStep } from '../../schema' const meta = { - title: 'escalation-policies/PolicyStepEditDialogDest', + title: 'Escalation Policies/Steps/Edit Dialog', component: PolicyStepEditDialogDest, render: function Component(args) { - return + return }, tags: ['autodocs'], + args: { + onClose: fn(), + escalationPolicyID: 'policy1', + stepID: 'step1', + }, parameters: { docs: { story: { inline: false, - iframeHeight: 400, + iframeHeight: 600, }, }, msw: { @@ -28,11 +33,27 @@ const meta = { graphql.query('ValidateDestination', ({ variables: vars }) => { return HttpResponse.json({ data: { - destinationFieldValidate: vars.input.value === '+12225558989', + destinationFieldValidate: vars.input.value.length === 12, }, }) }), graphql.query('DestDisplayInfo', ({ variables: vars }) => { + if (vars.input.values[0].value.length !== 12) { + return HttpResponse.json({ + errors: [ + { message: 'generic error' }, + { + message: 'Invalid number', + path: ['destinationDisplayInfo', 'input'], + extensions: { + code: 'INVALID_DEST_FIELD_VALUE', + fieldID: 'phone-number', + }, + } satisfies DestFieldValueError, + ], + }) + } + return HttpResponse.json({ data: { destinationDisplayInfo: { @@ -44,100 +65,103 @@ const meta = { }) }), - graphql.mutation( - 'updateEscalationPolicyStep', - ({ variables: vars }) => { - console.log(vars) - if (vars.input.escalationPolicyID === '1') { - return HttpResponse.json({ - data: { - updateEscalationPolicyStep: true, - }, - }) - } + graphql.query('GetEPStep', () => { + return HttpResponse.json({ + data: { + escalationPolicy: { + id: 'policy1', + steps: [ + { + id: 'step1', + delayMinutes: 17, + actions: [ + { + type: 'single-field', + values: [ + { fieldID: 'phone-number', value: '+19995550123' }, + ], + }, + ], + } as EscalationPolicyStep, + ], + }, + }, + }) + }), + + graphql.mutation('UpdateEPStep', ({ variables: vars }) => { + if (vars.input.delayMinutes === 999) { return HttpResponse.json({ - data: null, - errors: [ - { - message: 'This is a generic input field error', - path: ['editEscalationPolicyStep', 'input', 'name'], - extensions: { - code: 'INVALID_INPUT_VALUE', - }, - } satisfies InputFieldError, - ], + errors: [{ message: 'generic dialog error' }], }) - }, - ), + } + + return HttpResponse.json({ + data: { + updateEscalationPolicyStep: true, + }, + }) + }), ], }, }, } satisfies Meta -const action: DestinationInput[] = [ - { - type: 'builtin-twilio-sms', - values: [{ fieldID: 'phone-number', value: '11235550123' }], - }, -] - -const stepInput: UpdateEscalationPolicyStepInput = { - actions: action, - delayMinutes: 10, - id: '1', -} - export default meta type Story = StoryObj -export const EditPolicyStep: Story = { +export const UpdatePolicyStep: Story = { argTypes: { onClose: { action: 'onClose' }, }, args: { escalationPolicyID: '1', - step: stepInput, - onClose: fn(), }, - play: async ({ args }) => { - await userEvent.type( - await screen.findByPlaceholderText('11235550123'), - '12225558989', - ) + play: async ({ args, canvasElement }) => { + const canvas = within(canvasElement) + + // validate existing step data + // 1. delay should be 17 + // 2. phone number should be +19995550123 + + await waitFor(async function ExistingChip() { + await expect(await canvas.findByLabelText('Delay (minutes)')).toHaveValue( + 17, + ) + await expect(await canvas.findByText('+19995550123')).toBeVisible() + }) + + const phoneInput = await canvas.findByLabelText('Phone Number') + await userEvent.clear(phoneInput) + await userEvent.type(phoneInput, '1222') + await userEvent.click(await canvas.findByText('Add Action')) + + await expect(await canvas.findByText('Invalid number')).toBeVisible() + await expect(await canvas.findByText('generic error')).toBeVisible() - await userEvent.click(await screen.findByText('Add Action')) + await userEvent.clear(phoneInput) + await userEvent.type(phoneInput, '12225550123') + await userEvent.click(await canvas.findByText('Add Action')) await waitFor(async function Icon() { - await userEvent.click(await screen.findByTestId('destination-chip')) + await userEvent.click(await canvas.findByTestId('destination-chip')) }) - await userEvent.click(await screen.findByText('Submit')) + + const delayField = await canvas.findByLabelText('Delay (minutes)') + await userEvent.clear(delayField) + await userEvent.type(delayField, '999') + await userEvent.click(await canvas.findByText('Submit')) + + await expect(await canvas.findByText('generic dialog error')).toBeVisible() + + await expect(args.onClose).not.toHaveBeenCalled() // should not close on error + + await userEvent.clear(delayField) + await userEvent.type(delayField, '15') + await userEvent.click(await canvas.findByText('Retry')) await waitFor(async function Close() { - expect(args.onClose).toHaveBeenCalled() + await expect(args.onClose).toHaveBeenCalled() }) }, } - -// export const GenericError: Story = { -// args: { -// escalationPolicyID: '2', -// step: stepInput, -// onClose: fn(), -// }, -// play: async () => { -// await userEvent.click(await screen.findByText('Dest Type Error EP Step')) -// await userEvent.click(await screen.findByText('Generic Error EP Step')) -// await userEvent.type( -// await screen.findByPlaceholderText('11235550123'), -// '12225558989', -// ) - -// await userEvent.click(await screen.findByText('Add Action')) -// await userEvent.click(await screen.findByText('Submit')) - -// // expect error message -// await expect( -// await screen.findByText('This is a generic input field error'), -// ).toBeVisible() -// }, -// } diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx index 363191a562..c1422c7559 100644 --- a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx @@ -19,13 +19,13 @@ interface PolicyStepEditDialogDestProps { } const mutation = gql` - mutation ($input: UpdateEscalationPolicyStepInput!) { + mutation UpdateEPStep($input: UpdateEscalationPolicyStepInput!) { updateEscalationPolicyStep(input: $input) } ` const query = gql` - query GetEscalationPolicyStep($id: ID!) { + query GetEPStep($id: ID!) { escalationPolicy(id: $id) { id steps { From 52633486414af4d7ae90b98ae7106b350b5f948c Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 26 Mar 2024 13:57:48 -0500 Subject: [PATCH 8/8] fix error handling --- .../PolicyStepEditDialogDest.tsx | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx index c1422c7559..211ac13d9d 100644 --- a/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx +++ b/web/src/app/escalation-policies/PolicyStepEditDialogDest.tsx @@ -1,9 +1,8 @@ -import React, { useEffect, useState } from 'react' -import { CombinedError, gql, useMutation, useQuery } from 'urql' +import React, { useState } from 'react' +import { gql, useMutation, useQuery } from 'urql' import { splitErrorsByPath } from '../util/errutil' import FormDialog from '../dialogs/FormDialog' import PolicyStepFormDest, { FormValue } from './PolicyStepFormDest' -import { errorPaths } from '../users/UserContactMethodFormDest' import { Destination, EscalationPolicy, @@ -69,26 +68,19 @@ function PolicyStepEditDialogDest( }) const [editStepStatus, editStep] = useMutation(mutation) - const [stepErr, setStepErr] = useState(null) - const [formErrors, otherErrs] = splitErrorsByPath( - stepErr, - errorPaths('destinationDisplayInfo.input'), - ) - - useEffect(() => { - setStepErr(null) - }, [value]) - - useEffect(() => { - setStepErr(editStepStatus.error || null) - }, [editStepStatus.error]) + // Edit dialog has no errors to be handled by the form: + // - actions field has it's own validation + // - errors on existing actions are not handled specially, and just display in the dialog (i.e., duplicates) + // - the delay field has no validation, and is automatically clamped to the min/max values by the backend + const [a, errs] = splitErrorsByPath(editStepStatus.error, []) + console.log(a, errs, editStepStatus.error) return ( setValue(value)}