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

SALTO-582: custom obj translation reference fields and validation rules #813

Merged
merged 9 commits into from
Mar 12, 2020

Conversation

hadard
Copy link
Contributor

@hadard hadard commented Mar 11, 2020

No description provided.

Comment on lines 73 to 77
_.clone(objTranslationInstance),
_.clone(customObject),
_.clone(objTranslationType),
_.clone(validationRuleType),
_.clone(validationRuleInstance),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should probably use element.clone() instead of _.clone(element).

@codecov
Copy link

codecov bot commented Mar 12, 2020

Codecov Report

Merging #813 into master will increase coverage by 0.06%.
The diff coverage is 98.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #813      +/-   ##
==========================================
+ Coverage   89.27%   89.33%   +0.06%     
==========================================
  Files         204      205       +1     
  Lines        9168     9220      +52     
  Branches     1634     1640       +6     
==========================================
+ Hits         8185     8237      +52     
  Misses        983      983
Flag Coverage Δ
#unit_test 89.33% <98.33%> (+0.06%) ⬆️
Impacted Files Coverage Δ
packages/adapter-api/src/elements.ts 86.79% <0%> (ø) ⬆️
packages/salesforce-adapter/src/adapter.ts 96.88% <100%> (ø) ⬆️
packages/salesforce-adapter/src/filters/utils.ts 86.9% <100%> (+1.97%) ⬆️
packages/salesforce-adapter/src/constants.ts 100% <100%> (ø) ⬆️
...s/salesforce-adapter/src/filters/custom_objects.ts 96.98% <100%> (-0.02%) ⬇️
...e-adapter/src/filters/custom_object_translation.ts 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fc6522...4c74c1a. Read the comment docs.

@@ -5,9 +5,6 @@
"workspaces": {
"packages": [
"packages/*"
],
"nohoist": [
"salto-vscode/**"
Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link

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

Copy link
Contributor Author

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

@hadard hadard changed the title custom obj translation reference fields and validation rules SALTO-582: custom obj translation reference fields and validation rules Mar 12, 2020

const apiNameToCustomObject = generateApiNameToCustomObject(elements)
const apiNameToRules = _.groupBy(
getInstancesOfMetadataType(elements, VALIDATION_RULES_METADATA_TYPE),
ruleObj
customObjectApiName
Copy link
Contributor

Choose a reason for hiding this comment

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

this function is being applied on things that are not a custom object, is this a bug?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand now what this is supposed to be.
If I am right the name is very confusing. maybe rename customObjectApiName -> parentApiName and instanceShortName -> relativeApiName?
Then it can be used on field api names as well

onFetch: async (elements: Element[]) => {
const allCustomObjectFields = (elemID: ElemID): Iterable<Field> =>
wu(findElements(elements, elemID))
.map(elem => Object.values((elem as ObjectType).fields))
Copy link
Contributor

Choose a reason for hiding this comment

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

casting is dangerous

Suggested change
.map(elem => Object.values((elem as ObjectType).fields))
.filter(isObjectType)
.map(elem => Object.values(elem).fields))


// Change fields to reference
makeArray(customTranslation.value[FIELDS]).forEach(field => {
const customField = wu(allCustomObjectFields(customObjectElemId))
Copy link
Contributor

Choose a reason for hiding this comment

The 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?
same applies to the next loop over validation rules

@hadard hadard merged commit f987a7d into salto-io:master Mar 12, 2020
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.

3 participants