Skip to content
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

Merged
merged 1 commit into from
Feb 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ v1.4.2 [unreleased]
- [#38](https://github.com/latera/camunda-ext/pull/38) Add method for job updating into PlanadoV2 class
- [#33](https://github.com/latera/camunda-ext/pull/33) Allow to pass goodValueId into hid.Hydra#getGoodAddParamsBy
- [#24](https://github.com/latera/camunda-ext/pull/24) Add methods for files to document attaching
- [#39](https://github.com/latera/camunda-ext/pull/39) Add custom fields in create and update methods in PlanadoV2 class

### Bugfixes
- [#27](https://github.com/latera/camunda-ext/pull/27) Remove redundant get methods from hid.Hydra class
- [#27](https://github.com/latera/camunda-ext/pull/27) Remove redundant get methods from hid.Hydra class
- [#28](https://github.com/latera/camunda-ext/pull/28) Fix return types of HID class methods
- [#30](https://github.com/latera/camunda-ext/pull/30) Fix wrong Self-Care app id passing into method calls
- [#26](https://github.com/latera/camunda-ext/pull/26) Fix passing appCode into hid.Hydra#mainInit method
Expand Down
28 changes: 16 additions & 12 deletions src/org/camunda/latera/bss/connectors/PlanadoV2.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,17 @@ class PlanadoV2 {
LinkedHashMap payload = [
external_id : extId,
organization : true,
organization_name : data.companyName ?: '',
organization_name : data.companyName ?: '',
site_address : [
formatted : data.addressStreet ?: '',
entrance_no : data.addressEntrance ?: '',
floor : data.addressFloor ?: '',
apartment : data.addressApartment ?: '',
description : data.addressDescription ?: ''
formatted : data.addressStreet ?: '',
entrance_no : data.addressEntrance ?: '',
floor : data.addressFloor ?: '',
apartment : data.addressApartment ?: '',
description : data.addressDescription ?: ''
],
contacts : [[
name : data.contactName ?: '',
value : data.phone ?: '',
name : data.contactName ?: '',
value : data.phone ?: '',
type : 'phone'
]]
]
Expand All @@ -203,10 +203,11 @@ class PlanadoV2 {
}

LinkedHashMap payload = [
template_id : toIntSafe(data.templateId),
client_id : data.clientId,
scheduled_at : data.startDate,
description : data.description ?: ''
template_id : toIntSafe(data.templateId),
client_id : data.clientId,
scheduled_at : data.startDate,
description : data.description ?: '',
custom_fields : data.customFields ?: []
Copy link
Contributor

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

Copy link
Contributor

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)

Copy link
Contributor

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

Copy link
Contributor

@dolfinus dolfinus Feb 19, 2020

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:

  1. 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 or query arg.
  2. [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.

Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Why next time?

Copy link
Contributor

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

]

logger.info('Creating new job')
Expand All @@ -221,6 +222,9 @@ class PlanadoV2 {
if (data.containsKey('description')) {
description = data.description
}
if (data.containsKey('customFields')) {
custom_fields = data.customFields
}

it
}
Expand Down