personio, hibob (new) Connector contribution program [Maesn]#167
personio, hibob (new) Connector contribution program [Maesn]#167lennart-sve wants to merge 6 commits intoAppmixer-ai:devfrom
Conversation
jirihofman
left a comment
jirihofman
left a comment
There was a problem hiding this comment.
Thank you for your contribution @lennart-sve, much appreciated! This looks very promising. I made some suggestions to bring the code style more in line with our codebase.
There might be some changes required further. Are you ok with us pushing into this PR? One example is GetEmployees component. For a listing components we expect a standard set of outputTypes (object, first, array and file) but we haven't made these standards public yet. See example here.
| @@ -0,0 +1,75 @@ | |||
| 'use strict'; | |||
| const Promise = require('bluebird'); | |||
There was a problem hiding this comment.
Please remove bluebird dependency.
|
|
||
|
|
There was a problem hiding this comment.
Please remove empty lines.
| await Promise.map(diff, employee => { | ||
| // TODO: Add logging here | ||
| return context.sendJson({ employee }, 'out'); | ||
| }); |
There was a problem hiding this comment.
Use native await Promise.all and not bluebird's Promise.map.
| /** | ||
| * Component which triggers whenever an employee is deleted. | ||
| */ | ||
| class DeletedEmployee { |
There was a problem hiding this comment.
Is there a reason for defining a class and later exporting it? Can it be simplified instead like this?
| class DeletedEmployee { | |
| module.exports = { |
There was a problem hiding this comment.
The reason for this is simply that it was required for our logging framework, which I removed from the components as it would not be relevant to you
There was a problem hiding this comment.
This file can be removed completely:
axios->context.httpRequestbluebird-> node Promiserequest-promise- not used
| } catch (error) { | ||
| // TODO: Add logging here | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
Either add a more detailed logging (eg context.log) or remove the try/catch and Appmixer will handle the exception itself and do retries.
There was a problem hiding this comment.
Please rename this to lib.js.
| @@ -0,0 +1,10 @@ | |||
| { | |||
| "name": "maesn.hibob", | |||
There was a problem hiding this comment.
Appmixer vendor is expected when the connector is in src/appmixer folder.
| "name": "maesn.hibob", | |
| "name": "appmixer.hibob", |
| @@ -0,0 +1,25 @@ | |||
| { | |||
| "name": "maesn.hibob.employees.DeletedEmployee", | |||
There was a problem hiding this comment.
Appmixer vendor is expected when the connector is in src/appmixer folder.
| "name": "maesn.hibob.employees.DeletedEmployee", | |
| "name": "appmixer.hibob.employees.DeletedEmployee", |
| "name": "maesn.personio", | ||
| "label": "Personio", | ||
| "category": "applications", | ||
| "description": "Personio is an HR system ", |
There was a problem hiding this comment.
| "description": "Personio is an HR system ", | |
| "description": "Personio is an HR system.", |
No problem at all, feel free to make any changes you deem necessary. We would then wait for the appmixer version and branch them of with our changes. |
DavidDurman
changed the title
3 participants
No description provided.