feat: allow users to add auto reply triggers #1664
Conversation
|
This is ready for review! See notes about failing test in #1663 |
ajohn25
force-pushed
the
feat-auto-reply-handling
branch
from
7cd7433
to
063fda4
Compare
| @@ -25,5 +25,20 @@ export const schema = ` | |||
| createdAt: Date | |||
| interactionSteps: [InteractionStepInput] | |||
| } | |||
|
|
|||
| type InteractionStepWithChildren { | |||
There was a problem hiding this comment.
Question: is this necessary in the GraphQL schema? It looks like it is only being used for TypeScript typing. And the children are already captured in the interactionSteps array property.
|
|
||
| return ( | ||
| <div> | ||
| <br /> |
There was a problem hiding this comment.
Required: remove this -- styling should be done with CSS not <br>'s
| return; | ||
| } | ||
| switch (event.key) { | ||
| /* eslint-disable no-case-declarations */ |
There was a problem hiding this comment.
Suggestion: prefer an empty no-op default
| onChange={handleChange} | ||
| onInputChange={handleInputChange} | ||
| onKeyDown={handleKeyDown} | ||
| onPas |
There was a problem hiding this comment.
Question: intentional onPas prop?
| return ( | ||
| <div> | ||
| <br /> | ||
| Auto Replies |
There was a problem hiding this comment.
Question: should the label be part of the field itself? Or added where the field is used? I think separating form inputs from labels is the right way to do it but can't remember what Spoke conventions are (and would rather be consistent with conventions).
| }); | ||
| const autoReplyCount = parseInt(rowCount, 10); | ||
|
|
||
| if (handleOptOut || Number.isNaN(autoReplyCount) || autoReplyCount === 0) { |
There was a problem hiding this comment.
Suggestion: use variable to improve clarity
| if (handleOptOut || Number.isNaN(autoReplyCount) || autoReplyCount === 0) { | |
| const hasNoAutoReplies = Number.isNaN(autoReplyCount) || autoReplyCount === 0; | |
| if (handleOptOut || hasNoAutoReplies) { |
| }); | ||
| } | ||
|
|
||
| await updateQuery; |
There was a problem hiding this comment.
Question: execution of this query was moved inside an if block -- intentional?
| await assignContacts(client, assignment.id, campaign.id, 1); | ||
| await createMessage(client, { | ||
| assignmentId: assignment.id, | ||
| campaignContactId: contact.id, | ||
| contactNumber: contact.cell, | ||
| text: "Hi! Want to attend my cool event?" | ||
| }); |
There was a problem hiding this comment.
nit: mirror actual flow and assign contact and send message after campaign creation (interaction steps and auto-reply triggers)
|
|
||
| test("does not respond to a contact who says YES! with no auto reply configured for the campaign", async () => { | ||
| const testbed = await withClient(pool, async (client) => { | ||
| return createTestBed(client, agent); |
There was a problem hiding this comment.
Question: doesn't createTestBed() configure a yes auto reply token for the campaign?
| ); | ||
|
|
||
| const msgCount = parseInt(msgs.count, 10); | ||
| expect(msgCount).toBe(2); |
There was a problem hiding this comment.
Question: should this also check retry-interaction-step jobs?
Description
This PR allows admins to add auto reply triggers when editing a campaign. It also adds logic for marking contacts ineligible for auto replies, when a message is sent to them from Message Review
Motivation and Context
Closes #1663
How Has This Been Tested?
This has been tested locally:
Screenshots (if appropriate):
Documentation Changes
Checklist: