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-3176 SALTO-7190 - salesforce - create dependencies based on formulas in metadata instances #7222

Conversation

rotem531
Copy link
Contributor

@rotem531 rotem531 commented Feb 9, 2025

create dependencies based on formulas in metadata instances


_genereated_dependencies will be created for every metadata instance that contains a formula field, if the field's name is in the FORMULA_FIELDS constant.


Release Notes:
Salesforce:

  • generate dependencies of metadata instances by formula fields

User Notifications:
none

@rotem531 rotem531 requested a review from yelly February 9, 2025 13:43
@coveralls
Copy link

coveralls commented Feb 9, 2025

Coverage Status

coverage: 95.764%. first build
when pulling 1df0afe on rotem531:SALTO-3176-salesforce-formula-fields-in-some-types-are-saved-as-string-type
into 22a8a78 on salto-io:main.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks a lot like formula_deps.ts. Can they not be the same filter?

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, "no" is an OK answer, just explain why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both filters at the bottom line parse formulas to create dependencies, but each works on different types of elements and therefore have different approach to how to get to the formula. in addition formulas in custom objects (parsed in formula_dpes) are formula types (have an inner field named formula and other fields), and formulas in metadata instances are just strings that their value happens to be a formula.
this is mainly why @tamtamirr and I decided to have a separate filter.
@tamtamirr also suggested to rename formula_deps to something like custom_object_formula_dependencies, so i will probably change it in the next round of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, let's do the rename (watch out for the adapter config though).

@rotem531 rotem531 requested a review from yelly February 12, 2025 08:53
}

const filter: FilterCreator = ({ config }) => ({
name: 'addFormulaDependencies',
Copy link
Contributor

Choose a reason for hiding this comment

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

I would align the filter name with the file name.

const filter: FilterCreator = ({ config }) => ({
name: 'addFormulaDependencies',
onFetch: ensureSafeFilterFetch({
warningMessage: 'Error while parsing formulas',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add that this is specifically in metadata instances.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here I meant add this to the warning message.

warningMessage: 'Error while parsing formulas',
config,
fetchFilterFunc: async fetchedElements => {
const instanceElements = fetchedElements.filter(isInstanceElement)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why define this? You can just fetchedElements.filter(isInstanceElement).forEach(...).

Comment on lines 21 to 22
const config = { ...defaultFilterContext }
filter = getFormulaDependencies({ config }) as FilterWith<'onFetch'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just:

Suggested change
const config = { ...defaultFilterContext }
filter = getFormulaDependencies({ config }) as FilterWith<'onFetch'>
filter = getFormulaDependencies({ config: defaultFilterContext }) as FilterWith<'onFetch'>

[CORE_ANNOTATIONS.PARENT]: [customObjectTypeMock1],
},
)
const workFlowRule = createInstanceElement(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is one validationRuleInstance and the other workFlowRule? Be consistent.

[CORE_ANNOTATIONS.PARENT]: [customObjectTypeMock1],
},
)
const workFlowRule = createInstanceElement(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's "Workflow" the camel case here should be workflowRule.

})
it('should add dependencies to the instances', async () => {
await filter.onFetch([...instanceElements, ...types])
// eslint-disable-next-line no-underscore-dangle
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of all of these eslint ignores, use instance.annotations[CORE_ANNOTATIONS.GENERATED_DEPENDENCIES].

…176-salesforce-formula-fields-in-some-types-are-saved-as-string-type
@rotem531 rotem531 requested a review from yelly February 12, 2025 16:00
@rotem531 rotem531 merged commit 633bbce into salto-io:main Feb 13, 2025
55 checks passed
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