Feature/1681 reverse dependency lookup for deploy#2547
Feature/1681 reverse dependency lookup for deploy#2547yuliialikhyt wants to merge 11 commits intodevelopfrom
Conversation
Coverage ReportCommit:0e10944Base: develop@f69499e Details (changed files):
|
There was a problem hiding this comment.
Pull request overview
This PR extends the existing --refresh deploy behavior so that refreshing triggered send definitions also works when deploying/updating Content Builder blocks (and other non-email assets) by reverse-looking-up emails that reference those blocks.
Changes:
- Replaces the post-deploy refresh hook to call a new
Asset.refresh()entrypoint instead of refreshing only when the deployed asset is an email. - Adds reverse dependency lookup for blocks via an Asset query (
MUSTCONTAIN) to find emails referencing the updated block, then refreshes related TriggeredSendDefinitions. - Updates/extends the test suite and mock REST/SOAP fixtures to cover the new refresh behavior and request patterns.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/metadataTypes/Asset.js | Implements Asset.refresh() and _findEmailsUsingBlock() and wires refresh into postDeployTasks() |
| @types/lib/metadataTypes/Asset.d.ts | Updates typings to include new helper and refresh() return type |
| @types/lib/metadataTypes/Asset.d.ts.map | Regenerated source map for updated typings |
| test/resourceFactory.js | Extends request-to-fixture mapping to support MUSTCONTAIN query operator in POST body |
| test/type.asset.test.js | Adds test case for deploying a block with refresh: true and validates request count |
| test/type.triggeredSend.test.js | Extends expected refreshed triggered send keys and request count |
| test/type.journey.test.js | Updates expected journey count and request count due to additional mocked interaction |
| test/resources/9999999/triggeredSendDefinition/retrieve-TriggeredSendStatusINdummy,Active-response.xml | Adds an additional TriggeredSendDefinition result to mock “Active” retrieval |
| test/resources/9999999/triggeredSendDefinition/retrieve-CustomerKey=testExistingRefresh_triggeredSend-response.xml | New SOAP fixture for retrieving a specific TriggeredSendDefinition by CustomerKey |
| test/resources/9999999/interaction/v1/interactions/key_testExisting_email_block_refresh/get-response.json | New REST fixture for a specific published interaction containing the triggered send key |
| test/resources/9999999/interaction/v1/interactions/get-response-status=Published.json | Extends published interaction listing to include the new interaction |
| test/resources/9999999/asset/v1/content/assets/query/post-response-contentMUSTCONTAINtestExisting_block_refresh.json | New REST fixture for reverse lookup of emails referencing a block key |
| test/resources/9999999/asset/v1/content/assets/query/post-response-assetType.idIN219,220,221,222,223,224,225,226,227,228,230,232,240,241,242,243,244,245.json | Extends existing asset query fixture to include the new email asset |
| test/resources/9999999/asset/v1/content/assets/334322/patch-response.json | New REST fixture for patching the updated block asset |
| test/mockRoot/deploy/testInstance/testBU/asset/block/testExisting_block_refresh.asset-block-meta.json | Adds mock deploy metadata for the test block asset |
| test/mockRoot/deploy/testInstance/testBU/asset/block/testExisting_block_refresh.asset-block-meta.html | Adds mock deploy HTML content for the test block asset |
| toPublish.push(...Object.values(await this._findEmailsUsingBlock(codeBlockKeys))); | ||
| } | ||
| Util.logger.info(` - Found ${toPublish.length} assets to publish`); | ||
| return toPublish.length ? await this._refreshTriggeredSend(toPublish) : []; |
There was a problem hiding this comment.
refresh() passes toPublish (an array) into _refreshTriggeredSend(), but _refreshTriggeredSend() expects a MetadataTypeMap keyed by customerKey and iterates Object.keys(metadata) to access items. With an array, keys become "0", "1", ... which is brittle and can break if callers ever pass a non-array iterable. Consider normalizing toPublish into an object keyed by customerKey (and ideally de-dupe) before calling _refreshTriggeredSend(), or update _refreshTriggeredSend() to explicitly accept an array and iterate items directly.
| return toPublish.length ? await this._refreshTriggeredSend(toPublish) : []; | |
| const toPublishMap = | |
| toPublish.length > 0 | |
| ? toPublish.reduce( | |
| /** | |
| * @param {MetadataTypeMap} acc | |
| * @param {MetadataTypeItem} item | |
| * @returns {MetadataTypeMap} | |
| */ | |
| (acc, item) => { | |
| if (item && item.customerKey) { | |
| acc[item.customerKey] = item; | |
| } | |
| return acc; | |
| }, | |
| /** @type {MetadataTypeMap} */ ({}) | |
| ) | |
| : /** @type {MetadataTypeMap} */ ({}); | |
| return Object.keys(toPublishMap).length | |
| ? await this._refreshTriggeredSend(toPublishMap) | |
| : []; |
| static async refresh(keyArr) { | ||
| const codeBlockKeys = []; | ||
| const toPublish = []; | ||
| Util.logger.info(' - Caching dependent Metadata: asset'); | ||
| const assets = await this.retrieveForCache(undefined, null, undefined, false); | ||
| cache.mergeMetadata('asset', assets.metadata); | ||
| Util.logger.info(' - Caching dependent Metadata: shared-asset'); | ||
| const sharedAssets = await this.retrieveForCache(undefined, null, undefined, true); | ||
| cache.mergeMetadata('asset', sharedAssets.metadata); | ||
| for (const key of keyArr) { | ||
| const item = cache.getByKey(this.definition.type, key); | ||
| if (!item) { |
There was a problem hiding this comment.
refresh() declares keyArr as optional in the JSDoc/typings, but the implementation does for (const key of keyArr) which will throw if keyArr is undefined (e.g., if a caller invokes refresh without keys). Either make the parameter required everywhere (update docs/types) or add a default/guard (treat missing keys as an empty list or throw a clear error).
| payload.query = { | ||
| property: 'id', | ||
| simpleOperator: 'greaterThan', | ||
| value: items.at(-1).id, | ||
| }; | ||
| lastPage = 0; | ||
| moreResults = true; |
There was a problem hiding this comment.
In _findEmailsUsingBlock(), the pagination fallback for "all shards failed" sets value: items.at(-1).id. If the first response triggers this condition and returns no items, items.at(-1) is undefined and this will throw, aborting the refresh. Add a guard (e.g., only switch to the greaterThan strategy once at least one item has been collected, otherwise log/return).
| payload.query = { | |
| property: 'id', | |
| simpleOperator: 'greaterThan', | |
| value: items.at(-1).id, | |
| }; | |
| lastPage = 0; | |
| moreResults = true; | |
| if (!items.length) { | |
| // cannot apply greaterThan pagination without at least one item | |
| Util.logger.warn( | |
| 'Asset._findEmailsUsingBlock: received "all shards failed" response without any items; stopping pagination.' | |
| ); | |
| moreResults = false; | |
| } else { | |
| payload.query = { | |
| property: 'id', | |
| simpleOperator: 'greaterThan', | |
| value: items.at(-1).id, | |
| }; | |
| lastPage = 0; | |
| moreResults = true; | |
| } |
| } | ||
| } while (moreResults); | ||
| } catch (ex) { | ||
| Util.logger.error(`An error occured while attempting to query assets. ${ex.message}`); |
There was a problem hiding this comment.
Typo in log message: "An error occured" should be "An error occurred".
| Util.logger.error(`An error occured while attempting to query assets. ${ex.message}`); | |
| Util.logger.error(`An error occurred while attempting to query assets. ${ex.message}`); |
| * | ||
| * @private | ||
| * @param {string[]} keys metadata keys | ||
| * @returns {Promise.<object[]>} - array of assets |
There was a problem hiding this comment.
The JSDoc for _findEmailsUsingBlock() says it returns an array (Promise.<object[]>), but the implementation returns an object map (via Object.fromEntries(...)). Please update the JSDoc/typings to match the actual return type (or change the implementation to return an array).
| * @returns {Promise.<object[]>} - array of assets | |
| * @returns {Promise.<AssetMap>} - map of assets keyed by their keyField |
|
two things that caught my attention:
|
|
I've created a refresh function specifically for the Asset type. Journey and TriggeredSend already have similar functionality, no changes were necessary. Initially, I considered retrieving published journeys as part of this, but I reverted that approach. I realized that the existing TriggeredSend refresh logic already checks whether a TriggeredSend linked to an email activity is Active. So, the process looks up email keys based on code block keys and then runs refresh on the active TriggeredSends associated with those emails. The logic I've added: Refresh command on asset:
Deploy with refresh: Deploy as usual To find emails referencing a content block, I used the POST I'll refine the condition to something more specific, and add recursion to handle embedded blocks. |
PR details
Introduced refresh option for assets. Added a function to lookup emails that are referencing the block.
Can be run after deploy or separately. Can refresh triggeredSends linked to an email, or if the user runs the function for a code block (asset-other or asset-block), then the function will lookup emails that are using the code block. The function refreshes emails on the business unit it's been invoked on.
Examples:
What changes did you make? (Give an overview)
Further details (optional)
Tested on a couple of different code blocks (htmlblock, buttonblock, freeformblock and other), on a Parent BU and on a child BU. Tested refresh of a shared block.
Retrieve of published journeys was not necessary, the existing functionality of
_refreshTriggeredSendfunction already validates triggeredSend status. Also, journeys with a status Finishing are not accepting new contacts, but could be still sending emails.Checklist