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-7399: advanced fields references #7209

Merged
merged 7 commits into from
Mar 2, 2025
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
25 changes: 11 additions & 14 deletions packages/jira-adapter/e2e_test/adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
import {
Change,
CORE_ANNOTATIONS,
DeployResult,
Element,
getChangeData,
Expand Down Expand Up @@ -67,6 +66,9 @@ const excludedTypes = [
const nullProgressReporter: ProgressReporter = {
reportProgress: () => {},
}

const deleteElementsAtTheEnd = true // when debugging you can change to false to keep the created elements and see the deployed elements in the service

each([
['Cloud', false],
['Data Center', true],
Expand Down Expand Up @@ -250,6 +252,9 @@ each([
})

afterAll(async () => {
if (!deleteElementsAtTheEnd) {
return
}
const removalChangeGroups = addDeployResults
.map(res => res.appliedChanges)
.map(changeGroup =>
Expand Down Expand Up @@ -297,22 +302,14 @@ each([
.filter(isInstanceElement)
.filter(instance => instance.elemID.name.includes('createdByOssE2e'))
.filter(instance => !removalInstancesNames.includes(instance.elemID.getFullName()))
.filter(instance => instance.elemID.typeName !== FIELD_TYPE_NAME || instance.value.isLocked === false) // do not delete locked fields
.filter(
instance =>
instance.elemID.typeName !== FIELD_CONTEXT_TYPE_NAME ||
instance.annotations[CORE_ANNOTATIONS.PARENT]?.[0].value.isLocked === false,
) // do not delete contexts of locked fields
.filter(instance => instance.elemID.typeName !== FIELD_TYPE_NAME || !instance.value.isLocked) // do not delete locked fields, isLocked can also be undefined
.filter(instance => instance.elemID.typeName !== FIELD_CONTEXT_TYPE_NAME) // do not delete contexts as they will be deleted with their fields
.filter(instance => instance.elemID.typeName !== FIELD_CONTEXT_OPTION_TYPE_NAME) // do not delete options, they will be deleted with their contexts
.map(instance => toChange({ before: instance }))

if (!isDataCenter) {
const allRemovalErrors = await deployChanges([allOssCreatedElements], () => true) // do not fail on errors
if (allRemovalErrors.length) {
throw new Error(
`Failed to clean older e2e changes: ${allRemovalErrors.map(e => safeJsonStringify(e)).join(', ')}`,
)
}
const allRemovalErrors = await deployChanges([allOssCreatedElements], () => true) // do not fail on errors
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a part of the E2E where we clean old instances that were not deleted due to aborted E2E. We had problems with DC so we omitted it, but there is no reason as we do not fail the E2E if this stage fails

if (allRemovalErrors.length) {
log.error(`Failed to clean e2e changes: ${allRemovalErrors.map(e => safeJsonStringify(e)).join(', ')}`)
}
})
})
Expand Down
27 changes: 27 additions & 0 deletions packages/jira-adapter/e2e_test/instances/cloud/automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,33 @@ export const createAutomationValues = (name: string, allElements: Element[]): Va
],
conditions: [],
},
{
component: 'ACTION',
type: 'jira.issue.edit',
value: {
operations: [
{
field: {
type: 'ID',
value: createReference(new ElemID(JIRA, FIELD_TYPE_NAME, 'instance', 'Assignee__user'), allElements),
},
fieldType: 'assignee',
type: 'SET',
value: {
type: 'CLEAR',
value: 'clear',
},
},
],
advancedFields: new TemplateExpression({
parts: [
'{\n"update": {\n"',
createReference(new ElemID(JIRA, FIELD_TYPE_NAME, 'instance', 'Sprint__gh_sprint__c@uubuu'), allElements),
'" : [\n{\n"remove": {\n"value":"Pre-ship containment"\n}\n}\n]\n}\n}',
],
}),
},
},
],
canOtherRuleTrigger: false,
notifyOnError: 'FIRSTERROR',
Expand Down
35 changes: 33 additions & 2 deletions packages/jira-adapter/e2e_test/instances/datacenter/automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
*
* CERTAIN THIRD PARTY SOFTWARE MAY BE CONTAINED IN PORTIONS OF THE SOFTWARE. See NOTICE FILE AT https://github.com/salto-io/salto/blob/main/NOTICES
*/
import { ElemID, ReferenceExpression, StaticFile, Values } from '@salto-io/adapter-api'
import { ElemID, ReferenceExpression, StaticFile, TemplateExpression, Values, Element } from '@salto-io/adapter-api'
import * as path from 'path'
import * as fs from 'fs'
import { AUTOMATION_TYPE, JIRA } from '../../../src/constants'
import { FIELD_TYPE_NAME } from '../../../src/filters/fields/constants'
import { createReference } from '../../utils'

export const createAutomationValues = (name: string): Values => ({
export const createAutomationValues = (name: string, allElements: Element[]): Values => ({
name,
state: 'ENABLED',
canOtherRuleTrigger: false,
Expand Down Expand Up @@ -222,6 +223,36 @@ export const createAutomationValues = (name: string): Values => ({
optimisedIds: [],
newComponent: false,
},
{
component: 'ACTION',
type: 'jira.issue.edit',
value: {
operations: [
{
fieldId: 'assignee',
fieldType: 'assignee',
type: 'SET',
value: {
type: 'CLEAR',
value: 'clear',
},
},
],
advancedFields: new TemplateExpression({
parts: [
'{\n"update": {\n"',
createReference(new ElemID(JIRA, FIELD_TYPE_NAME, 'instance', 'Sprint__gh_sprint__c@uubuu'), allElements),
'" : [\n{\n"remove": {\n"value":"Pre-ship containment"\n}\n}\n]\n}\n}',
],
}),
sendNotifications: true,
useLegacyRendering: false,
},
children: [],
conditions: [],
optimisedIds: [],
newComponent: false,
},
],
labels: [],
})
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const createInstances = (randomString: string, fetchedElements: Element[]
const automation = new InstanceElement(
randomString,
findType(AUTOMATION_TYPE, fetchedElements),
createAutomationValues(randomString),
createAutomationValues(randomString, fetchedElements),
)

const workflow = new InstanceElement(
Expand Down
4 changes: 4 additions & 0 deletions packages/jira-adapter/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type JiraFetchConfig = definitions.UserFetchConfig<{ fetchCriteria: JiraFetchFil
splitFieldContextOptions?: boolean
enableRequestTypeFieldNameAlignment?: boolean
removeFieldConfigurationDefaultValues?: boolean
walkOnReferences?: boolean
remove10KOptionsContexts: boolean
}

Expand Down Expand Up @@ -187,6 +188,7 @@ const PARTIAL_DEFAULT_CONFIG: Omit<JiraConfig, 'apiDefinitions'> = {
removeFieldConfigurationDefaultValues: false,
splitFieldContextOptions: true,
remove10KOptionsContexts: false, // starting value, to be changed
walkOnReferences: true,
},
deploy: {
forceDelete: false,
Expand Down Expand Up @@ -355,6 +357,7 @@ const fetchConfigType = definitions.createUserFetchConfigType({
enableRequestTypeFieldNameAlignment: { refType: BuiltinTypes.BOOLEAN },
removeFieldConfigurationDefaultValues: { refType: BuiltinTypes.BOOLEAN },
remove10KOptionsContexts: { refType: BuiltinTypes.BOOLEAN },
walkOnReferences: { refType: BuiltinTypes.BOOLEAN },
},
fetchCriteriaType: fetchFiltersType,
omitElemID: true,
Expand Down Expand Up @@ -427,6 +430,7 @@ export const configType = createMatchingObjectType<Partial<JiraConfig>>({
'fetch.parseAdditionalAutomationExpressions',
'fetch.enableRequestTypeFieldNameAlignment',
'fetch.removeFieldConfigurationDefaultValues',
'fetch.walkOnReferences',
'deploy.taskMaxRetries',
'deploy.taskRetryDelay',
'deploy.ignoreMissingExtensions',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type Component = {
children?: Component[]
}

type AutomationInstance = InstanceElement & {
export type AutomationInstance = InstanceElement & {
value: {
trigger: Component
components?: Component[]
Expand All @@ -65,7 +65,7 @@ const AUTOMATION_INSTANCE_SCHEME = Joi.object({
}).unknown(true),
}).unknown(true)

const isAutomationInstance = createSchemeGuard<AutomationInstance>(
export const isAutomationInstance = createSchemeGuard<AutomationInstance>(
AUTOMATION_INSTANCE_SCHEME,
'Received an invalid automation',
)
Expand Down
42 changes: 42 additions & 0 deletions packages/jira-adapter/src/filters/automation/walk_on_automation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2025 Salto Labs Ltd.
* Licensed under the Salto Terms of Use (the "License");
* You may not use this file except in compliance with the License. You may obtain a copy of the License at https://www.salto.io/terms-of-use
*
* CERTAIN THIRD PARTY SOFTWARE MAY BE CONTAINED IN PORTIONS OF THE SOFTWARE. See NOTICE FILE AT https://github.com/salto-io/salto/blob/main/NOTICES
*/
import { WALK_NEXT_STEP, WalkOnFunc, walkOnValue } from '@salto-io/adapter-utils'
import { InstanceElement, Value } from '@salto-io/adapter-api'
import { AutomationInstance, isAutomationInstance } from './smart_values/smart_value_reference_filter'

const FIELDS_TO_WALK_ON = (): string[] => ['trigger', 'components']
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it should be a constant and not a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something I learned from Seggev, they use it in Netsuite.
As a constant array can be changed (it is not really a constant) when creating a constant array if you use a function the original array cannot be changed

Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

const ADVANCED_FIELDS = 'advancedFields'

export const walkOnAutomation = ({ instance, func }: { instance: AutomationInstance; func: WalkOnFunc }): void => {
FIELDS_TO_WALK_ON().forEach(fieldName => {
if (instance.value[fieldName] !== undefined) {
walkOnValue({
elemId: instance.elemID.createNestedID(fieldName),
value: instance.value[fieldName],
func,
})
}
})
}

export const walkOnAutomations = (instances: InstanceElement[], func: WalkOnFunc): void => {
instances.filter(isAutomationInstance).forEach(instance => walkOnAutomation({ instance, func }))
}

export const advancedFieldsReferenceFunc =
(func: (value: Value, fieldName: string) => void): WalkOnFunc =>
({ value }): WALK_NEXT_STEP => {
if (value == null) {
return WALK_NEXT_STEP.SKIP
}
if (value[ADVANCED_FIELDS] !== undefined) {
func(value, ADVANCED_FIELDS)
return WALK_NEXT_STEP.SKIP
}
return WALK_NEXT_STEP.RECURSE
}
37 changes: 30 additions & 7 deletions packages/jira-adapter/src/filters/field_references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,52 @@
*
* CERTAIN THIRD PARTY SOFTWARE MAY BE CONTAINED IN PORTIONS OF THE SOFTWARE. See NOTICE FILE AT https://github.com/salto-io/salto/blob/main/NOTICES
*/
import { Element, isInstanceElement } from '@salto-io/adapter-api'
import { InstanceElement, isInstanceElement } from '@salto-io/adapter-api'
import { references as referenceUtils } from '@salto-io/adapter-components'
import _ from 'lodash'
import { referencesRules, JiraFieldReferenceResolver, contextStrategyLookup } from '../reference_mapping'
import { FilterCreator } from '../filter'
import { FIELD_CONFIGURATION_TYPE_NAME, PROJECT_COMPONENT_TYPE } from '../constants'
import { AUTOMATION_TYPE, FIELD_CONFIGURATION_TYPE_NAME, PROJECT_COMPONENT_TYPE } from '../constants'
import { JiraConfig } from '../config/config'
import { FIELD_TYPE_NAME } from './fields/constants'
import { advancedFieldsReferenceFunc, walkOnAutomations } from './automation/walk_on_automation'
import { addFieldsTemplateReferences } from './fields/reference_to_fields'

/**
* Convert field values into references, based on predefined rules.
*/

const noReferencesTypes = [PROJECT_COMPONENT_TYPE, FIELD_CONFIGURATION_TYPE_NAME]
const NO_REFERENCES_TYPES = (): string[] => [PROJECT_COMPONENT_TYPE, FIELD_CONFIGURATION_TYPE_NAME]
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need it as a function?
I agree that it should be a constant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as I answered in the previous comment


const addWalkOnReferences = (instances: InstanceElement[], config: JiraConfig): void => {
if (!config.fetch.walkOnReferences) {
return
}
const fieldInstances = instances.filter(instance => instance.elemID.typeName === FIELD_TYPE_NAME)
const fieldInstancesById = new Map(
fieldInstances
.filter(instance => typeof instance.value.id === 'string')
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
I guess you prefer is that way, but IMO _.isString is cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to go with a natural solution over a package one

.map(instance => [instance.value.id, instance] as [string, InstanceElement]),
)
walkOnAutomations(
instances.filter(instance => instance.elemID.typeName === AUTOMATION_TYPE),
advancedFieldsReferenceFunc(
addFieldsTemplateReferences(fieldInstancesById, config.fetch.enableMissingReferences ?? true),
),
)
}

const filter: FilterCreator = ({ config }) => ({
name: 'fieldReferencesFilter',
onFetch: async (elements: Element[]) => {
onFetch: async elements => {
const instances = elements.filter(isInstanceElement)
addWalkOnReferences(instances, config)

const fixedDefs = referencesRules.map(def =>
config.fetch.enableMissingReferences ? def : _.omit(def, 'missingRefStrategy'),
)
// Remove once SALTO-6889 is done: ProjectComponents have no references, so don't need to scan them
const relevantElements = elements
.filter(isInstanceElement)
.filter(instance => !noReferencesTypes.includes(instance.elemID.typeName))
const relevantElements = instances.filter(instance => !NO_REFERENCES_TYPES().includes(instance.elemID.typeName))
await referenceUtils.addReferences({
elements: relevantElements,
contextElements: elements,
Expand Down
46 changes: 46 additions & 0 deletions packages/jira-adapter/src/filters/fields/reference_to_fields.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2025 Salto Labs Ltd.
* Licensed under the Salto Terms of Use (the "License");
* You may not use this file except in compliance with the License. You may obtain a copy of the License at https://www.salto.io/terms-of-use
*
* CERTAIN THIRD PARTY SOFTWARE MAY BE CONTAINED IN PORTIONS OF THE SOFTWARE. See NOTICE FILE AT https://github.com/salto-io/salto/blob/main/NOTICES
*/

import { ElemID, InstanceElement, ReferenceExpression, TemplateExpression, Value } from '@salto-io/adapter-api'
import { references as referenceUtils } from '@salto-io/adapter-components'
import { extractTemplate } from '@salto-io/adapter-utils'
import { referenceFunc } from '../script_runner/walk_on_scripts'
import { JIRA } from '../../constants'
import { FIELD_TYPE_NAME } from './constants'

const CUSTOM_FIELD_PATTERN = /(customfield_\d+)/

const referenceCustomFields = ({
text,
fieldInstancesById,
enableMissingReferences,
}: {
text: string
fieldInstancesById: Map<string, InstanceElement>
enableMissingReferences: boolean
}): TemplateExpression | string =>
extractTemplate(text, [CUSTOM_FIELD_PATTERN], expression => {
if (!expression.match(CUSTOM_FIELD_PATTERN)) {
return expression
}
const instance = fieldInstancesById.get(expression)
if (instance !== undefined) {
return new ReferenceExpression(instance.elemID, instance)
}
return enableMissingReferences
? referenceUtils.createMissingValueReference(new ElemID(JIRA, FIELD_TYPE_NAME, 'instance'), expression)
: expression
})

export const addFieldsTemplateReferences =
(fieldInstancesById: Map<string, InstanceElement>, enableMissingReferences: boolean): referenceFunc =>
(value: Value, fieldName: string): void => {
if (typeof value[fieldName] === 'string') {
value[fieldName] = referenceCustomFields({ text: value[fieldName], fieldInstancesById, enableMissingReferences })
}
}
Loading