-
Notifications
You must be signed in to change notification settings - Fork 4
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
HCX-10 Add custom fields in create and update in PlanadoV2 class #39
Conversation
client_id : data.clientId, | ||
scheduled_at : data.startDate, | ||
description : data.description ?: '', | ||
custom_fields : data.customFields ?: [] |
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.
This requires to pass custom fields as list:
Map newJob = planadl.createJob(
templateId: ...,
clientId: ...,
customFields: [[
name: 'someField',
value: 'Some value'
]])
But it's really more convenient better to pass custom fields as Map:
Map newJob = planadl.createJob(
templateId: ...,
clientId: ...,
customFields: [
someField: 'Some value'
])
It's how current integration with OTRS or Hydra's REST API working
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 have list of hashes in Planado REST, why don't we pass it? (Option 1)
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.
@dolfinus, if option 1 works properly then we use it
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.
Current solution is not really wrong or not working, but I think if we are making connectors classes which are going to help with implementing new business processes:
- They should not be just wrappers on API calls without any logic. If you need to call some API at once, you can simply use HTTPRestProcessor and pass everything you want as
body
orquery
arg. - [key: value] as a way to pass custom fields which is used in all classes of this project. It's quite strange to see in one of them another approach. It's better to have some similarities between different connectors for faster learning.
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.
okay, maybe we do it next time
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 next time?
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.
thanks a lot for your help with our codebase, let it go now
No description provided.