-
Notifications
You must be signed in to change notification settings - Fork 1
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
Customer.io Trigger Node #44
Conversation
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.
Nice! I am having trouble w tests but here is some initial feedback
@@ -19,8 +19,8 @@ export async function awsApiRequest(this: IHookFunctions | IExecuteFunctions | I | |||
const endpoint = `${service}.${credentials.region}.amazonaws.com`; | |||
|
|||
// Sign AWS API request with the user credentials | |||
const signOpts = {headers: headers || {}, host: endpoint, method, path, body}; | |||
sign(signOpts, {accessKeyId: `${credentials.accessKeyId}`, secretAccessKey: `${credentials.secretAccessKey}`}); | |||
const signOpts = { headers: headers || {}, host: endpoint, method, path, body }; |
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.
Make sure not to change other files in your PRs!
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.
oh ya I'll make sure to remove it
import { OptionsWithUri } from 'request'; | ||
import { IDataObject } from 'n8n-workflow'; | ||
|
||
export async function apiRequest(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions, method: string, endpoint: string, body: object, query?: IDataObject): Promise<any> { // tslint:disable-line:no-any |
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 go w codebase conventions, name this customerIoApiRequest
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.
ya, I'll make the changes
}, | ||
method, | ||
body, | ||
qs: query, |
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.
name the param qs so that you can write qs,
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.
yes!
@@ -1,19 +1,19 @@ | |||
import { | |||
IHookFunctions, | |||
IWebhookFunctions, | |||
} from 'n8n-core'; | |||
} from 'n8n-core'; |
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.
Also make sure to leave out this file!
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.
Hey @Shraddha2104! Small details.
Linter errors:
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:242:9 - Identifier 'events_list' is never reassigned; use 'const' instead of 'var'.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:242:9 - variable name must be in lowerCamelCase or UPPER_CASE
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:242:39 - Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:244:6 - Forbidden 'var' keyword, use 'let' or 'const' instead
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:244:10 - Identifier 'obj' is never reassigned; use 'const' instead of 'var'.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:245:6 - Forbidden 'var' keyword, use 'let' or 'const' instead
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:245:10 - Identifier 'key' is never reassigned; use 'const' instead of 'var'.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:246:6 - Forbidden 'var' keyword, use 'let' or 'const' instead
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:246:10 - Identifier 'val' is never reassigned; use 'const' instead of 'var'.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:249:7 - Forbidden 'var' keyword, use 'let' or 'const' instead
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:249:11 - Identifier 'value' is never reassigned; use 'const' instead of 'var'.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:253:7 - Forbidden 'var' keyword, use 'let' or 'const' instead
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/CustomerIo/CustomerIoTrigger.node.ts:253:11 - Identifier 'obj1' is never reassigned; use 'const' instead of 'var'.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/Google/Task/GenericFunctions.ts:19:8 - Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/Google/Task/GenericFunctions.ts:23:12 - Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/Google/Task/GenericFunctions.ts:68:8 - Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type.
ERROR: C:/Users/IvanOv/Desktop/third/n8n/packages/nodes-base/nodes/Google/Task/GenericFunctions.ts:70:12 - Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type.
} catch (err) { | ||
if (err.statusCode === 404) { | ||
return false; | ||
} | ||
throw new Error(`Customer.io Error: ${err}`); |
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.
err
→ error
const webhookUrl = this.getNodeWebhookUrl('default'); | ||
const events = this.getNodeParameter('events', []) as string[]; | ||
const endpoint = '/reporting_webhooks'; | ||
var events_list: { [key: string]: any } = {}; |
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.
camelCase
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.
let
or const
var obj = event.split('.'); //push.attempted | ||
var key = obj[0]; //push | ||
var val = obj[1]; //attempted | ||
|
||
if (events_list.hasOwnProperty(key)) { | ||
var value = events_list[key]; | ||
value[val] = true; | ||
} | ||
else { | ||
var obj1: { [val: string]: boolean; } = { [val]: true }; //{'attempted':true} | ||
events_list[key] = obj1; | ||
} |
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.
Would it be possible to rename obj
, key
and val
using the API names pushAttempted
, push
and attempted
?
} catch (e) { | ||
throw e; | ||
} |
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.
e
→ error
const endpoint = `/reporting_webhooks/${webhookData.webhookId}`; | ||
try { | ||
await customerIoApiRequest.call(this, 'DELETE', endpoint, {}, {}); | ||
} catch (e) { |
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.
e
→ error
import { OptionsWithUri } from 'request'; | ||
import { IDataObject } from 'n8n-workflow'; |
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.
Staggered lines and trailing commas.
Submitted to maintainers here! |
Hi,
✨ Added a Customer.io trigger node.
Let me know if it can be refactored anywhere!