-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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: V2 API endpoint create phone call #16528
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.ts
Outdated
Show resolved
Hide resolved
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.ts
Outdated
Show resolved
Hide resolved
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (09/10/24)1 reviewer was added to this PR based on Keith Williams's automation. |
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.
LGTM 🚀
|
||
@IsBoolean() | ||
@DocsProperty({ description: "Enabled status", default: true }) | ||
enabled = true; |
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.
To make typescript happy need to add !
aka enabled!
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.
I don't see any ts error on this
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.
since the type is 'true' it should be fine without the ! indeed, ! would be needed if the type was boolean
apps/api/v2/src/ee/event-types/event-types_2024_06_14/outputs/create-phone-call.output.ts
Outdated
Show resolved
Hide resolved
timeZone: user.timeZone, | ||
profile: { organization: { id: orgId } }, | ||
}, | ||
input: { ...body, eventTypeId }, |
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.
We are passing eventTypeId
to handleCreatePhoneCall
which has TCreatePhoneCallSchema
for input, but eventTypeId
does not exist on that schema.
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.
It has eventTypeId. check this code
const requiredFields = z.object({
yourPhoneNumber: z.string().refine((val) => isValidPhoneNumber(val)),
numberToCall: z.string().refine((val) => isValidPhoneNumber(val)),
calApiKey: z.string().trim().min(1, {
message: "Please enter CAL API Key",
}),
eventTypeId: z.number(),
enabled: z.boolean().default(false),
templateType: templateTypeEnum,
});
export const createPhoneCallSchema = requiredFields.merge(
z.object({
schedulerName: z.string().min(1).optional().nullable(),
guestName: z
.string()
.optional()
.transform((val) => {
return !!val ? val : undefined;
}),
guestEmail: z
.string()
.optional()
.transform((val) => {
return !!val ? val : undefined;
}),
guestCompany: z
.string()
.optional()
.transform((val) => {
return !!val ? val : undefined;
}),
beginMessage: z.string().optional(),
generalPrompt: z.string().optional(),
})
);
Notice we are using merge
...v2/src/modules/organizations/controllers/event-types/organizations-event-types.controller.ts
Show resolved
Hide resolved
user: { timeZone: string; id: number; profile?: { organization?: { id?: number } } }; | ||
input: TCreatePhoneCallSchema; | ||
}) => { | ||
if (!!!user?.profile?.organization) { |
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.
Why a tripple shabang !!!
? We could just have if(!user?.profile?.organization) {
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.
Both are same. Let me replace it if it's harder to read.
|
||
// If no retell LLM is associated with the event type, create one | ||
if (!aiPhoneCallConfig.llmId) { | ||
const createdRetellLLM = await retellAI.createRetellLLMAndUpdateWebsocketUrl(); |
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.
All the code in this block statement could be abstracted in function to make code more readable.
E2E results are ready! |
* refactor: V2 API endpoint create phone call * fix: type err * fix: type and build err * chore: change default value * chore: move it to another route * test: for create phone call * chore: undo constant * chore: remove test * fix: make begin_message optional * chore: improvements * chore: begin message * chore: remove unused import * chore: bump platform libraries with handleCreatePhoneCall * chore: improvement --------- Co-authored-by: Keith Williams <keithwillcode@gmail.com> Co-authored-by: Benny Joo <sldisek783@gmail.com> Co-authored-by: Morgan Vernay <morgan@cal.com> Co-authored-by: Peer Richelsen <peeroke@gmail.com>
What does this PR do?
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
DM me for input body and api key
POST:-
/api/v2/organizations/{org.id}/teams/{team.id}/event-types/{eventType.id}/create-phone-call
RETELL_AI_KEY
requiredChecklist