-
Notifications
You must be signed in to change notification settings - Fork 90
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
SALTO-582: custom obj translation reference fields and validation rules #813
Changes from 7 commits
d17a0cc
d1b24d2
9aac21f
afb212c
9323598
a79fe11
e3a7eb4
3c34d47
4c74c1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
"packages/*" | ||
], | ||
"nohoist": [ | ||
"salto-vscode/**" | ||
"vscode/**" | ||
] | ||
}, | ||
"devDependencies": { | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,87 @@ | ||||||||
/* | ||||||||
* Copyright 2020 Salto Labs Ltd. | ||||||||
* | ||||||||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||
* you may not use this file except in compliance with | ||||||||
* the License. You may obtain a copy of the License at | ||||||||
* | ||||||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||||||
* | ||||||||
* Unless required by applicable law or agreed to in writing, software | ||||||||
* distributed under the License is distributed on an "AS IS" BASIS, | ||||||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||
* See the License for the specific language governing permissions and | ||||||||
* limitations under the License. | ||||||||
*/ | ||||||||
import wu from 'wu' | ||||||||
import { | ||||||||
Element, ElemID, ReferenceExpression, Field, ObjectType, | ||||||||
} from '@salto-io/adapter-api' | ||||||||
import { findInstances, findElements } from '@salto-io/adapter-utils' | ||||||||
import { collections } from '@salto-io/lowerdash' | ||||||||
import _ from 'lodash' | ||||||||
import { logger } from '@salto-io/logging' | ||||||||
import { apiName } from '../transformers/transformer' | ||||||||
import { FilterWith } from '../filter' | ||||||||
import { instanceShortName, instanceParent, customObjectToMetadataTypeInstances } from './utils' | ||||||||
import { SALESFORCE, CUSTOM_OBJECT_TRANSLATION_METADATA_TYPE, VALIDATION_RULES_METADATA_TYPE } from '../constants' | ||||||||
|
||||||||
const log = logger(module) | ||||||||
|
||||||||
const { makeArray } = collections.array | ||||||||
|
||||||||
const FIELDS = 'fields' | ||||||||
const NAME = 'name' | ||||||||
const VALIDATION_RULES = 'validationRules' | ||||||||
|
||||||||
/** | ||||||||
* This filter change CustomObjectTranslation logical references to fields and validation rules to | ||||||||
* Salto referenes | ||||||||
*/ | ||||||||
const filterCreator = (): FilterWith<'onFetch'> => ({ | ||||||||
onFetch: async (elements: Element[]) => { | ||||||||
const allCustomObjectFields = (elemID: ElemID): Iterable<Field> => | ||||||||
wu(findElements(elements, elemID)) | ||||||||
.map(elem => Object.values((elem as ObjectType).fields)) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. casting is dangerous
Suggested change
|
||||||||
.flatten() | ||||||||
|
||||||||
const customToRule = customObjectToMetadataTypeInstances( | ||||||||
elements, VALIDATION_RULES_METADATA_TYPE | ||||||||
) | ||||||||
|
||||||||
wu(findInstances(elements, new ElemID(SALESFORCE, CUSTOM_OBJECT_TRANSLATION_METADATA_TYPE))) | ||||||||
.forEach(customTranslation => { | ||||||||
const customObjectElemId = instanceParent(customTranslation) | ||||||||
if (_.isUndefined(customObjectElemId)) { | ||||||||
log.warn('failed to find custom object for custom translation %s', | ||||||||
apiName(customTranslation)) | ||||||||
return | ||||||||
} | ||||||||
|
||||||||
// Change fields to reference | ||||||||
makeArray(customTranslation.value[FIELDS]).forEach(field => { | ||||||||
const customField = wu(allCustomObjectFields(customObjectElemId)) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: create once as a map of apiName -> elemID instead of iterating all fields every time? |
||||||||
.find(f => apiName(f, true) === field[NAME]) | ||||||||
if (customField) { | ||||||||
field[NAME] = new ReferenceExpression(customField.elemID) | ||||||||
} else { | ||||||||
log.warn('failed to find field %s in %s', field[NAME], customObjectElemId.getFullName()) | ||||||||
} | ||||||||
}) | ||||||||
|
||||||||
// Change validation rules to refs | ||||||||
const objRules = customToRule[customObjectElemId.getFullName()] | ||||||||
makeArray(customTranslation.value[VALIDATION_RULES]).forEach(rule => { | ||||||||
const ruleInstance = objRules?.find(r => instanceShortName(r) === rule[NAME]) | ||||||||
if (ruleInstance) { | ||||||||
rule[NAME] = new ReferenceExpression(ruleInstance.elemID) | ||||||||
} else { | ||||||||
log.warn('failed to validation rule %s for %s', rule[NAME], | ||||||||
customObjectElemId.getFullName()) | ||||||||
} | ||||||||
}) | ||||||||
}) | ||||||||
}, | ||||||||
}) | ||||||||
|
||||||||
export default filterCreator |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
/* | ||
* Copyright 2020 Salto Labs Ltd. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
import { | ||
ObjectType, InstanceElement, Field, BuiltinTypes, ElemID, ReferenceExpression, | ||
INSTANCE_ANNOTATIONS, | ||
} from '@salto-io/adapter-api' | ||
import filterCreator from '../../src/filters/custom_object_translation' | ||
import { | ||
SALESFORCE, CUSTOM_OBJECT_TRANSLATION_METADATA_TYPE, METADATA_TYPE, CUSTOM_OBJECT, API_NAME, | ||
VALIDATION_RULES_METADATA_TYPE, | ||
INSTANCE_FULL_NAME_FIELD, | ||
} from '../../src/constants' | ||
import { FilterWith } from '../../src/filter' | ||
|
||
describe('custom object translation filter', () => { | ||
const customObjName = 'MockCustomObject' | ||
const customFieldName = 'MockField' | ||
const customObjElemID = new ElemID(SALESFORCE, customObjName) | ||
const customObjectAnno = new ObjectType( | ||
{ | ||
elemID: customObjElemID, | ||
annotations: { | ||
[METADATA_TYPE]: CUSTOM_OBJECT, | ||
[API_NAME]: customObjName, | ||
}, | ||
} | ||
) | ||
const customObjectField = new ObjectType( | ||
{ | ||
elemID: customObjElemID, | ||
fields: { | ||
[customFieldName]: new Field(customObjElemID, customFieldName, BuiltinTypes.STRING, | ||
{ [API_NAME]: `${customObjName}.${customFieldName}` }), | ||
}, | ||
} | ||
) | ||
const customObjectAdditionalField = new ObjectType( | ||
{ | ||
elemID: customObjElemID, | ||
fields: { | ||
additional: new Field(customObjElemID, 'additional', BuiltinTypes.STRING, | ||
{ [API_NAME]: `${customObjName}.additional` }), | ||
}, | ||
} | ||
) | ||
const validationRuleName = 'Last_price_must_for_recently_sold' | ||
const validationRuleType = new ObjectType({ | ||
elemID: new ElemID(SALESFORCE, VALIDATION_RULES_METADATA_TYPE), | ||
annotations: { [METADATA_TYPE]: VALIDATION_RULES_METADATA_TYPE }, | ||
}) | ||
const validationRuleInstance = new InstanceElement( | ||
`${customObjName}_${validationRuleName}`, | ||
validationRuleType, | ||
{ [INSTANCE_FULL_NAME_FIELD]: `${customObjName}.${validationRuleName}` }, | ||
undefined, | ||
{ [INSTANCE_ANNOTATIONS.PARENT]: new ReferenceExpression(customObjElemID) } | ||
) | ||
const fakeValidationRuleInstance = new InstanceElement( | ||
`${customObjName}_BLA`, | ||
validationRuleType, | ||
{ [INSTANCE_FULL_NAME_FIELD]: `${customObjName}.BLA` } | ||
) | ||
const objTranslationType = new ObjectType({ | ||
elemID: new ElemID(SALESFORCE, CUSTOM_OBJECT_TRANSLATION_METADATA_TYPE), | ||
}) | ||
const objTranslationInstance = new InstanceElement( | ||
`${customObjName}-en_US`, | ||
objTranslationType, | ||
{ | ||
[INSTANCE_FULL_NAME_FIELD]: `${customObjName}-en_US`, | ||
fields: [{ name: customFieldName }, { name: 'not-exists' }], | ||
validationRules: [{ name: validationRuleName }, { name: 'not-exists' }], | ||
}, | ||
undefined, | ||
{ [INSTANCE_ANNOTATIONS.PARENT]: new ReferenceExpression(customObjElemID) } | ||
) | ||
const objTranslationNoCustomObjInstance = new InstanceElement( | ||
'BLA-en_US', | ||
objTranslationType, | ||
{ | ||
[INSTANCE_FULL_NAME_FIELD]: 'BLA.en_US', | ||
// Use here also single element and not a list | ||
fields: { name: customFieldName }, | ||
validationRules: { name: validationRuleName }, | ||
}, | ||
) | ||
|
||
describe('on fetch', () => { | ||
let postFilter: InstanceElement | ||
let postFilterNoObj: InstanceElement | ||
|
||
beforeAll(async () => { | ||
const filter = filterCreator() as FilterWith<'onFetch'> | ||
const testElements = [ | ||
objTranslationInstance.clone(), | ||
objTranslationNoCustomObjInstance.clone(), | ||
customObjectAnno.clone(), | ||
customObjectField.clone(), | ||
customObjectAdditionalField.clone(), | ||
objTranslationType.clone(), | ||
validationRuleType.clone(), | ||
validationRuleInstance.clone(), | ||
fakeValidationRuleInstance.clone(), | ||
] | ||
await filter.onFetch(testElements) | ||
postFilter = testElements[0] as InstanceElement | ||
postFilterNoObj = testElements[1] as InstanceElement | ||
}) | ||
|
||
describe('fields reference', () => { | ||
it('should transform fields to reference', async () => { | ||
expect(postFilter.value.fields[0].name) | ||
.toEqual(new ReferenceExpression(customObjectField.fields[customFieldName].elemID)) | ||
}) | ||
it('should keep name as is if no referenced field was found', async () => { | ||
expect(postFilter.value.fields[1].name).toBe('not-exists') | ||
}) | ||
|
||
it('should keep name as is if no custom object', async () => { | ||
expect(postFilterNoObj.value.fields.name).toBe(customFieldName) | ||
}) | ||
}) | ||
|
||
describe('validation rules reference', () => { | ||
it('should transform validation rules to reference', async () => { | ||
expect(postFilter.value.validationRules[0].name) | ||
.toEqual(new ReferenceExpression(validationRuleInstance.elemID)) | ||
}) | ||
it('should keep name as is if no referenced validation rules was found', async () => { | ||
expect(postFilter.value.validationRules[1].name).toBe('not-exists') | ||
}) | ||
|
||
it('should keep name as is if no custom object', async () => { | ||
expect(postFilterNoObj.value.validationRules.name).toBe(validationRuleName) | ||
}) | ||
}) | ||
}) | ||
}) |
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.
Seems unrelated to this PR.
maybe a separate commit?
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.
Also, I wonder if it was actually needed so it should be fixed instead of removed?
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.
per my understanding, it should be removed as “nohoist” enables workspaces to consume 3rd-party libraries not yet compatible with its hoisting scheme." and AFAIK we don't have such a case.
Notice that practically it's not working for a long time (not sure if it means something as vscode extension is also not working)
@royra @roironn WDYT?
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.
I think it's needed for packaging of the vscode extension: microsoft/vscode-vsce#300
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.
ok so I'll change
salto-vscode
->vscode
instead of removing