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

Consistent list order #348

Merged
merged 5 commits into from
Nov 6, 2019
Merged

Consistent list order #348

merged 5 commits into from
Nov 6, 2019

Conversation

ybro
Copy link
Contributor

@ybro ybro commented Nov 4, 2019

List ordering for ObjectTypes and records (InstanceElements)

@ybro ybro requested review from ori-moisis, hadard and a team November 4, 2019 17:34
@ybro ybro changed the title Initial implementation Consistent list order Nov 4, 2019
packages/salesforce-adapter/src/adapter.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
)
const arrayPropertyToSort = sortFieldInfo.fieldsToSortHierarchy.pop() as string
fieldsToSort.forEach(field => {
field[arrayPropertyToSort] = _.orderBy(
Copy link
Contributor

Choose a reason for hiding this comment

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

To support order of lists of primitives

        field[arrayPropertyToSort] = _.orderBy(
          field[arrayPropertyToSort],
          sortFieldInfo.fieldToSortBy ? sortFieldInfo.fieldToSortBy : undefined
        )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added another method as the object structure is entirely different

packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
packages/salesforce-adapter/src/filters/list_order.ts Outdated Show resolved Hide resolved
return
}
// const sortInfo = _.clone(sortFieldInfo)
instancesToChange.forEach(elem => {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I understand the content of the forEach, I think we can just do:

instancesToChange.forEach(elem => {
  const path =  fieldsToSortHierarchy.join('.') // IMO we can also put in SortField path instead of building it
  const sorted =  _.orderBy(_.get(elem.value, path), sortFieldInfo.fieldToSortBy)
  _.set(elem.value, path, sorted)
})

am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your suggestion doesn't work.
Also, added comments inline, to explain the logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

weird, I don't understand why... if you have time I'll be happy to look on it together. Anyway approving this.

return
}
// const sortInfo = _.clone(sortFieldInfo)
instancesToChange.forEach(elem => {
Copy link
Contributor

Choose a reason for hiding this comment

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

weird, I don't understand why... if you have time I'll be happy to look on it together. Anyway approving this.

@ybro ybro merged commit 7a53a76 into salto-io:master Nov 6, 2019
@ybro ybro deleted the consistent_list_order branch November 6, 2019 08:52
orendig added a commit that referenced this pull request Nov 6, 2019
* Added settings filter

* End help message with EOL (#340)

* removed extension readme badges (#345)

* fixes command title (#339)

* added event listener to file changes in extension (#338)

* added error severity in extension (#344)

* added error severity in extension

* changed event filter out non-bp file changes (#341)

* added wrapper to run aggr set function (fixes SALTO-264) (#342)

* try #1 to fix Capriza failure: "TypeError: Cannot read property 'lookupFilter' of undefined" (#346)

* SALTO-218: apply changes from state to bps post deploy (#343)

* Replaced async-file package (#347)

* upgraded inquirer.js to fix hang in Windows 10
* moved accesses to fs module to centralized file module
* using tmp-promise instead of tmp

* change default fetch behavior to auto approve incoming changes (#350)

* Added settings filter

* Hadar CR

* Consistent list order (#348)

List ordering for specific ObjectTypes and records (InstanceElements)

* missing await in apply changes post deploy (#356)

* added missing await in fetch (#353)

* Consistent list order (#348)

List ordering for specific ObjectTypes and records (InstanceElements)

* missing await in apply changes post deploy (#356)

* Added settings filter

* Hadar CR

* add debug logs (#355)

* blacklisted static resources (#352)

* shouldn't add default ObjectType annotations upon adapter.update (#357)

* Hadar comments

* rename elemId fieldType name to field_name since it collides with a Custom object that named Name and it causes undesired merge of the ObjectTypes (#354)

* Fixed e2e

* Fixed e2e

* Added settings filter

* Hadar CR

* Hadar comments

* Fixed e2e

* Fixed e2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants