Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

INT-441: Add Onfleet nodes to n8n #1

Merged
merged 3 commits into from
Dec 21, 2021

Conversation

YokySantiago
Copy link

Added Onfleet nodes for working with different endpoints like: organizations, administrators, workers, hubs, teams,
destinations, recipients, containers and webhooks.

Added Onfleet nodes for working with different endpoints like:
organizations, administrators, workers, hubs, teams, destinations, recipients,
containers and webhooks.
@jamesliupenn jamesliupenn self-requested a review December 15, 2021 17:50
Copy link

@jamesliupenn jamesliupenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good so far, just some nitpicking and typos to fix especially language-wise.

packages/nodes-base/nodes/Onfleet/Onfleet.node.json Outdated Show resolved Hide resolved
packages/nodes-base/nodes/Onfleet/Onfleet.node.json Outdated Show resolved Hide resolved
packages/nodes-base/nodes/Onfleet/Onfleet.node.json Outdated Show resolved Hide resolved
packages/nodes-base/nodes/Onfleet/Onfleet.node.ts Outdated Show resolved Hide resolved
packages/nodes-base/nodes/Onfleet/Onfleet.node.ts Outdated Show resolved Hide resolved
packages/nodes-base/nodes/Onfleet/Onfleet.node.ts Outdated Show resolved Hide resolved
@jamesliupenn jamesliupenn changed the title feat: added Onfleet nodes INT-441: Add Onfleet nodes to n8n Dec 16, 2021
@jamesliupenn
Copy link

Will be working on functional testing as well today, I'll comment on the JIRA ticket for any additional change requests

@jamesliupenn jamesliupenn self-requested a review December 20, 2021 17:27
Copy link

@jamesliupenn jamesliupenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some final nitpicking

@@ -595,14 +591,14 @@ export class Onfleet implements INodeType {
try {
if (operation === 'create') {
/* -------------------------------------------------------------------------- */
/* Add new task */
/* Add a new task */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be create a new task

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed "add" to "create"

/* -------------------------------------------------------------------------- */
responseData.push(...await onfleetApiRequest.call(this, 'GET', encodedApiKey, resource));
} else if (operation === 'create') {
/* -------------------------------------------------------------------------- */
/* Add new admin */
/* Add a new admin */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a new admin

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed "add" to "create"

@@ -894,7 +890,7 @@ export class Onfleet implements INodeType {
responseData.push(...await onfleetApiRequest.call(this, 'GET', encodedApiKey, resource));
} else if (operation === 'create') {
/* -------------------------------------------------------------------------- */
/* Add new hub */
/* Add a new hub */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed "add" to "create"

@@ -959,7 +955,7 @@ export class Onfleet implements INodeType {
responseData.push(await onfleetApiRequest.call(this, 'GET', encodedApiKey, path, {}, workerFilters));
} else if (operation === 'create') {
/* -------------------------------------------------------------------------- */
/* Add new worker */
/* Add a new worker */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed "add" to "create"

},
},
options: [
{
name: 'Add',
value: 'create',
description: 'Add new Onfleet admin.',
description: 'Add a new Onfleet admin.',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed "add" to "create"

Copy link

@jamesliupenn jamesliupenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else looks great, except for something that probably I failed to communicate. We should update the language to say Create a <Onfleet_object> instead of Add a <Onfleet_object> so we stay true to ourselves. I see that the value has been updated but the displayed language to n8n users still uses add

You can see https://docs.onfleet.com/reference#workers & https://docs.onfleet.com/reference#create-team, we all specify Create a worker or Create a team instead of Add.

Once fixed, I think I will merge this into master ;)

IGNORE me, I didn't see the latest commit, this is approved

@jamesliupenn jamesliupenn self-requested a review December 21, 2021 19:45
@jamesliupenn jamesliupenn merged commit 1c4a6ab into onfleet:master Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants