-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
Onfleet node #2593
Onfleet node #2593
Conversation
Added Onfleet nodes for working with different endpoints like: organizations, administrators, workers, hubs, teams, destinations, recipients, containers and webhooks.
INT-441: Add Onfleet nodes to n8n
feat: added name field to onfleet trigger node
@ivov hi there, do you know what's the typical lead time to get a new node added? We're anticipating this release and hope to get an estimate of when this will be live. |
Thanks a lot for the contribution @jamesliupenn. The node is one of the next ones that will get reviewed. So should not be much longer. |
Thanks! I realized we missed a few operations so we'll be making some updates tomorrow |
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.
Got reviewed. Let me know if you have questions about any comments. @jamesliupenn
body: any = {}, // tslint:disable-line:no-any | ||
qs?: any, // tslint:disable-line:no-any | ||
uri?: string, | ||
headers: IDataObject = {}, |
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.
Header is a parameter but it's never used in the function?
const apiError = error as IDataObject; | ||
const { message: messageError } = apiError.error as IDataObject; | ||
if (messageError) { | ||
const { message = '', cause = '' } = messageError as IDataObject; |
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 don't you simply pass the error object to the NodeApiError?
value: 'admins', | ||
}, | ||
{ | ||
name: 'Containers', |
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.
Resources names and values need to be singular.
value: 'webhooks', | ||
}, | ||
{ | ||
name: 'Hubs', |
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.
Resources need to be ordered alphabetically
description: INodeTypeDescription = { | ||
displayName: 'Onfleet Trigger', | ||
name: 'onfleetTrigger', | ||
icon: 'file:Onfleet.png', |
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 are two different images? Onfleet and Onfleet@2x? Can you please optimize the png using https://tinypng.com/.
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.
Or preferably use an SVG logo instead.
displayName: 'End', | ||
name: 'end', | ||
type: 'dateTime', | ||
default: Date.now(), |
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.
Set default to ''
|
||
export const workerFields = [ | ||
{ | ||
displayName: 'ID', |
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.
Worker ID
vehicleColorField, | ||
], | ||
}, | ||
{ |
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.
Title case
], | ||
}, | ||
{ | ||
displayName: 'Additional fields', |
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.
Update Fields
{ | ||
displayName: 'Additional fields', | ||
name: 'additionalFields', | ||
type: 'collection', |
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.
The analytics fields need to be optional.
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.
Rename the collection to Options.
Added team auto-dispatch and driver time estimate endpoints to Onfleet node
INT-483: Add missing endpoints to n8n-onfleet node
We'll be prioritizing this in the ongoing sprint, thanks for the feedback @RicardoE105, we'll keep you posted once we finish developing it internally. |
added new fixed collections, refactored the code according to comments made on the n8n pr
added docstrings for new functions addded for formatting the destination and recipient objects
Refactor according to n8n PR comments
@RicardoE105 this is ready for another round of review as we have resolved the issues you mentioned. Please keep us posted👍👍 |
@jamesliupenn perfect. Will review it again as soon as I can. |
@jamesliupenn had a chance to review again. This time I did a deeper review than last time and found the following. Can you please address these issues? Thanks. Onfleet review Admin resource Admin:getAll
Admin:delete
Task resource
Task:create
Task:clone
Task:complete
Task:delete
Task:getAll
Task:get
Container resource
Container:insertTasks
Hub resource Hub:getAll
Hub:update
Team resource Team:auto-dispatch
Team:create
Team:delete
Team:getAll
Team:getTimeStimates
Webhook resource
Worker resource Worker:create
worker:getAll
|
* Fixed some error creating a worker, moving some fields under additional fields collection * Fixed returned values for delete operations, making some changes for style code * Added operational error since required property is not working for dateTime fields
Admin resource
Admin:delete
Task resource
Task:create
Task:clone
Task:complete
Task:delete
Task:getAll
Task:get
Container resource
Container:insertTasks
Hub resource
Hub:update
Team resource
Team:create
Team:delete
Team:getAll
Team:getTimeEstimates
Webhook resource
Worker resource
worker:getAll
|
@YokySantiago to finish up the checklist items, @RicardoE105 this is over to you for review |
In order to complete what James mentioned Task:getAll
Container resource Team:auto-dispatch
Team:create
|
@jamesliupenn @YokySantiago I'm trying to test the team resource, but it says "payment is required". Can you provide a test account with access to the team endpoints? Also, to create a container I need a container ID. Where can I get a container ID from? |
What endpoint on the teams endpoint are you trying to access? If you are hitting specific endpoints like auto dispatch, you will need that feature enabled on your account and I can help you with that. Containers are abstractions of task assignments, so if you assign a task to a worker/org/team, there will be a container created. The container IDs are be available via the worker/org/team ID, so basically depending on which entity you are accessing the container for, use that entity ID. An example would be accessing a team's container, simply use that team ID |
Only get the payment required error when using auto dispatch |
I created an account with my email: [email protected]. If you can enable it for that account would be great. Thanks. |
@RicardoE105 you should be on a test account and has the auto-dispatch endpoint enabled now |
* feat: added Onfleet nodes Added Onfleet nodes for working with different endpoints like: organizations, administrators, workers, hubs, teams, destinations, recipients, containers and webhooks. * style: fixed typos, arrays uniformity, unnecesary files * refactor: changed add to create in comments and labels * feat: added name field to onfleet trigger node * feat: added team endpoints to onfleet node Added team auto-dispatch and driver time estimate endpoints to Onfleet node * style: remove dots in descriptions and fixed some typos * feat: added fixes according to comments made on the n8n PR added new fixed collections, refactored the code according to comments made on the n8n pr * fix: fixed recipient and destination cretion * docs: added docstrings for format some functions added docstrings for new functions addded for formatting the destination and recipient objects * style: formatting the code according to n8n nodelinter * fix: typos and better descriptions * [INT-510] n8n: Address additional problems from n8n code review (#5) * Fixed some error creating a worker, moving some fields under additional fields collection * Fixed returned values for delete operations, making some changes for style code * Added operational error since required property is not working for dateTime fields * ⚡ Improvements to #2593 * ⚡ Improvements * 🐛 Fix issue with wrong interface * ⚡ Improvements * ⚡ Improvements * ⚡ Minor improvement Co-authored-by: Santiago Botero Ruiz <[email protected]> Co-authored-by: ilsemaj <[email protected]> Co-authored-by: Santiago Botero Ruiz <[email protected]> Co-authored-by: Jan Oberhauser <[email protected]>
Thanks a lot for your contribution. Got merged with #2845 |
Got released with |
Preparation
Development
create
,delete
) have been added.true
, keep it astrue
. Changing default values can break the existing workflows of the users.Testing
create
andupdate
operations with all fields/operations.continueOnFail
option with a Function node. (For example, a Widget node has a GET operation that takes a widgetId and returns information on the widget. To test that the workflow continues on fail, set the Widget node to continue on fail, create a Function node, return a valid and an invalid widgetId, connect the Function node to Widget node, and run the workflow. The Widget node should show two items: one with information on the widget and another one with the error from having passed an invalid ID.)Code formatting
editorconfig
.editorconfig
.create
in several cases to be uniform with our API documentation)Errors and Outputs
Create
is consistent withGet
.Get All
is consistent withGet
.Presentation
GetAll
operations have the fields return and limit.Writing
displayNames
are capitalized (i.e.: "IDs", not "ids" or "Ids").description
in the node class is written in camelCase.Branding
Nice-to-haves (optional)
continueOnFail
. This feature is included in some of the newest nodes (e.g Lemlist node) to continue the workflow even if the node's execution fails.false
anddescription: ''
in the node descriptions (e.g., Lemlist node).body
and thenqs
./
(e.g., "/campaign").