-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow users to add auto reply triggers #1664
base: feat-auto-reply-handling
Are you sure you want to change the base?
Changes from all commits
89439ab
15ef52d
0bf25d0
cf0905b
6f09ddc
bc69354
8c3c013
fd1e938
95d1240
12e4d70
7cd7433
43ac6a6
36a05f5
1d1e82c
4cde457
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,21 @@ | ||
import type { | ||
Assignment, | ||
Campaign, | ||
CampaignContact, | ||
InteractionStep, | ||
Message, | ||
Organization, | ||
User | ||
} from "@spoke/spoke-codegen"; | ||
import faker from "faker"; | ||
import AuthHasher from "passport-local-authenticate"; | ||
import type { PoolClient } from "pg"; | ||
|
||
import type { Assignment } from "../../src/api/assignment"; | ||
import type { Campaign } from "../../src/api/campaign"; | ||
import type { CampaignContact } from "../../src/api/campaign-contact"; | ||
import type { InteractionStep } from "../../src/api/interaction-step"; | ||
import type { Message } from "../../src/api/message"; | ||
import type { Organization } from "../../src/api/organization"; | ||
import { UserRoleType } from "../../src/api/organization-membership"; | ||
import type { User } from "../../src/api/user"; | ||
import { DateTime } from "../../src/lib/datetime"; | ||
import type { | ||
AssignmentRecord, | ||
AutoReplyTriggerRecord, | ||
CampaignContactRecord, | ||
CampaignRecord, | ||
InteractionStepRecord, | ||
|
@@ -388,7 +391,7 @@ export const createMessage = async ( | |
.then(({ rows: [message] }) => message); | ||
|
||
export interface CreateCompleteCampaignOptions { | ||
organization?: CreateOrganizationOptions; | ||
organization?: CreateOrganizationOptions & { id: number }; | ||
campaign?: Omit<CreateCampaignOptions, "organizationId">; | ||
texters?: number | CreateTexterOptions[]; | ||
contacts?: number | Omit<CreateCampaignContactOptions, "campaignId">[]; | ||
|
@@ -398,10 +401,10 @@ export const createCompleteCampaign = async ( | |
client: PoolClient, | ||
options: CreateCompleteCampaignOptions | ||
) => { | ||
const organization = await createOrganization( | ||
client, | ||
options.organization ?? {} | ||
); | ||
const optOrg = options.organization; | ||
const organization = optOrg?.id | ||
? { id: optOrg?.id } | ||
: await createOrganization(client, options.organization ?? {}); | ||
|
||
const campaign = await createCampaign(client, { | ||
...(options.campaign ?? {}), | ||
|
@@ -524,3 +527,44 @@ export const createQuestionResponse = async ( | |
[options.value, options.campaignContactId, options.interactionStepId] | ||
) | ||
.then(({ rows: [questionResponse] }) => questionResponse); | ||
|
||
export type CreateAutoReplyTriggerOptions = { | ||
token: string; | ||
interactionStepId: number; | ||
}; | ||
|
||
export const createAutoReplyTrigger = async ( | ||
Comment on lines
+531
to
+536
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: move this to its own |
||
client: PoolClient, | ||
options: CreateAutoReplyTriggerOptions | ||
) => | ||
client | ||
.query<AutoReplyTriggerRecord>( | ||
` | ||
insert into public.auto_reply_trigger (token, interaction_step_id) | ||
values ($1, $2) | ||
returning * | ||
`, | ||
[options.token, options.interactionStepId] | ||
) | ||
.then(({ rows: [trigger] }) => trigger); | ||
|
||
export const assignContacts = async ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: move this to its own file, not in |
||
client: PoolClient, | ||
assignmentId: number, | ||
campaignId: number, | ||
count: number | ||
) => { | ||
await client.query( | ||
` | ||
update campaign_contact | ||
set assignment_id = $1 | ||
where id in ( | ||
select id from campaign_contact | ||
where campaign_id = $2 | ||
and assignment_id is null | ||
limit $3 | ||
) | ||
`, | ||
[assignmentId, campaignId, count] | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ export const schema = ` | |
scriptOptions: [String]! | ||
answerOption: String | ||
parentInteractionId: String | ||
autoReplyTokens: [String] | ||
isDeleted: Boolean | ||
answerActions: String | ||
questionResponse(campaignContactId: String): QuestionResponse | ||
|
@@ -18,10 +19,26 @@ export const schema = ` | |
scriptOptions: [String]! | ||
answerOption: String | ||
answerActions: String | ||
autoReplyTokens: [String] | ||
parentInteractionId: String | ||
isDeleted: Boolean | ||
createdAt: Date | ||
interactionSteps: [InteractionStepInput] | ||
} | ||
|
||
type InteractionStepWithChildren { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
id: ID! | ||
question: Question | ||
questionText: String | ||
scriptOptions: [String]! | ||
answerOption: String | ||
parentInteractionId: String | ||
autoReplyTokens: [String] | ||
isDeleted: Boolean | ||
answerActions: String | ||
questionResponse(campaignContactId: String): QuestionResponse | ||
createdAt: Date! | ||
interactionSteps: [InteractionStep] | ||
} | ||
`; | ||
export default schema; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -371,6 +371,7 @@ const rootSchema = ` | |
bulkOptOut(organizationId: String!, csvFile: Upload, numbersList: String): Int! | ||
bulkOptIn(organizationId: String!, csvFile: Upload, numbersList: String): Int! | ||
exportOptOuts(organizationId: String!, campaignIds: [String!]): Boolean! | ||
markForManualReply(campaignContactId: String!): CampaignContact! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: rename the root mutation to mutation MarkForManualReply($campaignContactId: String!) {
setAutoReplyEligible(campaignContactId: $campaignContactId, autoReplyEligible: false) {
id
autoReplyEligible
}
} |
||
} | ||
|
||
schema { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/** | ||
* @param { import("knex").Knex } knex | ||
* @returns { Promise<void> } | ||
*/ | ||
exports.up = function up(knex) { | ||
return knex.schema | ||
.createTable("auto_reply_trigger", (table) => { | ||
table.increments("id"); | ||
table.text("token").notNullable(); | ||
table.integer("interaction_step_id").notNullable(); | ||
table.foreign("interaction_step_id").references("interaction_step.id"); | ||
table.timestamp("created_at").notNull().defaultTo(knex.fn.now()); | ||
table.timestamp("updated_at").notNull().defaultTo(knex.fn.now()); | ||
table.unique(["interaction_step_id", "token"]); | ||
}) | ||
.raw( | ||
` | ||
create or replace function auto_reply_trigger_before_insert() returns trigger as $$ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: rename to describe what the function is actually doing (e.g. |
||
begin | ||
if exists( | ||
select 1 from auto_reply_trigger | ||
where token = NEW.token | ||
and interaction_step_id in ( | ||
select id from interaction_step child_steps | ||
where parent_interaction_id = ( | ||
select parent_interaction_id from interaction_step | ||
where id = NEW.interaction_step_id | ||
) | ||
) | ||
and interaction_step_id <> NEW.id | ||
) then | ||
raise exception 'Each interaction step can only have 1 child step assigned to any particular auto reply token'; | ||
end if; | ||
|
||
return NEW; | ||
end; | ||
$$ language plpgsql; | ||
|
||
create trigger _500_auto_reply_trigger_insert | ||
before insert | ||
on auto_reply_trigger | ||
for each row | ||
execute procedure auto_reply_trigger_before_insert(); | ||
|
||
create trigger _500_auto_reply_trigger_updated_at | ||
before update | ||
on public.auto_reply_trigger | ||
for each row | ||
execute procedure universal_updated_at(); | ||
` | ||
) | ||
.alterTable("campaign_contact", (table) => { | ||
table.boolean("auto_reply_eligible").notNullable().defaultTo(false); | ||
}) | ||
.alterTable("campaign_contact", (table) => { | ||
table | ||
.boolean("auto_reply_eligible") | ||
.notNullable() | ||
.defaultTo(true) | ||
.alter(); | ||
Comment on lines
+52
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: add comment that the goal of this is to backfill with |
||
}); | ||
}; | ||
/** | ||
* @param { import("knex").Knex } knex | ||
* @returns { Promise<void> } | ||
*/ | ||
exports.down = function down(knex) { | ||
return knex.schema | ||
.dropTable("auto_reply_trigger") | ||
.alterTable("campaign_contact", (table) => { | ||
table.dropColumn("auto_reply_eligible"); | ||
}); | ||
}; | ||
Comment on lines
+67
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Required: must drop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: use a discriminated union instead. See spoke-portal for example:
https://github.com/politics-rewired/spoke-portal/blob/main/apps/api/__tests__/lib/instance.ts#L17-L56